Repository navigation
playground: a streaming loop returns once a flush reports the peer gone, so a client that leaves no longer pins its reactor - #407
Merged
Conversation
…ne, so a client that leaves no longer pins its reactor The HTTP/2 and HTTP/3 response writers report a peer that has gone - a reset stream or a closed connection - as FlushResult.IsCompleted, and return it synchronously. The samples' /feed loops ignored it: once a client left, every flush returned at once and the loop kept the reactor thread forever, so no other connection on that reactor was served again (#404). Every loop that flushes in the six streaming samples now returns on IsCompleted: the /feed loops, and the echo and chunked-download loops in the same handlers, which have the same shape. The docs' panes are regenerated from the samples with scripts/gen-docs-panes.py. Checked on each /feed sample with 2 reactors: 16 h2load connections stream /feed for 3 s, then h2load exits. On main the server stays at 200% CPU and answers none of 4 later requests; with this change it drops to 0% and answers all 4.
This was referenced Oct 25, 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 #404.
The HTTP/2 and HTTP/3 response writers report a peer that has gone (a reset stream or a closed connection) as
FlushResult.IsCompleted, and return it synchronously. The samples'/feedloops ignored that result. Once a client left, every flush returned at once, so the loop never yielded and kept the reactor thread forever. No other connection on that reactor was served again.The fix: every loop that flushes in the six streaming samples returns on
IsCompleted:/feedloops inHttp2/ManagedStreamedBoth,Http2/ManagedStreamedResponse,Http2/Nghttp2Response,Http3/ManagedStreamedBothandHttp3/Nghttp3Response;Http3/Nghttp3StreamedBothhas no/feed, only those.The docs' panes are generated from the samples, so
docs/index.htmlis regenerated withscripts/gen-docs-panes.py. That covers nine loops: five/feedloops (the four the issue lists, and the HTTP/2ManagedStreamedBothone), three echo loops and one chunked download.Checked
Each
/feedsample ran with 2 reactors, pinned. 16 h2load connections streamed/feedfor 3 s, then h2load exited. Then: the server's CPU over the next 3 s, and 4 bounded requests (/, or/echowhere every other path is/feed).Http2/ManagedStreamedBothHttp2/ManagedStreamedResponseHttp2/Nghttp2ResponseHttp3/ManagedStreamedBothHttp3/Nghttp3ResponseHttp2/Nghttp2Responsewas driven with one stream per connection (-m 1). With two endless streams on one nghttp2 connection, the reactor is stuck during the load itself, on main too. That is a separate bug, filed as #409: the second stream's flush returns synchronously while the first one's drain is in flight.All six samples build. No suite runs them.
Not in this PR: the issue's other direction, a
FlushAsyncthat throws (or completes asynchronously) once it has reported the peer gone, so a handler that forgets the check can't take a reactor down. That changes the writers' behaviour for every consumer, so it's left for its own decision.Merges cleanly with #406, #393, #386, #384 and #363. Against #307 the only conflict is in
HardeningTests.cs, which #307 already has with main.API: none.
🤖 Generated with Claude Code