andreacavalli/netty5
1
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

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

Browse Source 
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 committed by GitHub
parent baab215f66
commit e01c4bce08
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 68 additions and 2 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
buffer/src/main/java/io/netty/buffer/CompositeByteBuf.java
View File

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

66
buffer/src/test/java/io/netty/buffer/AbstractCompositeByteBufTest.java
View File

@ -1443,4 +1443,70 @@ public abstract class AbstractCompositeByteBufTest extends AbstractByteBufTest {
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());
}
}