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 a310ec4649..442bc0f192 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 @@ -668,6 +668,7 @@ public class Http2ConnectionHandler extends ByteToMessageDecoder implements Http @Override public ChannelFuture goAway(final ChannelHandlerContext ctx, final int lastStreamId, final long errorCode, final ByteBuf debugData, ChannelPromise promise) { + boolean release = false; try { final Http2Connection connection = connection(); if (connection.goAwaySent() && lastStreamId > connection.remote().lastStreamKnownByPeer()) { @@ -680,6 +681,7 @@ public class Http2ConnectionHandler extends ByteToMessageDecoder implements Http // Need to retain before we write the buffer because if we do it after the refCnt could already be 0 and // result in an IllegalRefCountException. debugData.retain(); + release = true; ChannelFuture future = frameWriter().writeGoAway(ctx, lastStreamId, errorCode, debugData, promise); if (future.isDone()) { @@ -695,7 +697,9 @@ public class Http2ConnectionHandler extends ByteToMessageDecoder implements Http return future; } catch (Throwable cause) { // Make sure to catch Throwable because we are doing a retain() in this method. - debugData.release(); + if (release) { + debugData.release(); + } return promise.setFailure(cause); } } 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 0cb19bd29c..0e23e13ca8 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 @@ -50,7 +50,6 @@ import static io.netty.handler.codec.http2.Http2Stream.State.IDLE; import static io.netty.util.CharsetUtil.UTF_8; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; -import static org.junit.Assert.assertNull; import static org.junit.Assert.assertTrue; import static org.mockito.Matchers.any; import static org.mockito.Matchers.anyBoolean; @@ -419,21 +418,25 @@ public class Http2ConnectionHandlerTest { public void cannotSendGoAwayFrameWithIncreasingLastStreamIds() throws Exception { handler = newHandler(); ByteBuf data = dummyData(); - long errorCode = Http2Error.INTERNAL_ERROR.code(); + try { + long errorCode = Http2Error.INTERNAL_ERROR.code(); - handler.goAway(ctx, STREAM_ID, errorCode, data.retain(), promise); - verify(connection).goAwaySent(eq(STREAM_ID), eq(errorCode), eq(data)); - verify(frameWriter).writeGoAway(eq(ctx), eq(STREAM_ID), eq(errorCode), eq(data), eq(promise)); - // The frameWriter is only mocked, so it should not have interacted with the promise. - assertFalse(promise.isDone()); + handler.goAway(ctx, STREAM_ID, errorCode, data.retain(), promise); + verify(connection).goAwaySent(eq(STREAM_ID), eq(errorCode), eq(data)); + verify(frameWriter).writeGoAway(eq(ctx), eq(STREAM_ID), eq(errorCode), eq(data), eq(promise)); + // The frameWriter is only mocked, so it should not have interacted with the promise. + assertFalse(promise.isDone()); - when(connection.goAwaySent()).thenReturn(true); - when(remote.lastStreamKnownByPeer()).thenReturn(STREAM_ID); - handler.goAway(ctx, STREAM_ID + 2, errorCode, data, promise); - assertTrue(promise.isDone()); - assertFalse(promise.isSuccess()); - assertEquals(0, data.refCnt()); - verifyNoMoreInteractions(frameWriter); + when(connection.goAwaySent()).thenReturn(true); + when(remote.lastStreamKnownByPeer()).thenReturn(STREAM_ID); + handler.goAway(ctx, STREAM_ID + 2, errorCode, data, promise); + assertTrue(promise.isDone()); + assertFalse(promise.isSuccess()); + assertEquals(1, data.refCnt()); + verifyNoMoreInteractions(frameWriter); + } finally { + data.release(); + } } @Test