From 833a4cec28689d1ae7d021697af3270b6211977e Mon Sep 17 00:00:00 2001 From: Nils Eriksson Date: Wed, 29 Jul 2026 09:38:54 +0100 Subject: [PATCH] Fix connection window credit loss in HTTP/2 backlog accounting backLogSize was incremented when a stream added a reservation to the backlog, but never reduced when allocate() granted connection window capacity to a backlogged stream. Only the full-clear branch of releaseBackLog() reset it, so after any partial allocation the aggregate stayed larger than the sum of the outstanding per-stream requests. The stale aggregate forced later releaseBackLog() calls down the partial-allocation path even when the WINDOW_UPDATE covered every outstanding request. That path discards any surplus instead of returning it to the connection flow control window, so the server's view of the window ended up permanently smaller than the credit the client had granted. Streams created afterwards could stall waiting for window capacity the client believed it had already provided, until the write timeout closed the connection with ENHANCE_YOUR_CALM. Decrement backLogSize as allocations are made so it always equals the sum of the outstanding connection allocation requests, and extend the RFC 9218 priority test to assert that surplus window update credit remains usable by a subsequent stream. Also fix a digit transposition in an existing comment (5641 -> 5461). --- .../coyote/http2/Http2UpgradeHandler.java | 4 +++ test/org/apache/coyote/http2/TestRfc9218.java | 25 ++++++++++++++++++- 2 files changed, 28 insertions(+), 1 deletion(-) diff --git a/java/org/apache/coyote/http2/Http2UpgradeHandler.java b/java/org/apache/coyote/http2/Http2UpgradeHandler.java index 4bd58bc289c4..0d7c9e8d087e 100644 --- a/java/org/apache/coyote/http2/Http2UpgradeHandler.java +++ b/java/org/apache/coyote/http2/Http2UpgradeHandler.java @@ -172,6 +172,9 @@ class Http2UpgradeHandler extends AbstractStream implements InternalHttpUpgradeH private final PingManager pingManager = getPingManager(); private volatile int newStreamsSinceLastPrune = 0; private final Set backLogStreams = new HashSet<>(); + // The sum of the connection allocations requested by the streams in backLogStreams. Any change to a stream's + // requested allocation must be mirrored here so releaseBackLog() can correctly determine whether an increment + // clears the backlog. private long backLogSize = 0; // The time at which the connection will timeout unless data arrives before // then. -1 means no timeout. @@ -1332,6 +1335,7 @@ private int allocate(AbstractStream stream, int allocation) { int allocatedThisTime = Math.min(allocation, stream.getConnectionAllocationRequested()); stream.setConnectionAllocationRequested(stream.getConnectionAllocationRequested() - allocatedThisTime); stream.setConnectionAllocationMade(stream.getConnectionAllocationMade() + allocatedThisTime); + backLogSize -= allocatedThisTime; leftToAllocate = leftToAllocate - allocatedThisTime; } diff --git a/test/org/apache/coyote/http2/TestRfc9218.java b/test/org/apache/coyote/http2/TestRfc9218.java index 9ed9540b2ea0..4734c2ff057e 100644 --- a/test/org/apache/coyote/http2/TestRfc9218.java +++ b/test/org/apache/coyote/http2/TestRfc9218.java @@ -187,7 +187,7 @@ public void testPriority() throws Exception { trace = trace.replace("21-Body-1365\n", ""); Assert.assertEquals(0, trace.length()); - // 19 - 5641 body left + // 19 - 5461 body left // 21 - 4778 body left // Add 16k to the connection window. Should fully allocate 19 and 21. @@ -200,6 +200,29 @@ public void testPriority() throws Exception { // Dump for debugging purposes ioe.printStackTrace(); } + + output.clearTrace(); + + /* + * The final window update was 6145 bytes larger than the outstanding backlog. Those bytes must remain available + * in the connection window for the next stream. + */ + sendSimpleGetRequest(23); + parser.readFrame(); + // The headers for stream 23 will always arrive. If the surplus was lost, no body will be sent and the read + // below would block for the default timeout - so use a short timeout to fail quickly. + s.setSoTimeout(1000); + parser.readFrame(); + + Assert.assertTrue(output.getTrace().contains("23-Body-6145\n")); + output.clearTrace(); + + // 2047 completes the 8192 byte response body for stream 23, confirming that exactly 6145 surplus bytes were + // available in the connection window. + sendWindowUpdate(0, 2047); + parser.readFrame(); + + Assert.assertEquals("23-Body-2047\n23-EndOfStream\n", output.getTrace()); }