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 Health
Coverage
|
| File | Coverage |
|---|---|
| pkgs/http2/lib/src/flowcontrol/connection_queues.dart | 💔 94 % ⬇️ 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.
License Headers ✔️
// Copyright (c) 2026, the Dart project authors. Please see the AUTHORS file
// for details. All rights reserved. Use of this source code is governed by a
// BSD-style license that can be found in the LICENSE file.
| Files |
|---|
| no missing headers |
All source files should start with a license header.
Unrelated files missing license headers
| Files |
|---|
| pkgs/http_multi_server/test/cert.dart |
This check can be disabled by tagging the PR with skip-license-check.
Breaking changes ✔️
| Package | Change | Current Version | New Version | Needed Version | Looking good? |
|---|---|---|---|---|---|
| http2 | Non-Breaking | 3.1.0 | 3.2.0-wip | 3.2.0-wip | ✔️ |
This check can be disabled by tagging the PR with skip-breaking-check.
Unused Dependencies ✔️
| Package | Status |
|---|---|
| http2 | ✔️ All dependencies utilized correctly. |
For details on how to fix these, see dependency_validator.
This check can be disabled by tagging the PR with skip-unused-dependencies-check.
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.
| Package | Leaked API symbol | Leaking sources |
|---|
This check can be disabled by tagging the PR with skip-leaking-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.
…behind window-exhausted DATA > [!NOTE] > This PR was generated by an AI coding agent (Jetski) on behalf of @mosuem. ### Summary `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. ### 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 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, `_nothingSendableWhileBlocked` caches a negative scan result until the queue changes in a way that could unblock something (a non-DATA message is enqueued, a message is sent, or a stream's messages are cancelled). With 50-byte `sendData()` calls against an exhausted window, 10k/20k/40k calls take 41/50/62 ms with this cache (same as before this change) versus 439/1 799/10 635 ms without it. 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'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! ```
a995620 to
b321312
Compare
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,_nothingSendableWhileBlockedcaches a negative scan result until the queue changes in a way that could unblock something (a non-DATA message is enqueued, a message is sent, or a stream's messages are cancelled). With 50-bytesendData()calls against an exhausted window, 10k/20k/40k calls take 41/50/62 ms with this cache (same as before this change) versus 439/1 799/10 635 ms without it. 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: