From 306a855d931f0419319b32905215d771a4ecd13a Mon Sep 17 00:00:00 2001 From: Scott Mitchell Date: Thu, 25 Apr 2019 16:26:07 -0700 Subject: [PATCH] DefaultHttp2ConnectionEncoder async SETTINGS ACK SimpleChannelPromiseAggregator promise usage Motivaiton: DefaultHttp2ConnectionEncoder uses SimpleChannelPromiseAggregator to combine two operations into a single future status. However it directly uses the SimpleChannelPromiseAggregator object instead of using the newPromise() method in one case. This may result in premature completion of the aggregated future. Modifications: - DefaultHttp2ConnectionEncoder to use SimpleChannelPromiseAggregator#newPromise() instead of directly using the SimpleChannelPromiseAggregator instance when writing the settings ACK frame Result: More correct status for the SETTING ACK frame writing when auto settings ACK is disabled. --- .../handler/codec/http2/DefaultHttp2ConnectionEncoder.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/codec-http2/src/main/java/io/netty/handler/codec/http2/DefaultHttp2ConnectionEncoder.java b/codec-http2/src/main/java/io/netty/handler/codec/http2/DefaultHttp2ConnectionEncoder.java index 29116d17c0..1c9f3dd2fa 100644 --- a/codec-http2/src/main/java/io/netty/handler/codec/http2/DefaultHttp2ConnectionEncoder.java +++ b/codec-http2/src/main/java/io/netty/handler/codec/http2/DefaultHttp2ConnectionEncoder.java @@ -291,7 +291,7 @@ public class DefaultHttp2ConnectionEncoder implements Http2ConnectionEncoder, Ht // Acknowledge receipt of the settings. We should do this before we process the settings to ensure our // remote peer applies these settings before any subsequent frames that we may send which depend upon // these new settings. See https://github.com/netty/netty/issues/6520. - frameWriter.writeSettingsAck(ctx, aggregator); + frameWriter.writeSettingsAck(ctx, aggregator.newPromise()); // We create a "new promise" to make sure that status from both the write and the application are taken into // account independently.