From 31ef2370854734e61881cd6ad99820979fb0aecc Mon Sep 17 00:00:00 2001 From: Norman Maurer Date: Fri, 16 Oct 2015 13:38:10 +0200 Subject: [PATCH] Always return a real slice even when the length is 0 Motivation: We need to always return a real slice even when the requested length is 0. This is needed as otherwise we not correctly share the reference count and so may leak a buffer if the user call release() on the returned slice and expect it to decrement the reference count of the "parent" buffer. Modifications: - Always return a real slice - Add unit test for the bug. Result: No more leak possible when a user requests a slice of length 0 of a SlicedByteBuf. --- .../java/io/netty/buffer/SlicedByteBuf.java | 3 --- .../io/netty/buffer/SlicedByteBufTest.java | 25 ++++++++++++++++++- 2 files changed, 24 insertions(+), 4 deletions(-) diff --git a/buffer/src/main/java/io/netty/buffer/SlicedByteBuf.java b/buffer/src/main/java/io/netty/buffer/SlicedByteBuf.java index efecfd014a..f5fd4d5469 100644 --- a/buffer/src/main/java/io/netty/buffer/SlicedByteBuf.java +++ b/buffer/src/main/java/io/netty/buffer/SlicedByteBuf.java @@ -153,9 +153,6 @@ public class SlicedByteBuf extends AbstractDerivedByteBuf { @Override public ByteBuf slice(int index, int length) { checkIndex(index, length); - if (length == 0) { - return Unpooled.EMPTY_BUFFER; - } return buffer.slice(idx(index), length); } diff --git a/buffer/src/test/java/io/netty/buffer/SlicedByteBufTest.java b/buffer/src/test/java/io/netty/buffer/SlicedByteBufTest.java index 7aecaadd62..7f139c44d5 100644 --- a/buffer/src/test/java/io/netty/buffer/SlicedByteBufTest.java +++ b/buffer/src/test/java/io/netty/buffer/SlicedByteBufTest.java @@ -20,7 +20,7 @@ import org.junit.Test; import java.io.IOException; import java.util.Random; -import static org.junit.Assert.assertEquals; +import static org.junit.Assert.*; /** * Tests sliced channel buffers @@ -137,4 +137,27 @@ public class SlicedByteBufTest extends AbstractByteBufTest { wrapped.release(); } } + + @Test + public void sliceEmptyNotLeak() { + ByteBuf buffer = Unpooled.buffer(8).retain(); + assertEquals(2, buffer.refCnt()); + + ByteBuf slice1 = buffer.slice(); + assertEquals(2, slice1.refCnt()); + + ByteBuf slice2 = slice1.slice(); + assertEquals(2, slice2.refCnt()); + + assertFalse(slice2.release()); + assertEquals(1, buffer.refCnt()); + assertEquals(1, slice1.refCnt()); + assertEquals(1, slice2.refCnt()); + + assertTrue(slice2.release()); + + assertEquals(0, buffer.refCnt()); + assertEquals(0, slice1.refCnt()); + assertEquals(0, slice2.refCnt()); + } }