Skip to content

Commit

Permalink
vm: fix crash on fatal error in debug context
Browse files Browse the repository at this point in the history
Ensure that the debug context has an Environment assigned in case
a fatal error is raised.

The fatal exception handler in node.cc is not equipped to deal with
contexts that don't have one and can't easily be taught that due to
a deficiency in the V8 API: there is no way for the embedder to tell
if the data index is in use.

Fixes: #1190
PR-URL: #1229
Reviewed-By: Fedor Indutny <fedor@indutny.com>
  • Loading branch information
bnoordhuis committed Mar 22, 2015
1 parent d8f383b commit cf081a4
Show file tree
Hide file tree
Showing 4 changed files with 52 additions and 5 deletions.
6 changes: 2 additions & 4 deletions src/env.h
Original file line number Diff line number Diff line change
Expand Up @@ -474,6 +474,8 @@ class Environment {
inline HandleWrapQueue* handle_wrap_queue() { return &handle_wrap_queue_; }
inline ReqWrapQueue* req_wrap_queue() { return &req_wrap_queue_; }

static const int kContextEmbedderDataIndex = NODE_CONTEXT_EMBEDDER_DATA_INDEX;

private:
static const int kIsolateSlot = NODE_ISOLATE_SLOT;

Expand All @@ -482,10 +484,6 @@ class Environment {
inline ~Environment();
inline IsolateData* isolate_data() const;

enum ContextEmbedderDataIndex {
kContextEmbedderDataIndex = NODE_CONTEXT_EMBEDDER_DATA_INDEX
};

v8::Isolate* const isolate_;
IsolateData* const isolate_data_;
uv_check_t immediate_check_handle_;
Expand Down
24 changes: 23 additions & 1 deletion src/node_contextify.cc
Original file line number Diff line number Diff line change
Expand Up @@ -233,10 +233,32 @@ class ContextifyContext {


static void RunInDebugContext(const FunctionCallbackInfo<Value>& args) {
// Ensure that the debug context has an Environment assigned in case
// a fatal error is raised. The fatal exception handler in node.cc
// is not equipped to deal with contexts that don't have one and
// can't easily be taught that due to a deficiency in the V8 API:
// there is no way for the embedder to tell if the data index is
// in use.
struct ScopedEnvironment {
ScopedEnvironment(Local<Context> context, Environment* env)
: context_(context) {
const int index = Environment::kContextEmbedderDataIndex;
context->SetAlignedPointerInEmbedderData(index, env);
}
~ScopedEnvironment() {
const int index = Environment::kContextEmbedderDataIndex;
context_->SetAlignedPointerInEmbedderData(index, nullptr);
}
Local<Context> context_;
};

Local<String> script_source(args[0]->ToString(args.GetIsolate()));
if (script_source.IsEmpty())
return; // Exception pending.
Context::Scope context_scope(Debug::GetDebugContext());
Local<Context> debug_context = Debug::GetDebugContext();
Environment* env = Environment::GetCurrent(args);
ScopedEnvironment env_scope(debug_context, env);
Context::Scope context_scope(debug_context);
Local<Script> script = Script::Compile(script_source);
if (script.IsEmpty())
return; // Exception pending.
Expand Down
4 changes: 4 additions & 0 deletions test/fixtures/vm-run-in-debug-context.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
if (process.argv[2] === 'handle-fatal-exception')
process._fatalException = process.exit.bind(null, 42);

require('vm').runInDebugContext('*');
23 changes: 23 additions & 0 deletions test/parallel/test-vm-debug-context.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
var common = require('../common');
var assert = require('assert');
var vm = require('vm');
var spawn = require('child_process').spawn;

assert.throws(function() {
vm.runInDebugContext('*');
Expand All @@ -25,3 +26,25 @@ assert.strictEqual(vm.runInDebugContext(), undefined);
assert.strictEqual(vm.runInDebugContext(0), 0);
assert.strictEqual(vm.runInDebugContext(null), null);
assert.strictEqual(vm.runInDebugContext(undefined), undefined);

// See https://github.com/iojs/io.js/issues/1190, fatal errors should not
// crash the process.
var script = common.fixturesDir + '/vm-run-in-debug-context.js';
var proc = spawn(process.execPath, [script]);
var data = [];
proc.stdout.on('data', assert.fail);
proc.stderr.on('data', data.push.bind(data));
proc.once('exit', common.mustCall(function(exitCode, signalCode) {
assert.equal(exitCode, 1);
assert.equal(signalCode, null);
var haystack = Buffer.concat(data).toString('utf8');
assert(/SyntaxError: Unexpected token \*/.test(haystack));
}));

var proc = spawn(process.execPath, [script, 'handle-fatal-exception']);
proc.stdout.on('data', assert.fail);
proc.stderr.on('data', assert.fail);
proc.once('exit', common.mustCall(function(exitCode, signalCode) {
assert.equal(exitCode, 42);
assert.equal(signalCode, null);
}));

0 comments on commit cf081a4

Please sign in to comment.