Reset markers when obtain PooledByteBuf.

normanmaurer · normanmaurer · commit 979fdfd3d3e6 · 2015-05-20T07:29:21.000+02:00
Motivation:

When allocate a PooledByteBuf we need to ensure to also reset the markers for the readerIndex and writerIndex.

Modifications:

- Correct reset the markers
- Add test-case for it

Result:

Correctly reset markers.
diff --git a/buffer/src/main/java/io/netty/buffer/AbstractByteBuf.java b/buffer/src/main/java/io/netty/buffer/AbstractByteBuf.java
@@ -1178,4 +1178,8 @@ protected final void ensureAccessible() {
             throw new IllegalReferenceCountException(0);
         }
     }
+
+    void discardMarks() {
+        markedReaderIndex = markedWriterIndex = 0;
+    }
 }
diff --git a/buffer/src/main/java/io/netty/buffer/PooledByteBuf.java b/buffer/src/main/java/io/netty/buffer/PooledByteBuf.java
@@ -50,6 +50,7 @@ void init(PoolChunk<T> chunk, long handle, int offset, int length, int maxLength
         this.length = length;
         this.maxLength = maxLength;
         setIndex(0, 0);
+        discardMarks();
         tmpNioBuf = null;
         initThread = Thread.currentThread();
     }
diff --git a/buffer/src/test/java/io/netty/buffer/AbstractPooledByteBufTest.java b/buffer/src/test/java/io/netty/buffer/AbstractPooledByteBufTest.java
@@ -0,0 +1,72 @@
+/*
+ * Copyright 2015 The Netty Project
+ *
+ * The Netty Project licenses this file to you under the Apache License,
+ * version 2.0 (the "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at:
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ */
+package io.netty.buffer;
+
+import org.junit.Test;
+
+import static org.junit.Assert.*;
+
+public abstract class AbstractPooledByteBufTest extends AbstractByteBufTest {
+
+    private ByteBuf buffer;
+
+    protected abstract ByteBuf alloc(int length);
+
+    @Override
+    protected ByteBuf newBuffer(int length) {
+        buffer = alloc(length);
+        assertEquals(0, buffer.writerIndex());
+        assertEquals(0, buffer.readerIndex());
+        return buffer;
+    }
+
+    @Override
+    protected ByteBuf[] components() {
+        return new ByteBuf[] { buffer };
+    }
+
+    @Test
+    public void testDiscardMarks() {
+        ByteBuf buf = newBuffer(4);
+        buf.writeShort(1);
+
+        buf.skipBytes(1);
+
+        buf.markReaderIndex();
+        buf.markWriterIndex();
+        assertTrue(buf.release());
+
+        ByteBuf buf2 = newBuffer(4);
+
+        assertSame(unwrapIfNeeded(buf), unwrapIfNeeded(buf2));
+
+        buf2.writeShort(1);
+
+        buf2.resetReaderIndex();
+        buf2.resetWriterIndex();
+
+        assertEquals(0, buf2.readerIndex());
+        assertEquals(0, buf2.writerIndex());
+        assertTrue(buf2.release());
+    }
+
+    private static ByteBuf unwrapIfNeeded(ByteBuf buf) {
+        if (buf instanceof AdvancedLeakAwareByteBuf || buf instanceof SimpleLeakAwareByteBuf) {
+            return buf.unwrap();
+        }
+        return buf;
+    }
+}
diff --git a/buffer/src/test/java/io/netty/buffer/PooledBigEndianDirectByteBufTest.java b/buffer/src/test/java/io/netty/buffer/PooledBigEndianDirectByteBufTest.java
@@ -22,20 +22,12 @@
 /**
  * Tests big-endian direct channel buffers
  */
