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
Sorry, something went wrong.
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.objectGroup to Runtime.awaitPromise in ReplInspectorChannel.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.
Sorry, something went wrong.
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>
| // 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', { |
This comment was marked as spam.
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.
Sorry, something went wrong.
Codecov Report✅ All modified and coverable lines are covered by tests. @@ 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
... and 31 files with indirect coverage changes 🚀 New features to boost your workflow:
|
Sorry, something went wrong.
Summary
Fixes #64762.
When ReplInspectorChannel.evaluate() evaluates an expression that returns a promise, it issues two inspector commands in sequence:
Runtime.evaluate already passes objectGroup, placing the remote promise object into the REPL's named object group. However, Runtime.awaitPromise does not pass objectGroup.
Under GC pressure, the remote promise object can be collected before Runtime.awaitPromise completes, causing the inspector to return:
Instead of reporting the user's original exception, the REPL surfaces ERR_INSPECTOR_COMMAND.
Change
Pass objectGroup to Runtime.awaitPromise so 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.awaitPromise to receive an objectGroup. Passing the same object group used during Runtime.evaluate keeps 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