Repository navigation
Conversation
Member
|
What it the reason for keeping closed |
Author
|
Mostly our own code. They weren't kept forever, only for the rest of a longer-lived object's life:
Neither is a bug in ws. What made it expensive is that the sockets were The root cause is in Node.js, and I've proposed a fix there: nodejs/node#67327. If it lands, this PR only matters for the Node.js versions without it. We're also dropping the reference on our side. If you'd rather not take the copy, that's fine. |
lpinca
reviewed
Nov 8, 2026
The close reason is emitted with `'conclude'` as a view into the buffer the stream delivered, and the `WebSocket` keeps it as `_closeMessage`. So a closed `WebSocket` that is still referenced keeps that whole buffer, commonly 64 KiB, alive. Copy the reason when it is assigned to `_closeMessage`. A pooled copy would pin the `Buffer` pool the same way, since `Buffer.poolSize` is 64 KiB on recent Node.js versions. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
orgads
force-pushed
the
copy-close-reason
branch
from
November 8, 2026 07:27
da0efbc to
8dcfe56
Compare
This branch has not been deployed
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
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.
Summary
The close reason reaches
'conclude'as a view into the buffer the stream delivered, andreceiverOnConclude()stores it aswebsocket._closeMessage. A closedWebSocketthat is still referenced therefore keeps that whole buffer alive. On Node.js 24+, socket reads are carved out of a shared 64 KiB slab, so one 3-byte reason can pin 64 KiB, plus the other reads in that slab.The
'conclude'event still gets the view; only the copy that theWebSocketkeeps is compacted.The copy is unpooled because
Buffer.poolSizeis 65536 on Node.js 24.22.2, 26.12.1 and current main, so aBuffer.from()copy would pin the pool the same way. It costs one small allocation per close frame with a non-empty reason; an empty reason usesEMPTY_BUFFER(a zero-length view would still pin the buffer).This is the same pattern as #2350 (the frame mask).
Evidence
does not retain the read buffer via the close reasonchecks that the reason passed to'close'hasreason.buffer.byteLength === reason.length.Before: fails. After: passes, and the full suite has 446 passing.
'bye':Before:
ws._closeMessage.buffer.byteLengthis 65536.After: it is 3.
_closeMessage, roughly 17 MB in total.Merge Danger
Door: two-way.
Blast Radius: close frames. The reason passed to
'close'has the same contents; only its backing memory changes.🤖 Generated with Claude Code