Repository: netty/netty
Issue: Investigate use of HTTP/2 inbound flow control on decompressor listener
Labels: improvement

Once #3098 is cherry-picked, the DelegatingDecompressorFrameListener will have the ability to participate in application-level flow control.  It currently just opts-out by returning all of the bytes as "processed".  We should investigate whether or not it's worth while to make use of inbound flow control in this class. 


Issue comments:
@Scottmitch I created this based on your suggestion in review of #3098.  I figured it made sense to assign to you since you've done the work on the HTTP layer.

@nmittler - Thanks.  I don't think this is tied to the HTTP layer but it is related to other code I have done.  I'll put something together after your PR is resolved.

See PR https://github.com/netty/netty/pull/3144.

Cherry picked as b83f3850179ecb55dac382b7ade9d5d0ffbff375.


Pull request review comments:

Patch that resolved the issue:
diff --git a/codec-http2/src/main/java/io/netty/handler/codec/http2/CompressorHttp2ConnectionEncoder.java b/codec-http2/src/main/java/io/netty/handler/codec/http2/CompressorHttp2ConnectionEncoder.java
index 3548609eacb..5adc5e0af51 100644
--- a/codec-http2/src/main/java/io/netty/handler/codec/http2/CompressorHttp2ConnectionEncoder.java
+++ b/codec-http2/src/main/java/io/netty/handler/codec/http2/CompressorHttp2ConnectionEncoder.java
@@ -111,8 +111,8 @@ public ChannelFuture writeData(final ChannelHandlerContext ctx, final int stream
         }
 
         try {
-            // call retain here as it will call release after its written to the channel
-            channel.writeOutbound(data.retain());
+            // The channel will release the buffer after being written
+            channel.writeOutbound(data);
             ByteBuf buf = nextReadableBuf(channel);
             if (buf == null) {
                 if (endOfStream) {


Test patch:
diff --git a/codec-http2/src/test/java/io/netty/handler/codec/http2/DataCompressionHttp2Test.java b/codec-http2/src/test/java/io/netty/handler/codec/http2/DataCompressionHttp2Test.java
index 756c2559215..41921516289 100644
--- a/codec-http2/src/test/java/io/netty/handler/codec/http2/DataCompressionHttp2Test.java
+++ b/codec-http2/src/test/java/io/netty/handler/codec/http2/DataCompressionHttp2Test.java
@@ -150,7 +150,7 @@ public void gzipEncodingSingleEmptyMessage() throws Exception {
                 @Override
                 public void run() {
                     clientEncoder.writeHeaders(ctxClient(), 3, headers, 0, false, newPromiseClient());
-                    clientEncoder.writeData(ctxClient(), 3, data, 0, true, newPromiseClient());
+                    clientEncoder.writeData(ctxClient(), 3, data.retain(), 0, true, newPromiseClient());
                     ctxClient().flush();
                 }
             });
@@ -179,7 +179,7 @@ public void gzipEncodingSingleMessage() throws Exception {
                 @Override
                 public void run() {
                     clientEncoder.writeHeaders(ctxClient(), 3, headers, 0, false, newPromiseClient());
-                    clientEncoder.writeData(ctxClient(), 3, data, 0, true, newPromiseClient());
+                    clientEncoder.writeData(ctxClient(), 3, data.retain(), 0, true, newPromiseClient());
                     ctxClient().flush();
                 }
             });
@@ -210,8 +210,8 @@ public void gzipEncodingMultipleMessages() throws Exception {
                 @Override
                 public void run() {
                     clientEncoder.writeHeaders(ctxClient(), 3, headers, 0, false, newPromiseClient());
-                    clientEncoder.writeData(ctxClient(), 3, data1, 0, false, newPromiseClient());
-                    clientEncoder.writeData(ctxClient(), 3, data2, 0, true, newPromiseClient());
+                    clientEncoder.writeData(ctxClient(), 3, data1.retain(), 0, false, newPromiseClient());
+                    clientEncoder.writeData(ctxClient(), 3, data2.retain(), 0, true, newPromiseClient());
                     ctxClient().flush();
                 }
             });
@@ -258,7 +258,7 @@ public void run() {
                 public void run() {
                     clientEncoder.writeSettings(ctxClient(), settings, newPromiseClient());
                     clientEncoder.writeHeaders(ctxClient(), 3, headers, 0, false, newPromiseClient());
-                    clientEncoder.writeData(ctxClient(), 3, data, 0, true, newPromiseClient());
+                    clientEncoder.writeData(ctxClient(), 3, data.retain(), 0, true, newPromiseClient());
                     ctxClient().flush();
                 }
             });
@@ -302,7 +302,7 @@ public void run() {
                 public void run() {
                     clientEncoder.writeSettings(ctxClient(), settings, newPromiseClient());
                     clientEncoder.writeHeaders(ctxClient(), 3, headers, 0, false, newPromiseClient());
-                    clientEncoder.writeData(ctxClient(), 3, data, 0, true, newPromiseClient());
+                    clientEncoder.writeData(ctxClient(), 3, data.retain(), 0, true, newPromiseClient());
                     ctxClient().flush();
                 }
             });