vm: add run-after-evaluate microtask mode

This allows timeouts to apply to e.g. `Promise`s and `async function`s
from code running inside of `vm.Context`s, by giving the Context its
own microtasks queue.

Fixes: #3020

PR-URL: #34023
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Denys Otrishko <[email protected]>

doc/api/vm.md

@@ -188,6 +188,9 @@ overhead.
 <!-- YAML
 added: v0.3.1
 changes:
+  - version: REPLACEME
+    pr-url: https://github.com/nodejs/node/pull/34023
+    description: The `microtaskMode` option is supported now.
   - version: v10.0.0
     pr-url: https://github.com/nodejs/node/pull/19016
     description: The `contextCodeGeneration` option is supported now.
@@ -225,6 +228,10 @@ changes:
       `EvalError`. **Default:** `true`.
     * `wasm` {boolean} If set to false any attempt to compile a WebAssembly
       module will throw a `WebAssembly.CompileError`. **Default:** `true`.
+  * `microtaskMode` {string} If set to `afterEvaluate`, microtasks (tasks
+    scheduled through `Promise`s any `async function`s) will be run immediately
+    after the script has run. They are included in the `timeout` and
+    `breakOnSigint` scopes in that case.
 * Returns: {any} the result of the very last statement executed in the script.
 
 First contextifies the given `contextObject`, runs the compiled code contained
@@ -842,6 +849,9 @@ function with the given `params`.
 <!-- YAML
 added: v0.3.1
 changes:
+  - version: REPLACEME
+    pr-url: https://github.com/nodejs/node/pull/34023
+    description: The `microtaskMode` option is supported now.
   - version: v10.0.0
     pr-url: https://github.com/nodejs/node/pull/19398
     description: The first argument can no longer be a function.
@@ -867,6 +877,10 @@ changes:
       `EvalError`. **Default:** `true`.
     * `wasm` {boolean} If set to false any attempt to compile a WebAssembly
       module will throw a `WebAssembly.CompileError`. **Default:** `true`.
