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: https://github.com/iojs/io.js/issues/1190
PR-URL: https://github.com/iojs/io.js/pull/1229
Reviewed-By: Fedor Indutny <fedor@indutny.com>

diff --git a/src/env.h b/src/env.h
index 73940ad..cad03c4 100644 (file)
--- a/src/env.h
+++ b/src/env.h
@@ -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;
 
@@ -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_;

diff --git a/src/node_contextify.cc b/src/node_contextify.cc
index 6985a33..81d0388 100644 (file)
--- a/src/node_contextify.cc
+++ b/src/node_contextify.cc
@@ -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.

diff --git a/test/fixtures/vm-run-in-debug-context.js b/test/fixtures/vm-run-in-debug-context.js
new file mode 100644 (file)
index 0000000..94611c7
--- /dev/null
+++ b/test/fixtures/vm-run-in-debug-context.js
@@ -0,0 +1,4 @@
+if (process.argv[2] === 'handle-fatal-exception')
+  process._fatalException = process.exit.bind(null, 42);
+
+require('vm').runInDebugContext('*');

diff --git a/test/parallel/test-vm-debug-context.js b/test/parallel/test-vm-debug-context.js
index a648777..8f0a274 100644 (file)
--- a/test/parallel/test-vm-debug-context.js
+++ b/test/parallel/test-vm-debug-context.js
@@ -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('*');
@@ -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);
+}));