Repository navigation
httpclient: HandshakeTimeoutMs ends a TLS handshake whose peer accepted and went silent - #366
Merged
Merged
Conversation
…ed and went silent The handshake deadline was checked only between flights, so a TLS connect parked in RecvAsync for the server's first flight waited forever when the server accepted and never answered. Through the pool only the acquire timeout bounded it, and each such open kept _live and _opening raised, so PoolSize of them stopped the origin's pool for as long as the origin held the sockets. A direct TlsClientContext.ConnectAsync caller had no bound at all. The handshake now arms one ring timeout of HandshakeTimeoutMs on the socket's own ring. If it fires while the stream is not disposed and SSL_in_init still says the handshake is running, it shuts the socket down (SHUT_RDWR), the move the server's handshake sweep makes. The parked recv then completes, and the loop reports "TLS handshake to '<name>' did not complete within <n> ms". The between-flights check goes: the timer bounds a dribbling peer too. RingSocket gains a public Host accessor, so the stream reaches the ring from the socket it is given, and direct ConnectAsync callers get the deadline as well as the pool. The pending test "tls client: HandshakeTimeoutMs bounds a handshake whose peer accepted and went silent" is now a Test, and "tls client: HandshakeTimeoutMs bounds a direct ConnectAsync whose peer accepted and went silent" pins the path with no pool behind it.
MDA2AV
force-pushed
the
fix/344-handshake-deadline
branch
from
October 12, 2026 23:16
c929ad1 to
b0d38a3
Compare
This was referenced Oct 12, 2026
Merged
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 #344.
A TLS server that accepted the connection and then sent nothing parked the client's connect forever, and
HandshakeTimeoutMsnever fired.RunHandshakeAsyncchecked its deadline only between flights, so a connect parked inRecvAsyncfor the first flight never got back to the check. Through the pool only the acquire timeout bounded it, with no reason attached, and each parked open kept_liveand_openingraised, soPoolSizeof them stopped that origin's pool for as long as the origin held the sockets. A directTlsClientContext.ConnectAsynccaller had no bound at all.What changed
RunHandshakeAsyncnow arms one ring timeout (IRingHost.SubmitTimeout) forHandshakeTimeoutMs. When it fires, it checks that the stream is not disposed (afterDispose,SSL_freehas run) and thatSSL_in_initstill says the handshake is running. If both hold, it callsshutdown(fd, SHUT_RDWR), the same move as the server's handshake sweep. The parked recv completes, and the loop throwsTLS handshake to '<name>' did not complete within <n> ms.HandshakeDeadline, the shape of pg'sOpenDeadline(pg: a connect and its startup end at CommandTimeoutMs instead of waiting forever #318), holding the stream and anExpiredbool. The loop reads that bool after each recv to name the deadline. AMonotonicMscomparison can't do this reliably: it readsCLOCK_MONOTONIC_COARSEwhile the ring timeout runs onCLOCK_MONOTONIC, so at the moment the timeout fires the coarse clock can still be up to a tick short of the deadline. Checking the flag after every recv, not just a failed one, also means a flight that arrives after the shutdown never completes a handshake on a socket that is already shut down.RingSocketgains the approved one-linepublic IRingHost Host => _host;. The stream reaches the ring from the socket it is given, so directConnectAsynccallers get the deadline too, not just the pool.Proof
The pending test
tls client: HandshakeTimeoutMs bounds a handshake whose peer accepted and went silentis now aTest(). A new test,tls client: HandshakeTimeoutMs bounds a direct ConnectAsync whose peer accepted and went silent, does the same against the same tarpit throughTlsClientContext.ConnectAsyncwith no pool. Both useHandshakeTimeoutMs = 750.src/reverted)FAIL ... got: 599|no connection to 127.0.0.1:33375 within 4000 msPASS:599|no connection to 127.0.0.1:<port> within 4000 ms: TLS handshake to 'localhost' did not complete within 750 ms(the pool's four opens each failed at the deadline)ConnectAsyncFAIL ... Unable to read data from the transport connection: Connection timed out.(the handshake never completed; the test's 20 s read gave up)PASS:TLS handshake to 'localhost' did not complete within 750 ms, measured at 750 msThe 3 pending are the tests for #342, #343 and #340, as on main (main: 83 passed, 4 pending). The timings in the right-hand column come from a run with temporary prints, which were removed before the runs counted above.
The file's remarks listed both of its Pending defects, this one and #340's ALPN cast. The paragraph is removed rather than edited down to one bullet, because each Pending test states its own reason. #340's PR removes the same lines, so the two merge cleanly in either order.
With its siblings
This batch is #356 (#342), #358 (#343), #366 (#344) and #370 (#340). They merge with each other without conflicts, checked pairwise and all four together. #358 and #366 both edit
RunHandshakeAsync, in separate hunks. With all four merged, the Http suite is 88 passed, 0 failed, 0 pending.