Always return SliceByteBuf on slice(...) to eliminate possible leak

normanmaurer · normanmaurer · commit fa079baf2962 · 2014-12-22T11:15:28.000+01:00
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 &gt; 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.
diff --git a/buffer/src/main/java/io/netty/buffer/AbstractByteBuf.java b/buffer/src/main/java/io/netty/buffer/AbstractByteBuf.java
@@ -931,10 +931,6 @@ public ByteBuf slice() {
 
     @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
@@ -2456,6 +2456,22 @@ public void testMemoryAddressAfterRelease() {
         }
     }
 
+    @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

Original file line number	Diff line number	Diff line change
`@@ -931,10 +931,6 @@ public ByteBuf slice() {`
`931`	`931`
`932`	`932`	`@Override`
`933`	`933`	`public ByteBuf slice(int index, int length) {`
`934`		`- if (length == 0) {`
`935`		`- return Unpooled.EMPTY_BUFFER;`
`936`		`- }`
`937`		`-`
`938`	`934`	`return new SlicedByteBuf(this, index, length);`
`939`	`935`	`}`
`940`	`936`