Skip to content

Websockets: Wrap the ongoingAutoResponse so it gets destructed on the…#6637

Merged
erikcorry merged 2 commits intomainfrom
erikcorry/websocket-autoresponse
Apr 30, 2026
Merged

Websockets: Wrap the ongoingAutoResponse so it gets destructed on the…#6637
erikcorry merged 2 commits intomainfrom
erikcorry/websocket-autoresponse

Conversation

@erikcorry
Copy link
Copy Markdown
Contributor

… IO thread.

The WebSocket::AutoResponse struct holds a bare kj::Promise (ongoingAutoResponse) directly on the JS heap. When V8's GC collects a WebSocketPair, the destructor chain reaches this promise under DISALLOW_KJ_IO_DESTRUCTORS_SCOPE, and if the promise holds a real async node (a ForkBranch from ws.send().fork()), destroying it triggers a fatal "KJ async object being destroyed when not allowed" error (Sentry 37914510). The fix wraps the promise in kj::Maybe<IoOwn<kj::Promise>> so that destruction is deferred to the IoContext's delete queue, following the same pattern already used by outgoingMessages.

@erikcorry erikcorry requested a review from jasnell April 22, 2026 14:49
@erikcorry erikcorry requested review from a team as code owners April 22, 2026 14:49
@ask-bonk
Copy link
Copy Markdown
Contributor

ask-bonk Bot commented Apr 22, 2026

UnknownError: ProviderInitError

github run

@ask-bonk
Copy link
Copy Markdown
Contributor

ask-bonk Bot commented Apr 22, 2026

@erikcorry Bonk workflow failed. Check the logs for details.

View workflow run · To retry, trigger Bonk again.

@erikcorry erikcorry force-pushed the erikcorry/websocket-autoresponse branch from 49a94e5 to 671791a Compare April 29, 2026 13:43
… IO thread.

The WebSocket::AutoResponse struct holds a bare kj::Promise<void>
(ongoingAutoResponse) directly on the JS heap. When V8's GC collects a
WebSocketPair, the destructor chain reaches this promise under
DISALLOW_KJ_IO_DESTRUCTORS_SCOPE, and if the promise holds a real async node (a
ForkBranch from ws.send().fork()), destroying it triggers a fatal "KJ async
object being destroyed when not allowed" error (Sentry 37914510). The fix wraps
the promise in kj::Maybe<IoOwn<kj::Promise<void>>> so that destruction is
deferred to the IoContext's delete queue, following the same pattern already
used by outgoingMessages.
@erikcorry erikcorry force-pushed the erikcorry/websocket-autoresponse branch from 671791a to 8b01b21 Compare April 30, 2026 12:50
@erikcorry erikcorry merged commit a99ab15 into main Apr 30, 2026
24 checks passed
@erikcorry erikcorry deleted the erikcorry/websocket-autoresponse branch April 30, 2026 16:29
Copy link
Copy Markdown

@SuperVip93 SuperVip93 left a comment

Choose a reason for hiding this comment

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

Fbk

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.

3 participants