-public class PooledBigEndianDirectByteBufTest extends AbstractByteBufTest {
-
-    private ByteBuf buffer;
+public class PooledBigEndianDirectByteBufTest extends AbstractPooledByteBufTest {
 
     @Override
-    protected ByteBuf newBuffer(int length) {
-        buffer = PooledByteBufAllocator.DEFAULT.directBuffer(length);
+    protected ByteBuf alloc(int length) {
+        ByteBuf buffer = PooledByteBufAllocator.DEFAULT.directBuffer(length);
         assertSame(ByteOrder.BIG_ENDIAN, buffer.order());
-        assertEquals(0, buffer.writerIndex());
         return buffer;
     }
-
-    @Override
-    protected ByteBuf[] components() {
-        return new ByteBuf[] { buffer };
-    }
 }
diff --git a/buffer/src/test/java/io/netty/buffer/PooledBigEndianHeapByteBufTest.java b/buffer/src/test/java/io/netty/buffer/PooledBigEndianHeapByteBufTest.java
@@ -15,24 +15,13 @@
  */
 package io.netty.buffer;
 
-import static org.junit.Assert.*;
-
 /**
  * Tests big-endian heap channel buffers
  */
-public class PooledBigEndianHeapByteBufTest extends AbstractByteBufTest {
-
-    private ByteBuf buffer;
-
-    @Override
-    protected ByteBuf newBuffer(int length) {
-        buffer = PooledByteBufAllocator.DEFAULT.heapBuffer(length);
-        assertEquals(0, buffer.writerIndex());
-        return buffer;
-    }
+public class PooledBigEndianHeapByteBufTest extends AbstractPooledByteBufTest {
 
     @Override
-    protected ByteBuf[] components() {
-        return new ByteBuf[] { buffer };
+    protected ByteBuf alloc(int length) {
+        return PooledByteBufAllocator.DEFAULT.heapBuffer(length);
     }
 }
diff --git a/buffer/src/test/java/io/netty/buffer/PooledLittleEndianDirectByteBufTest.java b/buffer/src/test/java/io/netty/buffer/PooledLittleEndianDirectByteBufTest.java
@@ -22,20 +22,12 @@
 /**
  * Tests little-endian direct channel buffers
  */
-public class PooledLittleEndianDirectByteBufTest extends AbstractByteBufTest {
-
-    private ByteBuf buffer;
+public class PooledLittleEndianDirectByteBufTest extends AbstractPooledByteBufTest {
 
     @Override
-    protected ByteBuf newBuffer(int length) {
-        buffer = PooledByteBufAllocator.DEFAULT.directBuffer(length).order(ByteOrder.LITTLE_ENDIAN);
+    protected ByteBuf alloc(int length) {
+        ByteBuf buffer = PooledByteBufAllocator.DEFAULT.directBuffer(length).order(ByteOrder.LITTLE_ENDIAN);
         assertSame(ByteOrder.LITTLE_ENDIAN, buffer.order());
-        assertEquals(0, buffer.writerIndex());
         return buffer;
     }
-
-    @Override
-    protected ByteBuf[] components() {
-        return new ByteBuf[] { buffer };
-    }
 }
diff --git a/buffer/src/test/java/io/netty/buffer/PooledLittleEndianHeapByteBufTest.java b/buffer/src/test/java/io/netty/buffer/PooledLittleEndianHeapByteBufTest.java
@@ -22,19 +22,12 @@
 /**
  * Tests little-endian heap channel buffers
  */
-public class PooledLittleEndianHeapByteBufTest extends AbstractByteBufTest {
-
-    private ByteBuf buffer;
+public class PooledLittleEndianHeapByteBufTest extends AbstractPooledByteBufTest {
 
     @Override
-    protected ByteBuf newBuffer(int length) {
-        buffer = PooledByteBufAllocator.DEFAULT.heapBuffer(length).order(ByteOrder.LITTLE_ENDIAN);
-        assertEquals(0, buffer.writerIndex());
+    protected ByteBuf alloc(int length) {
+        ByteBuf buffer = PooledByteBufAllocator.DEFAULT.heapBuffer(length).order(ByteOrder.LITTLE_ENDIAN);
+        assertSame(ByteOrder.LITTLE_ENDIAN, buffer.order());
         return buffer;
     }
-
-    @Override
-    protected ByteBuf[] components() {
-        return new ByteBuf[] { buffer };
-    }
 }

Original file line number	Diff line number	Diff line change
`@@ -1178,4 +1178,8 @@ protected final void ensureAccessible() {`
`1178`	`1178`	`throw new IllegalReferenceCountException(0);`
`1179`	`1179`	`}`
`1180`	`1180`	`}`
	`1181`	`+`
	`1182`	`+ void discardMarks() {`
	`1183`	`+ markedReaderIndex = markedWriterIndex = 0;`
	`1184`	`+ }`
`1181`	`1185`	`}`
Original file line number	Diff line number	Diff line change
`@@ -50,6 +50,7 @@ void init(PoolChunk<T> chunk, long handle, int offset, int length, int maxLength`
`50`	`50`	`this.length = length;`
`51`	`51`	`this.maxLength = maxLength;`
`52`	`52`	`setIndex(0, 0);`
	`53`	`+ discardMarks();`
`53`	`54`	`tmpNioBuf = null;`
`54`	`55`	`initThread = Thread.currentThread();`
`55`	`56`	`}`