Repository navigation
nghttp3: a late answer to a cancelled request is dropped instead of closing the connection - #291
Merged
Merged
Conversation
…losing the connection
An async handler, buffered or streaming, that was still running when the client cancelled its
request (STOP_SENDING and RESET_STREAM, as a closed tab sends) submitted its answer after the stream
had closed. nghttp3 had already dropped the stream and returned ERR_STREAM_NOT_FOUND, and
QueueResponse treated that as a protocol failure: the next packet ended the run loop and closed the
connection, taking every other request on it down with the one the client gave up on.
QueueResponse now drops an answer whose stream nghttp3 no longer knows, as the transport already
does with ngtcp2's STREAM_NOT_FOUND on a send. That covers both completion paths and FailStreamed's
500, with no new state.
Covered by "h3: a late buffered answer to a cancelled request leaves the connection serving", its
streaming twin, and an ioxide.http3 control next to them in H3Tests. The handler answers only once a
probe connection has seen its stream close on the server, and the tests assert that it did.
Confirmed both ways: on the unmodified source both nghttp3 tests failed in 5 of 5 runs
("submit_response failed: ERR_STREAM_NOT_FOUND", the next request got -1 and the server closed the
connection) while the control passed; with the fix all three pass.
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.
An nghttp3 handler that was still running when the client cancelled its request closed the whole connection when it answered.
A client that gives up on a request sends STOP_SENDING and RESET_STREAM, as a browser does when a tab closes. nghttp3 then drops the stream. The handler, buffered or streaming, still finishes and submits its answer, and
ih3_submit_responsereturnsERR_STREAM_NOT_FOUND.QueueResponsetreated that as a protocol failure: the connection closed, taking every other request on it down with the one the client abandoned. The pure C# stack (ioxide.http3) already handles this.QueueResponsenow drops an answer whose stream nghttp3 no longer knows, the same way the transport already treats ngtcp2's STREAM_NOT_FOUND on a send. That covers both completion paths andFailStreamed's 500, with no new state. nghttp3 copies the headers and body at submit, so nothing is left behind.Tests
New, in
H3Tests:h3: a late buffered answer to a cancelled request leaves the connection servingh3: a late streaming answer to a cancelled request leaves the connection servingcontrol: a late answer to a cancelled request leaves an ioxide.http3 connection servingThe handler waits on a
RingTimerand answers only after the server has seen the stream close (a probe overridesOnStreamClosed). The tests assert that it did, so they can't pass on an answer that wasn't actually late. The client then sends another request on the same connection.All suites pass: E2E 255, Unit 63.