Drain in-flight streams after a graceful GOAWAY#193
Merged
Conversation
const86
marked this pull request as ready for review
July 14, 2026 05:02
const86
force-pushed
the
goaway-draining
branch
4 times, most recently
from
July 15, 2026 19:18
41d0950 to
7523894
Compare
…reams A received GOAWAY moves the connection to :closed even when NO_ERROR means streams at or below last_stream_id may still complete (RFC 9113, Section 6.8). In :closed, connection-level WINDOW_UPDATE and SETTINGS are ignored - an in-flight upload stalls once the connection window drains - and escalate to a connection error 15s after the GOAWAY (igrigorik#140), while the peer is still serving the streams. A graceful GOAWAY received while such streams exist now moves the connection to :draining: connection-level frames are processed and the drainable streams - already begun, at or below last_stream_id - run to completion, while frames that would open a new stream (HEADERS for an untracked stream, PUSH_PROMISE, PRIORITY) are discarded and new_stream keeps raising ConnectionClosed. The connection closes once the last drainable stream finishes, or immediately on an error GOAWAY. Streams the GOAWAY refused (above last_stream_id), idle streams, and undelivered push reservations do not hold the drain open. Draining only affects frames the application feeds via receive - nothing is buffered behind its back. Callers gating teardown on closed? should use closed? || draining?. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
const86
changed the title
| expect(connected_conn).to be_closing | ||
|
|
||
| # Connection-level frames keep being processed while closing. | ||
| connected_conn << f.generate(window_update_frame.merge(stream: 0, increment: 1000)) |
Collaborator
There was a problem hiding this comment.
shouldn't this also be testing that frames for stream do not raise?
Contributor
Author
There was a problem hiding this comment.
DATA rather than HEADERS or a stream WINDOW_UPDATE: the example also runs as a server, where HEADERS on the even stream id is a connection error, and the helper SETTINGS pin windows at 2³¹-1, so any increment overflows.
|
|
||
| # Response HEADERS racing the local reset must not kill the | ||
| # closing connection. | ||
| client << f.generate(headers_frame.merge(payload: Compressor.new.encode(RESPONSE_HEADERS))) |
Collaborator
There was a problem hiding this comment.
I see you're doing it there. would it be possible to test this all in the same it block?
Contributor
Author
There was a problem hiding this comment.
One read carries both responses, so the discard-mid-read case covers separate-read delivery too — nothing from the three blocks is lost.
…te specs - PRIORITY for an untracked id at or below the last stream id is skipped only when the connection is closed or closing: live connections keep accepting PRIORITY-initiated streams, while a draining connection is not wedged open nor emits phantom :stream events after a GOAWAY. - Comment why the last closing stream moves the connection to :closed. - The shared drain example also proves stream-addressed frames are processed while :closing. - One client spec block covers delivery to a tracked stream and discard of an untracked one's response HEADERS while closing, same-read. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BqCFwHgYK7fFycnw1xCJde
The comment above the untracked-PRIORITY branch quoted the RFC 7540 priority-tree rationale, which RFC 9113 deprecates and the guard below no longer honors after a GOAWAY; state the actual behavior instead. The README's connection lifecycle section now spells out the closed?/closing? contract: closed? answers "no new work" from GOAWAY on, closing? marks the draining phase, refusals surface via :goaway. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BqCFwHgYK7fFycnw1xCJde
HoneyryderChuck
approved these changes
Jul 23, 2026
Collaborator
|
thx @const86 |
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
2 participants
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.
On a graceful GOAWAY the connection moved straight to
:closed, where connection-level WINDOW_UPDATE and SETTINGS are ignored — and escalate to a connection error 15s later (#140) — while the peer keeps completing streams at or below last_stream_id (RFC 9113 Section 6.8). In-flight uploads stall on the frozen connection window, and drains longer than 15s kill every promised stream.A graceful GOAWAY received while streams are tracked now enters
:closing: connection-level frames are processed as usual, tracked streams run to completion, and the connection moves to:closedwhen the last one ends (or immediately on an error GOAWAY). HEADERS that would open a new stream and PUSH_PROMISE are discarded (after HPACK decoding, keeping compression state consistent), andnew_streamkeeps raising ConnectionClosed.closed?now answers "does this connection accept new work?" — true from GOAWAY receipt on, exactly when it turned true before this change;closing?distinguishes a still-draining connection. Streams the GOAWAY refused (or that never started) hold nothing open: callers keep tracking refusals via the:goawayevent and retry on a new connection.After a GOAWAY, a PRIORITY frame for an already-used stream id no longer resurrects a phantom stream (previously it re-activated the id and emitted
:streamwith no HEADERS behind it — while draining, such a phantom would also hold the connection in:closingforever). Live-connection PRIORITY handling is unchanged.Draining only affects frames the application feeds via
#receive— nothing is buffered behind its back (the #184 concern). Subsumes the #192 HEADERS loss while draining; #192 still applies to an already-closed connection.Validated end-to-end against golang.org/x/net/http2 graceful shutdown (h2c): session streams keep serving mid-drain, a 3 MB upload crosses the connection window while :closing, and post-GOAWAY stream opens fail fast for retry on a fresh connection.
🤖 Generated with Claude Code