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
This commit is contained in:
parent
2d48ec4e3f
commit
371c431b6b
@ -32,6 +32,7 @@ import io.netty.buffer.ByteBuf;
|
|||||||
import io.netty.channel.ChannelHandlerContext;
|
import io.netty.channel.ChannelHandlerContext;
|
||||||
import io.netty.handler.codec.http2.Http2Exception.CompositeStreamException;
|
import io.netty.handler.codec.http2.Http2Exception.CompositeStreamException;
|
||||||
import io.netty.handler.codec.http2.Http2Exception.StreamException;
|
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.PlatformDependent;
|
||||||
import io.netty.util.internal.UnstableApi;
|
import io.netty.util.internal.UnstableApi;
|
||||||
|
|
||||||
@ -449,7 +450,9 @@ public class DefaultHttp2LocalFlowController implements Http2LocalFlowController
|
|||||||
|
|
||||||
@Override
|
@Override
|
||||||
public boolean writeWindowUpdateIfNeeded() throws Http2Exception {
|
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;
|
return false;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@ -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 io.netty.handler.codec.http2.Http2CodecUtil.DEFAULT_WINDOW_SIZE;
|
||||||
import static org.junit.Assert.assertEquals;
|
import static org.junit.Assert.assertEquals;
|
||||||
import static org.junit.Assert.assertFalse;
|
import static org.junit.Assert.assertFalse;
|
||||||
|
import static org.junit.Assert.assertNull;
|
||||||
import static org.junit.Assert.assertTrue;
|
import static org.junit.Assert.assertTrue;
|
||||||
import static org.mockito.Mockito.any;
|
import static org.mockito.Mockito.any;
|
||||||
import static org.mockito.Mockito.anyInt;
|
import static org.mockito.Mockito.anyInt;
|
||||||
@ -34,6 +35,7 @@ import io.netty.buffer.ByteBuf;
|
|||||||
import io.netty.buffer.Unpooled;
|
import io.netty.buffer.Unpooled;
|
||||||
import io.netty.channel.ChannelHandlerContext;
|
import io.netty.channel.ChannelHandlerContext;
|
||||||
import io.netty.channel.ChannelPromise;
|
import io.netty.channel.ChannelPromise;
|
||||||
|
import io.netty.handler.codec.http2.Http2Stream.State;
|
||||||
import io.netty.util.concurrent.EventExecutor;
|
import io.netty.util.concurrent.EventExecutor;
|
||||||
import junit.framework.AssertionFailedError;
|
import junit.framework.AssertionFailedError;
|
||||||
import org.junit.Before;
|
import org.junit.Before;
|
||||||
@ -138,6 +140,28 @@ public class DefaultHttp2LocalFlowControllerTest {
|
|||||||
verifyWindowUpdateNotSent(STREAM_ID);
|
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
|
@Test
|
||||||
public void halfWindowRemainingShouldUpdateAllWindows() throws Http2Exception {
|
public void halfWindowRemainingShouldUpdateAllWindows() throws Http2Exception {
|
||||||
int dataSize = (int) (DEFAULT_WINDOW_SIZE * DEFAULT_WINDOW_UPDATE_RATIO) + 1;
|
int dataSize = (int) (DEFAULT_WINDOW_SIZE * DEFAULT_WINDOW_UPDATE_RATIO) + 1;
|
||||||
|
Loading…
Reference in New Issue
Block a user