repl: pass objectGroup to Runtime.awaitPromise to prevent GC collection#64764
repl: pass objectGroup to Runtime.awaitPromise to prevent GC collection#64764DivyanshuX9 wants to merge 1 commit into
Conversation
There was a problem hiding this comment.
Pull request overview
This PR fixes a GC-related race in the inspector-backed REPL evaluator when awaiting promise results. It ensures the remote Promise handle created by Runtime.evaluate remains retained until Runtime.awaitPromise completes, preventing Inspector error -32000: Promise was collected from surfacing as ERR_INSPECTOR_COMMAND.
Changes:
- Pass
objectGroup: this.objectGrouptoRuntime.awaitPromiseinReplInspectorChannel.evaluate(). - Update the surrounding comment to document the GC-retention rationale for the two-step inspector flow.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
When the inspector-backed REPL evaluates a promise-returning expression, Runtime.evaluate stores the remote promise object in an objectGroup. Runtime.awaitPromise did not pass objectGroup. Under GC pressure, the remote promise object could be collected before the await completed, causing the inspector to report: -32000: Promise was collected This surfaced in the REPL as ERR_INSPECTOR_COMMAND instead of the user's original exception. Pass objectGroup to Runtime.awaitPromise so the promise remains retained until the await completes. Fixes: nodejs#64762 Signed-off-by: Divyanshu Sharma <divyanshu88999@gmail.com>
c51f3cd to
a16deb3
Compare
| // Pass objectGroup so the promise remote object stays retained for the | ||
| // duration of the await and cannot be collected by a GC cycle between | ||
| // the two inspector calls. | ||
| return this.postInterruptible('Runtime.awaitPromise', { |
There was a problem hiding this comment.
awaitPromise method never accept https://chromedevtools.github.io/devtoolsprotocol/tot/Runtime/#method-awaitPromise objectGroup
avivkeller
left a comment
There was a problem hiding this comment.
This is incorrect. See https://chromium-review.googlesource.com/c/v8/v8/+/8123081 for a proper fix for this issue.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #64764 +/- ##
=======================================
Coverage 90.15% 90.15%
=======================================
Files 744 744
Lines 242517 242520 +3
Branches 45688 45682 -6
=======================================
+ Hits 218642 218649 +7
- Misses 15358 15367 +9
+ Partials 8517 8504 -13
🚀 New features to boost your workflow:
|
Summary
Fixes #64762.
When
ReplInspectorChannel.evaluate()evaluates an expression that returns a promise, it issues two inspector commands in sequence:Runtime.evaluateRuntime.awaitPromiseRuntime.evaluatealready passesobjectGroup, placing the remote promise object into the REPL's named object group. However,Runtime.awaitPromisedoes not passobjectGroup.Under GC pressure, the remote promise object can be collected before
Runtime.awaitPromisecompletes, causing the inspector to return:Instead of reporting the user's original exception, the REPL surfaces
ERR_INSPECTOR_COMMAND.Change
Pass
objectGrouptoRuntime.awaitPromiseso the remote promise object remains retained for the duration of the await.return this.postInterruptible('Runtime.awaitPromise', { __proto__: null, promiseObjectId: response.result.objectId, returnByValue: params.returnByValue, generatePreview: params.generatePreview, + objectGroup: this.objectGroup, }, breakOnSigint);Why
The Inspector protocol allows
Runtime.awaitPromiseto receive anobjectGroup. Passing the same object group used duringRuntime.evaluatekeeps the remote promise object alive until the await operation completes, preventing it from being collected while the inspector request is in progress.Testing
The existing test,
test/parallel/test-repl-pretty-stack-custom-writer.mjs, already exercises this code path. This change makes that test reliable under GC pressure, so no additional test is required.Related