diff --git a/codec-http2/src/main/java/io/netty/handler/codec/http2/Http2ConnectionHandler.java b/codec-http2/src/main/java/io/netty/handler/codec/http2/Http2ConnectionHandler.java index 5b1e4aa2ec..062bc11d87 100644 --- a/codec-http2/src/main/java/io/netty/handler/codec/http2/Http2ConnectionHandler.java +++ b/codec-http2/src/main/java/io/netty/handler/codec/http2/Http2ConnectionHandler.java @@ -15,6 +15,7 @@ package io.netty.handler.codec.http2; import static io.netty.handler.codec.http2.Http2CodecUtil.HTTP_UPGRADE_STREAM_ID; +import static io.netty.handler.codec.http2.Http2CodecUtil.PING_FRAME_PAYLOAD_LENGTH; import static io.netty.handler.codec.http2.Http2CodecUtil.connectionPrefaceBuf; import static io.netty.handler.codec.http2.Http2CodecUtil.getEmbeddedHttp2Exception; import static io.netty.handler.codec.http2.Http2Error.INTERNAL_ERROR; @@ -99,8 +100,6 @@ public class Http2ConnectionHandler extends ByteToMessageDecoder implements Http if (encoder.connection() != decoder.connection()) { throw new IllegalArgumentException("Encoder and Decoder do not share the same connection object"); } - - clientPrefaceString = clientPrefaceString(encoder.connection()); } public Http2Connection connection() { @@ -159,14 +158,21 @@ public class Http2ConnectionHandler extends ByteToMessageDecoder implements Http @Override public void handlerAdded(ChannelHandlerContext ctx) throws Exception { + clientPrefaceString = clientPrefaceString(encoder.connection()); // This handler was just added to the context. In case it was handled after // the connection became active, send the connection preface now. sendPreface(ctx); } + /** + * Releases the {@code clientPrefaceString}. Any active streams will be left in the open. + */ @Override protected void handlerRemoved0(ChannelHandlerContext ctx) throws Exception { - dispose(); + if (clientPrefaceString != null) { + clientPrefaceString.release(); + clientPrefaceString = null; + } } @Override @@ -226,10 +232,18 @@ public class Http2ConnectionHandler extends ByteToMessageDecoder implements Http @Override public void channelInactive(ChannelHandlerContext ctx) throws Exception { - ChannelFuture future = ctx.newSucceededFuture(); - final Collection streams = connection().activeStreams(); - for (Http2Stream s : streams.toArray(new Http2Stream[streams.size()])) { - closeStream(s, future); + try { + ChannelFuture future = ctx.newSucceededFuture(); + final Collection streams = connection().activeStreams(); + for (Http2Stream s : streams.toArray(new Http2Stream[streams.size()])) { + closeStream(s, future); + } + } finally { + try { + encoder().close(); + } finally { + decoder().close(); + } } super.channelInactive(ctx); } @@ -450,18 +464,6 @@ public class Http2ConnectionHandler extends ByteToMessageDecoder implements Http ChannelFutureListener.CLOSE_ON_FAILURE); } - /** - * Disposes of all resources. - */ - private void dispose() { - encoder.close(); - decoder.close(); - if (clientPrefaceString != null) { - clientPrefaceString.release(); - clientPrefaceString = null; - } - } - /** * Decodes the client connection preface string from the input buffer. * 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 ea9844451c..6c0ebd6037 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 @@ -117,22 +117,27 @@ public class Http2ConnectionHandlerTest { when(ctx.newSucceededFuture()).thenReturn(future); when(ctx.newPromise()).thenReturn(promise); when(ctx.write(any())).thenReturn(future); - - handler = newHandler(); } - private Http2ConnectionHandler newHandler() { - return new Http2ConnectionHandler(decoderBuilder, encoderBuilder); + private Http2ConnectionHandler newHandler() throws Exception { + Http2ConnectionHandler handler = new Http2ConnectionHandler(decoderBuilder, encoderBuilder); + handler.handlerAdded(ctx); + return handler; } @After public void tearDown() throws Exception { - handler.handlerRemoved(ctx); + if (handler != null) { + handler.handlerRemoved(ctx); + } } @Test public void clientShouldSendClientPrefaceStringWhenActive() throws Exception { when(connection.isServer()).thenReturn(false); + when(channel.isActive()).thenReturn(false); + handler = newHandler(); + when(channel.isActive()).thenReturn(true); handler.channelActive(ctx); verify(ctx).write(eq(connectionPrefaceBuf())); } @@ -140,6 +145,9 @@ public class Http2ConnectionHandlerTest { @Test public void serverShouldNotSendClientPrefaceStringWhenActive() throws Exception { when(connection.isServer()).thenReturn(true); + when(channel.isActive()).thenReturn(false); + handler = newHandler(); + when(channel.isActive()).thenReturn(true); handler.channelActive(ctx); verify(ctx, never()).write(eq(connectionPrefaceBuf())); } @@ -158,18 +166,21 @@ public class Http2ConnectionHandlerTest { @Test public void serverReceivingValidClientPrefaceStringShouldContinueReadingFrames() throws Exception { when(connection.isServer()).thenReturn(true); + handler = newHandler(); handler.channelRead(ctx, connectionPrefaceBuf()); verify(decoder).decodeFrame(eq(ctx), any(ByteBuf.class), Matchers.>any()); } @Test public void channelInactiveShouldCloseStreams() throws Exception { + handler = newHandler(); handler.channelInactive(ctx); verify(stream).close(); } @Test public void connectionErrorShouldStartShutdown() throws Exception { + handler = newHandler(); Http2Exception e = new Http2Exception(PROTOCOL_ERROR); when(remote.lastStreamCreated()).thenReturn(STREAM_ID); handler.exceptionCaught(ctx, e); @@ -178,4 +189,14 @@ public class Http2ConnectionHandlerTest { captor.capture(), eq(promise)); captor.getValue().release(); } + + @Test + public void encoderAndDecoderAreClosedOnChannelInactive() throws Exception { + handler = newHandler(); + handler.channelActive(ctx); + when(channel.isActive()).thenReturn(false); + handler.channelInactive(ctx); + verify(encoder).close(); + verify(decoder).close(); + } }