Repository navigation
fix(http2): do not head-of-line block HEADERS, RST_STREAM and GOAWAY behind window-exhausted DATA - #2013
Conversation
778da70 to
f180b1d
Compare
f180b1d to
71eb4a0
Compare
71eb4a0 to
a995620
Compare
PR HealthLicense Headers ✔️
All source files should start with a license header. Unrelated files missing license headers
This check can be disabled by tagging the PR with Unused Dependencies ✔️
For details on how to fix these, see dependency_validator. This check can be disabled by tagging the PR with Breaking changes ✔️
This check can be disabled by tagging the PR with API leaks ✔️The following packages contain symbols visible in the public API, but not exported by the library. Export these symbols or remove them from your publicly visible API.
This check can be disabled by tagging the PR with
Coverage
|
| File | Coverage |
|---|---|
| pkgs/http2/lib/src/flowcontrol/connection_queues.dart | 💔 93 % ⬇️ 0 % |
This check for test coverage is informational (issues shown here will not fail the PR).
This check can be disabled by tagging the PR with skip-coverage-check.
Changelog Entry ✔️
| Package | Changed Files |
|---|
Changes to files need to be accounted for in their respective changelogs.
This check can be disabled by tagging the PR with skip-changelog-check (for example, for non-user-visible changes when the package version is already at -wip).
a995620 to
b321312
Compare
b321312 to
092b1b3
Compare
e373706 to
b5dd5d2
Compare
…behind window-exhausted DATA > [!NOTE] > This PR was generated by an AI coding agent (Jetski) on behalf of @mosuem. `ConnectionMessageQueueOut` is a strict FIFO. Once the connection-level send window is exhausted, a `DataMessage` at the head of the queue blocks *everything* behind it, including frames that are not subject to flow control (RFC 9113 Section 6.9: *"Flow control only applies to frames that are identified as being subject to flow control. Of the frame types defined in this document, this includes only DATA frames."*). In practice: while one stream was waiting for `WINDOW_UPDATE`, no new request could be opened (`HEADERS`), no stream could be cancelled (`RST_STREAM`), and the connection could not be shut down gracefully (`GOAWAY`) until the peer granted credit. - **`lib/src/flowcontrol/connection_queues.dart`**: - `_peekSendableMessage()` replaces the "first message or nothing" rule. When the window is exhausted and the head of the queue is a `DataMessage`, it returns the first `GOAWAY`, or the first `HEADERS` / `RST_STREAM` whose stream has no earlier queued message. Skipping a message of a stream blocks every later message of that stream (and, for a `PUSH_PROMISE`, of the promised stream), so per-stream frame order is preserved: trailers never overtake their own `DATA`. - The scan is linear in the queue length, and `StreamMessageQueueOut` keeps forwarding `DATA` into this queue while the connection window is exhausted (it only checks the stream window). To avoid rescanning on every `sendData()` call, `enqueueMessage` skips `_trySendMessages()` when a `DataMessage` is enqueued while `_connectionWindow.positiveWindow.wouldBuffer` is `true` (since the new `DataMessage` cannot be sent and cannot unblock any earlier queued message). Enqueueing a non-DATA message while blocked still costs one pass over the queue. Added to `test/client_test.dart`: - `headers-and-rst-not-blocked-or-reordered-on-exhausted-window`: stream 1 exhausts the connection window; stream 3's `HEADERS` and stream 5's `HEADERS` + `RST_STREAM` must still go out, in that order, before the window is replenished. - `trailers-stay-behind-blocked-data-while-other-headers-bypass`: stream 1 has blocked `DATA` *and* queued trailers; stream 3's `HEADERS` bypass them, stream 1's trailers are only written after its `DATA`. **Before fix:** ```text 00:05 +0 -1: client-tests client-errors headers-and-rst-not-blocked-or-reordered-on-exhausted-window [E] TimeoutException after 0:00:05.000000: Test timed out after 5 seconds. 00:10 +0 -2: client-tests client-errors trailers-stay-behind-blocked-data-while-other-headers-bypass [E] TimeoutException after 0:00:05.000000: Test timed out after 5 seconds. ``` (stream 3's `HEADERS` never leave the client while stream 1 waits for credit) **After fix:** ```text 00:00 +2: All tests passed! ```
b5dd5d2 to
655002d
Compare
Revisions updated by 19 package dependencies found. Moving forward revisions for: http. Commit message: [deps] rev http Revisions updated by `dart tools/rev_sdk_deps.dart http`. http (https://github.com/dart-lang/http/compare/c76dfb3..c76dfb3): c76dfb3 Wed Oct 28 10:21:55 2026 -0700 Brian Quinlan fix(http2): accept larger headers (dart-lang/http#2041) 23495b0 Mon Oct 26 15:17:28 2026 -0700 Brian Quinlan chore(cronet_http): ready 1.10.1 (dart-lang/http#2039) 27591d4 Mon Oct 26 15:08:10 2026 -0700 Brian Quinlan fix(cronet): avoid cronet namespace duplication (dart-lang/http#2034) 9cdb8da Fri Oct 23 11:14:21 2026 -0700 Kevin Moore Tighten pull_request_label workflow permissions (dart-lang/http#2035) 3998b87 Fri Oct 23 07:31:29 2026 +0200 Moritz fix(http2): do not head-of-line block HEADERS, RST_STREAM and GOAWAY behind window-exhausted DATA (dart-lang/http#2013) be2b51f Fri Oct 23 07:31:28 2026 +0200 Moritz fix(http2): Http2Client applies response-body backpressure to the HTTP/2 stream and uses 4 MiB / 16 MiB receive windows (dart-lang/http#2029) 3b504cf Thu Oct 22 06:28:12 2026 +0200 Moritz fix(http2): never write RST_STREAM ahead of the stream's still-queued HEADERS (dart-lang/http#2008) a641fac Thu Oct 22 06:28:12 2026 +0200 Moritz fix(http2): preserve the complete response on RST_STREAM(NO_ERROR) (dart-lang/http#2012) 5d896c7 Thu Oct 22 06:28:11 2026 +0200 Moritz fix(http2): keep flow-controlled end-stream DATA alive until it is flushed (`#1998`) (dart-lang/http#2007) c834aa0 Thu Oct 22 06:28:11 2026 +0200 Moritz fix(http2): ignore PRIORITY and unknown frames on streams and reject non-zero stream control frames (dart-lang/http#2006) 2d35515 Thu Oct 22 06:15:59 2026 +0200 Moritz fix(http2): replenish connection receive window when stream closes with unconsumed data (dart-lang/http#2005) 1217181 Tue Oct 20 19:17:59 2026 -0400 Mouad Debbar [http_multi_server] Prepare to release 3.2.3 (dart-lang/http#2028). http (https://github.com/dart-lang/http/compare/a8a24a0..c76dfb3): c76dfb3 Wed Oct 28 10:21:55 2026 -0700 Brian Quinlan fix(http2): accept larger headers (dart-lang/http#2041) 23495b0 Mon Oct 26 15:17:28 2026 -0700 Brian Quinlan chore(cronet_http): ready 1.10.1 (dart-lang/http#2039) 27591d4 Mon Oct 26 15:08:10 2026 -0700 Brian Quinlan fix(cronet): avoid cronet namespace duplication (dart-lang/http#2034) 9cdb8da Fri Oct 23 11:14:21 2026 -0700 Kevin Moore Tighten pull_request_label workflow permissions (dart-lang/http#2035) 3998b87 Fri Oct 23 07:31:29 2026 +0200 Moritz fix(http2): do not head-of-line block HEADERS, RST_STREAM and GOAWAY behind window-exhausted DATA (dart-lang/http#2013) be2b51f Fri Oct 23 07:31:28 2026 +0200 Moritz fix(http2): Http2Client applies response-body backpressure to the HTTP/2 stream and uses 4 MiB / 16 MiB receive windows (dart-lang/http#2029) 3b504cf Thu Oct 22 06:28:12 2026 +0200 Moritz fix(http2): never write RST_STREAM ahead of the stream's still-queued HEADERS (dart-lang/http#2008) a641fac Thu Oct 22 06:28:12 2026 +0200 Moritz fix(http2): preserve the complete response on RST_STREAM(NO_ERROR) (dart-lang/http#2012) 5d896c7 Thu Oct 22 06:28:11 2026 +0200 Moritz fix(http2): keep flow-controlled end-stream DATA alive until it is flushed () (dart-lang/http#2007) c834aa0 Thu Oct 22 06:28:11 2026 +0200 Moritz fix(http2): ignore PRIORITY and unknown frames on streams and reject non-zero stream control frames (dart-lang/http#2006) 2d35515 Thu Oct 22 06:15:59 2026 +0200 Moritz fix(http2): replenish connection receive window when stream closes with unconsumed data (dart-lang/http#2005) 1217181 Tue Oct 20 19:17:59 2026 -0400 Mouad Debbar [http_multi_server] Prepare to release 3.2.3 (dart-lang/http#2028) Change-Id: I259130f06771f4b7ba61c7610847e6677b5c8ed5 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/570641 Reviewed-by: Nate Bosch <[email protected]> Commit-Queue: Brian Quinlan <[email protected]>
Note
This PR was generated by an AI coding agent (Jetski) on behalf of @mosuem.
Summary
ConnectionMessageQueueOutis a strict FIFO. Once the connection-level send window is exhausted, aDataMessageat the head of the queue blocks everything behind it, including frames that are not subject to flow control (RFC 9113 Section 6.9: "Flow control only applies to frames that are identified as being subject to flow control. Of the frame types defined in this document, this includes only DATA frames."). In practice: while one stream was waiting forWINDOW_UPDATE, no new request could be opened (HEADERS), no stream could be cancelled (RST_STREAM), and the connection could not be shut down gracefully (GOAWAY) until the peer granted credit.Changes
lib/src/flowcontrol/connection_queues.dart:_peekSendableMessage()replaces the "first message or nothing" rule. When the window is exhausted and the head of the queue is aDataMessage, it returns the firstGOAWAY, or the firstHEADERS/RST_STREAMwhose stream has no earlier queued message. Skipping a message of a stream blocks every later message of that stream (and, for aPUSH_PROMISE, of the promised stream), so per-stream frame order is preserved: trailers never overtake their ownDATA.StreamMessageQueueOutkeeps forwardingDATAinto this queue while the connection window is exhausted (it only checks the stream window). To avoid rescanning on everysendData()call,enqueueMessageskips_trySendMessages()when aDataMessageis enqueued while_connectionWindow.positiveWindow.wouldBufferistrue(since the newDataMessagecannot be sent and cannot unblock any earlier queued message). Enqueueing a non-DATA message while blocked still costs one pass over the queue.Test Verification (Fails Before$\rightarrow$ Passes After)
Added to
test/client_test.dart:headers-and-rst-not-blocked-or-reordered-on-exhausted-window: stream 1 exhausts the connection window; stream 3'sHEADERSand stream 5'sHEADERS+RST_STREAMmust still go out, in that order, before the window is replenished.trailers-stay-behind-blocked-data-while-other-headers-bypass: stream 1 has blockedDATAand queued trailers; stream 3'sHEADERSbypass them, stream 1's trailers are only written after itsDATA.Before fix:
(stream 3's
HEADERSnever leave the client while stream 1 waits for credit)After fix: