diff --git a/codec-http2/src/main/java/io/netty/handler/codec/http2/AbstractHttp2ConnectionHandlerBuilder.java b/codec-http2/src/main/java/io/netty/handler/codec/http2/AbstractHttp2ConnectionHandlerBuilder.java index 9898732233..560005bdc4 100644 --- a/codec-http2/src/main/java/io/netty/handler/codec/http2/AbstractHttp2ConnectionHandlerBuilder.java +++ b/codec-http2/src/main/java/io/netty/handler/codec/http2/AbstractHttp2ConnectionHandlerBuilder.java @@ -16,6 +16,7 @@ package io.netty.handler.codec.http2; +import io.netty.channel.Channel; import io.netty.handler.codec.http2.Http2HeadersEncoder.SensitivityDetector; import io.netty.util.internal.UnstableApi; @@ -83,6 +84,7 @@ public abstract class AbstractHttp2ConnectionHandlerBuilder processGoAwayWriteResult(ctx, lastStreamId, errorCode, debugData, future1)); } + // if closeListener != null this means we have already initiated graceful closure. doGracefulShutdown will apply + // the gracefulShutdownTimeoutMillis on each invocation, however we only care to apply the timeout on the + // start of graceful shutdown. + if (errorCode == NO_ERROR.code() && closeListener == null) { + doGracefulShutdown(ctx, future, null); + } return future; } @@ -826,10 +854,10 @@ public class Http2ConnectionHandler extends ByteToMessageDecoder implements Http * Close the remote endpoint with with a {@code GO_AWAY} frame. Does not flush * immediately, this is the responsibility of the caller. */ - private ChannelFuture goAway(ChannelHandlerContext ctx, Http2Exception cause) { + private ChannelFuture goAway(ChannelHandlerContext ctx, Http2Exception cause, ChannelPromise promise) { long errorCode = cause != null ? cause.error().code() : NO_ERROR.code(); int lastKnownStream = connection().remote().lastStreamCreated(); - return goAway(ctx, lastKnownStream, errorCode, Http2CodecUtil.toByteBuf(ctx, cause), ctx.newPromise()); + return goAway(ctx, lastKnownStream, errorCode, Http2CodecUtil.toByteBuf(ctx, cause), promise); } private void processRstStreamWriteResult(ChannelHandlerContext ctx, Http2Stream stream, ChannelFuture future) { @@ -898,17 +926,23 @@ public class Http2ConnectionHandler extends ByteToMessageDecoder implements Http long timeout, TimeUnit unit) { this.ctx = ctx; this.promise = promise; - timeoutTask = ctx.executor().schedule(() -> { - ctx.close(promise); - }, timeout, unit); + timeoutTask = ctx.executor().schedule(this::doClose, timeout, unit); } @Override - public void operationComplete(ChannelFuture sentGoAwayFuture) throws Exception { + public void operationComplete(ChannelFuture sentGoAwayFuture) { if (timeoutTask != null) { timeoutTask.cancel(false); } - ctx.close(promise); + doClose(); + } + + private void doClose() { + if (promise == null) { + ctx.close(); + } else { + ctx.close(promise); + } } } } diff --git a/codec-http2/src/main/java/io/netty/handler/codec/http2/Http2ConnectionHandlerBuilder.java b/codec-http2/src/main/java/io/netty/handler/codec/http2/Http2ConnectionHandlerBuilder.java index 3e3020021c..e0b7ef82af 100644 --- a/codec-http2/src/main/java/io/netty/handler/codec/http2/Http2ConnectionHandlerBuilder.java +++ b/codec-http2/src/main/java/io/netty/handler/codec/http2/Http2ConnectionHandlerBuilder.java @@ -92,6 +92,11 @@ public final class Http2ConnectionHandlerBuilder return super.initialHuffmanDecodeCapacity(initialHuffmanDecodeCapacity); } + @Override + public Http2ConnectionHandlerBuilder decoupleCloseAndGoAway(boolean decoupleCloseAndGoAway) { + return super.decoupleCloseAndGoAway(decoupleCloseAndGoAway); + } + @Override public Http2ConnectionHandler build() { return super.build(); @@ -100,6 +105,6 @@ public final class Http2ConnectionHandlerBuilder @Override protected Http2ConnectionHandler build(Http2ConnectionDecoder decoder, Http2ConnectionEncoder encoder, Http2Settings initialSettings) { - return new Http2ConnectionHandler(decoder, encoder, initialSettings); + return new Http2ConnectionHandler(decoder, encoder, initialSettings, decoupleCloseAndGoAway()); } } diff --git a/codec-http2/src/main/java/io/netty/handler/codec/http2/Http2FrameCodec.java b/codec-http2/src/main/java/io/netty/handler/codec/http2/Http2FrameCodec.java index 2595ad2f99..3e00856d37 100644 --- a/codec-http2/src/main/java/io/netty/handler/codec/http2/Http2FrameCodec.java +++ b/codec-http2/src/main/java/io/netty/handler/codec/http2/Http2FrameCodec.java @@ -159,8 +159,9 @@ public class Http2FrameCodec extends Http2ConnectionHandler { private final IntObjectMap frameStreamToInitializeMap = new IntObjectHashMap(8); - Http2FrameCodec(Http2ConnectionEncoder encoder, Http2ConnectionDecoder decoder, Http2Settings initialSettings) { - super(decoder, encoder, initialSettings); + Http2FrameCodec(Http2ConnectionEncoder encoder, Http2ConnectionDecoder decoder, Http2Settings initialSettings, + boolean decoupleCloseAndGoAway) { + super(decoder, encoder, initialSettings, decoupleCloseAndGoAway); decoder.frameListener(new FrameListener()); connection().addListener(new ConnectionListener()); @@ -496,7 +497,7 @@ public class Http2FrameCodec extends Http2ConnectionHandler { void onHttp2UnknownStreamError(@SuppressWarnings("unused") ChannelHandlerContext ctx, Throwable cause, Http2Exception.StreamException streamException) { // Just log.... - LOG.warn("Stream exception thrown for unkown stream {}.", streamException.streamId(), cause); + LOG.warn("Stream exception thrown for unknown stream {}.", streamException.streamId(), cause); } @Override diff --git a/codec-http2/src/main/java/io/netty/handler/codec/http2/Http2FrameCodecBuilder.java b/codec-http2/src/main/java/io/netty/handler/codec/http2/Http2FrameCodecBuilder.java index 070d6300c0..b448fad171 100644 --- a/codec-http2/src/main/java/io/netty/handler/codec/http2/Http2FrameCodecBuilder.java +++ b/codec-http2/src/main/java/io/netty/handler/codec/http2/Http2FrameCodecBuilder.java @@ -31,6 +31,8 @@ public class Http2FrameCodecBuilder extends Http2FrameCodecBuilder(boolean server) { server(server); + // For backwards compatibility we should disable to timeout by default at this layer. + gracefulShutdownTimeoutMillis(0); } /** @@ -139,6 +141,11 @@ public class Http2FrameCodecBuilder extends return super.initialHuffmanDecodeCapacity(initialHuffmanDecodeCapacity); } + @Override + public Http2FrameCodecBuilder decoupleCloseAndGoAway(boolean decoupleCloseAndGoAway) { + return super.decoupleCloseAndGoAway(decoupleCloseAndGoAway); + } + /** * Build a {@link Http2FrameCodec} object. */ @@ -173,6 +180,8 @@ public class Http2FrameCodecBuilder extends @Override protected Http2FrameCodec build( Http2ConnectionDecoder decoder, Http2ConnectionEncoder encoder, Http2Settings initialSettings) { - return new Http2FrameCodec(encoder, decoder, initialSettings); + Http2FrameCodec codec = new Http2FrameCodec(encoder, decoder, initialSettings, decoupleCloseAndGoAway()); + codec.gracefulShutdownTimeoutMillis(gracefulShutdownTimeoutMillis()); + return codec; } } diff --git a/codec-http2/src/main/java/io/netty/handler/codec/http2/Http2MultiplexCodec.java b/codec-http2/src/main/java/io/netty/handler/codec/http2/Http2MultiplexCodec.java index da98a87c36..01dc4f76ea 100644 --- a/codec-http2/src/main/java/io/netty/handler/codec/http2/Http2MultiplexCodec.java +++ b/codec-http2/src/main/java/io/netty/handler/codec/http2/Http2MultiplexCodec.java @@ -155,8 +155,8 @@ public class Http2MultiplexCodec extends Http2FrameCodec { Http2ConnectionDecoder decoder, Http2Settings initialSettings, ChannelHandler inboundStreamHandler, - ChannelHandler upgradeStreamHandler) { - super(encoder, decoder, initialSettings); + ChannelHandler upgradeStreamHandler, boolean decoupleCloseAndGoAway) { + super(encoder, decoder, initialSettings, decoupleCloseAndGoAway); this.inboundStreamHandler = inboundStreamHandler; this.upgradeStreamHandler = upgradeStreamHandler; } diff --git a/codec-http2/src/main/java/io/netty/handler/codec/http2/Http2MultiplexCodecBuilder.java b/codec-http2/src/main/java/io/netty/handler/codec/http2/Http2MultiplexCodecBuilder.java index 71128e81af..33dd6d801d 100644 --- a/codec-http2/src/main/java/io/netty/handler/codec/http2/Http2MultiplexCodecBuilder.java +++ b/codec-http2/src/main/java/io/netty/handler/codec/http2/Http2MultiplexCodecBuilder.java @@ -35,6 +35,8 @@ public class Http2MultiplexCodecBuilder Http2MultiplexCodecBuilder(boolean server, ChannelHandler childHandler) { server(server); this.childHandler = checkSharable(requireNonNull(childHandler, "childHandler")); + // For backwards compatibility we should disable to timeout by default at this layer. + gracefulShutdownTimeoutMillis(0); } private static ChannelHandler checkSharable(ChannelHandler handler) { @@ -71,6 +73,14 @@ public class Http2MultiplexCodecBuilder return new Http2MultiplexCodecBuilder(true, childHandler); } + public Http2MultiplexCodecBuilder withUpgradeStreamHandler(ChannelHandler upgradeStreamHandler) { + if (this.isServer()) { + throw new IllegalArgumentException("Server codecs don't use an extra handler for the upgrade stream"); + } + this.upgradeStreamHandler = upgradeStreamHandler; + return this; + } + @Override public Http2Settings initialSettings() { return super.initialSettings(); @@ -91,14 +101,6 @@ public class Http2MultiplexCodecBuilder return super.gracefulShutdownTimeoutMillis(gracefulShutdownTimeoutMillis); } - public Http2MultiplexCodecBuilder withUpgradeStreamHandler(ChannelHandler upgradeStreamHandler) { - if (this.isServer()) { - throw new IllegalArgumentException("Server codecs don't use an extra handler for the upgrade stream"); - } - this.upgradeStreamHandler = upgradeStreamHandler; - return this; - } - @Override public boolean isServer() { return super.isServer(); @@ -170,6 +172,11 @@ public class Http2MultiplexCodecBuilder return super.autoAckSettingsFrame(autoAckSettings); } + @Override + public Http2MultiplexCodecBuilder decoupleCloseAndGoAway(boolean decoupleCloseAndGoAway) { + return super.decoupleCloseAndGoAway(decoupleCloseAndGoAway); + } + @Override public Http2MultiplexCodec build() { Http2FrameWriter frameWriter = this.frameWriter; @@ -201,6 +208,9 @@ public class Http2MultiplexCodecBuilder @Override protected Http2MultiplexCodec build( Http2ConnectionDecoder decoder, Http2ConnectionEncoder encoder, Http2Settings initialSettings) { - return new Http2MultiplexCodec(encoder, decoder, initialSettings, childHandler, upgradeStreamHandler); + Http2MultiplexCodec codec = new Http2MultiplexCodec(encoder, decoder, initialSettings, childHandler, + upgradeStreamHandler, decoupleCloseAndGoAway()); + codec.gracefulShutdownTimeoutMillis(gracefulShutdownTimeoutMillis()); + return codec; } } diff --git a/codec-http2/src/main/java/io/netty/handler/codec/http2/HttpToHttp2ConnectionHandler.java b/codec-http2/src/main/java/io/netty/handler/codec/http2/HttpToHttp2ConnectionHandler.java index bdec231204..bfaef776d2 100644 --- a/codec-http2/src/main/java/io/netty/handler/codec/http2/HttpToHttp2ConnectionHandler.java +++ b/codec-http2/src/main/java/io/netty/handler/codec/http2/HttpToHttp2ConnectionHandler.java @@ -45,6 +45,13 @@ public class HttpToHttp2ConnectionHandler extends Http2ConnectionHandler { this.validateHeaders = validateHeaders; } + protected HttpToHttp2ConnectionHandler(Http2ConnectionDecoder decoder, Http2ConnectionEncoder encoder, + Http2Settings initialSettings, boolean validateHeaders, + boolean decoupleCloseAndGoAway) { + super(decoder, encoder, initialSettings, decoupleCloseAndGoAway); + this.validateHeaders = validateHeaders; + } + /** * Get the next stream id either from the {@link HttpHeaders} object or HTTP/2 codec * diff --git a/codec-http2/src/main/java/io/netty/handler/codec/http2/HttpToHttp2ConnectionHandlerBuilder.java b/codec-http2/src/main/java/io/netty/handler/codec/http2/HttpToHttp2ConnectionHandlerBuilder.java index 877c958fea..f8f97fdcfb 100644 --- a/codec-http2/src/main/java/io/netty/handler/codec/http2/HttpToHttp2ConnectionHandlerBuilder.java +++ b/codec-http2/src/main/java/io/netty/handler/codec/http2/HttpToHttp2ConnectionHandlerBuilder.java @@ -84,6 +84,11 @@ public final class HttpToHttp2ConnectionHandlerBuilder extends return super.initialHuffmanDecodeCapacity(initialHuffmanDecodeCapacity); } + @Override + public HttpToHttp2ConnectionHandlerBuilder decoupleCloseAndGoAway(boolean decoupleCloseAndGoAway) { + return super.decoupleCloseAndGoAway(decoupleCloseAndGoAway); + } + @Override public HttpToHttp2ConnectionHandler build() { return super.build(); @@ -92,6 +97,7 @@ public final class HttpToHttp2ConnectionHandlerBuilder extends @Override protected HttpToHttp2ConnectionHandler build(Http2ConnectionDecoder decoder, Http2ConnectionEncoder encoder, Http2Settings initialSettings) { - return new HttpToHttp2ConnectionHandler(decoder, encoder, initialSettings, isValidateHeaders()); + return new HttpToHttp2ConnectionHandler(decoder, encoder, initialSettings, isValidateHeaders(), + decoupleCloseAndGoAway()); } } diff --git a/codec-http2/src/test/java/io/netty/handler/codec/http2/Http2ConnectionHandlerTest.java b/codec-http2/src/test/java/io/netty/handler/codec/http2/Http2ConnectionHandlerTest.java index 90c179fa88..fceb820da0 100644 --- a/codec-http2/src/test/java/io/netty/handler/codec/http2/Http2ConnectionHandlerTest.java +++ b/codec-http2/src/test/java/io/netty/handler/codec/http2/Http2ConnectionHandlerTest.java @@ -695,7 +695,7 @@ public class Http2ConnectionHandlerTest { final long expectedMillis = 1234; handler.gracefulShutdownTimeoutMillis(expectedMillis); handler.close(ctx, promise); - verify(executor).schedule(any(Runnable.class), eq(expectedMillis), eq(TimeUnit.MILLISECONDS)); + verify(executor, atLeastOnce()).schedule(any(Runnable.class), eq(expectedMillis), eq(TimeUnit.MILLISECONDS)); } @Test diff --git a/codec-http2/src/test/java/io/netty/handler/codec/http2/Http2FrameCodecTest.java b/codec-http2/src/test/java/io/netty/handler/codec/http2/Http2FrameCodecTest.java index fa489ed7a8..7b3ef43442 100644 --- a/codec-http2/src/test/java/io/netty/handler/codec/http2/Http2FrameCodecTest.java +++ b/codec-http2/src/test/java/io/netty/handler/codec/http2/Http2FrameCodecTest.java @@ -219,7 +219,7 @@ public class Http2FrameCodecTest { Http2Connection conn = new DefaultHttp2Connection(true); Http2ConnectionEncoder enc = new DefaultHttp2ConnectionEncoder(conn, new DefaultHttp2FrameWriter()); Http2ConnectionDecoder dec = new DefaultHttp2ConnectionDecoder(conn, enc, new DefaultHttp2FrameReader()); - Http2FrameCodec codec = new Http2FrameCodec(enc, dec, new Http2Settings()); + Http2FrameCodec codec = new Http2FrameCodec(enc, dec, new Http2Settings(), false); EmbeddedChannel em = new EmbeddedChannel(codec); // We call #consumeBytes on a stream id which has not been seen yet to emulate the case @@ -323,15 +323,15 @@ public class Http2FrameCodecTest { ByteBuf debugData = bb("debug"); ByteBuf expected = debugData.copy(); - Http2GoAwayFrame goAwayFrame = new DefaultHttp2GoAwayFrame(NO_ERROR.code(), debugData); + Http2GoAwayFrame goAwayFrame = new DefaultHttp2GoAwayFrame(NO_ERROR.code(), + debugData.retainedDuplicate()); goAwayFrame.setExtraStreamIds(2); channel.writeOutbound(goAwayFrame); verify(frameWriter).writeGoAway(eqFrameCodecCtx(), eq(7), eq(NO_ERROR.code()), eq(expected), anyChannelPromise()); - assertEquals(1, debugData.refCnt()); - assertEquals(State.OPEN, stream.state()); - assertTrue(channel.isActive()); + assertEquals(State.CLOSED, stream.state()); + assertFalse(channel.isActive()); expected.release(); debugData.release(); } @@ -386,16 +386,17 @@ public class Http2FrameCodecTest { assertEquals(State.OPEN, stream.state()); ByteBuf debugData = bb("debug"); - Http2GoAwayFrame goAwayFrame = new DefaultHttp2GoAwayFrame(NO_ERROR.code(), debugData.slice()); + Http2GoAwayFrame goAwayFrame = new DefaultHttp2GoAwayFrame(NO_ERROR.code(), + debugData.retainedDuplicate()); goAwayFrame.setExtraStreamIds(Integer.MAX_VALUE); channel.writeOutbound(goAwayFrame); // When the last stream id computation overflows, the last stream id should just be set to 2^31 - 1. verify(frameWriter).writeGoAway(eqFrameCodecCtx(), eq(Integer.MAX_VALUE), eq(NO_ERROR.code()), eq(debugData), anyChannelPromise()); - assertEquals(1, debugData.refCnt()); - assertEquals(State.OPEN, stream.state()); - assertTrue(channel.isActive()); + debugData.release(); + assertEquals(State.CLOSED, stream.state()); + assertFalse(channel.isActive()); } @Test