pjfanning opened a new pull request, #1259:
URL: https://github.com/apache/pekko-http/pull/1259
### Motivation
`totalBufferedData` feeds the connection-level flow controller, which emits
a WINDOW_UPDATE only while `outstanding + buffered` stays below half of
`incoming-connection-level-buffer-size`:
```scala
private def ifMoreThanHalfUsed(max: Int, outstanding: Int, buffered: Int):
Int = {
val totalReservedSpace = outstanding + buffered
if (totalReservedSpace < max / 2) max - totalReservedSpace else 0
}
```
It is incremented for every DATA frame received (`ReceivingData.handle`),
but was **not** decremented on the three paths that discard a non-empty buffer:
- `IncomingStreamBuffer.onRstStreamFrame` — cleared `buffer` after the peer
reset the stream;
- `IncomingStreamBuffer.onDownstreamFinish` — cleared it when the
application cancelled the entity stream;
- `CollectingIncomingData.onRstStreamFrame` — a no-op, even though the bytes
collected so far had already been counted.
A peer that sends data the handler never reads and then resets the stream
therefore ratchets `totalBufferedData` up permanently. Once the leaked total
reaches half the configured buffer size, the server stops replenishing the
connection window, it drains to zero, and **every stream on that connection
stalls**. Note the RST_STREAM throttle does not prevent this — even a
throttle-compliant reset rate leaks the counter.
### Modification
Add `IncomingStreamBuffer.discardBuffer()`, which subtracts what is still
buffered from `totalBufferedData` before clearing it, and use it on both
discard paths. Subtract the collected bytes in
`CollectingIncomingData.onRstStreamFrame` for the same reason.
### Result
Resetting or cancelling a stream releases the connection-level window its
buffered data reserved, so the connection keeps being replenished and no longer
stalls.
### Tests
- `sbt "http2-tests/testOnly
org.apache.pekko.http.impl.engine.http2.Http2ServerSpec
org.apache.pekko.http.impl.engine.http2.Http2ClientSpec
org.apache.pekko.http.impl.engine.http2.Http2ClientServerSpec"` — pass (174
tests). A new test buffers request data the handler never reads and resets the
stream six times over, with `incoming-connection-level-buffer-size` lowered to
the initial window so the effect is reached quickly, then asserts the
connection still accepts a request. Verified it fails with the fix stashed (the
peer runs out of connection window).
- `sbt http-core/mimaReportBinaryIssues` — pass (internal
`impl.engine.http2` change, no public API).
- Native `scalafmt` on the changed files — clean.
### References
None - releases buffered-data accounting when a stream is reset or cancelled
🤖 Generated with [Claude Code](https://claude.com/claude-code)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]