Skip to content

fix(client): close the socket when the browser dies before the close handshake - #639

Merged
route merged 1 commit into
mainfrom
fix-dead-browser-socket
Sep 28, 2026
Merged

route merged 1 commit into
mainfrom
fix-dead-browser-socket

Conversation

@route

@route route commented Sep 28, 2026

Copy link
Copy Markdown
Member

WebSocket#close only sends a close frame. The socket itself is closed by on_close, which runs when the browser answers with a close frame of its own — and a browser that is already dead never answers. So #quit and #restart left the socket open until GC finalized the TCPSocket: one leaked descriptor per call, silently, with nothing raised.

That is enough to raise Errno::EMFILE from Process.spawn when browsers are restarted in a loop under a low nofile limit, which is what #637 hit.

Fix is to close the socket explicitly at the end of Client#close, after the grace period the dispatch thread already gets. A healthy quit still completes the handshake through on_close inside that window, so the forced close is a no-op there; it only does work when the peer is gone.

Before

healthy #restart x5:                    fds flat at 8
SIGKILL the browser tree, #restart x6:  9, 10, 11, 12, 13, 14

After

SIGKILL the browser tree, #restart x6:  8, 8, 8, 8, 8, 8

The new spec kills the browser's process group, quits, and asserts the socket is closed. It fails on main with expected #<TCPSocket:fd 13, AF_INET, 127.0.0.1, 60322>.closed? to be truthy, got false.

Refs #637

…handshake

`WebSocket#close` only sends a close frame; the socket is closed by the driver's
`on_close`, which runs when the browser answers with a close frame of its own. A
browser that is already dead never answers, so `#quit` and `#restart` left the
socket open until GC finalized it -- one leaked descriptor per call, enough to
raise `Errno::EMFILE` from `Process.spawn` when browsers are restarted in a loop
under a low `nofile` limit.

Close the socket explicitly at the end of `Client#close`, after the existing
grace period for the dispatch thread, so a healthy quit still completes the
handshake through `on_close` and the forced close is a no-op there.
@route
route merged commit bd7d823 into main Sep 28, 2026
7 checks passed
@route
route deleted the fix-dead-browser-socket branch September 28, 2026 18:40
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.

1 participant