Fix regression in CompositeByteBuf.discard*ReadBytes() (#9068)

Motivation:

1f93bd3 introduced a regression that could lead to not have the lastAccessed field correctly null'ed out when the endOffset of the internal Component == CompositeByteBuf.readerIndex()

Modifications:

- Correctly null out the lastAccessed field in any case
- Add unit tests

Result:

Fixes regression in CompositeByteBuf.discard*ReadBytes()
This commit is contained in:
Norman Maurer 2019-04-17 18:03:08 +02:00
parent 7e469b764e
commit 7e6d1bbcfd
2 changed files with 68 additions and 2 deletions

View File

@ -1762,7 +1762,7 @@ public class CompositeByteBuf extends AbstractReferenceCountedByteBuf implements
return this; // Nothing to discard return this; // Nothing to discard
} }
Component la = lastAccessed; Component la = lastAccessed;
if (la != null && la.endOffset < readerIndex) { if (la != null && la.endOffset <= readerIndex) {
lastAccessed = null; lastAccessed = null;
} }
removeCompRange(0, firstComponentId); removeCompRange(0, firstComponentId);
@ -1816,7 +1816,7 @@ public class CompositeByteBuf extends AbstractReferenceCountedByteBuf implements
c.slice = slice.slice(trimmedBytes, c.length()); c.slice = slice.slice(trimmedBytes, c.length());
} }
Component la = lastAccessed; Component la = lastAccessed;
if (la != null && la.endOffset < readerIndex) { if (la != null && la.endOffset <= readerIndex) {
lastAccessed = null; lastAccessed = null;
} }

View File

@ -1433,4 +1433,70 @@ public abstract class AbstractCompositeByteBufTest extends AbstractByteBufTest {
assertTrue(buf.release()); assertTrue(buf.release());
} }
@Test
public void testDiscardSomeReadBytesCorrectlyUpdatesLastAccessed() {
testDiscardCorrectlyUpdatesLastAccessed(true);
}
@Test
public void testDiscardReadBytesCorrectlyUpdatesLastAccessed() {
testDiscardCorrectlyUpdatesLastAccessed(false);
}
private static void testDiscardCorrectlyUpdatesLastAccessed(boolean discardSome) {
CompositeByteBuf cbuf = compositeBuffer();
List<ByteBuf> buffers = new ArrayList<ByteBuf>(4);
for (int i = 0; i < 4; i++) {
ByteBuf buf = buffer().writeInt(i);
cbuf.addComponent(true, buf);
buffers.add(buf);
}
// Skip the first 2 bytes which means even if we call discard*ReadBytes() later we can no drop the first
// component as it is still used.
cbuf.skipBytes(2);
if (discardSome) {
cbuf.discardSomeReadBytes();
} else {
cbuf.discardReadBytes();
}
assertEquals(4, cbuf.numComponents());
// Now skip 3 bytes which means we should be able to drop the first component on the next discard*ReadBytes()
// call.
cbuf.skipBytes(3);
if (discardSome) {
cbuf.discardSomeReadBytes();
} else {
cbuf.discardReadBytes();
}
assertEquals(3, cbuf.numComponents());
// Now skip again 3 bytes which should bring our readerIndex == start of the 3 component.
cbuf.skipBytes(3);
// Read one int (4 bytes) which should bring our readerIndex == start of the 4 component.
assertEquals(2, cbuf.readInt());
if (discardSome) {
cbuf.discardSomeReadBytes();
} else {
cbuf.discardReadBytes();
}
// Now all except the last component should have been dropped / released.
assertEquals(1, cbuf.numComponents());
assertEquals(3, cbuf.readInt());
if (discardSome) {
cbuf.discardSomeReadBytes();
} else {
cbuf.discardReadBytes();
}
assertEquals(0, cbuf.numComponents());
// These should have been released already.
for (ByteBuf buffer: buffers) {
assertEquals(0, buffer.refCnt());
}
assertTrue(cbuf.release());
}
} }