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

Fixing bug in HTTP/2 ping handling.

Browse Source 
Motivation:

The current HTTP/2 ping handling replies with an ack using the same
buffer as the received ping. It doesn't call retain(), however, which
causes a ReferenceCountException since the buffer ends up getting
released twice (once by the write and once by the decoder).

Modifications:

Modified AbstractHttp2ConnectionHandler to retain() the buffer. Added a
ping to Http2ConnectionRoundtripTest.stressTest() to verify the problem
and that this fixes it.

Result:

Ping should no longer cause an exception.
This commit is contained in:
nmittler 2014-07-22 08:36:17 -07:00 committed by Norman Maurer
parent eab9bcb9d3
commit e3795330bf
2 changed files with 7 additions and 2 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

3
codec-http2/src/main/java/io/netty/handler/codec/http2/AbstractHttp2ConnectionHandler.java
View File

@ -968,7 +968,8 @@ public abstract class AbstractHttp2ConnectionHandler extends ByteToMessageDecode
verifyPrefaceReceived();
// Send an ack back to the remote client.
frameWriter.writePing(ctx, ctx.newPromise(), true, data);
// Need to retain the buffer here since it will be released after the write completes.
frameWriter.writePing(ctx, ctx.newPromise(), true, data.retain());
AbstractHttp2ConnectionHandler.this.onPingRead(ctx, data);
}

6
codec-http2/src/test/java/io/netty/handler/codec/http2/Http2ConnectionRoundtripTest.java
View File

@ -60,7 +60,7 @@ public class Http2ConnectionRoundtripTest {
private Channel serverChannel;
private Channel clientChannel;
private static final int NUM_STREAMS = 1000;
private final CountDownLatch requestLatch = new CountDownLatch(NUM_STREAMS * 2);
private final CountDownLatch requestLatch = new CountDownLatch(NUM_STREAMS * 3);
@Before
public void setup() throws Exception {
@ -111,12 +111,14 @@ public class Http2ConnectionRoundtripTest {
new DefaultHttp2Headers.Builder().method("GET").scheme("https")
.authority("example.org").path("/some/path/resource2").build();
final String text = "hello world";
final String pingMsg = "12345678";
runInChannel(clientChannel, new Http2Runnable() {
@Override
public void run() {
for (int i = 0, nextStream = 3; i < NUM_STREAMS; ++i, nextStream += 2) {
http2Client.writeHeaders(
ctx(), newPromise(), nextStream, headers, 0, (short) 16, false, 0, false, false);
http2Client.writePing(ctx(), newPromise(), Unpooled.copiedBuffer(pingMsg.getBytes()));
http2Client.writeData(
ctx(), newPromise(), nextStream,
Unpooled.copiedBuffer(text.getBytes()), 0, true, true, false);
@ -128,6 +130,8 @@ public class Http2ConnectionRoundtripTest {
verify(serverObserver, times(NUM_STREAMS)).onHeadersRead(any(ChannelHandlerContext.class),
anyInt(), eq(headers), eq(0), eq((short) 16), eq(false), eq(0), eq(false),
eq(false));
verify(serverObserver, times(NUM_STREAMS)).onPingRead(any(ChannelHandlerContext.class),
eq(Unpooled.copiedBuffer(pingMsg.getBytes())));
verify(serverObserver, times(NUM_STREAMS)).onDataRead(any(ChannelHandlerContext.class),
anyInt(), eq(Unpooled.copiedBuffer(text.getBytes())), eq(0), eq(true), eq(true));
}