WebSocket client: resume a paused socket when the connection closes

[robobun]

Problem

• ws.pause(); ws.close() on a client WebSocket leaks the socket's fd for the life of the process. The close event fires (1000, wasClean). 200 of 200 on 1.4.2, canary and main.
• After the Close frame the client waits for a read to see EOF. A paused socket never reads, and uSockets takes it out of epoll on EPOLLHUP (packages/bun-usockets/src/loop.c:849). send_close_with_body (src/http_jsc/websocket_client.rs:1100) has already dispatched the close, so JS cannot call resume().
• Same on wss, through proxies, and with a deferred Close frame.

Fix

• send_close_with_body resumes the socket before it writes the Close frame. JS gets no message after a close.
• While a Close frame waits to be flushed, pause() returns false and isPaused stays false.
• The fix is in the WebSocket client because the wait is there. On TLS the client calls no shutdown, so uSockets sees no event.
• Verified: test/js/web/websocket/websocket-pause.test.ts (9 new tests, all fail on 1.4.3-canary). Also test/js/web/websocket/ and test/js/first_party/ws. Self-reviewed: 3 concerns raised, 1 addressed, 2 rejected (Notes).

Background

• pause() calls us_socket_pause. It removes the readable interest from the poll and sets is_paused.
• After the Close frame the plain-TCP client shuts both directions down. The kernel reports EPOLLHUP, the read returns 0, and the dispatcher closes the fd. The TLS client waits for the peer's FIN.
• The dispatcher defers the EOF of a paused socket so that its owner can resume() and read the tail. On EPOLLHUP it also removes the fd from epoll.

Notes

History. #40566 added pause(), resume() and isPaused in 1.4.2. The leak exists since then.

Deferred Close frame. With unsent data queued, the Close frame queues behind it and the close is dispatched when it drains. Both orders leak on 1.4.3-canary: pause(); close() and close(); pause(). In the second order C++ still holds the native client, so pause() reached the socket. The same leak occurs when a message handler calls pause() and the peer's Close is in the same read: the parse loop reaches the Close frame, echoes it and shuts the socket down while it is paused.

Trace of one pause(); close() on a debug build of main (BUN_DEBUG_WebSocketClient=1): Sending close with code 1000, clearData, the close event, and nothing more. Without pause() the same run continues with onClose (handle_close), which releases the socket's ref on the client. With the fix the paused run logs onClose too.

Results of the fd tests (4 connections per test, in the test process; the value is the set of fds that are open afterwards and were not open before):

