[wip] src, inspector: support opted-in VM contexts

doc/api/inspector.md

@@ -10,6 +10,45 @@ It can be accessed using:
 const inspector = require('inspector');
 ```
 
+## inspector.attachContext(contextifiedSandbox[, options])
+
+* `contextifiedSandbox` {Object} The [contextified][] object to attach to the
+  inspector.
+* `options` {Object}
+  * `name` {string} Name of the context. If not specified, a default name `vm
+    Module Context ${idx}` where `idx` is the incrementing index corresponding
+    to this call will be used.
+  * `origin` {string} URL of the webpage this context corresponds to. Defaults
+    to `false`.
+<!--
+  TODO
+  * `console` {string} Controls whether the V8 console will be available.
+    Defaults to `'merge'`.
+-->
+
+Make inspector aware of the specified context. This allows code running in that
+context to be debugged, including support for [`debugger` statement][]. If the
+context is already being tracked by inspector, this function is a no-op.
+
+*Note:* Calling this function will prevent `contextifiedSandbox` from being
+garbage collected. To prevent memory leaks, [`inspector.detachContext()`][]
+*must* be called after work on the context has been completed.
+
+## inspector.contextAttached(contextifiedSandbox)
+
+* `contextifiedSandbox` {Object} A [contextified][] object
+* Returns: {boolean}
+
+Check if the specified context is attached to V8 inspector.
+
+## inspector.detachContext(contextifiedSandbox)
+
+* `contextifiedSandbox` {Object} The [contextified][] object to detach from
+  inspector
+
+Inform the V8 inspector that the context has been destroyed. If the context is
+not being tracked by inspector, this function is a no-op.
+
 ## inspector.open([port[, host[, wait]]])
 
 * port {number} Port to listen on for inspector connections. Optional,
@@ -135,6 +174,8 @@ messages again. Reconnected session will lose all inspector state, such as
 enabled agents or configured breakpoints.
 
 
+[contextified]: vm.html#vm_what_does_it_mean_to_contextify_an_object
+[`debugger` statement]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Statements/debugger
 [`session.connect()`]: #inspector_session_connect
 [`Debugger.paused`]: https://chromedevtools.github.io/devtools-protocol/v8/Debugger/#event-paused
 [`EventEmitter`]: events.html#events_class_eventemitter

doc/api/vm.md

@@ -59,6 +59,11 @@ changes:
     `cachedData` property of the returned `vm.Script` instance.
     The `cachedDataProduced` value will be set to either `true` or `false`
     depending on whether code cache data is produced successfully.
+  * `contextifiedSandbox` {Object} A [contextified][] sandbox to associate with
+    the current script. This does not bind the created script to this context
+    (i.e. `runInContext` still works with all contexts), but merely informs V8
+    (and V8 inspector) that the script is associated with the context. Defaults
+    to the current context.
 
 Creating a new `vm.Script` object compiles `code` but does not run it. The
 compiled `vm.Script` can be run later multiple times. It is important to note
@@ -94,12 +99,28 @@ changes:
     event that have been attached via `process.on("SIGINT")` will be disabled
     during script execution, but will continue to work after that.
     If execution is terminated, an [`Error`][] will be thrown.
+  * `doNotInformInspector` {boolean} If `true`, do not try to attach the
+    context to inspector. Debugging with `--inspect` will not work for code in
+    the context in that case.
 
 
 Runs the compiled code contained by the `vm.Script` object within the given
 `contextifiedSandbox` and returns the result. Running code does not have access
 to local scope.
 
+By default, this function checks if inspector was already aware of the context
+(if support for inspector is available in the Node.js build). If not, and if
+`doNotInformInspector` is not specified, this function attaches it to inspector
+using [`inspector.attachContext()`][], and detaches it before returning.
+However, *it is still recommended that you handle context attachment and
+detachment manually,* since:
+
+1. Asynchronous code running in the context would still not have inspector
+   debugging support, if relying on automatic attachment provided by this
+   function.
+2. Attaching and detaching per `runInContext()` run can have a significant
+   performance cost if this is done for the same context again and again.
+
 The following example compiles code that increments a global variable, sets
 the value of another global variable, then execute the code multiple times.
 The globals are contained in the `sandbox` object.
@@ -149,11 +170,23 @@ added: v0.3.1
   * `timeout` {number} Specifies the number of milliseconds to execute `code`
     before terminating execution. If execution is terminated, an [`Error`][]
     will be thrown.
+  * `doNotInformInspector` {boolean} If `true`, do not try to attach the
+    context to inspector. Debugging with `--inspect` will not work for code in
+    the context in that case.
 
 First contextifies the given `sandbox`, runs the compiled code contained by
 the `vm.Script` object within the created sandbox, and returns the result.
 Running code does not have access to local scope.
 
+By default, if support for inspector is available in the Node.js build, and if
+`doNotInformInspector` is not specified, this function attaches the new context
+to inspector using [`inspector.attachContext()`][], and detaches it before
+returning. However, if debugging supported is desired, *it is recommended that
+you handle context creation, attachment to inspector, and detachment from
+inspector manually,* since asynchronous code running in the context would still
+not have inspector debugging support, if relying on automatic attachment
+provided by this function.
+
 The following example compiles code that sets a global variable, then executes
 the code multiple times in different contexts. The globals are set on and
 contained within each individual `sandbox`.
@@ -284,12 +317,28 @@ Returns `true` if the given `sandbox` object has been [contextified][] using
   * `timeout` {number} Specifies the number of milliseconds to execute `code`
     before terminating execution. If execution is terminated, an [`Error`][]
     will be thrown.
+  * `doNotInformInspector` {boolean} If `true`, do not try to attach the
+    context to inspector. Debugging with `--inspect` will not work for code in
+    the context in that case.
 
 The `vm.runInContext()` method compiles `code`, runs it within the context of
 the `contextifiedSandbox`, then returns the result. Running code does not have
 access to the local scope. The `contextifiedSandbox` object *must* have been
 previously [contextified][] using the [`vm.createContext()`][] method.
 
+By default, this function checks if inspector was already aware of the context
+(if support for inspector is available in the Node.js build). If not, and if
+`doNotInformInspector` is not specified, this function attaches it to inspector
+using [`inspector.attachContext()`][], and detaches it before returning.
+However, *it is still recommended that you handle context attachment and
+detachment manually,* since:
+
+1. Asynchronous code running in the context would still not have inspector
+   debugging support, if relying on automatic attachment provided by this
+   function.
+2. Attaching and detaching per `runInContext()` run can have a significant
+   performance cost if this is done for the same context again and again.
+
 The following example compiles and executes different scripts using a single
 [contextified][] object:
 

lib/inspector.js

@@ -1,8 +1,17 @@
 'use strict';
 
+const errors = require('internal/errors');
 const EventEmitter = require('events');
 const util = require('util');
-const { connect, open, url } = process.binding('inspector');
+const { isContext } = process.binding('contextify');
+const {
+  connect,
+  open,
+  url,
+  contextAttached: _contextAttached,
+  attachContext: _attachContext,
+  detachContext: _detachContext
+} = process.binding('inspector');
 
 if (!connect)
   throw new Error('Inspector is not available');
@@ -86,9 +95,49 @@ class Session extends EventEmitter {
   }
 }
 
+function checkSandbox(sandbox) {
+  if (typeof sandbox !== 'object') {
+    throw new errors.TypeError('ERR_INVALID_ARG_TYPE', 'sandbox',
+                               'object', sandbox);
+  }
+  if (!isContext(sandbox)) {
+    throw new errors.TypeError('ERR_SANDBOX_NOT_CONTEXTIFIED');
+  }
+}
+
+let ctxIdx = 1;
+function attachContext(contextifiedSandbox, {
+  name = `vm Module Context ${ctxIdx++}`,
+  origin
+} = {}) {
+  checkSandbox(contextifiedSandbox);
+  if (typeof name !== 'string') {
+    throw new errors.TypeError('ERR_INVALID_ARG_TYPE', 'options.name',
+                               'string', name);
+  }
+  if (origin !== undefined && typeof origin !== 'string') {
+    throw new errors.TypeError('ERR_INVALID_ARG_TYPE', 'options.origin',
+                               'string', origin);
+  }
+  _attachContext(contextifiedSandbox, name, origin);
+}
+
+function detachContext(contextifiedSandbox) {
+  checkSandbox(contextifiedSandbox);
+  _detachContext(contextifiedSandbox);
+}
+
+function contextAttached(contextifiedSandbox) {
+  checkSandbox(contextifiedSandbox);
+  return _contextAttached(contextifiedSandbox);
+}
+
 module.exports = {
   open: (port, host, wait) => open(port, host, !!wait),
   close: process._debugEnd,
   url: url,
-  Session
+  Session,
+  attachContext,
+  detachContext,
+  contextAttached
 };

lib/internal/errors.js

@@ -165,6 +165,8 @@ E('ERR_NAPI_CONS_FUNCTION', 'Constructor must be a function');
 E('ERR_NAPI_CONS_PROTOTYPE_OBJECT', 'Constructor.prototype must be an object');
 E('ERR_NO_CRYPTO', 'Node.js is not compiled with OpenSSL crypto support');
 E('ERR_PARSE_HISTORY_DATA', 'Could not parse history data in %s');
+E('ERR_SANDBOX_NOT_CONTEXTIFIED',
+  'Provided sandbox must have been converted to a context');
 E('ERR_SOCKET_ALREADY_BOUND', 'Socket is already bound');
 E('ERR_SOCKET_BAD_TYPE',
   'Bad socket type specified. Valid types are: udp4, udp6');

lib/vm.js

@@ -23,6 +23,13 @@
 
 const binding = process.binding('contextify');
 const Script = binding.ContextifyScript;
+const { contextAttached, attachContext, detachContext } = (() => {
+  try {
+    return require('inspector');
+  } catch (err) {
+    return {};
+  }
+})();
 
 // The binding provides a few useful primitives:
 // - Script(code, { filename = "evalmachine.anonymous",
@@ -46,11 +53,23 @@ Script.prototype.runInThisContext = function(options) {
 };
 
 Script.prototype.runInContext = function(contextifiedSandbox, options) {
-  if (options && options.breakOnSigint && process._events.SIGINT) {
-    return sigintHandlersWrap(realRunInContext, this,
-                              [contextifiedSandbox, options]);
-  } else {
-    return realRunInContext.call(this, contextifiedSandbox, options);
+  const needToAttach = contextAttached &&
+                       !(options && options.doNotInformInspector) &&
+                       !contextAttached(contextifiedSandbox);
+  if (needToAttach) {
+    attachContext(contextifiedSandbox);
+  }
+  try {
+    if (options && options.breakOnSigint && process._events.SIGINT) {
+      return sigintHandlersWrap(realRunInContext, this,
+                                [contextifiedSandbox, options]);
+    } else {
+      return realRunInContext.call(this, contextifiedSandbox, options);
+    }
+  } finally {
+    if (needToAttach) {
+      detachContext(contextifiedSandbox);
+    }
   }
 };
 
@@ -104,12 +123,32 @@ function runInDebugContext(code) {
 }
 
 function runInContext(code, contextifiedSandbox, options) {
+  options = Object.assign({}, options, {
+    contextifiedSandbox
+  });
   return createScript(code, options)
     .runInContext(contextifiedSandbox, options);
 }
 
 function runInNewContext(code, sandbox, options) {
-  return createScript(code, options).runInNewContext(sandbox, options);
+  sandbox = createContext(sandbox);
+  const needToAttach = contextAttached &&
+                       !(options && options.doNotInformInspector) &&
+                       !contextAttached(sandbox);
+  if (needToAttach) {
+    attachContext(sandbox);
+  }
+  try {
+    options = Object.assign({}, options, {
+      contextifiedSandbox: sandbox,
+      doNotInformInspector: true
+    });
+    return createScript(code, options).runInNewContext(sandbox, options);
+  } finally {
+    if (needToAttach) {
+      detachContext(sandbox);
+    }
+  }
 }
 
 function runInThisContext(code, options) {

node.gyp

@@ -175,6 +175,7 @@
         'src/node_config.cc',
         'src/node_constants.cc',
         'src/node_contextify.cc',
+        'src/node_contextify.h',
         'src/node_debug_options.cc',
         'src/node_file.cc',
         'src/node_http_parser.cc',
@@ -624,8 +625,10 @@
         '<(OBJ_PATH)<(OBJ_SEPARATOR)env.<(OBJ_SUFFIX)',
         '<(OBJ_PATH)<(OBJ_SEPARATOR)node.<(OBJ_SUFFIX)',
         '<(OBJ_PATH)<(OBJ_SEPARATOR)node_buffer.<(OBJ_SUFFIX)',
+        '<(OBJ_PATH)<(OBJ_SEPARATOR)node_contextify.<(OBJ_SUFFIX)',
         '<(OBJ_PATH)<(OBJ_SEPARATOR)node_i18n.<(OBJ_SUFFIX)',
         '<(OBJ_PATH)<(OBJ_SEPARATOR)node_url.<(OBJ_SUFFIX)',
+        '<(OBJ_PATH)<(OBJ_SEPARATOR)node_watchdog.<(OBJ_SUFFIX)',
         '<(OBJ_PATH)<(OBJ_SEPARATOR)util.<(OBJ_SUFFIX)',
         '<(OBJ_PATH)<(OBJ_SEPARATOR)string_bytes.<(OBJ_SUFFIX)',
         '<(OBJ_PATH)<(OBJ_SEPARATOR)string_search.<(OBJ_SUFFIX)',