From e1becd214777c99ac7b91b0614ba9cdc34c1751c Mon Sep 17 00:00:00 2001 From: Norman Maurer Date: Tue, 6 May 2014 10:03:24 +0200 Subject: [PATCH] Fix buffer leak introduced by #2462 Motivation: Because of not correctly release a buffer before null out the reference a memory leak shows up. Modifications: Correct call buffer.release() before null out reference. Result: No more leak --- .../codec/spdy/SpdyHeaderBlockRawDecoder.java | 2 +- .../spdy/SpdyHeaderBlockRawDecoderTest.java | 63 ++++++++++--------- .../spdy/SpdyHeaderBlockZlibDecoderTest.java | 21 ++++--- 3 files changed, 50 insertions(+), 36 deletions(-) diff --git a/codec-http/src/main/java/io/netty/handler/codec/spdy/SpdyHeaderBlockRawDecoder.java b/codec-http/src/main/java/io/netty/handler/codec/spdy/SpdyHeaderBlockRawDecoder.java index d0fa052cd3..750d1f4f6f 100644 --- a/codec-http/src/main/java/io/netty/handler/codec/spdy/SpdyHeaderBlockRawDecoder.java +++ b/codec-http/src/main/java/io/netty/handler/codec/spdy/SpdyHeaderBlockRawDecoder.java @@ -81,7 +81,7 @@ public class SpdyHeaderBlockRawDecoder extends SpdyHeaderBlockDecoder { if (cumulation.isReadable()) { cumulation.discardReadBytes(); } else { - cumulation = null; + releaseBuffer(); } } } diff --git a/codec-http/src/test/java/io/netty/handler/codec/spdy/SpdyHeaderBlockRawDecoderTest.java b/codec-http/src/test/java/io/netty/handler/codec/spdy/SpdyHeaderBlockRawDecoderTest.java index 754aca58bb..6ca1df23af 100644 --- a/codec-http/src/test/java/io/netty/handler/codec/spdy/SpdyHeaderBlockRawDecoderTest.java +++ b/codec-http/src/test/java/io/netty/handler/codec/spdy/SpdyHeaderBlockRawDecoderTest.java @@ -17,6 +17,8 @@ package io.netty.handler.codec.spdy; import io.netty.buffer.ByteBuf; import io.netty.buffer.Unpooled; +import io.netty.util.ReferenceCountUtil; +import org.junit.After; import org.junit.Before; import org.junit.Test; @@ -42,9 +44,14 @@ public class SpdyHeaderBlockRawDecoderTest { frame = new DefaultSpdyHeadersFrame(1); } + @After + public void tearDown() { + decoder.end(); + } + @Test public void testEmptyHeaderBlock() throws Exception { - ByteBuf headerBlock = Unpooled.EMPTY_BUFFER; + ByteBuf headerBlock = ReferenceCountUtil.releaseLater(Unpooled.EMPTY_BUFFER); decoder.decode(headerBlock, frame); decoder.endHeaderBlock(frame); @@ -55,7 +62,7 @@ public class SpdyHeaderBlockRawDecoderTest { @Test public void testZeroNameValuePairs() throws Exception { - ByteBuf headerBlock = Unpooled.buffer(4); + ByteBuf headerBlock = ReferenceCountUtil.releaseLater(Unpooled.buffer(4)); headerBlock.writeInt(0); decoder.decode(headerBlock, frame); decoder.endHeaderBlock(frame); @@ -67,7 +74,7 @@ public class SpdyHeaderBlockRawDecoderTest { @Test public void testNegativeNameValuePairs() throws Exception { - ByteBuf headerBlock = Unpooled.buffer(4); + ByteBuf headerBlock = ReferenceCountUtil.releaseLater(Unpooled.buffer(4)); headerBlock.writeInt(-1); decoder.decode(headerBlock, frame); @@ -78,7 +85,7 @@ public class SpdyHeaderBlockRawDecoderTest { @Test public void testOneNameValuePair() throws Exception { - ByteBuf headerBlock = Unpooled.buffer(21); + ByteBuf headerBlock = ReferenceCountUtil.releaseLater(Unpooled.buffer(21)); headerBlock.writeInt(1); headerBlock.writeInt(4); headerBlock.writeBytes(nameBytes); @@ -97,7 +104,7 @@ public class SpdyHeaderBlockRawDecoderTest { @Test public void testMissingNameLength() throws Exception { - ByteBuf headerBlock = Unpooled.buffer(4); + ByteBuf headerBlock = ReferenceCountUtil.releaseLater(Unpooled.buffer(4)); headerBlock.writeInt(1); decoder.decode(headerBlock, frame); decoder.endHeaderBlock(frame); @@ -109,7 +116,7 @@ public class SpdyHeaderBlockRawDecoderTest { @Test public void testZeroNameLength() throws Exception { - ByteBuf headerBlock = Unpooled.buffer(8); + ByteBuf headerBlock = ReferenceCountUtil.releaseLater(Unpooled.buffer(8)); headerBlock.writeInt(1); headerBlock.writeInt(0); decoder.decode(headerBlock, frame); @@ -121,7 +128,7 @@ public class SpdyHeaderBlockRawDecoderTest { @Test public void testNegativeNameLength() throws Exception { - ByteBuf headerBlock = Unpooled.buffer(8); + ByteBuf headerBlock = ReferenceCountUtil.releaseLater(Unpooled.buffer(8)); headerBlock.writeInt(1); headerBlock.writeInt(-1); decoder.decode(headerBlock, frame); @@ -133,7 +140,7 @@ public class SpdyHeaderBlockRawDecoderTest { @Test public void testMissingName() throws Exception { - ByteBuf headerBlock = Unpooled.buffer(8); + ByteBuf headerBlock = ReferenceCountUtil.releaseLater(Unpooled.buffer(8)); headerBlock.writeInt(1); headerBlock.writeInt(4); decoder.decode(headerBlock, frame); @@ -146,7 +153,7 @@ public class SpdyHeaderBlockRawDecoderTest { @Test public void testIllegalNameOnlyNull() throws Exception { - ByteBuf headerBlock = Unpooled.buffer(18); + ByteBuf headerBlock = ReferenceCountUtil.releaseLater(Unpooled.buffer(18)); headerBlock.writeInt(1); headerBlock.writeInt(1); headerBlock.writeByte(0); @@ -161,7 +168,7 @@ public class SpdyHeaderBlockRawDecoderTest { @Test public void testMissingValueLength() throws Exception { - ByteBuf headerBlock = Unpooled.buffer(12); + ByteBuf headerBlock = ReferenceCountUtil.releaseLater(Unpooled.buffer(12)); headerBlock.writeInt(1); headerBlock.writeInt(4); headerBlock.writeBytes(nameBytes); @@ -175,7 +182,7 @@ public class SpdyHeaderBlockRawDecoderTest { @Test public void testZeroValueLength() throws Exception { - ByteBuf headerBlock = Unpooled.buffer(16); + ByteBuf headerBlock = ReferenceCountUtil.releaseLater(Unpooled.buffer(16)); headerBlock.writeInt(1); headerBlock.writeInt(4); headerBlock.writeBytes(nameBytes); @@ -193,7 +200,7 @@ public class SpdyHeaderBlockRawDecoderTest { @Test public void testNegativeValueLength() throws Exception { - ByteBuf headerBlock = Unpooled.buffer(16); + ByteBuf headerBlock = ReferenceCountUtil.releaseLater(Unpooled.buffer(16)); headerBlock.writeInt(1); headerBlock.writeInt(4); headerBlock.writeBytes(nameBytes); @@ -207,7 +214,7 @@ public class SpdyHeaderBlockRawDecoderTest { @Test public void testMissingValue() throws Exception { - ByteBuf headerBlock = Unpooled.buffer(16); + ByteBuf headerBlock = ReferenceCountUtil.releaseLater(Unpooled.buffer(16)); headerBlock.writeInt(1); headerBlock.writeInt(4); headerBlock.writeBytes(nameBytes); @@ -222,7 +229,7 @@ public class SpdyHeaderBlockRawDecoderTest { @Test public void testIllegalValueOnlyNull() throws Exception { - ByteBuf headerBlock = Unpooled.buffer(17); + ByteBuf headerBlock = ReferenceCountUtil.releaseLater(Unpooled.buffer(17)); headerBlock.writeInt(1); headerBlock.writeInt(4); headerBlock.writeBytes(nameBytes); @@ -237,7 +244,7 @@ public class SpdyHeaderBlockRawDecoderTest { @Test public void testIllegalValueStartsWithNull() throws Exception { - ByteBuf headerBlock = Unpooled.buffer(22); + ByteBuf headerBlock = ReferenceCountUtil.releaseLater(Unpooled.buffer(22)); headerBlock.writeInt(1); headerBlock.writeInt(4); headerBlock.writeBytes(nameBytes); @@ -253,7 +260,7 @@ public class SpdyHeaderBlockRawDecoderTest { @Test public void testIllegalValueEndsWithNull() throws Exception { - ByteBuf headerBlock = Unpooled.buffer(22); + ByteBuf headerBlock = ReferenceCountUtil.releaseLater(Unpooled.buffer(22)); headerBlock.writeInt(1); headerBlock.writeInt(4); headerBlock.writeBytes(nameBytes); @@ -269,7 +276,7 @@ public class SpdyHeaderBlockRawDecoderTest { @Test public void testMultipleValues() throws Exception { - ByteBuf headerBlock = Unpooled.buffer(27); + ByteBuf headerBlock = ReferenceCountUtil.releaseLater(Unpooled.buffer(27)); headerBlock.writeInt(1); headerBlock.writeInt(4); headerBlock.writeBytes(nameBytes); @@ -291,7 +298,7 @@ public class SpdyHeaderBlockRawDecoderTest { @Test public void testMultipleValuesEndsWithNull() throws Exception { - ByteBuf headerBlock = Unpooled.buffer(28); + ByteBuf headerBlock = ReferenceCountUtil.releaseLater(Unpooled.buffer(28)); headerBlock.writeInt(1); headerBlock.writeInt(4); headerBlock.writeBytes(nameBytes); @@ -312,7 +319,7 @@ public class SpdyHeaderBlockRawDecoderTest { @Test public void testIllegalValueMultipleNulls() throws Exception { - ByteBuf headerBlock = Unpooled.buffer(28); + ByteBuf headerBlock = ReferenceCountUtil.releaseLater(Unpooled.buffer(28)); headerBlock.writeInt(1); headerBlock.writeInt(4); headerBlock.writeBytes(nameBytes); @@ -331,7 +338,7 @@ public class SpdyHeaderBlockRawDecoderTest { @Test public void testMissingNextNameValuePair() throws Exception { - ByteBuf headerBlock = Unpooled.buffer(21); + ByteBuf headerBlock = ReferenceCountUtil.releaseLater(Unpooled.buffer(21)); headerBlock.writeInt(2); headerBlock.writeInt(4); headerBlock.writeBytes(nameBytes); @@ -350,7 +357,7 @@ public class SpdyHeaderBlockRawDecoderTest { @Test public void testMultipleNames() throws Exception { - ByteBuf headerBlock = Unpooled.buffer(38); + ByteBuf headerBlock = ReferenceCountUtil.releaseLater(Unpooled.buffer(38)); headerBlock.writeInt(2); headerBlock.writeInt(4); headerBlock.writeBytes(nameBytes); @@ -372,7 +379,7 @@ public class SpdyHeaderBlockRawDecoderTest { @Test public void testExtraData() throws Exception { - ByteBuf headerBlock = Unpooled.buffer(22); + ByteBuf headerBlock = ReferenceCountUtil.releaseLater(Unpooled.buffer(22)); headerBlock.writeInt(1); headerBlock.writeInt(4); headerBlock.writeBytes(nameBytes); @@ -391,7 +398,7 @@ public class SpdyHeaderBlockRawDecoderTest { @Test public void testMultipleDecodes() throws Exception { - ByteBuf headerBlock = Unpooled.buffer(21); + ByteBuf headerBlock = ReferenceCountUtil.releaseLater(Unpooled.buffer(21)); headerBlock.writeInt(1); headerBlock.writeInt(4); headerBlock.writeBytes(nameBytes); @@ -415,14 +422,14 @@ public class SpdyHeaderBlockRawDecoderTest { @Test public void testContinueAfterInvalidHeaders() throws Exception { - ByteBuf numHeaders = Unpooled.buffer(4); + ByteBuf numHeaders = ReferenceCountUtil.releaseLater(Unpooled.buffer(4)); numHeaders.writeInt(1); - ByteBuf nameBlock = Unpooled.buffer(8); + ByteBuf nameBlock = ReferenceCountUtil.releaseLater(Unpooled.buffer(8)); nameBlock.writeInt(4); nameBlock.writeBytes(nameBytes); - ByteBuf valueBlock = Unpooled.buffer(9); + ByteBuf valueBlock = ReferenceCountUtil.releaseLater(Unpooled.buffer(9)); valueBlock.writeInt(5); valueBlock.writeBytes(valueBytes); @@ -443,7 +450,7 @@ public class SpdyHeaderBlockRawDecoderTest { @Test public void testTruncatedHeaderName() throws Exception { - ByteBuf headerBlock = Unpooled.buffer(maxHeaderSize + 18); + ByteBuf headerBlock = ReferenceCountUtil.releaseLater(Unpooled.buffer(maxHeaderSize + 18)); headerBlock.writeInt(1); headerBlock.writeInt(maxHeaderSize + 1); for (int i = 0; i < maxHeaderSize + 1; i++) { @@ -462,7 +469,7 @@ public class SpdyHeaderBlockRawDecoderTest { @Test public void testTruncatedHeaderValue() throws Exception { - ByteBuf headerBlock = Unpooled.buffer(maxHeaderSize + 13); + ByteBuf headerBlock = ReferenceCountUtil.releaseLater(Unpooled.buffer(maxHeaderSize + 13)); headerBlock.writeInt(1); headerBlock.writeInt(4); headerBlock.writeBytes(nameBytes); diff --git a/codec-http/src/test/java/io/netty/handler/codec/spdy/SpdyHeaderBlockZlibDecoderTest.java b/codec-http/src/test/java/io/netty/handler/codec/spdy/SpdyHeaderBlockZlibDecoderTest.java index 4a9efa32a9..cc06893559 100644 --- a/codec-http/src/test/java/io/netty/handler/codec/spdy/SpdyHeaderBlockZlibDecoderTest.java +++ b/codec-http/src/test/java/io/netty/handler/codec/spdy/SpdyHeaderBlockZlibDecoderTest.java @@ -17,6 +17,8 @@ package io.netty.handler.codec.spdy; import io.netty.buffer.ByteBuf; import io.netty.buffer.Unpooled; +import io.netty.util.ReferenceCountUtil; +import org.junit.After; import org.junit.Before; import org.junit.Test; @@ -46,9 +48,14 @@ public class SpdyHeaderBlockZlibDecoderTest { frame = new DefaultSpdyHeadersFrame(1); } + @After + public void tearDown() { + decoder.end(); + } + @Test public void testHeaderBlock() throws Exception { - ByteBuf headerBlock = Unpooled.buffer(37); + ByteBuf headerBlock = ReferenceCountUtil.releaseLater(Unpooled.buffer(37)); headerBlock.writeBytes(zlibHeader); headerBlock.writeByte(0); // Non-compressed block headerBlock.writeByte(0x15); // little-endian length (21) @@ -74,7 +81,7 @@ public class SpdyHeaderBlockZlibDecoderTest { @Test public void testHeaderBlockMultipleDecodes() throws Exception { - ByteBuf headerBlock = Unpooled.buffer(37); + ByteBuf headerBlock = ReferenceCountUtil.releaseLater(Unpooled.buffer(37)); headerBlock.writeBytes(zlibHeader); headerBlock.writeByte(0); // Non-compressed block headerBlock.writeByte(0x15); // little-endian length (21) @@ -105,7 +112,7 @@ public class SpdyHeaderBlockZlibDecoderTest { @Test public void testLargeHeaderName() throws Exception { - ByteBuf headerBlock = Unpooled.buffer(8220); + ByteBuf headerBlock = ReferenceCountUtil.releaseLater(Unpooled.buffer(8220)); headerBlock.writeBytes(zlibHeader); headerBlock.writeByte(0); // Non-compressed block headerBlock.writeByte(0x0c); // little-endian length (8204) @@ -130,7 +137,7 @@ public class SpdyHeaderBlockZlibDecoderTest { @Test public void testLargeHeaderValue() throws Exception { - ByteBuf headerBlock = Unpooled.buffer(8220); + ByteBuf headerBlock = ReferenceCountUtil.releaseLater(Unpooled.buffer(8220)); headerBlock.writeBytes(zlibHeader); headerBlock.writeByte(0); // Non-compressed block headerBlock.writeByte(0x0c); // little-endian length (8204) @@ -157,7 +164,7 @@ public class SpdyHeaderBlockZlibDecoderTest { @Test(expected = SpdyProtocolException.class) public void testHeaderBlockExtraData() throws Exception { - ByteBuf headerBlock = Unpooled.buffer(37); + ByteBuf headerBlock = ReferenceCountUtil.releaseLater(Unpooled.buffer(37)); headerBlock.writeBytes(zlibHeader); headerBlock.writeByte(0); // Non-compressed block headerBlock.writeByte(0x15); // little-endian length (21) @@ -179,7 +186,7 @@ public class SpdyHeaderBlockZlibDecoderTest { @Test(expected = SpdyProtocolException.class) public void testHeaderBlockInvalidDictionary() throws Exception { - ByteBuf headerBlock = Unpooled.buffer(7); + ByteBuf headerBlock = ReferenceCountUtil.releaseLater(Unpooled.buffer(7)); headerBlock.writeByte(0x78); headerBlock.writeByte(0x3f); headerBlock.writeByte(0x01); // Unknown dictionary @@ -192,7 +199,7 @@ public class SpdyHeaderBlockZlibDecoderTest { @Test(expected = SpdyProtocolException.class) public void testHeaderBlockInvalidDeflateBlock() throws Exception { - ByteBuf headerBlock = Unpooled.buffer(11); + ByteBuf headerBlock = ReferenceCountUtil.releaseLater(Unpooled.buffer(11)); headerBlock.writeBytes(zlibHeader); headerBlock.writeByte(0); // Non-compressed block headerBlock.writeByte(0x00); // little-endian length (0)