Repository navigation
fix(http2): replenish connection receive window when stream closes with unconsumed data - #2005
Conversation
260a474 to
8f57234
Compare
8f57234 to
4f0f2d1
Compare
PR Health
Coverage
|
| File | Coverage |
|---|---|
| pkgs/http2/lib/src/flowcontrol/connection_queues.dart | 💔 93 % ⬇️ 0 % |
| pkgs/http2/lib/src/flowcontrol/stream_queues.dart | 💚 90 % ⬆️ 1 % |
| pkgs/http2/lib/src/streams/stream_handler.dart | 💔 91 % ⬇️ 1 % |
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 (for example, for non-user-visible changes when the package version is already at -wip).
…th unconsumed data > [!NOTE] > This PR was generated by an AI coding agent (Jetski) on behalf of @mosuem. ### Summary Per **RFC 9113 Section 5.2 (Flow Control)** and **Section 6.9 (`WINDOW_UPDATE`)**, a receiver that receives a flow-controlled `DATA` frame must always account for its contribution against the connection flow-control window, even when the stream terminates abnormally or its incoming messages are discarded before the application consumes them. Previously: 1. `ConnectionMessageQueueIn.removeStreamMessageQueue(streamId)` discarded buffered `DataMessage`s in `_stream2pendingMessages[streamId]` without calling `_windowUpdateHandler.dataProcessed(ignoredBytes)`. If a stream was paused (or had not yet been listened to) when `DATA` frames arrived and was subsequently terminated (`stream.terminate()` or `RST_STREAM`), those bytes were permanently leaked from the connection-level receive window. Once 65,535 bytes (or `connectionWindowSize`) accumulated across terminated streams, all future streams on the connection deadlocked waiting for connection window credit. 2. `StreamMessageQueueIn.cancel()` only completed `_onCancelCompleter` without draining `_pendingMessages`, updating `bufferIndicator`, or running `onCloseCheck()`. Cancelling `stream.incomingMessages` while paused left `bufferIndicator.wouldBuffer == true`, stranding buffered `DATA` in `ConnectionMessageQueueIn` and keeping the stream's active slot open indefinitely. 3. `StreamHandler._terminateStream` was a no-op when `stream.state == StreamState.Closed`, so calling `stream.terminate()` on a stream whose remote end had already sent `END_STREAM` while the local listener was paused failed to clean up the stream or replenish the connection window. ### Changes - **`lib/src/flowcontrol/connection_queues.dart`**: In `ConnectionMessageQueueIn.removeStreamMessageQueue`, sum the byte lengths of any undelivered `DataMessage`s and call `_windowUpdateHandler.dataProcessed(ignoredBytes)` when `!wasTerminated`. Also terminate any remaining stream message queues in `ConnectionMessageQueueIn.onTerminated`. - **`lib/src/flowcontrol/stream_queues.dart`**: Override `StreamMessageQueueIn.cancel()` to drain `_pendingMessages`, update `bufferIndicator` so `ConnectionMessageQueueIn` flushes any held messages and replenishes the connection window, and run `onCloseCheck()`. - **`lib/src/streams/stream_handler.dart`**: Allow `_terminateStream` to clean up `StreamState.Closed` streams whose `incomingQueue` has not yet closed. ### Test Verification (Fails Before $\rightarrow$ Passes After) Added two regression tests in `test/flow_control_test.dart`, driving the raw peer from #1999: - `terminating a stream with unconsumed buffered DATA replenishes the connection receive window` - `cancelling incomingMessages on a paused stream replenishes the connection window and releases the active stream slot` Wherever a test needs to know that the client has processed the frames sent so far (or finished reacting to a cancel), it uses the peer's PING round trip (`peer.barrier()`) rather than a wall-clock sleep. **Before fix** (tests on `master`): ```text 00:05 +0 -1: terminating a stream with unconsumed buffered DATA replenishes the connection receive window [E] TimeoutException after 0:00:05.000000: Future not completed test/flow_control_test.dart 126:7 _RawPeer.until test/flow_control_test.dart 427:5 main.<fn> 00:10 +0 -2: cancelling incomingMessages on a paused stream replenishes the connection window and releases the active stream slot [E] TimeoutException after 0:00:05.000000: Future not completed test/flow_control_test.dart 126:7 _RawPeer.until test/flow_control_test.dart 469:5 main.<fn> ``` **After fix:** ```text 00:00 +9: All tests passed! ```
4f0f2d1 to
9e69975
Compare
…window-replenish # Conflicts: # pkgs/http2/CHANGELOG.md
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
Per RFC 9113 Section 5.2 (Flow Control) and Section 6.9 (
WINDOW_UPDATE), a receiver that receives a flow-controlledDATAframe must always account for its contribution against the connection flow-control window, even when the stream terminates abnormally or its incoming messages are discarded before the application consumes them.Previously:
ConnectionMessageQueueIn.removeStreamMessageQueue(streamId)discarded bufferedDataMessages in_stream2pendingMessages[streamId]without calling_windowUpdateHandler.dataProcessed(ignoredBytes). If a stream was paused (or had not yet been listened to) whenDATAframes arrived and was subsequently terminated (stream.terminate()orRST_STREAM), those bytes were permanently leaked from the connection-level receive window. Once 65,535 bytes (orconnectionWindowSize) accumulated across terminated streams, all future streams on the connection deadlocked waiting for connection window credit.StreamMessageQueueIn.cancel()only completed_onCancelCompleterwithout draining_pendingMessages, updatingbufferIndicator, or runningonCloseCheck(). Cancellingstream.incomingMessageswhile paused leftbufferIndicator.wouldBuffer == true, stranding bufferedDATAinConnectionMessageQueueInand keeping the stream's active slot open indefinitely.StreamHandler._terminateStreamwas a no-op whenstream.state == StreamState.Closed, so callingstream.terminate()on a stream whose remote end had already sentEND_STREAMwhile the local listener was paused failed to clean up the stream or replenish the connection window.Changes
lib/src/flowcontrol/connection_queues.dart: InConnectionMessageQueueIn.removeStreamMessageQueue, sum the byte lengths of any undeliveredDataMessages and call_windowUpdateHandler.dataProcessed(ignoredBytes)when!wasTerminated. Also terminate any remaining stream message queues inConnectionMessageQueueIn.onTerminated.lib/src/flowcontrol/stream_queues.dart: OverrideStreamMessageQueueIn.cancel()to drain_pendingMessages, updatebufferIndicatorsoConnectionMessageQueueInflushes any held messages and replenishes the connection window, and runonCloseCheck().lib/src/streams/stream_handler.dart: Allow_terminateStreamto clean upStreamState.Closedstreams whoseincomingQueuehas not yet closed.Test Verification (Fails Before$\rightarrow$ Passes After)
Added two regression tests in
test/flow_control_test.dart, driving the raw peer from #1999:terminating a stream with unconsumed buffered DATA replenishes the connection receive windowcancelling incomingMessages on a paused stream replenishes the connection window and releases the active stream slotWherever a test needs to know that the client has processed the frames sent so far (or finished reacting to a cancel), it uses the peer's PING round trip (
peer.barrier()) rather than a wall-clock sleep.Before fix (tests on
master):After fix: