vm: "afterEvaluate", make module.evaluate() return a promise from the outer context

doc/api/vm.md

@@ -1916,6 +1916,63 @@ 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.
 
+### When `microtaskMode` is `'afterEvaluate'`, beware sharing Promises between Contexts
+
+In `'afterEvaluate'` mode, the `Context` has its own microtask queue, separate
+from the global microtask queue used by the outer (main) context. While this
+mode is necessary to enforce `timeout` and enable `breakOnSigint` with
+asynchronous tasks, it also makes sharing promises between contexts challenging.
+
+In the example below, a promise is created in the inner context and shared with
+the outer context. When the outer context `await` on the promise, the execution
+flow of the outer context is disrupted in a surprising way: the log statement
+is never executed.
+
+```mjs
+import * as vm from 'node:vm';
+
+const inner_context = vm.createContext({}, { microtaskMode: 'afterEvaluate' });
+
+// runInContext() returns a Promise created in the inner context.
+const inner_promise = vm.runInContext(
+  'Promise.resolve()',
+  context,
+);
+
+// As part of performing `await`, the JavaScript runtime must enqueue a task
+// on the microtask queue of the context where `inner_promise` was created.
+// A task is added on the inner microtask queue, but **it will not be run
+// automatically**: this task will remain pending indefinitely.
+//
+// Since the outer microtask queue is empty, execution in the outer module
+// falls through, and the log statement below is never executed.
+await inner_promise;
+
+console.log('this will NOT be printed');
+```
+
+To successfully share promises between contexts with different microtask queues,
+it is necessary to ensure that tasks on the inner microtask queue will be run
+**whenever** the outer context enqueues a task on the inner microtask queue.
+
+The tasks on the microtask queue of a given context are run whenever
+`runInContext()` or `SourceTextModule.evaluate()` are invoked on a script or
+module using this context. In our example, the normal execution flow can be
+restored by scheduling a second call to `runInContext()` _before_ `await
+inner_promise`.
+
+```mjs
+// Schedule `runInContext()` to manually drain the inner context microtask
+// queue; it will run after the `await` statement below.
+setImmediate(() => {
+  vm.runInContext('', context);
+});
+
+await inner_promise;
+
+console.log('OK');
+```
+
 ## Support of dynamic `import()` in compilation APIs
 
 The following APIs support an `importModuleDynamically` option to enable dynamic

src/module_wrap.cc

