Repository navigation
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.
…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
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: