Skip to content

Commit

Permalink
Fix all leaks reported during tests
Browse files Browse the repository at this point in the history 
- One notable leak is from WebSocketFrameAggregator
- All other leaks are from tests
  • Loading branch information
@trustin
Norman Maurer authored and trustin committed Dec 6, 2013
1 parent 5142800 commit b3d8c81
Show file tree
Hide file tree
Showing 32 changed files with 294 additions and 132 deletions.
2 changes: 1 addition & 1 deletion buffer/src/test/java/io/netty/buffer/AbstractByteBufTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1532,7 +1532,7 @@ public void testCompareTo() {
@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));
}

Expand Down
74 changes: 39 additions & 35 deletions buffer/src/test/java/io/netty/buffer/AbstractCompositeByteBufTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -101,8 +101,8 @@ protected boolean discardReadBytesDoesNotMoveWritableBytes() {
*/
@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());
Expand Down Expand Up @@ -161,7 +161,7 @@ public void testDiscardReadBytes3() {

@Test
public void testAutoConsolidation() {
CompositeByteBuf buf = compositeBuffer(2);
CompositeByteBuf buf = releaseLater(compositeBuffer(2));

buf.addComponent(wrappedBuffer(new byte[] { 1 }));
assertEquals(1, buf.numComponents());
Expand All @@ -179,7 +179,7 @@ public void testAutoConsolidation() {

@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());
Expand Down Expand Up @@ -229,13 +229,13 @@ public void testRangedConsolidation() {

@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());
Expand Down Expand Up @@ -362,77 +362,81 @@ public void testWrittenBuffersEquals() {
//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));
}

Expand Down
7 changes: 5 additions & 2 deletions buffer/src/test/java/io/netty/buffer/ByteBufProcessorTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,12 +19,14 @@
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));
Expand All @@ -42,7 +44,8 @@ public void testForward() {

@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));
Expand Down
2 changes: 1 addition & 1 deletion codec-http/src/main/java/io/netty/handler/codec/http/multipart/HttpPostRequestDecoder.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -393,7 +393,7 @@ public HttpPostRequestDecoder offer(HttpContent content) throws ErrorDataDecoder
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) {
Expand Down
6 changes: 4 additions & 2 deletions ...c-http/src/main/java/io/netty/handler/codec/http/websocketx/WebSocketFrameAggregator.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@ protected void decode(ChannelHandlerContext ctx, WebSocketFrame msg, List<Object
} else if (msg instanceof BinaryWebSocketFrame) {
currentFrame = new BinaryWebSocketFrame(true, msg.rsv(), buf);
} else {
buf.release();
throw new IllegalStateException(
"WebSocket frame was not of type TextWebSocketFrame or BinaryWebSocketFrame");
}
Expand All @@ -77,6 +78,8 @@ protected void decode(ChannelHandlerContext ctx, WebSocketFrame msg, List<Object
}
CompositeByteBuf content = (CompositeByteBuf) currentFrame.content();
if (content.readableBytes() > maxFrameSize - msg.content().readableBytes()) {
// release the current frame
currentFrame.release();
tooLongFrameFound = true;
throw new TooLongFrameException(
"WebSocketFrame length exceeded " + content +
Expand All @@ -102,7 +105,6 @@ protected void decode(ChannelHandlerContext ctx, WebSocketFrame msg, List<Object
@Override
public void channelInactive(ChannelHandlerContext ctx) throws Exception {
super.channelInactive(ctx);

// release current frame if it is not null as it may be a left-over
if (currentFrame != null) {
currentFrame.release();
Expand All @@ -113,7 +115,7 @@ public void channelInactive(ChannelHandlerContext ctx) throws Exception {
@Override
public void handlerRemoved(ChannelHandlerContext ctx) throws Exception {
super.handlerRemoved(ctx);
// release current frane if it is not null as it may be a left-over as there is not much more we can do in
// release current frame if it is not null as it may be a left-over as there is not much more we can do in
// this case
if (currentFrame != null) {
currentFrame.release();
Expand Down
46 changes: 42 additions & 4 deletions codec-http/src/test/java/io/netty/handler/codec/http/HttpClientCodecTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@
import io.netty.util.CharsetUtil;
import org.junit.Test;

import static io.netty.util.ReferenceCountUtil.*;
import static org.hamcrest.CoreMatchers.*;
import static org.junit.Assert.*;

public class HttpClientCodecTest {
Expand All @@ -42,6 +44,21 @@ public void testFailsNotOnRequestResponse() {
ch.writeOutbound(new DefaultFullHttpRequest(HttpVersion.HTTP_1_1, HttpMethod.GET, "http://localhost/"));
ch.writeInbound(Unpooled.copiedBuffer(RESPONSE, CharsetUtil.ISO_8859_1));
ch.finish();

for (;;) {
Object msg = ch.readOutbound();
if (msg == null) {
break;
}
release(msg);
}
for (;;) {
Object msg = ch.readInbound();
if (msg == null) {
break;
}
release(msg);
}
}

@Test
Expand All @@ -52,6 +69,20 @@ public void testFailsNotOnRequestResponseChunked() {
ch.writeOutbound(new DefaultFullHttpRequest(HttpVersion.HTTP_1_1, HttpMethod.GET, "http://localhost/"));
ch.writeInbound(Unpooled.copiedBuffer(CHUNKED_RESPONSE, CharsetUtil.ISO_8859_1));
ch.finish();
for (;;) {
Object msg = ch.readOutbound();
if (msg == null) {
break;
}
release(msg);
}
for (;;) {
Object msg = ch.readInbound();
if (msg == null) {
break;
}
release(msg);
}
}

@Test
Expand All @@ -61,8 +92,7 @@ public void testFailsOnMissingResponse() {

assertTrue(ch.writeOutbound(new DefaultFullHttpRequest(HttpVersion.HTTP_1_1, HttpMethod.GET,
"http://localhost/")));
assertNotNull(ch.readOutbound());

assertNotNull(releaseLater(ch.readOutbound()));
try {
ch.finish();
fail();
Expand All @@ -76,8 +106,16 @@ public void testFailsOnIncompleteChunkedResponse() {
HttpClientCodec codec = new HttpClientCodec(4096, 8192, 8192, true);
EmbeddedChannel ch = new EmbeddedChannel(codec);

ch.writeOutbound(new DefaultFullHttpRequest(HttpVersion.HTTP_1_1, HttpMethod.GET, "http://localhost/"));
ch.writeInbound(Unpooled.copiedBuffer(INCOMPLETE_CHUNKED_RESPONSE, CharsetUtil.ISO_8859_1));
ch.writeOutbound(releaseLater(
new DefaultFullHttpRequest(HttpVersion.HTTP_1_1, HttpMethod.GET, "http://localhost/")));
assertNotNull(releaseLater(ch.readOutbound()));
assertNull(ch.readInbound());
ch.writeInbound(releaseLater(
Unpooled.copiedBuffer(INCOMPLETE_CHUNKED_RESPONSE, CharsetUtil.ISO_8859_1)));
assertThat(releaseLater(ch.readInbound()), instanceOf(HttpResponse.class));
assertThat(releaseLater(ch.readInbound()), instanceOf(HttpContent.class)); // Chunk 'first'
assertThat(releaseLater(ch.readInbound()), instanceOf(HttpContent.class)); // Chunk 'second'
assertNull(ch.readInbound());

try {
ch.finish();
Expand Down
8 changes: 7 additions & 1 deletion codec-http/src/test/java/io/netty/handler/codec/http/HttpContentEncoderTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -59,10 +59,15 @@ public void testSplitContent() throws Exception {
HttpContent chunk;
chunk = (HttpContent) ch.readOutbound();
assertThat(chunk.content().toString(CharsetUtil.US_ASCII), is("3"));
chunk.release();

chunk = (HttpContent) ch.readOutbound();
assertThat(chunk.content().toString(CharsetUtil.US_ASCII), is("2"));
chunk.release();

chunk = (HttpContent) ch.readOutbound();
assertThat(chunk.content().toString(CharsetUtil.US_ASCII), is("1"));
chunk.release();

chunk = (HttpContent) ch.readOutbound();
assertThat(chunk.content().isReadable(), is(false));
Expand Down Expand Up @@ -98,8 +103,9 @@ public void testChunkedContent() throws Exception {

chunk = (HttpContent) ch.readOutbound();
assertThat(chunk.content().toString(CharsetUtil.US_ASCII), is("1"));

assertThat(chunk, is(instanceOf(HttpContent.class)));
chunk.release();

chunk = (HttpContent) ch.readOutbound();
assertThat(chunk.content().isReadable(), is(false));
assertThat(chunk, is(instanceOf(LastHttpContent.class)));
Expand Down
9 changes: 5 additions & 4 deletions codec-http/src/test/java/io/netty/handler/codec/http/HttpObjectAggregatorTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@

import java.util.List;

import static io.netty.util.ReferenceCountUtil.releaseLater;
import static org.junit.Assert.*;

public class HttpObjectAggregatorTest {
Expand Down Expand Up @@ -99,7 +100,6 @@ public void testAggregateWithTrailer() {
assertEquals(aggratedMessage.headers().get("X-Test"), Boolean.TRUE.toString());
assertEquals(aggratedMessage.headers().get("X-Trailer"), Boolean.TRUE.toString());
checkContentBuffer(aggratedMessage);

assertNull(embedder.readInbound());
}

Expand All @@ -109,9 +109,9 @@ public void testTooLongFrameException() {
EmbeddedChannel embedder = new EmbeddedChannel(aggr);
HttpRequest message = new DefaultHttpRequest(HttpVersion.HTTP_1_1,
HttpMethod.GET, "http://localhost");
HttpContent chunk1 = new DefaultHttpContent(Unpooled.copiedBuffer("test", CharsetUtil.US_ASCII));
HttpContent chunk2 = new DefaultHttpContent(Unpooled.copiedBuffer("test2", CharsetUtil.US_ASCII));
HttpContent chunk3 = new DefaultHttpContent(Unpooled.copiedBuffer("test3", CharsetUtil.US_ASCII));
HttpContent chunk1 = releaseLater(new DefaultHttpContent(Unpooled.copiedBuffer("test", CharsetUtil.US_ASCII)));
HttpContent chunk2 = releaseLater(new DefaultHttpContent(Unpooled.copiedBuffer("test2", CharsetUtil.US_ASCII)));
HttpContent chunk3 = releaseLater(new DefaultHttpContent(Unpooled.copiedBuffer("test3", CharsetUtil.US_ASCII)));
HttpContent chunk4 = LastHttpContent.EMPTY_LAST_CONTENT;

assertFalse(embedder.writeInbound(message));
Expand All @@ -135,6 +135,7 @@ public void testTooLongFrameException() {
}
assertFalse(embedder.writeInbound(chunk3.copy()));
assertFalse(embedder.writeInbound(chunk4.copy()));
embedder.finish();
}

@Test(expected = IllegalArgumentException.class)
Expand Down
Loading

0 comments on commit b3d8c81

Please sign in to comment.