fix(nghttp3): a streamed handler that awaits anything no longer hangs - #178
Merged
Conversation
Nghttp3ResponseWriter.FlushAsync parked on a pass waiter, and pass waiters are released only by DrainStreamed - a loop that runs while the peer is sending. A peer waiting for our response sends nothing, so a handler that resumed from outside that loop waited for a pass its own output was supposed to provoke. Which is every real handler. The existing streamed tests, the samples and the benchmarks all write their chunks in a tight loop, so the handler never leaves the pass that dispatched it and the drain resumes it inline. Await a file read, a query or an upstream first and you come back somewhere else: the first chunk stalls with the bytes sitting in nghttp3, and the second hangs outright waiting for the first to be taken. DrainStreamed now records that it is on the stack, and a flush that finds no drain running drives one itself instead of parking. Inside the pass nothing changes - the loop still resumes writers inline and a one-chunk response still pays no reactor round trip, which was the point of the original design. The regression test awaits between chunks, which no existing test does. Without the fix it reports "expected [chunkchunkchunkchunk], got []" - the same empty body a static file server hit when it read the file off io_uring mid-response. Also bumps all eleven packages to 0.4.178, since the fix is only useful to anyone consuming it from NuGet. Unit 36, Chaos 47, Http 38, E2E 47.
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.
A streamed nghttp3 response whose handler
awaits anything - a file read, a query, an upstream - never reached the client. First chunk stalled, second hung.What was wrong
Nghttp3ResponseWriter.FlushAsyncparks on a pass waiter, and pass waiters are released only byDrainStreamed- a loop that runs while the peer is sending. A peer waiting for our response sends nothing, so the handler waited for a pass that its own output was supposed to provoke.Two symptoms, depending on chunk count: the first chunk stalls (promoted, resumed, and nothing pumps it), and the second hangs outright in the
while (_inFlightLength > 0)loop waiting for the first to be taken.Why it survived this long
Every existing exercise of that writer returns a fixed, in-memory response with no
awaitbetween chunks - the streamed tests, the Playground samples, the benchmarks, HttpArena. A handler that never leaves the pass gets resumed inline byDrainStreamed, exactly as designed. Nothing had tried the combination that every production handler has.The fix
DrainStreamedrecords that it is on the stack. A flush that finds no drain running drives one itself instead of parking.Inside the pass nothing changes. The loop still resumes writers inline, and a one-chunk response still pays no reactor round trip - that was the point of the original design and the comment in
CompleteAsyncexplaining it.The test
The regression test
awaits between chunks, which no existing test does. Disabling only the outside-pass escape:An empty body - the same thing a static file server got when it read the file off io_uring mid-response, which is how this was found.
Scope
ioxide.nghttp3only. The other three streamed writers were checked and are fine, because of what each waits on:ioxide.nghttp3ioxide.http3ioxide.http2ioxide.nghttp2Buffered responses were never affected, in any module.
Release
0.4.178 across all eleven packages - the fix is only useful to someone consuming it from NuGet.
Unit 36, Chaos 47, Http 38, E2E 47 - all 0 failed, 0 skipped.