Skip to content

Commit 764e6c3

Browse files
committed
Change HTTP/2 tests to retain rather than copy ByteBufs
Motivation: The current build is showing potential leaks in the HTTP/2 tests that use Http2TestUtil.FrameCountDown, which copies the buffers when it receives them from the decoder. The leak detecor sees this copy as the source of a leak. It would be better all around to just retain, rather than copying the buffer. This should help to lower the overall memory footprint of the tests as well as potentially getting rid of the reported "leaks". Modifications: Modified Http2TestUtil to use ByteBuf.retain() everywhere that was previously calling ByteBuf.copy(). Result: Smaller memory footprint for tests and hopefully getting rid of reported leaks.
1 parent 8bc7dee commit 764e6c3

File tree

1 file changed

+16
-15
lines changed

1 file changed

+16
-15
lines changed

codec-http2/src/test/java/io/netty/handler/codec/http2/Http2TestUtil.java

Lines changed: 16 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -85,26 +85,27 @@ private Http2TestUtil() {
8585
}
8686

8787
static class FrameAdapter extends ByteToMessageDecoder {
88-
private final boolean copyBufs;
88+
private final boolean retainBufs;
8989
private final Http2Connection connection;
9090
private final Http2FrameListener listener;
9191
private final DefaultHttp2FrameReader reader;
9292
private CountDownLatch latch;
9393

94-
FrameAdapter(Http2FrameListener listener, CountDownLatch latch, boolean copyBufs) {
95-
this(null, listener, latch, copyBufs);
94+
FrameAdapter(Http2FrameListener listener, CountDownLatch latch, boolean retainBufs) {
95+
this(null, listener, latch, retainBufs);
9696
}
9797

98-
FrameAdapter(Http2Connection connection, Http2FrameListener listener, CountDownLatch latch, boolean copyBufs) {
99-
this(connection, new DefaultHttp2FrameReader(), listener, latch, copyBufs);
98+
FrameAdapter(Http2Connection connection, Http2FrameListener listener, CountDownLatch latch,
99+
boolean retainBufs) {
100+
this(connection, new DefaultHttp2FrameReader(), listener, latch, retainBufs);
100101
}
101102

102103
FrameAdapter(Http2Connection connection, DefaultHttp2FrameReader reader, Http2FrameListener listener,
103-
CountDownLatch latch, boolean copyBufs) {
104+
CountDownLatch latch, boolean retainBufs) {
104105
this.connection = connection;
105106
this.listener = listener;
106107
this.reader = reader;
107-
this.copyBufs = copyBufs;
108+
this.retainBufs = retainBufs;
108109
latch(latch);
109110
}
110111

@@ -149,7 +150,7 @@ protected void decode(ChannelHandlerContext ctx, ByteBuf in, List<Object> out) t
149150
public void onDataRead(ChannelHandlerContext ctx, int streamId, ByteBuf data, int padding,
150151
boolean endOfStream) throws Http2Exception {
151152
Http2Stream stream = getOrCreateStream(streamId, endOfStream);
152-
listener.onDataRead(ctx, streamId, copyBufs ? data.copy() : data, padding, endOfStream);
153+
listener.onDataRead(ctx, streamId, retainBufs ? data.retain() : data, padding, endOfStream);
153154
if (endOfStream) {
154155
closeStream(stream, true);
155156
}
@@ -217,13 +218,13 @@ public void onSettingsRead(ChannelHandlerContext ctx, Http2Settings settings) th
217218

218219
@Override
219220
public void onPingRead(ChannelHandlerContext ctx, ByteBuf data) throws Http2Exception {
220-
listener.onPingRead(ctx, copyBufs ? data.copy() : data);
221+
listener.onPingRead(ctx, retainBufs ? data.retain() : data);
221222
latch.countDown();
222223
}
223224

224225
@Override
225226
public void onPingAckRead(ChannelHandlerContext ctx, ByteBuf data) throws Http2Exception {
226-
listener.onPingAckRead(ctx, copyBufs ? data.copy() : data);
227+
listener.onPingAckRead(ctx, retainBufs ? data.retain() : data);
227228
latch.countDown();
228229
}
229230

@@ -238,7 +239,7 @@ public void onPushPromiseRead(ChannelHandlerContext ctx, int streamId, int promi
238239
@Override
239240
public void onGoAwayRead(ChannelHandlerContext ctx, int lastStreamId, long errorCode, ByteBuf debugData)
240241
throws Http2Exception {
241-
listener.onGoAwayRead(ctx, lastStreamId, errorCode, copyBufs ? debugData.copy() : debugData);
242+
listener.onGoAwayRead(ctx, lastStreamId, errorCode, retainBufs ? debugData.retain() : debugData);
242243
latch.countDown();
243244
}
244245

@@ -290,7 +291,7 @@ public void dataLatch(CountDownLatch latch) {
290291
@Override
291292
public void onDataRead(ChannelHandlerContext ctx, int streamId, ByteBuf data, int padding, boolean endOfStream)
292293
throws Http2Exception {
293-
listener.onDataRead(ctx, streamId, data.copy(), padding, endOfStream);
294+
listener.onDataRead(ctx, streamId, data.retain(), padding, endOfStream);
294295
messageLatch.countDown();
295296
if (dataLatch != null) {
296297
for (int i = 0; i < data.readableBytes(); ++i) {
@@ -340,13 +341,13 @@ public void onSettingsRead(ChannelHandlerContext ctx, Http2Settings settings) th
340341

341342
@Override
342343
public void onPingRead(ChannelHandlerContext ctx, ByteBuf data) throws Http2Exception {
343-
listener.onPingRead(ctx, data.copy());
344+
listener.onPingRead(ctx, data.retain());
344345
messageLatch.countDown();
345346
}
346347

347348
@Override
348349
public void onPingAckRead(ChannelHandlerContext ctx, ByteBuf data) throws Http2Exception {
349-
listener.onPingAckRead(ctx, data.copy());
350+
listener.onPingAckRead(ctx, data.retain());
350351
messageLatch.countDown();
351352
}
352353

@@ -360,7 +361,7 @@ public void onPushPromiseRead(ChannelHandlerContext ctx, int streamId, int promi
360361
@Override
361362
public void onGoAwayRead(ChannelHandlerContext ctx, int lastStreamId, long errorCode, ByteBuf debugData)
362363
throws Http2Exception {
363-
listener.onGoAwayRead(ctx, lastStreamId, errorCode, debugData.copy());
364+
listener.onGoAwayRead(ctx, lastStreamId, errorCode, debugData.retain());
364365
messageLatch.countDown();
365366
}
366367

0 commit comments

Comments
 (0)