From d2b137649d753b593ad00318c8b0cdba13b77447 Mon Sep 17 00:00:00 2001 From: Trustin Lee Date: Thu, 14 Mar 2013 15:25:22 +0900 Subject: [PATCH] Fix more memory leaks in the buffer tests --- .../io/netty/buffer/AbstractByteBufTest.java | 54 +++++++------ .../buffer/AbstractCompositeByteBufTest.java | 77 ++++++++----------- .../io/netty/buffer/ConsolidationTest.java | 19 +++-- 3 files changed, 77 insertions(+), 73 deletions(-) diff --git a/buffer/src/test/java/io/netty/buffer/AbstractByteBufTest.java b/buffer/src/test/java/io/netty/buffer/AbstractByteBufTest.java index eb6d7b93f1..c243486b8f 100644 --- a/buffer/src/test/java/io/netty/buffer/AbstractByteBufTest.java +++ b/buffer/src/test/java/io/netty/buffer/AbstractByteBufTest.java @@ -24,8 +24,10 @@ import org.junit.Test; import java.io.ByteArrayInputStream; import java.io.ByteArrayOutputStream; import java.nio.ByteBuffer; +import java.util.ArrayDeque; import java.util.Arrays; import java.util.HashSet; +import java.util.Queue; import java.util.Random; import java.util.Set; @@ -42,6 +44,13 @@ public abstract class AbstractByteBufTest { private static final int BLOCK_SIZE = 128; private static final byte[] EMPTY_ARRAY = new byte[0]; + private static final Queue freeLaterQueue = new ArrayDeque(); + + protected ByteBuf freeLater(ByteBuf buf) { + freeLaterQueue.add(buf); + return buf; + } + private long seed; private Random random; private ByteBuf buffer; @@ -78,6 +87,17 @@ public abstract class AbstractByteBufTest { } buffer = null; } + + for (;;) { + ByteBuf buf = freeLaterQueue.poll(); + if (buf == null) { + break; + } + + if (buf.refCnt() > 0) { + buf.release(buf.refCnt()); + } + } } @Test @@ -862,7 +882,7 @@ public abstract class AbstractByteBufTest { @Test public void testRandomDirectBufferTransfer() { byte[] tmp = new byte[BLOCK_SIZE * 2]; - ByteBuf value = directBuffer(BLOCK_SIZE * 2); + ByteBuf value = freeLater(directBuffer(BLOCK_SIZE * 2)); for (int i = 0; i < buffer.capacity() - BLOCK_SIZE + 1; i += BLOCK_SIZE) { random.nextBytes(tmp); value.setBytes(0, tmp, 0, value.capacity()); @@ -870,7 +890,7 @@ public abstract class AbstractByteBufTest { } random.setSeed(seed); - ByteBuf expectedValue = directBuffer(BLOCK_SIZE * 2); + ByteBuf expectedValue = freeLater(directBuffer(BLOCK_SIZE * 2)); for (int i = 0; i < buffer.capacity() - BLOCK_SIZE + 1; i += BLOCK_SIZE) { random.nextBytes(tmp); expectedValue.setBytes(0, tmp, 0, expectedValue.capacity()); @@ -880,9 +900,6 @@ public abstract class AbstractByteBufTest { assertEquals(expectedValue.getByte(j), value.getByte(j)); } } - - value.release(); - expectedValue.release(); } @Test @@ -1029,7 +1046,7 @@ public abstract class AbstractByteBufTest { @Test public void testSequentialDirectBufferTransfer1() { byte[] valueContent = new byte[BLOCK_SIZE * 2]; - ByteBuf value = directBuffer(BLOCK_SIZE * 2); + ByteBuf value = freeLater(directBuffer(BLOCK_SIZE * 2)); buffer.writerIndex(0); for (int i = 0; i < buffer.capacity() - BLOCK_SIZE + 1; i += BLOCK_SIZE) { random.nextBytes(valueContent); @@ -1043,7 +1060,7 @@ public abstract class AbstractByteBufTest { random.setSeed(seed); byte[] expectedValueContent = new byte[BLOCK_SIZE * 2]; - ByteBuf expectedValue = wrappedBuffer(expectedValueContent); + ByteBuf expectedValue = freeLater(wrappedBuffer(expectedValueContent)); for (int i = 0; i < buffer.capacity() - BLOCK_SIZE + 1; i += BLOCK_SIZE) { random.nextBytes(expectedValueContent); int valueOffset = random.nextInt(BLOCK_SIZE); @@ -1057,14 +1074,12 @@ public abstract class AbstractByteBufTest { assertEquals(0, value.readerIndex()); assertEquals(0, value.writerIndex()); } - - value.release(); } @Test public void testSequentialDirectBufferTransfer2() { byte[] valueContent = new byte[BLOCK_SIZE * 2]; - ByteBuf value = directBuffer(BLOCK_SIZE * 2); + ByteBuf value = freeLater(directBuffer(BLOCK_SIZE * 2)); buffer.writerIndex(0); for (int i = 0; i < buffer.capacity() - BLOCK_SIZE + 1; i += BLOCK_SIZE) { random.nextBytes(valueContent); @@ -1082,7 +1097,7 @@ public abstract class AbstractByteBufTest { random.setSeed(seed); byte[] expectedValueContent = new byte[BLOCK_SIZE * 2]; - ByteBuf expectedValue = wrappedBuffer(expectedValueContent); + ByteBuf expectedValue = freeLater(wrappedBuffer(expectedValueContent)); for (int i = 0; i < buffer.capacity() - BLOCK_SIZE + 1; i += BLOCK_SIZE) { random.nextBytes(expectedValueContent); value.setBytes(0, valueContent); @@ -1098,8 +1113,6 @@ public abstract class AbstractByteBufTest { assertEquals(valueOffset, value.readerIndex()); assertEquals(valueOffset + BLOCK_SIZE, value.writerIndex()); } - - value.release(); } @Test @@ -1288,7 +1301,7 @@ public abstract class AbstractByteBufTest { for (int i = 0; i < buffer.capacity(); i += 4) { buffer.writeInt(i); } - ByteBuf copy = copiedBuffer(buffer); + ByteBuf copy = freeLater(copiedBuffer(buffer)); // Make sure there's no effect if called when readerIndex is 0. buffer.readerIndex(CAPACITY / 4); @@ -1406,7 +1419,7 @@ public abstract class AbstractByteBufTest { buffer.setIndex(readerIndex, writerIndex); // Make sure all properties are copied. - ByteBuf copy = buffer.copy(); + ByteBuf copy = freeLater(buffer.copy()); assertEquals(0, copy.readerIndex()); assertEquals(buffer.readableBytes(), copy.writerIndex()); assertEquals(buffer.readableBytes(), copy.capacity()); @@ -1604,8 +1617,8 @@ public abstract class AbstractByteBufTest { @Test public void testHashCode() { - ByteBuf elemA = buffer(15); - ByteBuf elemB = directBuffer(15); + ByteBuf elemA = freeLater(buffer(15)); + ByteBuf elemB = freeLater(directBuffer(15)); elemA.writeBytes(new byte[] { 1, 2, 3, 4, 5, 6, 7, 8, 9, 0, 1, 2, 3, 4, 5 }); elemB.writeBytes(new byte[] { 6, 7, 8, 9, 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 9 }); @@ -1614,9 +1627,9 @@ public abstract class AbstractByteBufTest { set.add(elemB); assertEquals(2, set.size()); - assertTrue(set.contains(elemA.copy())); + assertTrue(set.contains(freeLater(elemA.copy()))); - ByteBuf elemBCopy = elemB.copy(); + ByteBuf elemBCopy = freeLater(elemB.copy()); assertTrue(set.contains(elemBCopy)); buffer.clear(); @@ -1631,9 +1644,6 @@ public abstract class AbstractByteBufTest { assertTrue(set.remove(buffer)); assertFalse(set.contains(elemB)); assertEquals(0, set.size()); - - elemB.release(); - elemBCopy.release(); } // Test case for https://github.com/netty/netty/issues/325 diff --git a/buffer/src/test/java/io/netty/buffer/AbstractCompositeByteBufTest.java b/buffer/src/test/java/io/netty/buffer/AbstractCompositeByteBufTest.java index 826068b079..ec1d8464d5 100644 --- a/buffer/src/test/java/io/netty/buffer/AbstractCompositeByteBufTest.java +++ b/buffer/src/test/java/io/netty/buffer/AbstractCompositeByteBufTest.java @@ -225,78 +225,69 @@ public abstract class AbstractCompositeByteBufTest extends ByteBuf a, b; // XXX Same tests with several buffers in wrappedCheckedBuffer // Different length. - a = wrappedBuffer(new byte[] { 1 }).order(order); - b = wrappedBuffer( + a = freeLater(wrappedBuffer(new byte[] { 1 }).order(order)); + b = freeLater(wrappedBuffer( wrappedBuffer(new byte[] { 1 }).order(order), - wrappedBuffer(new byte[] { 2 }).order(order)); + wrappedBuffer(new byte[] { 2 }).order(order))); assertFalse(BufUtil.equals(a, b)); - b.release(); // Same content, same firstIndex, short length. - a = wrappedBuffer(new byte[] { 1, 2, 3 }).order(order); - b = wrappedBuffer( + a = freeLater(wrappedBuffer(new byte[] { 1, 2, 3 }).order(order)); + b = freeLater(wrappedBuffer( wrappedBuffer(new byte[]{1}).order(order), wrappedBuffer(new byte[]{2}).order(order), - wrappedBuffer(new byte[]{3}).order(order)); + wrappedBuffer(new byte[]{3}).order(order))); assertTrue(BufUtil.equals(a, b)); - b.release(); // Same content, different firstIndex, short length. - a = wrappedBuffer(new byte[] { 1, 2, 3 }).order(order); - b = wrappedBuffer( + a = freeLater(wrappedBuffer(new byte[] { 1, 2, 3 }).order(order)); + b = freeLater(wrappedBuffer( wrappedBuffer(new byte[] { 0, 1, 2, 3, 4 }, 1, 2).order(order), - wrappedBuffer(new byte[] { 0, 1, 2, 3, 4 }, 3, 1).order(order)); + wrappedBuffer(new byte[] { 0, 1, 2, 3, 4 }, 3, 1).order(order))); assertTrue(BufUtil.equals(a, b)); - b.release(); // Different content, same firstIndex, short length. - a = wrappedBuffer(new byte[] { 1, 2, 3 }).order(order); - b = wrappedBuffer( + a = freeLater(wrappedBuffer(new byte[] { 1, 2, 3 }).order(order)); + b = freeLater(wrappedBuffer( wrappedBuffer(new byte[] { 1, 2 }).order(order), - wrappedBuffer(new byte[] { 4 }).order(order)); + wrappedBuffer(new byte[] { 4 }).order(order))); assertFalse(BufUtil.equals(a, b)); - b.release(); // Different content, different firstIndex, short length. - a = wrappedBuffer(new byte[] { 1, 2, 3 }).order(order); - b = wrappedBuffer( + a = freeLater(wrappedBuffer(new byte[] { 1, 2, 3 }).order(order)); + b = freeLater(wrappedBuffer( wrappedBuffer(new byte[] { 0, 1, 2, 4, 5 }, 1, 2).order(order), - wrappedBuffer(new byte[] { 0, 1, 2, 4, 5 }, 3, 1).order(order)); + wrappedBuffer(new byte[] { 0, 1, 2, 4, 5 }, 3, 1).order(order))); assertFalse(BufUtil.equals(a, b)); - b.release(); // Same content, same firstIndex, long length. - a = wrappedBuffer(new byte[] { 1, 2, 3, 4, 5, 6, 7, 8, 9, 10 }).order(order); - b = wrappedBuffer( + a = freeLater(wrappedBuffer(new byte[] { 1, 2, 3, 4, 5, 6, 7, 8, 9, 10 }).order(order)); + b = freeLater(wrappedBuffer( wrappedBuffer(new byte[] { 1, 2, 3 }).order(order), wrappedBuffer(new byte[] { 4, 5, 6 }).order(order), - wrappedBuffer(new byte[] { 7, 8, 9, 10 }).order(order)); + wrappedBuffer(new byte[] { 7, 8, 9, 10 }).order(order))); assertTrue(BufUtil.equals(a, b)); - b.release(); // Same content, different firstIndex, long length. - a = wrappedBuffer(new byte[] { 1, 2, 3, 4, 5, 6, 7, 8, 9, 10 }).order(order); - b = wrappedBuffer( + a = freeLater(wrappedBuffer(new byte[] { 1, 2, 3, 4, 5, 6, 7, 8, 9, 10 }).order(order)); + b = freeLater(wrappedBuffer( wrappedBuffer(new byte[] { 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11}, 1, 5).order(order), - wrappedBuffer(new byte[] { 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11}, 6, 5).order(order)); + wrappedBuffer(new byte[] { 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11}, 6, 5).order(order))); assertTrue(BufUtil.equals(a, b)); - b.release(); // Different content, same firstIndex, long length. - a = wrappedBuffer(new byte[] { 1, 2, 3, 4, 5, 6, 7, 8, 9, 10 }).order(order); - b = wrappedBuffer( + a = freeLater(wrappedBuffer(new byte[] { 1, 2, 3, 4, 5, 6, 7, 8, 9, 10 }).order(order)); + b = freeLater(wrappedBuffer( wrappedBuffer(new byte[] { 1, 2, 3, 4, 6 }).order(order), - wrappedBuffer(new byte[] { 7, 8, 5, 9, 10 }).order(order)); + wrappedBuffer(new byte[] { 7, 8, 5, 9, 10 }).order(order))); assertFalse(BufUtil.equals(a, b)); - b.release(); // Different content, different firstIndex, long length. - a = wrappedBuffer(new byte[] { 1, 2, 3, 4, 5, 6, 7, 8, 9, 10 }).order(order); - b = wrappedBuffer( + a = freeLater(wrappedBuffer(new byte[] { 1, 2, 3, 4, 5, 6, 7, 8, 9, 10 }).order(order)); + b = freeLater(wrappedBuffer( wrappedBuffer(new byte[] { 0, 1, 2, 3, 4, 6, 7, 8, 5, 9, 10, 11 }, 1, 5).order(order), - wrappedBuffer(new byte[] { 0, 1, 2, 3, 4, 6, 7, 8, 5, 9, 10, 11 }, 6, 5).order(order)); + wrappedBuffer(new byte[] { 0, 1, 2, 3, 4, 6, 7, 8, 5, 9, 10, 11 }, 6, 5).order(order))); assertFalse(BufUtil.equals(a, b)); - b.release(); } @Test @@ -441,15 +432,11 @@ public abstract class AbstractCompositeByteBufTest extends // Test for https://github.com/netty/netty/issues/1060 @Test public void testReadWithEmptyCompositeBuffer() { - ByteBuf buf = compositeBuffer(); - try { - int n = 65; - for (int i = 0; i < n; i ++) { - buf.writeByte(1); - assertEquals(1, buf.readByte()); - } - } finally { - buf.release(); + ByteBuf buf = freeLater(compositeBuffer()); + int n = 65; + for (int i = 0; i < n; i ++) { + buf.writeByte(1); + assertEquals(1, buf.readByte()); } } } diff --git a/buffer/src/test/java/io/netty/buffer/ConsolidationTest.java b/buffer/src/test/java/io/netty/buffer/ConsolidationTest.java index 57eb439cc1..addb356490 100644 --- a/buffer/src/test/java/io/netty/buffer/ConsolidationTest.java +++ b/buffer/src/test/java/io/netty/buffer/ConsolidationTest.java @@ -15,23 +15,26 @@ */ package io.netty.buffer; -import static io.netty.buffer.Unpooled.wrappedBuffer; - import org.junit.Test; + +import static io.netty.buffer.Unpooled.*; import static org.junit.Assert.*; /** * Tests buffer consolidation */ public class ConsolidationTest { - @Test public void shouldWrapInSequence() { ByteBuf currentBuffer = wrappedBuffer(wrappedBuffer("a".getBytes()), wrappedBuffer("=".getBytes())); currentBuffer = wrappedBuffer(currentBuffer, wrappedBuffer("1".getBytes()), wrappedBuffer("&".getBytes())); - - String s = new String(currentBuffer.copy().array()); + + ByteBuf copy = currentBuffer.copy(); + String s = new String(copy.array()); assertEquals("a=1&", s); + + currentBuffer.release(); + copy.release(); } @Test @@ -51,7 +54,11 @@ public class ConsolidationTest { currentBuffer = wrappedBuffer(currentBuffer, wrappedBuffer("e".getBytes()), wrappedBuffer("=".getBytes())); currentBuffer = wrappedBuffer(currentBuffer, wrappedBuffer("5".getBytes()), wrappedBuffer("&".getBytes())); - String s = new String(currentBuffer.copy().array()); + ByteBuf copy = currentBuffer.copy(); + String s = new String(copy.array()); assertEquals("a=1&b=2&c=3&d=4&e=5&", s); + + currentBuffer.release(); + copy.release(); } }