From a69a39c84949c6093182858092d57139a128e2bf Mon Sep 17 00:00:00 2001 From: Norman Maurer Date: Thu, 11 Dec 2014 20:50:54 +0100 Subject: [PATCH] Always return SliceByteBuf on slice(...) to eliminate possible leak Motivation: When calling slice(...) on a ByteBuf the returned ByteBuf should be the slice of a ByteBuf and shares it's reference count. This is important as it is perfect legal to use buf.slice(...).release() and have both, the slice and the original ByteBuf released. At the moment this is only the case if the requested slice size is > 0. This makes the behavior inconsistent and so may lead to a memory leak. Modifications: - Never return Unpooled.EMPTY_BUFFER when calling slice(...). - Adding test case for buffer.slice(...).release() and buffer.duplicate(...).release() Result: Consistent behaviour and so no more leaks possible. --- .../java/io/netty/buffer/AbstractByteBuf.java | 4 ---- .../io/netty/buffer/AbstractByteBufTest.java | 16 ++++++++++++++++ 2 files changed, 16 insertions(+), 4 deletions(-) diff --git a/buffer/src/main/java/io/netty/buffer/AbstractByteBuf.java b/buffer/src/main/java/io/netty/buffer/AbstractByteBuf.java index b67d3d1b97..3f97be30c8 100644 --- a/buffer/src/main/java/io/netty/buffer/AbstractByteBuf.java +++ b/buffer/src/main/java/io/netty/buffer/AbstractByteBuf.java @@ -903,10 +903,6 @@ public abstract class AbstractByteBuf extends ByteBuf { @Override public ByteBuf slice(int index, int length) { - if (length == 0) { - return Unpooled.EMPTY_BUFFER; - } - return new SlicedByteBuf(this, index, length); } diff --git a/buffer/src/test/java/io/netty/buffer/AbstractByteBufTest.java b/buffer/src/test/java/io/netty/buffer/AbstractByteBufTest.java index 71f4ae9d42..0eabfef988 100644 --- a/buffer/src/test/java/io/netty/buffer/AbstractByteBufTest.java +++ b/buffer/src/test/java/io/netty/buffer/AbstractByteBufTest.java @@ -2456,6 +2456,22 @@ public abstract class AbstractByteBufTest { } } + @Test + public void testSliceRelease() { + ByteBuf buf = newBuffer(8); + assertEquals(1, buf.refCnt()); + assertTrue(buf.slice().release()); + assertEquals(0, buf.refCnt()); + } + + @Test + public void testDuplicateRelease() { + ByteBuf buf = newBuffer(8); + assertEquals(1, buf.refCnt()); + assertTrue(buf.duplicate().release()); + assertEquals(0, buf.refCnt()); + } + // Test-case trying to reproduce: // https://github.com/netty/netty/issues/2843 @Test