[#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
This commit is contained in:
Norman Maurer 2015-09-23 11:25:20 +02:00
parent 115d3576be
commit 956a757d37
6 changed files with 31 additions and 14 deletions

View File

@ -101,8 +101,7 @@ public abstract class AbstractByteBuf extends ByteBuf {
"readerIndex: %d, writerIndex: %d (expected: 0 <= readerIndex <= writerIndex <= capacity(%d))", "readerIndex: %d, writerIndex: %d (expected: 0 <= readerIndex <= writerIndex <= capacity(%d))",
readerIndex, writerIndex, capacity())); readerIndex, writerIndex, capacity()));
} }
this.readerIndex = readerIndex; setIndex0(readerIndex, writerIndex);
this.writerIndex = writerIndex;
return this; 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; markedReaderIndex = markedWriterIndex = 0;
} }
} }

View File

@ -51,8 +51,6 @@ abstract class PooledByteBuf<T> extends AbstractReferenceCountedByteBuf {
this.offset = offset; this.offset = offset;
this.length = length; this.length = length;
this.maxLength = maxLength; this.maxLength = maxLength;
setIndex(0, 0);
discardMarks();
tmpNioBuf = null; tmpNioBuf = null;
this.cache = cache; this.cache = cache;
} }
@ -65,11 +63,20 @@ abstract class PooledByteBuf<T> extends AbstractReferenceCountedByteBuf {
memory = chunk.memory; memory = chunk.memory;
offset = 0; offset = 0;
this.length = maxLength = length; this.length = maxLength = length;
setIndex(0, 0);
tmpNioBuf = null; tmpNioBuf = null;
cache = 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 @Override
public final int capacity() { public final int capacity() {
return length; return length;

View File

@ -37,8 +37,7 @@ final class PooledDirectByteBuf extends PooledByteBuf<ByteBuffer> {
static PooledDirectByteBuf newInstance(int maxCapacity) { static PooledDirectByteBuf newInstance(int maxCapacity) {
PooledDirectByteBuf buf = RECYCLER.get(); PooledDirectByteBuf buf = RECYCLER.get();
buf.setRefCnt(1); buf.reuse(maxCapacity);
buf.maxCapacity(maxCapacity);
return buf; return buf;
} }

View File

@ -36,8 +36,7 @@ final class PooledHeapByteBuf extends PooledByteBuf<byte[]> {
static PooledHeapByteBuf newInstance(int maxCapacity) { static PooledHeapByteBuf newInstance(int maxCapacity) {
PooledHeapByteBuf buf = RECYCLER.get(); PooledHeapByteBuf buf = RECYCLER.get();
buf.setRefCnt(1); buf.reuse(maxCapacity);
buf.maxCapacity(maxCapacity);
return buf; return buf;
} }

View File

@ -41,8 +41,7 @@ final class PooledUnsafeDirectByteBuf extends PooledByteBuf<ByteBuffer> {
static PooledUnsafeDirectByteBuf newInstance(int maxCapacity) { static PooledUnsafeDirectByteBuf newInstance(int maxCapacity) {
PooledUnsafeDirectByteBuf buf = RECYCLER.get(); PooledUnsafeDirectByteBuf buf = RECYCLER.get();
buf.setRefCnt(1); buf.reuse(maxCapacity);
buf.maxCapacity(maxCapacity);
return buf; return buf;
} }

View File

@ -40,7 +40,16 @@ public abstract class AbstractPooledByteBufTest extends AbstractByteBufTest {
@Test @Test
public void testDiscardMarks() { 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.writeShort(1);
buf.skipBytes(1); buf.skipBytes(1);
@ -49,7 +58,7 @@ public abstract class AbstractPooledByteBufTest extends AbstractByteBufTest {
buf.markWriterIndex(); buf.markWriterIndex();
assertTrue(buf.release()); assertTrue(buf.release());
ByteBuf buf2 = newBuffer(4); ByteBuf buf2 = newBuffer(capacity);
assertSame(unwrapIfNeeded(buf), unwrapIfNeeded(buf2)); assertSame(unwrapIfNeeded(buf), unwrapIfNeeded(buf2));