Handle SocketException while configuring an accepted socket in TcpListener - #1165
Open
follesoe wants to merge 1 commit into
Open
Handle SocketException while configuring an accepted socket in TcpListener#1165follesoe wants to merge 1 commit into
follesoe wants to merge 1 commit into
Conversation
…tener Problem: a peer that connects and immediately resets the connection can kill the whole process. When the RST lands between the proactor completing an accept and InCompleted applying socket options to the accepted socket, the option calls throw (EINVAL on macOS) on the proactor thread, where nothing catches the exception. Solution: treat setup failure as a failed accept, the way libzmq treats tune_socket failure: dispose the accepted socket, raise EventAcceptFailed and keep accepting. The guarded region includes the StreamEngine construction, whose constructor also touches the socket (send/receive buffer sizes). The monitor event reports ConnectionReset rather than mapping the raw error through ToErrorCode, which Debug.Asserts on unmapped values. Accept-side sibling of the outbound-socket fix in zeromq#1161. Adds a regression test that floods a listener with immediately-reset connects and asserts it stays alive and accepting; without the fix it crashes the test host 12/12 times on macOS arm64. Co-Authored-By: Claude Opus 4.8 <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.
Problem
A TCP peer that connects and immediately resets the connection can kill the whole process. When the RST lands between the proactor completing an accept and
TcpListener.InCompletedapplying socket options to the accepted socket, the option calls throw — on macOS asSocketException (22): Invalid argumentfromacceptedSocket.NoDelay = true:InCompletedruns on the proactor thread, where nothing catches the exception, so it terminates the process. Any bound socket is exposed to this through a flapping or rapidly reconnecting client — a test suite that hammers connect/disconnect against a bound PUB socket crashed its test host roughly 1 run in 10 on macOS arm64.This is the accept-side sibling of #1161, which handled the same macOS
EINVALbehaviour for outbound sockets inTcpConnector.Solution
Treat a failure to set up the accepted socket as a failed accept, the way libzmq's
tcp_listenertreatstune_socketfailure: dispose the accepted socket, raiseEventAcceptFailed, and keep accepting. The guarded region also covers theStreamEngineconstruction, whose constructor touches the socket too (send/receive buffer sizes) and shares the same failure window.Two deliberate choices, called out for review:
SocketExceptionbroadly, not Handle macOSEINVALwhen enablingTCP_NODELAYon outbound TCP sockets #1161'swhen (SocketErrorCode == InvalidArgument)filter. On the connector path the socket is kept afterwards, so that filter must be narrow; here the socket is being dropped on any setup failure, and a peer-reset socket does not necessarily fail withEINVALon every OS. Dropping is also safe for a hypothetically healthy socket — the peer's reconnect logic recovers — whereas an escaped exception is fatal.EventAcceptFailedreportsErrorCode.ConnectionResetinstead of mapping the caughtSocketErrorCodethroughToErrorCode(), becauseToErrorCodehitsDebug.Assert(false)on unmapped values — an assertion failure inside crash recovery would defeat the purpose. Connection reset is the effective outcome regardless of the exact errno.Proof
The new regression test floods a bound
PublisherSocketwith 400 raw TCP connects that reset immediately (SO_LINGER=0+ close), then asserts the listener still accepts and serves a real subscriber. On macOS arm64 (.NET 10):The liveness assertion keeps the test meaningful on platforms that never throw here, and would also catch the alternative failure mode where the exception is swallowed without re-arming
Accept(), which leaves the listener permanently deaf.The full NetMQ.Tests suite passes locally on net10.0 (267 passed, 4 pre-existing skips).
🤖 Generated with Claude Code