Skip to content

Commit cf081a4

Browse files
committed
vm: fix crash on fatal error in debug context
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 <[email protected]>
1 parent d8f383b commit cf081a4

File tree

4 files changed

+52
-5
lines changed

4 files changed

+52
-5
lines changed

src/env.h

+2-4
Original file line numberDiff line numberDiff line change
@@ -474,6 +474,8 @@ class Environment {
474474
inline HandleWrapQueue* handle_wrap_queue() { return &handle_wrap_queue_; }
475475
inline ReqWrapQueue* req_wrap_queue() { return &req_wrap_queue_; }
476476

477+
static const int kContextEmbedderDataIndex = NODE_CONTEXT_EMBEDDER_DATA_INDEX;
478+
477479
private:
478480
static const int kIsolateSlot = NODE_ISOLATE_SLOT;
479481

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

485-
enum ContextEmbedderDataIndex {
486-
kContextEmbedderDataIndex = NODE_CONTEXT_EMBEDDER_DATA_INDEX
487-
};
488-
489487
v8::Isolate* const isolate_;
490488
IsolateData* const isolate_data_;
491489
uv_check_t immediate_check_handle_;

src/node_contextify.cc

+23-1
Original file line numberDiff line numberDiff line change
@@ -233,10 +233,32 @@ class ContextifyContext {
233233

234234

235235
static void RunInDebugContext(const FunctionCallbackInfo<Value>& args) {
236+
// Ensure that the debug context has an Environment assigned in case
237+
// a fatal error is raised. The fatal exception handler in node.cc
238+
// is not equipped to deal with contexts that don't have one and
239+
// can't easily be taught that due to a deficiency in the V8 API:
240+
// there is no way for the embedder to tell if the data index is
241+
// in use.
242+
struct ScopedEnvironment {
243+
ScopedEnvironment(Local<Context> context, Environment* env)
244+
: context_(context) {
245+
const int index = Environment::kContextEmbedderDataIndex;
246+
context->SetAlignedPointerInEmbedderData(index, env);
247+
}
248+
~ScopedEnvironment() {
249+
const int index = Environment::kContextEmbedderDataIndex;
250+
context_->SetAlignedPointerInEmbedderData(index, nullptr);
251+
}
252+
Local<Context> context_;
253+
};
254+
236255
Local<String> script_source(args[0]->ToString(args.GetIsolate()));
237256
if (script_source.IsEmpty())
238257
return; // Exception pending.
239-
Context::Scope context_scope(Debug::GetDebugContext());
258+
Local<Context> debug_context = Debug::GetDebugContext();
259+
Environment* env = Environment::GetCurrent(args);
260+
ScopedEnvironment env_scope(debug_context, env);
261+
Context::Scope context_scope(debug_context);
240262
Local<Script> script = Script::Compile(script_source);
241263
if (script.IsEmpty())
242264
return; // Exception pending.
+4
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
if (process.argv[2] === 'handle-fatal-exception')
2+
process._fatalException = process.exit.bind(null, 42);
3+
4+
require('vm').runInDebugContext('*');

test/parallel/test-vm-debug-context.js

+23
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
var common = require('../common');
22
var assert = require('assert');
33
var vm = require('vm');
4+
var spawn = require('child_process').spawn;
45

56
assert.throws(function() {
67
vm.runInDebugContext('*');
@@ -25,3 +26,25 @@ assert.strictEqual(vm.runInDebugContext(), undefined);
2526
assert.strictEqual(vm.runInDebugContext(0), 0);
2627
assert.strictEqual(vm.runInDebugContext(null), null);
2728
assert.strictEqual(vm.runInDebugContext(undefined), undefined);
29+
30+
// See https://github.com/iojs/io.js/issues/1190, fatal errors should not
31+
// crash the process.
32+
var script = common.fixturesDir + '/vm-run-in-debug-context.js';
33+
var proc = spawn(process.execPath, [script]);
34+
var data = [];
35+
proc.stdout.on('data', assert.fail);
36+
proc.stderr.on('data', data.push.bind(data));
37+
proc.once('exit', common.mustCall(function(exitCode, signalCode) {
38+
assert.equal(exitCode, 1);
39+
assert.equal(signalCode, null);
40+
var haystack = Buffer.concat(data).toString('utf8');
41+
assert(/SyntaxError: Unexpected token \*/.test(haystack));
42+
}));
43+
44+
var proc = spawn(process.execPath, [script, 'handle-fatal-exception']);
45+
proc.stdout.on('data', assert.fail);
46+
proc.stderr.on('data', assert.fail);
47+
proc.once('exit', common.mustCall(function(exitCode, signalCode) {
48+
assert.equal(exitCode, 42);
49+
assert.equal(signalCode, null);
50+
}));

0 commit comments

Comments
 (0)