+  * `microtaskMode` {string} If set to `afterEvaluate`, microtasks (tasks
+    scheduled through `Promise`s any `async function`s) will be run immediately
+    after a script has run through [`script.runInContext()`][].
+    They are included in the `timeout` and `breakOnSigint` scopes in that case.
 * Returns: {Object} contextified object.
 
 If given a `contextObject`, the `vm.createContext()` method will [prepare
@@ -998,6 +1012,9 @@ console.log(contextObject);
 <!-- YAML
 added: v0.3.1
 changes:
+  - version: REPLACEME
+    pr-url: https://github.com/nodejs/node/pull/34023
+    description: The `microtaskMode` option is supported now.
   - version: v10.0.0
     pr-url: https://github.com/nodejs/node/pull/19016
     description: The `contextCodeGeneration` option is supported now.
@@ -1064,6 +1081,10 @@ changes:
     * Returns: {Module Namespace Object|vm.Module} Returning a `vm.Module` is
       recommended in order to take advantage of error tracking, and to avoid
       issues with namespaces that contain `then` function exports.
+  * `microtaskMode` {string} If set to `afterEvaluate`, microtasks (tasks
+    scheduled through `Promise`s any `async function`s) will be run immediately
+    after the script has run. They are included in the `timeout` and
+    `breakOnSigint` scopes in that case.
 * Returns: {any} the result of the very last statement executed in the script.
 
 The `vm.runInNewContext()` first contextifies the given `contextObject` (or
@@ -1220,13 +1241,13 @@ within which it can operate. The process of creating the V8 Context and
 associating it with the `contextObject` is what this document refers to as
 "contextifying" the object.
 
-## Timeout limitations when using `process.nextTick()`, promises, and `queueMicrotask()`
+## Timeout interactions with asynchronous tasks and Promises
 
-Because of the internal mechanics of how the `process.nextTick()` queue and
-the microtask queue that underlies Promises are implemented within V8 and
-Node.js, it is possible for code running within a context to "escape" the
-`timeout` set using `vm.runInContext()`, `vm.runInNewContext()`, and
-`vm.runInThisContext()`.
+`Promise`s and `async function`s can schedule tasks run by the JavaScript
+engine asynchronously. By default, these tasks are run after all JavaScript
+functions on the current stack are done executing.
+This allows escaping the functionality of the `timeout` and
+`breakOnSigint` options.
 
 For example, the following code executed by `vm.runInNewContext()` with a
 timeout of 5 milliseconds schedules an infinite loop to run after a promise
@@ -1236,21 +1257,52 @@ resolves. The scheduled loop is never interrupted by the timeout:
 const vm = require('vm');
 
 function loop() {
+  console.log('entering loop');
   while (1) console.log(Date.now());
 }
 
 vm.runInNewContext(
-  'Promise.resolve().then(loop);',
+  'Promise.resolve().then(() => loop());',
   { loop, console },
   { timeout: 5 }
 );
+// This prints *before* 'entering loop' (!)
+console.log('done executing');
 ```
 
-This issue also occurs when the `loop()` call is scheduled using
-the `process.nextTick()` and `queueMicrotask()` functions.
+This can be addressed by passing `microtaskMode: 'afterEvaluate'` to the code
+that creates the `Context`:
 
-This issue occurs because all contexts share the same microtask and nextTick
-queues.
+```js
+const vm = require('vm');
+
+function loop() {
+  while (1) console.log(Date.now());
+}
+
+vm.runInNewContext(
+  'Promise.resolve().then(() => loop());',
+  { loop, console },
+  { timeout: 5, microtaskMode: 'afterEvaluate' }
+);
+```
+
+In this case, the microtask scheduled through `promise.then()` will be run
+before returning from `vm.runInNewContext()`, and will be interrupted
+by the `timeout` functionality. This applies only to code running in a
+`vm.Context`, so e.g. [`vm.runInThisContext()`][] does not take this option.
+
+Promise callbacks are entered into the microtask queue of the context in which
+they were created. For example, if `() => loop()` is replaced with just `loop`
+in the above example, then `loop` will be pushed into the global microtask
+queue, because it is a function from the outer (main) context, and thus will
+also be able to escape the timeout.
+
+If asynchronous scheduling functions such as `process.nextTick()`,
+`queueMicrotask()`, `setTimeout()`, `setImmediate()`, etc. are made available
+inside a `vm.Context`, functions passed to them will be added to global queues,
+which are shared by all contexts. Therefore, callbacks passed to those functions
+are not controllable through the timeout either.
 
 [`ERR_VM_DYNAMIC_IMPORT_CALLBACK_MISSING`]: errors.html#ERR_VM_DYNAMIC_IMPORT_CALLBACK_MISSING
 [`ERR_VM_MODULE_STATUS`]: errors.html#ERR_VM_MODULE_STATUS

lib/vm.js

@@ -29,6 +29,7 @@ const {
 
 const {
   ContextifyScript,
+  MicrotaskQueue,
   makeContext,
   isContext: _isContext,
   constants,
@@ -186,6 +187,7 @@ function getContextOptions(options) {
     name: options.contextName,
     origin: options.contextOrigin,
     codeGeneration: undefined,
+    microtaskMode: options.microtaskMode,
   };
   if (contextOptions.name !== undefined)
     validateString(contextOptions.name, 'options.contextName');
@@ -201,6 +203,8 @@ function getContextOptions(options) {
       validateBoolean(wasm, 'options.contextCodeGeneration.wasm');
     contextOptions.codeGeneration = { strings, wasm };
   }
+  if (options.microtaskMode !== undefined)
+    validateString(options.microtaskMode, 'options.microtaskMode');
   return contextOptions;
 }
 
@@ -222,7 +226,8 @@ function createContext(contextObject = {}, options = {}) {
   const {
     name = `VM Context ${defaultContextNameIndex++}`,
     origin,
-    codeGeneration
+    codeGeneration,
+    microtaskMode
   } = options;
 
   validateString(name, 'options.name');
@@ -239,7 +244,22 @@ function createContext(contextObject = {}, options = {}) {
     validateBoolean(wasm, 'options.codeGeneration.wasm');
   }
 
-  makeContext(contextObject, name, origin, strings, wasm);
+  let microtaskQueue = null;
+  if (microtaskMode !== undefined) {
+    validateString(microtaskMode, 'options.microtaskMode');
+
+    if (microtaskMode === 'afterEvaluate') {
+      microtaskQueue = new MicrotaskQueue();
+    } else {
+      throw new ERR_INVALID_ARG_VALUE(
+        'options.microtaskQueue',
+        microtaskQueue,
+        'must be \'afterEvaluate\' or undefined'
+      );
+    }
+  }
+
+  makeContext(contextObject, name, origin, strings, wasm, microtaskQueue);
   return contextObject;
 }
 

src/env.h

@@ -419,6 +419,7 @@ constexpr size_t kFsStatsBufferLength =
   V(i18n_converter_template, v8::ObjectTemplate)                               \
   V(libuv_stream_wrap_ctor_template, v8::FunctionTemplate)                     \
   V(message_port_constructor_template, v8::FunctionTemplate)                   \
+  V(microtask_queue_ctor_template, v8::FunctionTemplate)                       \
   V(pipe_constructor_template, v8::FunctionTemplate)                           \
   V(promise_wrap_template, v8::ObjectTemplate)                                 \
   V(sab_lifetimepartner_constructor_template, v8::FunctionTemplate)            \

src/module_wrap.cc

@@ -35,6 +35,7 @@ using v8::IntegrityLevel;
 using v8::Isolate;
 using v8::Local;
 using v8::MaybeLocal;
+using v8::MicrotaskQueue;
 using v8::Module;
 using v8::Number;
 using v8::Object;
@@ -106,15 +107,15 @@ void ModuleWrap::New(const FunctionCallbackInfo<Value>& args) {
   Local<String> url = args[0].As<String>();
 
   Local<Context> context;
+  ContextifyContext* contextify_context = nullptr;
   if (args[1]->IsUndefined()) {
     context = that->CreationContext();
   } else {
     CHECK(args[1]->IsObject());
-    ContextifyContext* sandbox =
-        ContextifyContext::ContextFromContextifiedSandbox(
-            env, args[1].As<Object>());
-    CHECK_NOT_NULL(sandbox);
-    context = sandbox->context();
+    contextify_context = ContextifyContext::ContextFromContextifiedSandbox(
+        env, args[1].As<Object>());
+    CHECK_NOT_NULL(contextify_context);
+    context = contextify_context->context();
   }
 
   Local<Integer> line_offset;
@@ -224,6 +225,7 @@ void ModuleWrap::New(const FunctionCallbackInfo<Value>& args) {
   }
 
   obj->context_.Reset(isolate, context);
+  obj->contextify_context_ = contextify_context;
 
   env->hash_to_module_map.emplace(module->GetIdentityHash(), obj);
 
@@ -319,6 +321,11 @@ void ModuleWrap::Evaluate(const FunctionCallbackInfo<Value>& args) {
   Local<Context> context = obj->context_.Get(isolate);
   Local<Module> module = obj->module_.Get(isolate);
 
+  ContextifyContext* contextify_context = obj->contextify_context_;
+  std::shared_ptr<MicrotaskQueue> microtask_queue;
+  if (contextify_context != nullptr)
+      microtask_queue = contextify_context->microtask_queue();
+
   // module.evaluate(timeout, breakOnSigint)
   CHECK_EQ(args.Length(), 2);
 
@@ -334,18 +341,24 @@ void ModuleWrap::Evaluate(const FunctionCallbackInfo<Value>& args) {
   bool timed_out = false;
   bool received_signal = false;
   MaybeLocal<Value> result;
+  auto run = [&]() {
+    MaybeLocal<Value> result = module->Evaluate(context);
+    if (!result.IsEmpty() && microtask_queue)
+      microtask_queue->PerformCheckpoint(isolate);
+    return result;
+  };
   if (break_on_sigint && timeout != -1) {
     Watchdog wd(isolate, timeout, &timed_out);
     SigintWatchdog swd(isolate, &received_signal);
-    result = module->Evaluate(context);
+    result = run();
   } else if (break_on_sigint) {
     SigintWatchdog swd(isolate, &received_signal);
-    result = module->Evaluate(context);
+    result = run();
   } else if (timeout != -1) {
     Watchdog wd(isolate, timeout, &timed_out);
-    result = module->Evaluate(context);
+    result = run();
   } else {
-    result = module->Evaluate(context);
+    result = run();
   }
 
   if (result.IsEmpty()) {

src/module_wrap.h

@@ -12,6 +12,10 @@ namespace node {
 
 class Environment;
 
+namespace contextify {
+class ContextifyContext;
+}
+
 namespace loader {
 
 enum ScriptType : int {
@@ -82,12 +86,13 @@ class ModuleWrap : public BaseObject {
   static ModuleWrap* GetFromModule(node::Environment*, v8::Local<v8::Module>);
 
   v8::Global<v8::Function> synthetic_evaluation_steps_;
-  bool synthetic_ = false;
   v8::Global<v8::Module> module_;
   v8::Global<v8::String> url_;
-  bool linked_ = false;
   std::unordered_map<std::string, v8::Global<v8::Promise>> resolve_cache_;
   v8::Global<v8::Context> context_;
+  contextify::ContextifyContext* contextify_context_ = nullptr;
+  bool synthetic_ = false;
+  bool linked_ = false;
   uint32_t id_;
 };