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]

Reply via email to