From b3d8c81557315ac5b5d4b381fe7461ce276a3f9e Mon Sep 17 00:00:00 2001 From: Norman Maurer Date: Fri, 6 Dec 2013 12:40:11 +0100 Subject: [PATCH] Fix all leaks reported during tests - One notable leak is from WebSocketFrameAggregator - All other leaks are from tests --- .../io/netty/buffer/AbstractByteBufTest.java | 2 +- .../buffer/AbstractCompositeByteBufTest.java | 74 ++++++++++--------- .../io/netty/buffer/ByteBufProcessorTest.java | 7 +- .../multipart/HttpPostRequestDecoder.java | 2 +- .../websocketx/WebSocketFrameAggregator.java | 6 +- .../codec/http/HttpClientCodecTest.java | 46 +++++++++++- .../codec/http/HttpContentEncoderTest.java | 8 +- .../codec/http/HttpObjectAggregatorTest.java | 9 ++- .../multipart/HttpPostRequestDecoderTest.java | 23 +++--- .../WebSocketFrameAggregatorTest.java | 35 +++++++-- .../WebSocketServerHandshaker00Test.java | 6 +- .../WebSocketServerHandshaker08Test.java | 5 +- .../WebSocketServerHandshaker13Test.java | 5 +- .../WebSocketServerProtocolHandlerTest.java | 12 +-- .../codec/ByteToMessageDecoderTest.java | 4 + .../codec/DelimiterBasedFrameDecoderTest.java | 29 +++++--- .../LengthFieldBasedFrameDecoderTest.java | 6 +- .../codec/LineBasedFrameDecoderTest.java | 24 ++++-- .../codec/ReplayingDecoderBufferTest.java | 11 +-- .../handler/codec/ReplayingDecoderTest.java | 14 +++- .../compression/SnappyFramedDecoderTest.java | 5 +- .../compression/SnappyFramedEncoderTest.java | 7 +- .../compression/SnappyIntegrationTest.java | 18 +++++ .../handler/codec/compression/ZlibTest.java | 8 ++ .../frame/DelimiterBasedFrameDecoderTest.java | 5 +- .../LengthFieldBasedFrameDecoderTest.java | 5 +- .../ProtobufVarint32FrameDecoderTest.java | 7 +- ...tobufVarint32LengthFieldPrependerTest.java | 5 +- .../codec/string/StringEncoderTest.java | 1 + .../stream/ChunkedWriteHandlerTest.java | 6 +- .../socket/SocketBufReleaseTest.java | 7 ++ .../channel/ChannelOutboundBufferTest.java | 24 +++--- 32 files changed, 294 insertions(+), 132 deletions(-) diff --git a/buffer/src/test/java/io/netty/buffer/AbstractByteBufTest.java b/buffer/src/test/java/io/netty/buffer/AbstractByteBufTest.java index 74f420a040..60c31c748e 100644 --- a/buffer/src/test/java/io/netty/buffer/AbstractByteBufTest.java +++ b/buffer/src/test/java/io/netty/buffer/AbstractByteBufTest.java @@ -1532,7 +1532,7 @@ public abstract class AbstractByteBufTest { @Test public void testToString() { buffer.clear(); - buffer.writeBytes(copiedBuffer("Hello, World!", CharsetUtil.ISO_8859_1)); + buffer.writeBytes(releaseLater(copiedBuffer("Hello, World!", CharsetUtil.ISO_8859_1))); assertEquals("Hello, World!", buffer.toString(CharsetUtil.ISO_8859_1)); } diff --git a/buffer/src/test/java/io/netty/buffer/AbstractCompositeByteBufTest.java b/buffer/src/test/java/io/netty/buffer/AbstractCompositeByteBufTest.java index 678ec01902..a943f1f05d 100644 --- a/buffer/src/test/java/io/netty/buffer/AbstractCompositeByteBufTest.java +++ b/buffer/src/test/java/io/netty/buffer/AbstractCompositeByteBufTest.java @@ -101,8 +101,8 @@ public abstract class AbstractCompositeByteBufTest extends AbstractByteBufTest { */ @Test public void testComponentAtOffset() { - CompositeByteBuf buf = (CompositeByteBuf) wrappedBuffer(new byte[]{1, 2, 3, 4, 5}, - new byte[]{4, 5, 6, 7, 8, 9, 26}); + CompositeByteBuf buf = releaseLater((CompositeByteBuf) wrappedBuffer(new byte[]{1, 2, 3, 4, 5}, + new byte[]{4, 5, 6, 7, 8, 9, 26})); //Ensure that a random place will be fine assertEquals(5, buf.componentAtOffset(2).capacity()); @@ -161,7 +161,7 @@ public abstract class AbstractCompositeByteBufTest extends AbstractByteBufTest { @Test public void testAutoConsolidation() { - CompositeByteBuf buf = compositeBuffer(2); + CompositeByteBuf buf = releaseLater(compositeBuffer(2)); buf.addComponent(wrappedBuffer(new byte[] { 1 })); assertEquals(1, buf.numComponents()); @@ -179,7 +179,7 @@ public abstract class AbstractCompositeByteBufTest extends AbstractByteBufTest { @Test public void testCompositeToSingleBuffer() { - CompositeByteBuf buf = compositeBuffer(3); + CompositeByteBuf buf = releaseLater(compositeBuffer(3)); buf.addComponent(wrappedBuffer(new byte[] {1, 2, 3})); assertEquals(1, buf.numComponents()); @@ -229,13 +229,13 @@ public abstract class AbstractCompositeByteBufTest extends AbstractByteBufTest { @Test public void testCompositeWrappedBuffer() { - ByteBuf header = buffer(12).order(order); - ByteBuf payload = buffer(512).order(order); + ByteBuf header = releaseLater(buffer(12)).order(order); + ByteBuf payload = releaseLater(buffer(512)).order(order); header.writeBytes(new byte[12]); payload.writeBytes(new byte[512]); - ByteBuf buffer = wrappedBuffer(header, payload); + ByteBuf buffer = releaseLater(wrappedBuffer(header, payload)); assertEquals(12, header.readableBytes()); assertEquals(512, payload.readableBytes()); @@ -362,77 +362,81 @@ public abstract class AbstractCompositeByteBufTest extends AbstractByteBufTest { //XXX Same tests than testEquals with written AggregateChannelBuffers ByteBuf a, b; // Different length. - a = wrappedBuffer(new byte[] { 1 }).order(order); - b = wrappedBuffer(wrappedBuffer(new byte[] { 1 }, new byte[1]).order(order)); + a = releaseLater(wrappedBuffer(new byte[] { 1 })).order(order); + b = releaseLater(wrappedBuffer(wrappedBuffer(new byte[] { 1 }, new byte[1])).order(order)); // to enable writeBytes b.writerIndex(b.writerIndex() - 1); - b.writeBytes(wrappedBuffer(new byte[] { 2 }).order(order)); + b.writeBytes(releaseLater(wrappedBuffer(new byte[] { 2 })).order(order)); assertFalse(ByteBufUtil.equals(a, b)); // Same content, same firstIndex, short length. - a = wrappedBuffer(new byte[] { 1, 2, 3 }).order(order); - b = wrappedBuffer(wrappedBuffer(new byte[] { 1 }, new byte[2]).order(order)); + a = releaseLater(wrappedBuffer(new byte[] { 1, 2, 3 })).order(order); + b = releaseLater(wrappedBuffer(releaseLater(wrappedBuffer(new byte[] { 1 }, new byte[2]))).order(order)); // to enable writeBytes b.writerIndex(b.writerIndex() - 2); - b.writeBytes(wrappedBuffer(new byte[] { 2 }).order(order)); - b.writeBytes(wrappedBuffer(new byte[] { 3 }).order(order)); + b.writeBytes(releaseLater(wrappedBuffer(new byte[] { 2 })).order(order)); + b.writeBytes(releaseLater(wrappedBuffer(new byte[] { 3 })).order(order)); assertTrue(ByteBufUtil.equals(a, b)); // Same content, different firstIndex, short length. - a = wrappedBuffer(new byte[] { 1, 2, 3 }).order(order); - b = wrappedBuffer(wrappedBuffer(new byte[] { 0, 1, 2, 3, 4 }, 1, 3).order(order)); + a = releaseLater(wrappedBuffer(new byte[] { 1, 2, 3 })).order(order); + b = releaseLater(wrappedBuffer(releaseLater(wrappedBuffer(new byte[] { 0, 1, 2, 3, 4 }, 1, 3))).order(order)); // to enable writeBytes b.writerIndex(b.writerIndex() - 1); - b.writeBytes(wrappedBuffer(new byte[] { 0, 1, 2, 3, 4 }, 3, 1).order(order)); + b.writeBytes(releaseLater(wrappedBuffer(new byte[] { 0, 1, 2, 3, 4 }, 3, 1)).order(order)); assertTrue(ByteBufUtil.equals(a, b)); // Different content, same firstIndex, short length. - a = wrappedBuffer(new byte[] { 1, 2, 3 }).order(order); - b = releaseLater(wrappedBuffer(wrappedBuffer(new byte[] { 1, 2 }, new byte[1]).order(order))); + a = releaseLater(wrappedBuffer(new byte[] { 1, 2, 3 })).order(order); + b = releaseLater(wrappedBuffer(releaseLater(wrappedBuffer(new byte[] { 1, 2 }, new byte[1])).order(order))); // to enable writeBytes b.writerIndex(b.writerIndex() - 1); - b.writeBytes(wrappedBuffer(new byte[] { 4 }).order(order)); + b.writeBytes(releaseLater(wrappedBuffer(new byte[] { 4 })).order(order)); assertFalse(ByteBufUtil.equals(a, b)); // Different content, different firstIndex, short length. - a = wrappedBuffer(new byte[] { 1, 2, 3 }).order(order); - b = wrappedBuffer(wrappedBuffer(new byte[] { 0, 1, 2, 4, 5 }, 1, 3).order(order)); + a = releaseLater(wrappedBuffer(new byte[] { 1, 2, 3 })).order(order); + b = releaseLater(wrappedBuffer(releaseLater(wrappedBuffer(new byte[] { 0, 1, 2, 4, 5 }, 1, 3))).order(order)); // to enable writeBytes b.writerIndex(b.writerIndex() - 1); - b.writeBytes(wrappedBuffer(new byte[] { 0, 1, 2, 4, 5 }, 3, 1).order(order)); + b.writeBytes(releaseLater(wrappedBuffer(new byte[] { 0, 1, 2, 4, 5 }, 3, 1)).order(order)); assertFalse(ByteBufUtil.equals(a, b)); // Same content, same firstIndex, long length. - a = wrappedBuffer(new byte[] { 1, 2, 3, 4, 5, 6, 7, 8, 9, 10 }).order(order); - b = releaseLater(wrappedBuffer(wrappedBuffer(new byte[] { 1, 2, 3 }, new byte[7])).order(order)); + a = releaseLater(wrappedBuffer(new byte[] { 1, 2, 3, 4, 5, 6, 7, 8, 9, 10 })).order(order); + b = releaseLater(wrappedBuffer(releaseLater(wrappedBuffer(new byte[] { 1, 2, 3 }, new byte[7]))).order(order)); // to enable writeBytes b.writerIndex(b.writerIndex() - 7); - b.writeBytes(wrappedBuffer(new byte[] { 4, 5, 6 }).order(order)); - b.writeBytes(wrappedBuffer(new byte[] { 7, 8, 9, 10 }).order(order)); + b.writeBytes(releaseLater(wrappedBuffer(new byte[] { 4, 5, 6 })).order(order)); + b.writeBytes(releaseLater(wrappedBuffer(new byte[] { 7, 8, 9, 10 })).order(order)); assertTrue(ByteBufUtil.equals(a, b)); // Same content, different firstIndex, long length. - a = wrappedBuffer(new byte[] { 1, 2, 3, 4, 5, 6, 7, 8, 9, 10 }).order(order); - b = wrappedBuffer(wrappedBuffer(new byte[] { 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11}, 1, 10).order(order)); + a = releaseLater(wrappedBuffer(new byte[] { 1, 2, 3, 4, 5, 6, 7, 8, 9, 10 })).order(order); + b = releaseLater(wrappedBuffer(releaseLater( + wrappedBuffer(new byte[] { 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11}, 1, 10))).order(order)); // to enable writeBytes b.writerIndex(b.writerIndex() - 5); - b.writeBytes(wrappedBuffer(new byte[] { 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11}, 6, 5).order(order)); + b.writeBytes(releaseLater( + wrappedBuffer(new byte[] { 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11}, 6, 5)).order(order)); assertTrue(ByteBufUtil.equals(a, b)); // Different content, same firstIndex, long length. - a = wrappedBuffer(new byte[] { 1, 2, 3, 4, 5, 6, 7, 8, 9, 10 }).order(order); + a = releaseLater(wrappedBuffer(new byte[] { 1, 2, 3, 4, 5, 6, 7, 8, 9, 10 })).order(order); b = releaseLater(wrappedBuffer(wrappedBuffer(new byte[] { 1, 2, 3, 4, 6 }, new byte[5])).order(order)); // to enable writeBytes b.writerIndex(b.writerIndex() - 5); - b.writeBytes(wrappedBuffer(new byte[] { 7, 8, 5, 9, 10 }).order(order)); + b.writeBytes(releaseLater(wrappedBuffer(new byte[] { 7, 8, 5, 9, 10 })).order(order)); assertFalse(ByteBufUtil.equals(a, b)); // Different content, different firstIndex, long length. - a = wrappedBuffer(new byte[] { 1, 2, 3, 4, 5, 6, 7, 8, 9, 10 }).order(order); - b = wrappedBuffer(wrappedBuffer(new byte[] { 0, 1, 2, 3, 4, 6, 7, 8, 5, 9, 10, 11 }, 1, 10).order(order)); + a = releaseLater(wrappedBuffer(new byte[] { 1, 2, 3, 4, 5, 6, 7, 8, 9, 10 })).order(order); + b = releaseLater(wrappedBuffer(releaseLater( + wrappedBuffer(new byte[] { 0, 1, 2, 3, 4, 6, 7, 8, 5, 9, 10, 11 }, 1, 10))).order(order)); // to enable writeBytes b.writerIndex(b.writerIndex() - 5); - b.writeBytes(wrappedBuffer(new byte[] { 0, 1, 2, 3, 4, 6, 7, 8, 5, 9, 10, 11 }, 6, 5).order(order)); + b.writeBytes(releaseLater( + wrappedBuffer(new byte[] { 0, 1, 2, 3, 4, 6, 7, 8, 5, 9, 10, 11 }, 6, 5)).order(order)); assertFalse(ByteBufUtil.equals(a, b)); } diff --git a/buffer/src/test/java/io/netty/buffer/ByteBufProcessorTest.java b/buffer/src/test/java/io/netty/buffer/ByteBufProcessorTest.java index 8d9e76fd40..26e833489e 100644 --- a/buffer/src/test/java/io/netty/buffer/ByteBufProcessorTest.java +++ b/buffer/src/test/java/io/netty/buffer/ByteBufProcessorTest.java @@ -19,12 +19,14 @@ package io.netty.buffer; import io.netty.util.CharsetUtil; import org.junit.Test; +import static io.netty.util.ReferenceCountUtil.releaseLater; import static org.junit.Assert.*; public class ByteBufProcessorTest { @Test public void testForward() { - final ByteBuf buf = Unpooled.copiedBuffer("abc\r\n\ndef\r\rghi\n\njkl\0\0mno \t\tx", CharsetUtil.ISO_8859_1); + final ByteBuf buf = releaseLater( + Unpooled.copiedBuffer("abc\r\n\ndef\r\rghi\n\njkl\0\0mno \t\tx", CharsetUtil.ISO_8859_1)); final int length = buf.readableBytes(); assertEquals(3, buf.forEachByte(0, length, ByteBufProcessor.FIND_CRLF)); @@ -42,7 +44,8 @@ public class ByteBufProcessorTest { @Test public void testBackward() { - final ByteBuf buf = Unpooled.copiedBuffer("abc\r\n\ndef\r\rghi\n\njkl\0\0mno \t\tx", CharsetUtil.ISO_8859_1); + final ByteBuf buf = releaseLater( + Unpooled.copiedBuffer("abc\r\n\ndef\r\rghi\n\njkl\0\0mno \t\tx", CharsetUtil.ISO_8859_1)); final int length = buf.readableBytes(); assertEquals(27, buf.forEachByteDesc(0, length, ByteBufProcessor.FIND_LINEAR_WHITESPACE)); diff --git a/codec-http/src/main/java/io/netty/handler/codec/http/multipart/HttpPostRequestDecoder.java b/codec-http/src/main/java/io/netty/handler/codec/http/multipart/HttpPostRequestDecoder.java index b584cecfa9..fe21fb7262 100644 --- a/codec-http/src/main/java/io/netty/handler/codec/http/multipart/HttpPostRequestDecoder.java +++ b/codec-http/src/main/java/io/netty/handler/codec/http/multipart/HttpPostRequestDecoder.java @@ -393,7 +393,7 @@ public class HttpPostRequestDecoder { checkDestroyed(); // Maybe we should better not copy here for performance reasons but this will need - // more care by teh caller to release the content in a correct manner later + // more care by the caller to release the content in a correct manner later // So maybe something to optimize on a later stage ByteBuf buf = content.content(); if (undecodedChunk == null) { diff --git a/codec-http/src/main/java/io/netty/handler/codec/http/websocketx/WebSocketFrameAggregator.java b/codec-http/src/main/java/io/netty/handler/codec/http/websocketx/WebSocketFrameAggregator.java index 080bc7dcb0..c8cb395b61 100644 --- a/codec-http/src/main/java/io/netty/handler/codec/http/websocketx/WebSocketFrameAggregator.java +++ b/codec-http/src/main/java/io/netty/handler/codec/http/websocketx/WebSocketFrameAggregator.java @@ -63,6 +63,7 @@ public class WebSocketFrameAggregator extends MessageToMessageDecoder maxFrameSize - msg.content().readableBytes()) { + // release the current frame + currentFrame.release(); tooLongFrameFound = true; throw new TooLongFrameException( "WebSocketFrame length exceeded " + content + @@ -102,7 +105,6 @@ public class WebSocketFrameAggregator extends MessageToMessageDecoder input = new ChunkedInput() { private boolean done; - private final ByteBuf buffer = Unpooled.copiedBuffer("Test", CharsetUtil.ISO_8859_1); + private final ByteBuf buffer = releaseLater(Unpooled.copiedBuffer("Test", CharsetUtil.ISO_8859_1)); @Override public boolean isEndOfInput() throws Exception { @@ -141,7 +142,7 @@ public class ChunkedWriteHandlerTest { // the listener should have been notified assertTrue(listenerNotified.get()); - assertEquals(buffer, ch.readOutbound()); + assertEquals(releaseLater(buffer), releaseLater(ch.readOutbound())); assertNull(ch.readOutbound()); } @@ -203,6 +204,7 @@ public class ChunkedWriteHandlerTest { i = 0; } } + buffer.release(); } assertEquals(BYTES.length * inputs.length, read); diff --git a/testsuite/src/test/java/io/netty/testsuite/transport/socket/SocketBufReleaseTest.java b/testsuite/src/test/java/io/netty/testsuite/transport/socket/SocketBufReleaseTest.java index ae108a671f..dddadefc15 100644 --- a/testsuite/src/test/java/io/netty/testsuite/transport/socket/SocketBufReleaseTest.java +++ b/testsuite/src/test/java/io/netty/testsuite/transport/socket/SocketBufReleaseTest.java @@ -64,6 +64,9 @@ public class SocketBufReleaseTest extends AbstractSocketTest { serverHandler.check(); clientHandler.check(); + + serverHandler.release(); + clientHandler.release(); } private static class BufWriterHandler extends SimpleChannelInboundHandler { @@ -104,5 +107,9 @@ public class SocketBufReleaseTest extends AbstractSocketTest { latch.await(); assertEquals(1, buf.refCnt()); } + + void release() { + buf.release(); + } } } diff --git a/transport/src/test/java/io/netty/channel/ChannelOutboundBufferTest.java b/transport/src/test/java/io/netty/channel/ChannelOutboundBufferTest.java index b48d0dfc9e..f1cfdb3820 100644 --- a/transport/src/test/java/io/netty/channel/ChannelOutboundBufferTest.java +++ b/transport/src/test/java/io/netty/channel/ChannelOutboundBufferTest.java @@ -39,11 +39,7 @@ public class ChannelOutboundBufferTest { assertNull(b); } assertEquals(0, buffer.nioBufferCount()); - for (;;) { - if (!buffer.remove()) { - break; - } - } + release(buffer); } @Test @@ -78,11 +74,7 @@ public class ChannelOutboundBufferTest { assertNull(buffers[i]); } } - for (;;) { - if (!buffer.remove()) { - break; - } - } + release(buffer); } @Test @@ -107,11 +99,8 @@ public class ChannelOutboundBufferTest { for (int i = 0; i < buffers.length; i++) { assertEquals(buffers[i], buf.internalNioBuffer(0, buf.readableBytes())); } - for (;;) { - if (!buffer.remove()) { - break; - } - } + release(buffer); + buf.release(); } @Test @@ -143,6 +132,11 @@ public class ChannelOutboundBufferTest { assertNull(buffers[i]); } } + release(buffer); + buf.release(); + } + + private static void release(ChannelOutboundBuffer buffer) { for (;;) { if (!buffer.remove()) { break;