Skip to content

[perf] Do not retain the read buffer via the close reason - #2354

Open
orgads wants to merge 1 commit into
websockets:masterfrom
orgads:copy-close-reason
Open

orgads wants to merge 1 commit into
websockets:masterfrom
orgads:copy-close-reason

Conversation

@orgads

@orgads orgads commented Nov 7, 2026 •

Copy link
Copy Markdown

Summary

The close reason reaches 'conclude' as a view into the buffer the stream delivered, and receiverOnConclude() stores it as websocket._closeMessage. A closed WebSocket that 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.

 receiverOnConclude()
-  websocket._closeMessage = reason;
+  if (reason.length === 0) {
+    websocket._closeMessage = EMPTY_BUFFER;
+  } else {
+    websocket._closeMessage = Buffer.allocUnsafeSlow(reason.length);
+    reason.copy(websocket._closeMessage);
+  }

The 'conclude' event still gets the view; only the copy that the WebSocket keeps is compacted.

The copy is unpooled because Buffer.poolSize is 65536 on Node.js 24.22.2, 26.12.1 and current main, so a Buffer.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 uses EMPTY_BUFFER (a zero-length view would still pin the buffer).

This is the same pattern as #2350 (the frame mask).

Evidence

  • New test does not retain the read buffer via the close reason checks that the reason passed to 'close' has reason.buffer.byteLength === reason.length.
    Before: fails. After: passes, and the full suite has 446 passing.
  • Over TLS on Node.js 24.22.2, after the server closes with reason 'bye':
    Before: ws._closeMessage.buffer.byteLength is 65536.
    After: it is 3.
  • In a voice server under load (800 concurrent calls, heap snapshot), about 250 closed WebSockets each held a 64 KiB slab only through _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

@lpinca

lpinca commented Nov 7, 2026

Copy link
Copy Markdown
Member

What it the reason for keeping closed WebSockets alive?

@orgads

orgads commented Nov 7, 2026

Copy link
Copy Markdown
Author

Mostly our own code. They weren't kept forever, only for the rest of a longer-lived object's life:

  • Our STT client wraps the WebSocket in a reconnecting wrapper that keeps a reference to its last socket after the final close. The wrapper belongs to the call, and the STT stream often closes before the call ends, so the closed socket stayed reachable until the call ended.
  • A separate leak, since fixed, kept ended calls alive through a keep-alive HTTPS socket, and their closed WebSockets with them.

Neither is a bug in ws. What made it expensive is that the sockets were wss://. On Node.js 24+, TLS reads are views into a shared 64 KiB slab, so a close reason of a few bytes keeps the whole slab alive, along with the reads of other connections that landed in it. Holding a closed WebSocket a bit too long should cost a small object, not 64 KiB.

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.

Comment thread lib/receiver.js Resolved
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
orgads force-pushed the copy-close-reason branch from da0efbc to 8dcfe56 Compare November 8, 2026 07:27

This branch has not been deployed

No deployments
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