test	1.4.3-canary	this branch (debug, ASAN)
pause(), then close() (ws)	4 fds	none
(wss)	8	none
(wss via http:// proxy)	8	none
(wss via https:// proxy)	8	none
(ws via https:// proxy)	8	none
pause() in the message handler, the peer's Close in the same read	4	none
pause(), then close() with the Close frame behind unsent data	4	none
close(), then pause() with the Close frame behind unsent data	pause() returns true, and the fd leaks	pause() returns false, none

On TLS the count is 8 because the peer's socket stays open as well: the client never answers the peer's FIN.

Why the servers are in the test process. With the origin in another process, plain ws did not leak in my runs on Linux (0 of 50). The origin answers the Close frame before the client polls again. Data that arrives after shutdown(SHUT_RD) makes Linux reset the connection (TcpExtTCPAbortOnData goes up by 1 per connection), and the dispatcher closes a socket on an error event whether it is paused or not. In one process the client handles its own EPOLLHUP before the origin can answer, which is the case from the report. A proxy in between gives the same order.

Why not uSockets. The only uSockets call on the plain path is us_socket_shutdown_read. A rule there does not reach wss or the tunnel, where the client makes no call at all. It also changes Bun.Socket#shutdown(true) on a paused socket. That owner is still attached and keeps its own IS_PAUSED flag (src/runtime/socket/socket_body.rs), so the two flags diverge. The dispatcher defers a paused socket that is already shut down on purpose (see the comment above eof_deferrable in loop.c). #42352 is the sibling case: it resumes inside us_internal_ssl_close for Bun.listen({ tls }), because that wait is inside uSockets. Here the wait is in the WebSocket client.

Other cases checked by hand, all leak on 1.4.3-canary and are clean with the fix: close(1001, "reason"), an 8 MB send backlog before close() (the server still receives every message), the peer closes first and the application calls close() later, plain ws through an http:// proxy.

Windows. The fd tests read /proc/self/fd or /dev/fd, so those 8 tests skip on Windows. The wss test observes the origin's socket and runs everywhere. On windows-x64 it times out on 1.4.3-canary and passes on a debug build of this branch.

Related PRs. #42976 documents that a paused client answers no ping and does not see the peer's close until resume(). #42978 makes isPaused read false on a socket with no connection. This PR changes no docs and does not touch those cases. It only clears the flag when a connected client refuses pause() (WebSocket::pause in WebSocket.cpp), so that a pause() that returns false leaves isPaused false in the new case too. #42352 is the sibling fix for Bun.listen({ tls }) inside uSockets.

Self-review.

• Addressed: a first version added docs that said close() and terminate() work while paused, and no test covers terminate(). The docs changes are no longer part of this PR.
• Rejected, not part of this PR: after a graceful close the TLS client still never closes its own socket. Two leaks follow from that, and neither needs pause(). ws.terminate() on wss:// through a CONNECT proxy never closes the proxy socket (20 of 20, the server keeps 20 pendingWebSockets). A wss peer that answers the Close frame and keeps TCP open pins the fd (8 of 8 after 5 s). Both reproduce on 1.4.3-canary without this change and need their own fix (a close timeout, and a close of the tunnel's socket).
• Rejected: replace the per-owner resume with one structural rule. See "Why not uSockets".

Review follow-up. The fd tests first ran a fixture in a subprocess with a 30 s per-test timeout. They now run in the test process, compare the set of open fds, and need no timeout (under 1 s each on a debug build). Two multi-line comments in src/ are one line each now.

Suites. test/js/web/websocket/ and test/js/first_party/ws on a debug build: 392 pass, 8 skip, 4 fail. The 4 failures are not from this change. websocket.test.js "should connect over https" and "should send and receive messages" need the public internet. "should connect many times over https" exceeds 5 s on a debug build here, and fails the same way on a debug build of main. websocket-proxy-close-reentrancy.test.ts timed out once in the full run and passes alone (3 of 3).

---

no test proof · iteration 0 · platform-specific test(s) that do not run on this machine, deferring to CI, which covers all platforms: test/js/web/websocket/websocket-pause.test.ts

[coderabbitai]

Warning

Review limit reached

• Run on-demand review

_{On-demand reviews are free for the next 4 days. After that, they cost $0.25 per reviewed file.}

Or wait 6 minutes for your next included review.

Check out review usage here.

View limit details

Limit details: You’ve used all 10 included reviews currently available.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Essentials

Run ID: 767b9b94-bd03-47df-9e55-71e27d88fa86

📥 Commits

Reviewing files that changed from the base of the PR and between b8eacea and 96582e7.

📒 Files selected for processing (3)

• src/http_jsc/websocket_client.rs
• src/jsc/bindings/webcore/WebSocket.cpp
• test/js/web/websocket/websocket-pause.test.ts

---

_{Comment @coderabbitai help to get the list of available commands.}

[robobun]

^{Updated 2:46 PM PT - Sep 16th, 2026}

❌ @robobun, your commit 96582e7 has 1 failures in Build #116740 (All Failures):

• test/js/bun/spawn/spawn.test.ts - code 1: stdout reader of an unref'd child and process lifetime > an idle reader stopped at the highwater … on 🐧 13 x64-asan

---

🧪   To try this PR locally:

bunx bun-pr 42974

That installs a local version of the PR into your bun-42974 executable, so you can run:

bun-42974 --bun

[robobun]

Status: ready for review. The diff is green in CI. The one red job is not from this change (see CI below).

How I reproduced it: the loop from the report (open a client WebSocket to a Bun.serve echo server in the same process, ws.pause(), ws.close(), wait for the close event, count /proc/self/fd).

build	with pause()	without
1.4.3-canary.1+c6b7fcb5b	closeEvents: 20, fdGrowth: 20	fdGrowth: 0
debug build of main	closeEvents: 20, fdGrowth: 20	fdGrowth: 0
this branch (debug, ASAN)	closeEvents: 20, fdGrowth: 0	fdGrowth: 0

The new tests: bun bd test test/js/web/websocket/websocket-pause.test.ts -t "close\(\) while paused". With USE_SYSTEM_BUN=1 9 of 9 fail. With this branch 9 of 9 pass.

CI (build 116740, finished): 180 of 181 jobs pass. test/js/web/websocket/websocket-pause.test.ts passes on every lane: Linux glibc and musl (x64, aarch64), x64-asan, Windows 2019 x64 and Windows 11 aarch64 (15 pass, 8 skip: the fd tests are POSIX only), macOS aarch64 and x64 (23 pass). The red job is test/js/bun/spawn/spawn.test.ts on debian 13 x64-asan, 4 of 4 attempts. The child's stderr there is WARNING: ptrace appears to be blocked (is seccomp enabled?). LeakSanitizer may hang. and the test expects an empty stderr. This PR does not touch spawn or that test, and I reported it as a break on main. Three other tests failed once and passed on retry (test/bake/deinitialization.test.ts, test-watch-mode-kill-signal-override.mjs, serve-http2-lifecycle.test.ts).

[robobun]

Cross-reference: #42978 changes isPaused() on the client WebSocket. It returns false when the socket has no connection (no native client, and not CONNECTING).

This PR adds a window that #42978 does not cover on main. While a Close frame waits behind unsent data, C++ still holds the native client, close() has resumed the socket, and pause() returns false. In that window m_paused can still be true, so isPaused would read true.

The PR that merges second needs one more change: isPaused() keys on m_state (OPEN or CONNECTING), with a test for that window. If this PR merges first, I will make that change in #42978.

[claude]

Nothing blocking. The comments below are optional suggestions. There is no need to push a fix for them before merging.

Beyond the inline findings, I also checked the new self.resume() in send_close_with_body for synchronous teardown: us_socket_resume can call us_internal_socket_close_raw when us_poll_change fails, which reaches handle_close, but WebSocket::close holds a RefPtr guard on this and handle_close takes outgoing_websocket, so the follow-up dispatch_abrupt_close/clear_data in send_close_with_body are no-ops rather than a double dispatch or use-after-free. The other caller (peer Close frame parsed inside the read callback) runs under the socket's own data callback, where uSockets already tolerates a close.

Extended reasoning...

Two verified findings are posted inline (per-test timeouts on the new fixture tests, and m_paused/isPaused drifting from the socket state when pause() returns false during a pending close dispatch). This note only records what else was examined: the reentrancy risk of calling resume() from send_close_with_body, traced through packages/bun-usockets/src/socket.c:863-881 (us_socket_resume closes the socket on us_poll_change failure) and src/http_jsc/websocket_client.rs (close at line 1381 holds RefPtr::from_this(this); handle_close at line 287 takes outgoing_websocket so later dispatch_abrupt_close returns early). The only behavioral consequence on that failure path is a 1006 instead of a 1000 close, which is confined to an epoll re-registration failure. Not approving because the change alters socket lifecycle in the WebSocket client and the inline findings, one of which affects user-visible isPaused state, warrant a human look.

4 verified lower-impact observations (convention, logging or cleanup points) were not posted.

[robobun]

Review follow-up, pushed in 7a1469e and 9eb4c6f:

• Per-test timeouts: removed. The fd tests now run in the test process and compare the set of open file descriptors before and after. They need no fixture and no subprocess, and each one takes under 1 s on a debug build. The fixture file is gone.
• isPaused after a pause() that returns false: behavior kept, now documented. With the guard, a deferred close behaves like a close that is flushed at once: pause() returns false and isPaused follows the call, as it already does for a closed or terminated socket. bun.d.ts and websockets.mdx now say what isPaused reports. A value that follows the outcome is an API decision for the maintainers. It also needs a rule for isPaused after close().
• Multi-line comments in src/: both are one line now.

All four review threads are answered and resolved. bun bd test test/js/web/websocket/websocket-pause.test.ts: 23 pass. With USE_SYSTEM_BUN=1 the 9 new tests fail.

[robobun]

Correction to my follow-up above, pushed in 96582e7:

• isPaused: I no longer document the old behavior. WebSocket::pause() now clears the flag when a connected client refuses the call, so a pause() that returns false during a deferred close leaves isPaused false. The test for that order asserts it. The case of a socket with no connection is WebSocket: report isPaused as false when the socket has no connection #42978, and this PR does not touch it.
• Docs: this PR changes no docs now. docs: a paused client WebSocket does not answer pings or see the close until resume() #42976 documents what a paused client does not see, at the same two places, so my sentences were a duplicate and a merge conflict.

bun bd test test/js/web/websocket/websocket-pause.test.ts: 23 pass.

[claude]

I re-reviewed the latest push and found no bugs; both points from my earlier pass (the 30 s per-test timeouts and isPaused reporting true after a refused pause()) are addressed in the current code. Since this changes socket read-interest inside the WebSocket client's close path, a human look at the lifecycle reasoning is still worthwhile.

What was reviewed:

• The resume() call in send_close_with_body (src/http_jsc/websocket_client.rs:1109): it can synchronously close the socket if us_poll_change fails, but both callers (close() and handle_data via the Close-frame echo) hold a RefPtr guard, and dispatch_abrupt_close/dispatch_close take outgoing_websocket once, so a second dispatch is a no-op.
• The new pause() early return and the C++ m_paused revert: the kind-None case still latches for a pre-open pause, and the deferred-Close test asserts pause() returns false with isPaused === false.
• The fd-leak test: a leaked socket that reuses the baseline readdirSync dirfd number is still reported (the next dirfd lands on a new number), so the check cannot pass vacuously; the held connection keeps lazily-created fds in the baseline.
• One small thing I did not flag as a bug: the ws package wrapper (src/js/thirdparty/ws.js:721) sets its own #paused = true before calling native pause(), so its isPaused will still read true in the refused-during-pending-close case while the global WebSocket reads false.

Extended reasoning...

Overview

The PR changes two native files and one test file. In src/http_jsc/websocket_client.rs, send_close_with_body now calls self.resume() after the pending-close early return and before the has_tcp() check, and pause() returns false without touching the socket when a Close frame is queued behind unsent data. In src/jsc/bindings/webcore/WebSocket.cpp, WebSocket::pause() reverts m_paused to false when a connected client (kind != None) refuses. test/js/web/websocket/websocket-pause.test.ts gains a raw-TLS origin test that awaits the origin socket's close, and a Linux/macOS-only fd-leak suite that diffs /proc/self/fd (or /dev/fd) across the existing MODES matrix plus the deferred-Close orderings.

Security risks

None specific to this change. The code path is the client's own graceful close; it does not parse new untrusted input, change TLS verification, or alter proxy credential handling. The resumed reads after close go to dispatch_data, which drops data once outgoing_websocket is None, matching the pre-existing unpaused behaviour.

Level of scrutiny

Moderate-to-high, because the fix alters poll interest inside a socket lifecycle path with refcounted teardown. I traced the re-entrancy risk of us_socket_resume (it can call us_internal_socket_close_raw when us_poll_change fails): close() and handle_data both hold RefPtr::from_this guards, and the subsequent has_tcp() false branch calls dispatch_abrupt_close, which is idempotent through outgoing_websocket.replace(None). I also checked that the other terminal paths (terminate, finalize, drop_connection_without_callback) close the socket directly and do not wait on a read, so they do not need the same resume. The C++ revert only applies when a client is attached, preserving the pre-open latch (applyPauseToConnectedClient on connect). The tests are in the correct file, use port: 0, poll with a deadline instead of sleeping, and release servers via using/finally.

Other factors

The two inline comments from my prior pass were resolved by the author with follow-up commits that actually change the code (timeouts removed in 7a1469e; m_paused revert in 96582e7), so there is no outstanding objection from me. No third-party reviewer has requested changes. The changed paths are not covered by CODEOWNERS. I could not run the test suite in this environment (no debug build present), so the claim that all nine new tests fail on the unfixed build is the author's, though the fd-diff mechanism is sound by inspection. The minor ws-package isPaused divergence is pre-existing behaviour of that wrapper and not introduced here, so I noted it without treating it as a blocker.

[robobun]

Thanks. On the ws wrapper note: agreed, and it is the wrapper's own flag (#paused in src/js/thirdparty/ws.js), which already reads true after a pause() in CLOSING on main, because the native pause() returns false there once close() has run. This PR does not change that wrapper. I left it out to keep the change to the leak and to the one new false return it adds.