Fix H1 pool retry when recycled connection got close_notify after response#650
Merged
Conversation
…ponse Broaden the relaunch predicate in Http11PoolProcessing so a recycled connection that died while idle (TLS close_notify + FIN, then RST on write) maps IOException/SocketException to ConnectionCloseException when no response byte was received yet. Track RecycledConnection on Exchange so a fresh-connection failure still surfaces as 528 instead of looping. Also propagate Faulted=true through the async ReadAsync path of DisposeEventNotifierStream and broaden the cleanup branch in Http11ConnectionPool.Send so any dead-connection signal disposes the read stream.
…er write-path failure Two latent issues uncovered while running the previous fix under load. 1) Http11PoolProcessing's interim-response loop indexed the buffer with HeaderLength = -1 (the close_notify sentinel returned by GetNext) and threw ArgumentOutOfRangeException before the post-try CloseNotify branch could turn it into a relaunch. Skip the interim sniff when CloseNotify is set. 2) The recycled-and-no-response conversion only ran in the response-read try/catch. A request-write failure on a recycled connection (TCP RST already in flight) bypassed it entirely and surfaced as 528. Added the same conversion at the Http11ConnectionPool.Send catch boundary so both paths behave consistently.
This was referenced May 12, 2026
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
1 participant
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.
Summary
Production trace: HTTP/2 trace with
Http11ConnectionPool + BouncyCastleshowed the third request landing on a connection the upstream had already closed (TLS close_notify + FIN immediately after response #2, noConnection: closeheader). The pool blindly recycled the dead connection and the request surfaced as 528 instead of retrying on a fresh connection.What changed
Exchange.RecycledConnectionflag, set byHttp11ConnectionPool.Sendwhen pulling from the pool. Lets the failure path tell "stale pooled connection died on us" (safe to relaunch) apart from "fresh connection failed immediately" (genuine upstream issue, must surface as 528 instead of looping).Http11PoolProcessingresponse-read catch andHttp11ConnectionPool.Sendouter catch both convertIOException/SocketExceptiontoConnectionCloseException("Relaunch")whenRecycledConnection && ResponseHeaderStart == default. Covers both response-read and request-write failure paths.Http11PoolProcessingwas indexingbuffer.Buffer.AsSpan(0, HeaderLength)immediately afterGetNext, which crashed withArgumentOutOfRangeExceptionwhenGetNextreturned the close_notify sentinelHeaderLength = -1. Skip the sniff onCloseNotify.Http11ConnectionPool.Sendcleanup branch now disposesReadStreamand nullsexchange.ConnectionforConnectionCloseException | TlsFatalAlert | IOException | SocketException, not just the first.DisposeEventNotifierStream.ReadAsyncasync overload now setsFaulted = trueon exception, mirroring the sync overload.Reproducer
Http11PoolCloseNotifyAfterResponseTestsstands up a raw TLS listener that mimics the trace (two responses thenSslStream.ShutdownAsync+ close), pipes through a real Fluxzy proxy on both BouncyCastle and OS engines, and asserts the third request returns 200 and triggered a fresh upstream accept.