@@ -735,8 +735,48 @@ void ModuleWrap::Evaluate(const FunctionCallbackInfo<Value>& args) {
   MaybeLocal<Value> result;
   auto run = [&]() {
     MaybeLocal<Value> result = module->Evaluate(context);
-    if (!result.IsEmpty() && microtask_queue)
+
+    Local<Value> res;
+    if (result.ToLocal(&res) && microtask_queue) {
+      DCHECK(res->IsPromise());
+
+      // To address https://github.com/nodejs/node/issues/59541 when the
+      // module has its own separate microtask queue in microtaskMode
+      // "afterEvaluate", we avoid returning a promise built inside the
+      // module's own context.
+      //
+      // Instead, we build a promise in the outer context, which we resolve
+      // with {result}, then we checkpoint the module's own queue, and finally
+      // we return the outer-context promise.
+      //
+      // If we simply returned the inner promise {result} directly, per
+      // https://tc39.es/ecma262/#sec-newpromiseresolvethenablejob the outer
+      // context, when resolving a promise coming from a different context,
+      // would need to enqueue a task (known as a thenable job task) onto the
+      // queue of that different context (the module's context). But this queue
+      // will normally not be checkpointed after evaluate() returns.
+      //
+      // This means that the execution flow in the outer context would
+      // silently fall through at the statement:
+      //   await module.evaluate();
+      //
+      // This is true for any promises created inside the module's context
+      // and made available to the outer context, as the node:vm doc explains.
+      //
+      // We must handle this particular return value differently to make it
+      // possible to await on the result of evaluate().
+      Local<Context> outer_context = isolate->GetCurrentContext();
+      Local<Promise::Resolver> resolver;
+      if (!Promise::Resolver::New(outer_context).ToLocal(&resolver)) {
+        return MaybeLocal<Value>();
+      }
+      if (resolver->Resolve(outer_context, res).IsNothing()) {
+        return MaybeLocal<Value>();
+      }
+      result = resolver->GetPromise();
+
       microtask_queue->PerformCheckpoint(isolate);
+    }
     return result;
   };
   if (break_on_sigint && timeout != -1) {

test/parallel/test-vm-module-after-evaluate.js

@@ -0,0 +1,71 @@
+// Flags: --experimental-vm-modules
+'use strict';
+
+// https://github.com/nodejs/node/issues/59541
+//
+// Promises created in a context using microtaskMode: "aferEvaluate" (meaning it
+// has its own microtask queue), when resolved in the surrounding context, will
+// schedule a task back onto the inner context queue.
+
+const common = require('../common');
+const vm = require('vm');
+
+const microtaskMode = 'afterEvaluate';
+
+(async () => {
+  const mustNotCall1 = common.mustNotCall();
+  const mustNotCall2 = common.mustNotCall();
+  const mustCall1 = common.mustCall();
+
+  const inner = {};
+
+  const context = vm.createContext({ inner }, { microtaskMode });
+
+  const module = new vm.SourceTextModule(
+    'inner.promise = Promise.resolve();',
+    { context },
+  );
+
+  await module.link(mustNotCall1);
+  await module.evaluate();
+
+  // Prior to the fix for Issue 59541, the next statement was never executed.
+  mustCall1();
+
+  await inner.promise;
+
+  // This is expected: the await statement above enqueues a (thenable job) task
+  // onto the inner context microtask queue, but it will not be checkpointed,
+  // therefore we never make progress.
+  mustNotCall2();
+})().then(common.mustNotCall());
+
+(async () => {
+  const mustNotCall1 = common.mustNotCall();
+  const mustCall1 = common.mustCall();
+  const mustCall2 = common.mustCall();
+  const mustCall3 = common.mustCall();
+
+  const inner = {};
+
+  const context = vm.createContext({ inner }, { microtaskMode });
+
+  const module = new vm.SourceTextModule(
+    'inner.promise = Promise.resolve();',
+    { context },
+  );
+
+  await module.link(mustNotCall1);
+  await module.evaluate();
+  mustCall1();
+
+  setImmediate(() => {
+    mustCall2();
+    // This will checkpoint the inner context microtask queue, and allow the
+    // promise from the inner context to be resolved in the outer context.
+    module.evaluate();
+  });
+
+  await inner.promise;
+  mustCall3();
+})().then(common.mustCall());

test/parallel/test-vm-script-after-evaluate.js

@@ -0,0 +1,65 @@
+'use strict';
+
+// https://github.com/nodejs/node/issues/59541
+//
+// Promises created in a context using microtaskMode: "aferEvaluate" (meaning
+// it has its own microtask queue), when resolved in the surrounding context,
+// will schedule a task back onto the inner context queue. This test checks that
+// the async execution progresses normally.
+
+const common = require('../common');
+const vm = require('vm');
+
+const microtaskMode = 'afterEvaluate';
+
+(async () => {
+  const mustNotCall1 = common.mustNotCall();
+
+  await vm.runInNewContext(
+    `Promise.resolve()`,
+    {}, { microtaskMode });
+  // Expected behavior: resolving an promise created in the inner context, from
+  // the outer context results in the execution flow falling through, unless the
+  // inner context microtask queue is manually drained, which we don't do here.
+  mustNotCall1();
+})().then(common.mustNotCall());
+
+(async () => {
+  const mustCall1 = common.mustCall();
+  const mustCall2 = common.mustCall();
+  const mustCall3 = common.mustCall();
+
+  // Create a new context.
+  const context = vm.createContext({}, { microtaskMode });
+
+  setImmediate(() => {
+    // This will drain the context microtask queue, after the `await` statement
+    // below, and allow the promise from the inner context, created below, to be
+    // resolved in the outer context.
+    vm.runInContext('', context);
+    mustCall2();
+  });
+
+  const inner_promise = vm.runInContext(
+    `Promise.resolve()`,
+    context);
+  mustCall1();
+
+  await inner_promise;
+  mustCall3();
+})().then(common.mustCall());
+
+{
+  const mustNotCall1 = common.mustNotCall();
+  const mustCall1 = common.mustCall();
+
+  const context = vm.createContext({ setImmediate, mustNotCall1 }, { microtaskMode });
+
+  // setImmediate() will be run after runInContext() returns, and since the
+  // anonymous function passed to `then` is defined in the inner context, the
+  // thenable job task will be enqueued on the inner context microtask queue,
+  // but at this point, it will not be drained automatically.
+  vm.runInContext(`new Promise(setImmediate).then(() => mustNotCall1())`, context);
+
+  mustCall1();
+}