From 55d805e19935da161a3889d48d6abfc2b9308622 Mon Sep 17 00:00:00 2001 From: Scott Mitchell Date: Tue, 22 Sep 2015 10:41:31 -0700 Subject: [PATCH] DefaultHttp2RemoteFlowController may not write all pending bytes Motivation: DefaultHttp2RemoteFlowController attempts to write as many bytes as possible to transition the channel to not writable, and then relies on notification of channelWritabilityChange to continue writing. However the amount of bytes written by DefaultHttp2RemoteFlowController may not be the same number of bytes that is actually written to the channel due to other ChannelHandlers (SslHandler, compression, etc...) in the pipeline. This means there is a potential for the DefaultHttp2RemoteFlowController to be waiting for a channel writaiblity change event that will never come, and thus not write all queued data. Modifications: - DefaultHttp2RemoteFlowController should write pending bytes until there are no more, or until the channel is not writable. Result: DefaultHttp2RemoteFlowController will write all pending data. Fixes https://github.com/netty/netty/issues/4242 --- .../DefaultHttp2RemoteFlowController.java | 23 +++++++++++-------- 1 file changed, 14 insertions(+), 9 deletions(-) diff --git a/codec-http2/src/main/java/io/netty/handler/codec/http2/DefaultHttp2RemoteFlowController.java b/codec-http2/src/main/java/io/netty/handler/codec/http2/DefaultHttp2RemoteFlowController.java index 33b4262b00..3f7f326083 100644 --- a/codec-http2/src/main/java/io/netty/handler/codec/http2/DefaultHttp2RemoteFlowController.java +++ b/codec-http2/src/main/java/io/netty/handler/codec/http2/DefaultHttp2RemoteFlowController.java @@ -297,17 +297,22 @@ public class DefaultHttp2RemoteFlowController implements Http2RemoteFlowControll */ @Override public void writePendingBytes() throws Http2Exception { - Http2Stream connectionStream = connection.connectionStream(); - int connectionWindowSize = writableBytes(state(connectionStream).windowSize()); + AbstractState connectionState = connectionState(); + int connectionWindowSize; + do { + connectionWindowSize = writableBytes(connectionState.windowSize()); - if (connectionWindowSize > 0) { - // Allocate the bytes for the connection window to the streams, but do not write. - allocateBytesForTree(connectionStream, connectionWindowSize); - } + if (connectionWindowSize > 0) { + // Allocate the bytes for the connection window to the streams, but do not write. + allocateBytesForTree(connectionState.stream(), connectionWindowSize); + } - // Now write all of the allocated bytes, must write as there may be empty frames with - // EOS = true - connection.forEachActiveStream(WRITE_ALLOCATED_BYTES); + // Write all of allocated bytes. We must call this even if no bytes are allocated as it is possible there + // are empty frames indicating the End Of Stream. + connection.forEachActiveStream(WRITE_ALLOCATED_BYTES); + } while (connectionState.streamableBytesForTree() > 0 && + connectionWindowSize > 0 && + ctx.channel().isWritable()); } /**