Fix socket leak (CLOSE_WAIT) in NetworkTransport when reconnection is disabled - #283
Open
skirrellyjones wants to merge 1 commit into
Conversation
… disabled When reconnectionConfig.enabled is false and the receive loop's connection terminates for any reason -- including the peer closing gracefully -- the loop finishes the message stream without calling connection.cancel(). NWConnection.state never transitions to .cancelled or .failed on its own after a peer-initiated FIN, so nothing else cleans up the connection either. The socket is left stuck in CLOSE_WAIT until the process exits. The reconnecting branches already call connection.cancel() before retrying; this adds the same call to the two non-reconnecting "give up" branches so the underlying socket is always released. Verified against a server using reconnectionConfig: .disabled for per-client connections (mattt/iMCP): before this fix, a burst of ~250 client connect/disconnect cycles left 247+ sockets in CLOSE_WAIT within 10 seconds; after the fix, 0. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
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.
Fixes #282.
Problem
When
reconnectionConfig.enabled == falseand the receive loop'sNWConnectionterminates for any reason — including the peer closinggracefully — the loop finishes the message stream but never calls
connection.cancel().NWConnection.statenever transitions to.cancelledor.failedon its own after a peer-initiated FIN, sonothing downstream (e.g. code watching
connection.stateto decide whento clean up) fires either. The socket is left stuck in
CLOSE_WAITuntil the process itself exits.
This hits hardest for servers that intentionally disable reconnection
for their per-client connections (the correct choice — a server
shouldn't try to reconnect to a client). Every client disconnect leaks
one file descriptor permanently.
Fix
Call
connection.cancel()in both non-reconnecting "give up" branchesof the receive loop, mirroring what the reconnecting branches already do
a few lines above (they call
self.connection.cancel()beforeretrying).
Testing
swift buildsucceeds.swift test --filter NetworkTransportTests— all 16 existing testspass, including "Resource Cleanup" and "Disconnect During Receive".
which sets
reconnectionConfig: .disabledfor its server-sideconnections): before this fix, a burst of ~250 client connect/disconnect
cycles left 247+ sockets in
CLOSE_WAITwithin 10 seconds and hadalready crashed the process once from file-descriptor exhaustion
(
SIGABRT, "Too many open files"). After the fix: 0 leaked sockets,and the connections that live through the burst work correctly (I
exercised a live MCP
tools/callover one and got a real responseback).