Skip to content

Avoid inspector stack capture in RPC GC finalizers - #7255

Open
ruifigueira wants to merge 1 commit into
cloudflare:mainfrom
ruifigueira:rfigueira/rpc-stub-gc-crash
Open

Avoid inspector stack capture in RPC GC finalizers#7255
ruifigueira wants to merge 1 commit into
cloudflare:mainfrom
ruifigueira:rfigueira/rpc-stub-gc-crash

Conversation

@ruifigueira

Copy link
Copy Markdown

When V8 garbage-collects a leaked RPC stub, its C++ destructor logs a disposal warning. If the inspector is enabled, logging also captures the current JavaScript stack.

The garbage collection can interrupt optimized JavaScript at a point where V8 cannot safely reconstruct that stack. Attempting to capture it then triggers a V8 CHECK and crashes the process.

The fix keeps the warning but omits its inspector stack trace when emitted from RPC GC finalizers.

Skip V8 stack collection for RPC disposal warnings emitted during GC.
Add an inspector-enabled regression covering optimized string flattening and explicit disposal.
@ruifigueira
ruifigueira requested review from a team as code owners September 5, 2026 23:14
Comment thread src/workerd/io/worker.c++
params.setExecutionContextId(v8_inspector::V8ContextInfo::executionContextId(js.v8Context()));
params.setTimestamp(impl->inspectorClient->currentTimeMS());
stackTraceToCDP(js, params.initStackTrace());
if (captureStackTrace) {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of passing this parameter everywhere couldn't we just check with v8 whether JavaScript execution is disabled or whatever it's called and not capture stack trace in that case?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried automatic detection with jsg::isInGcDestructor() but it failed because the RPC finalizer runs from the GC epilogue, where that function returns false 😭

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants