vm: "afterEvaluate", make module.evaluate() return a promise from the outer context #59801
+234
−1
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Review requested:
|
nodejs-github-bot
added
c++
addaleax
reviewed
Sep 8, 2025
ericrannaud
force-pushed
the
vm-after-evaluate-link-queues
branch
from
9f3b583 to
9cb3575
Compare
legendecas
reviewed
Sep 8, 2025
addaleax
reviewed
Sep 8, 2025
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #59801 +/- ##
==========================================
+ Coverage 88.27% 88.28% +0.01%
==========================================
Files 701 701
Lines 206753 206784 +31
Branches 39757 39760 +3
==========================================
+ Hits 182512 182565 +53
+ Misses 16285 16251 -34
- Partials 7956 7968 +12
🚀 New features to boost your workflow:
|
Check that we lose the execution flow in the outer context, upon resolving a promise created in in the inner context.
… outer context
Consider the default context A with a microtask queue QA, and a
context B with its own microtask queue QB.
Context B is constructed with vm.createContext(..., {microtaskMode:
"afterEvaluate"}). The evaluation in context B can be performed via
vm.Script or vm.SourceTextModule.
The standard (https://tc39.es/ecma262/#sec-newpromiseresolvethenablejob)
dictates that, when resolving a {promise} with {resolution}, from any
context, the {then} method on {promise} should be called within a task
enqueued on the microtask queue from the context associated with {then}.
Specifically, after evaluating a script or module in context B, any
promises created within B, if later resolved within A, will result in a
task to be enqueued back onto QB, even long after we are done evaluating
any code within B.
This creates a challenge for users of node:vm in "afterEvaluate" mode.
In ContextifyScript::EvalMachine() and in ModuleWrap::Evaluate(), we
only drain the microtask queue QB a single time after running the script
or evaluating the module. After that point, the queue will not be
drained unless another script or module is evaluated in the same
context.
In the following scenario, prior to this patch, the log statement will
not be printed:
const microtaskMode = "afterEvaluate";
const context = vm.createContext({}, {microtaskMode});
const source = "";
const module = new vm.SourceTextModule(source, {context});
await module.link(() => null);
await module.evaluate();
console.log("NOT PRINTED");
To resolve the promise returned by evaluate(), which technically is the
top-level capability for {module}, a promise created within B, V8 will
enqueue a task on QB. But since this is after the PerformCheckpoint()
call in ModuleWrap::Evaluate(), the task in QB is never run. In the
meantime, since QA is empty, the Node process simply exits (with a
warning about the unsettled promise, if it happened to be a top-level
await).
While being unable to do `await module.evaluate()` is clearly a problem,
more generally, it is intended that in "afterEvaluate" mode, promises
created in the inner context cannot make progress if, and until, the
microtask queue of the inner context is checkpointed.
Therefore, to address this issue, the fix is narrow:
When the module has its own microtask queue, i.e. in "afterEvaluate"
mode, the inner-context promise returned by
v8::SourceTextModule::Evaluate() is first resolved to an outer-context
promise, then we checkpoint the microtask queue of the inner context,
then we return the outer-context promise we just built.
This ensures that in the statement `await module.evaluate()`, the
promise returned by `evaluate()` can be resolved within the outer
context, without involving the microtask queue in the inner context.
Fixes: nodejs#59541
Refs: https://issues.chromium.org/issues/441679231
Refs: https://groups.google.com/g/v8-dev/c/YIeRg8CUNS8/m/rEQdFuNZAAAJ
ericrannaud
force-pushed
the
vm-after-evaluate-link-queues
branch
from
9cb3575 to
abf9d31
Compare
ericrannaud
changed the title
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The underlying behavior conforms to the spec, see Refs. We need a workaround on the Node side to at least enable the use of
awaiton the return value ofmodule.evaluate().This series first adds a test that confirm the current incorrect behavior: I wrote the test so it passes with the current code.
Then I introduce a workaround (credit for the idea goes to @addaleax) and update the tests to verify that the behavior is now correct.
Then I add a subsection to
doc/vm.mdto explain how it can be a challenge to share promises between contexts with afterEvaluate; I also add more tests to demonstrate how promises behave in this case.Fixes: #59541
Refs: https://issues.chromium.org/issues/441679231
Refs: https://groups.google.com/g/v8-dev/c/YIeRg8CUNS8/m/rEQdFuNZAAAJ
Refs: https://tc39.es/ecma262/#sec-newpromiseresolvethenablejob