andreacavalli/netty5
1
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

Revert "Ensure we only call debugData.release() if we called debugData.retain()"

Browse Source 
This reverts commit 4beb075dd3244d940c34de07a77ab15b7555f3a4.
This commit is contained in:
Norman Maurer 2016-09-16 08:51:11 -07:00
parent 2c4a7a2539
commit 3c62a20519
2 changed files with 15 additions and 22 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
codec-http2/src/main/java/io/netty/handler/codec/http2/Http2ConnectionHandler.java
View File

@ -670,7 +670,6 @@ 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 {
promise = promise.unvoid();
final Http2Connection connection = connection();
@ -692,7 +691,6 @@ 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()) {
@ -708,9 +706,7 @@ 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.
if (release) {
debugData.release();
}
debugData.release();
return promise.setFailure(cause);
}
}

31
codec-http2/src/test/java/io/netty/handler/codec/http2/Http2ConnectionHandlerTest.java
View File

@ -53,6 +53,7 @@ import static io.netty.handler.codec.http2.Http2TestUtil.newVoidPromise;
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;
@ -425,25 +426,21 @@ public class Http2ConnectionHandlerTest {
public void cannotSendGoAwayFrameWithIncreasingLastStreamIds() throws Exception {
handler = newHandler();
ByteBuf data = dummyData();
try {
long errorCode = Http2Error.INTERNAL_ERROR.code();
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(1, data.refCnt());
verifyNoMoreInteractions(frameWriter);
} finally {
data.release();
}
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);
}
@Test