From 956a757d37c52d550aebdbfaf3d68863baa2a765 Mon Sep 17 00:00:00 2001 From: Norman Maurer Date: Wed, 23 Sep 2015 11:25:20 +0200 Subject: [PATCH] [#3789] Correctly reset markers for all allocations when using PooledByteBufAllocator Motivation: We need to ensure all markers are reset when doing an allocation via the PooledByteBufAllocator. This was not the always the case. Modifications: Move all logic that needs to get executed when reuse a PooledByteBuf into one place and call it. Result: Correct behavior --- .../main/java/io/netty/buffer/AbstractByteBuf.java | 10 +++++++--- .../main/java/io/netty/buffer/PooledByteBuf.java | 13 ++++++++++--- .../java/io/netty/buffer/PooledDirectByteBuf.java | 3 +-- .../java/io/netty/buffer/PooledHeapByteBuf.java | 3 +-- .../io/netty/buffer/PooledUnsafeDirectByteBuf.java | 3 +-- .../io/netty/buffer/AbstractPooledByteBufTest.java | 13 +++++++++++-- 6 files changed, 31 insertions(+), 14 deletions(-) diff --git a/buffer/src/main/java/io/netty/buffer/AbstractByteBuf.java b/buffer/src/main/java/io/netty/buffer/AbstractByteBuf.java index f4d69b8f2a..2fab7b8a39 100644 --- a/buffer/src/main/java/io/netty/buffer/AbstractByteBuf.java +++ b/buffer/src/main/java/io/netty/buffer/AbstractByteBuf.java @@ -101,8 +101,7 @@ public abstract class AbstractByteBuf extends ByteBuf { "readerIndex: %d, writerIndex: %d (expected: 0 <= readerIndex <= writerIndex <= capacity(%d))", readerIndex, writerIndex, capacity())); } - this.readerIndex = readerIndex; - this.writerIndex = writerIndex; + setIndex0(readerIndex, writerIndex); return this; } @@ -1152,7 +1151,12 @@ public abstract class AbstractByteBuf extends ByteBuf { } } - void discardMarks() { + final void setIndex0(int readerIndex, int writerIndex) { + this.readerIndex = readerIndex; + this.writerIndex = writerIndex; + } + + final 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 index 006358b7fd..6b348e4a35 100644 --- a/buffer/src/main/java/io/netty/buffer/PooledByteBuf.java +++ b/buffer/src/main/java/io/netty/buffer/PooledByteBuf.java @@ -51,8 +51,6 @@ abstract class PooledByteBuf extends AbstractReferenceCountedByteBuf { this.offset = offset; this.length = length; this.maxLength = maxLength; - setIndex(0, 0); - discardMarks(); tmpNioBuf = null; this.cache = cache; } @@ -65,11 +63,20 @@ abstract class PooledByteBuf extends AbstractReferenceCountedByteBuf { memory = chunk.memory; offset = 0; this.length = maxLength = length; - setIndex(0, 0); tmpNioBuf = null; cache = null; } + /** + * Method must be called before reuse this {@link PooledByteBufAllocator} + */ + final void reuse(int maxCapacity) { + maxCapacity(maxCapacity); + setRefCnt(1); + setIndex0(0, 0); + discardMarks(); + } + @Override public final int capacity() { return length; diff --git a/buffer/src/main/java/io/netty/buffer/PooledDirectByteBuf.java b/buffer/src/main/java/io/netty/buffer/PooledDirectByteBuf.java index bd07ddd489..94abd7f1cc 100644 --- a/buffer/src/main/java/io/netty/buffer/PooledDirectByteBuf.java +++ b/buffer/src/main/java/io/netty/buffer/PooledDirectByteBuf.java @@ -37,8 +37,7 @@ final class PooledDirectByteBuf extends PooledByteBuf { static PooledDirectByteBuf newInstance(int maxCapacity) { PooledDirectByteBuf buf = RECYCLER.get(); - buf.setRefCnt(1); - buf.maxCapacity(maxCapacity); + buf.reuse(maxCapacity); return buf; } diff --git a/buffer/src/main/java/io/netty/buffer/PooledHeapByteBuf.java b/buffer/src/main/java/io/netty/buffer/PooledHeapByteBuf.java index f873dbe7cf..77d78985a3 100644 --- a/buffer/src/main/java/io/netty/buffer/PooledHeapByteBuf.java +++ b/buffer/src/main/java/io/netty/buffer/PooledHeapByteBuf.java @@ -36,8 +36,7 @@ final class PooledHeapByteBuf extends PooledByteBuf { static PooledHeapByteBuf newInstance(int maxCapacity) { PooledHeapByteBuf buf = RECYCLER.get(); - buf.setRefCnt(1); - buf.maxCapacity(maxCapacity); + buf.reuse(maxCapacity); return buf; } diff --git a/buffer/src/main/java/io/netty/buffer/PooledUnsafeDirectByteBuf.java b/buffer/src/main/java/io/netty/buffer/PooledUnsafeDirectByteBuf.java index c9a7c8bc56..96f2792496 100644 --- a/buffer/src/main/java/io/netty/buffer/PooledUnsafeDirectByteBuf.java +++ b/buffer/src/main/java/io/netty/buffer/PooledUnsafeDirectByteBuf.java @@ -41,8 +41,7 @@ final class PooledUnsafeDirectByteBuf extends PooledByteBuf { static PooledUnsafeDirectByteBuf newInstance(int maxCapacity) { PooledUnsafeDirectByteBuf buf = RECYCLER.get(); - buf.setRefCnt(1); - buf.maxCapacity(maxCapacity); + buf.reuse(maxCapacity); return buf; } diff --git a/buffer/src/test/java/io/netty/buffer/AbstractPooledByteBufTest.java b/buffer/src/test/java/io/netty/buffer/AbstractPooledByteBufTest.java index fec371bc6e..5469a4d143 100644 --- a/buffer/src/test/java/io/netty/buffer/AbstractPooledByteBufTest.java +++ b/buffer/src/test/java/io/netty/buffer/AbstractPooledByteBufTest.java @@ -40,7 +40,16 @@ public abstract class AbstractPooledByteBufTest extends AbstractByteBufTest { @Test public void testDiscardMarks() { - ByteBuf buf = newBuffer(4); + testDiscardMarks(4); + } + + @Test + public void testDiscardMarksUnpooled() { + testDiscardMarks(32 * 1024 * 1024); + } + + private void testDiscardMarks(int capacity) { + ByteBuf buf = newBuffer(capacity); buf.writeShort(1); buf.skipBytes(1); @@ -49,7 +58,7 @@ public abstract class AbstractPooledByteBufTest extends AbstractByteBufTest { buf.markWriterIndex(); assertTrue(buf.release()); - ByteBuf buf2 = newBuffer(4); + ByteBuf buf2 = newBuffer(capacity); assertSame(unwrapIfNeeded(buf), unwrapIfNeeded(buf2));