Don't send window update frame for unconsumed bytes when stream is already closed (#9816)

Motivation:

At the moment we send a window update frame for the connection + stream when a stream is closed and there are unconsumed bytes left. While we need to do this for the connection it makes no sense to write a window update frame for the stream itself as it is already closed

Modifications:

- Don't write the window update frame for the stream when the stream is closed
- Add unit test

Result:

Don't write the window frame for closed streams

codec-http2/src/main/java/io/netty/handler/codec/http2/DefaultHttp2LocalFlowController.java

@@ -31,6 +31,7 @@ import io.netty.buffer.ByteBuf;
 import io.netty.channel.ChannelHandlerContext;
 import io.netty.handler.codec.http2.Http2Exception.CompositeStreamException;
 import io.netty.handler.codec.http2.Http2Exception.StreamException;
+import io.netty.handler.codec.http2.Http2Stream.State;
 import io.netty.util.internal.PlatformDependent;
 import io.netty.util.internal.UnstableApi;
 
@@ -448,7 +449,9 @@ public class DefaultHttp2LocalFlowController implements Http2LocalFlowController
 
         @Override
         public boolean writeWindowUpdateIfNeeded() throws Http2Exception {
-            if (endOfStream || initialStreamWindowSize <= 0) {
+            if (endOfStream || initialStreamWindowSize <= 0 ||
+                    // If the stream is already closed there is no need to try to write a window update for it.
+                    isClosed(stream)) {
                 return false;
             }
 

codec-http2/src/test/java/io/netty/handler/codec/http2/DefaultHttp2LocalFlowControllerTest.java

@@ -20,6 +20,7 @@ import static io.netty.handler.codec.http2.Http2CodecUtil.CONNECTION_STREAM_ID;
 import static io.netty.handler.codec.http2.Http2CodecUtil.DEFAULT_WINDOW_SIZE;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertNull;
 import static org.junit.Assert.assertTrue;
 import static org.mockito.Mockito.any;
 import static org.mockito.Mockito.anyInt;
@@ -34,6 +35,7 @@ import io.netty.buffer.ByteBuf;
 import io.netty.buffer.Unpooled;
 import io.netty.channel.ChannelHandlerContext;
 import io.netty.channel.ChannelPromise;
+import io.netty.handler.codec.http2.Http2Stream.State;
 import io.netty.util.concurrent.EventExecutor;
 import junit.framework.AssertionFailedError;
 import org.junit.Before;
@@ -138,6 +140,28 @@ public class DefaultHttp2LocalFlowControllerTest {
         verifyWindowUpdateNotSent(STREAM_ID);
     }
 
+    @Test
+    public void windowUpdateShouldNotBeSentAfterStreamIsClosedForUnconsumedBytes() throws Http2Exception {
+        int dataSize = (int) (DEFAULT_WINDOW_SIZE * DEFAULT_WINDOW_UPDATE_RATIO) + 1;
+
+        // Don't set end-of-stream on the frame as we want to verify that we not return the unconsumed bytes in this
+        // case once the stream was closed,
+        receiveFlowControlledFrame(STREAM_ID, dataSize, 0, false);
+        verifyWindowUpdateNotSent(CONNECTION_STREAM_ID);
+        verifyWindowUpdateNotSent(STREAM_ID);
+
+        // Close the stream
+        Http2Stream stream = connection.stream(STREAM_ID);
+        stream.close();
+        assertEquals(State.CLOSED, stream.state());
+        assertNull(connection.stream(STREAM_ID));
+
+        // The window update for the connection should made it through but not the update for the already closed
+        // stream
+        verifyWindowUpdateSent(CONNECTION_STREAM_ID, dataSize);
+        verifyWindowUpdateNotSent(STREAM_ID);
+    }
+
     @Test
     public void halfWindowRemainingShouldUpdateAllWindows() throws Http2Exception {
         int dataSize = (int) (DEFAULT_WINDOW_SIZE * DEFAULT_WINDOW_UPDATE_RATIO) + 1;