Motivation:
To improve the speed of ByteBuf with order LITTLE_ENDIAN and where the native order is also LITTLE_ENDIAN (intel) we introduces a new special SwappedByteBuf before in commit 4ad3984c8b
. Unfortunally the commit has a flaw which does not handle correctly the case when a ByteBuf expands. This was caused because the memoryAddress was cached and never changed again even if the underlying buffer expanded. This can lead to corrupt data or even to SEGFAULT the JVM if you are lucky enough.
Modification:
Always lookup the actual memoryAddress of the wrapped ByteBuf.
Result:
No more data-corruption for ByteBuf with order LITTLE_ENDIAN and no JVM crashes.
This commit is contained in:
parent
f341562b91
commit
4e36c0ae96
@ -384,6 +384,6 @@ final class PooledUnsafeDirectByteBuf extends PooledByteBuf<ByteBuffer> {
|
||||
|
||||
@Override
|
||||
protected SwappedByteBuf newSwappedByteBuf() {
|
||||
return new UnsafeDirectSwappedByteBuf(this, memoryAddress);
|
||||
return new UnsafeDirectSwappedByteBuf(this);
|
||||
}
|
||||
}
|
||||
|
@ -517,6 +517,6 @@ public class UnpooledUnsafeDirectByteBuf extends AbstractReferenceCountedByteBuf
|
||||
|
||||
@Override
|
||||
protected SwappedByteBuf newSwappedByteBuf() {
|
||||
return new UnsafeDirectSwappedByteBuf(this, memoryAddress);
|
||||
return new UnsafeDirectSwappedByteBuf(this);
|
||||
}
|
||||
}
|
||||
|
@ -27,17 +27,19 @@ final class UnsafeDirectSwappedByteBuf extends SwappedByteBuf {
|
||||
private static final boolean NATIVE_ORDER = ByteOrder.nativeOrder() == ByteOrder.BIG_ENDIAN;
|
||||
private final boolean nativeByteOrder;
|
||||
private final AbstractByteBuf wrapped;
|
||||
private final long memoryAddress;
|
||||
|
||||
UnsafeDirectSwappedByteBuf(AbstractByteBuf buf, long memoryAddress) {
|
||||
UnsafeDirectSwappedByteBuf(AbstractByteBuf buf) {
|
||||
super(buf);
|
||||
wrapped = buf;
|
||||
this.memoryAddress = memoryAddress;
|
||||
nativeByteOrder = NATIVE_ORDER == (order() == ByteOrder.BIG_ENDIAN);
|
||||
}
|
||||
|
||||
private long addr(int index) {
|
||||
return memoryAddress + index;
|
||||
// We need to call wrapped.memoryAddress() everytime and NOT cache it as it may change if the buffer expand.
|
||||
// See:
|
||||
// - https://github.com/netty/netty/issues/2587
|
||||
// - https://github.com/netty/netty/issues/2580
|
||||
return wrapped.memoryAddress() + index;
|
||||
}
|
||||
|
||||
@Override
|
||||
|
@ -1943,6 +1943,18 @@ public abstract class AbstractByteBufTest {
|
||||
}
|
||||
}
|
||||
|
||||
// See:
|
||||
// - https://github.com/netty/netty/issues/2587
|
||||
// - https://github.com/netty/netty/issues/2580
|
||||
@Test
|
||||
public void testLittleEndianWithExpand() {
|
||||
ByteBuf buffer = releaseLater(newBuffer(0)).order(LITTLE_ENDIAN);
|
||||
System.out.println(buffer.getClass());
|
||||
buffer.writeInt(0x12345678);
|
||||
System.out.println(ByteBufUtil.hexDump(buffer));
|
||||
assertEquals("78563412", ByteBufUtil.hexDump(buffer));
|
||||
}
|
||||
|
||||
static final class TestGatheringByteChannel implements GatheringByteChannel {
|
||||
private final ByteArrayOutputStream out = new ByteArrayOutputStream();
|
||||
private final WritableByteChannel channel = Channels.newChannel(out);
|
||||
|
@ -15,6 +15,8 @@
|
||||
*/
|
||||
package io.netty.buffer;
|
||||
|
||||
import static io.netty.buffer.Unpooled.LITTLE_ENDIAN;
|
||||
import static io.netty.util.ReferenceCountUtil.releaseLater;
|
||||
import static org.junit.Assert.*;
|
||||
|
||||
import java.util.Random;
|
||||
@ -94,4 +96,10 @@ public class SlicedByteBufTest extends AbstractByteBufTest {
|
||||
public void testNioBufferExposeOnlyRegion() {
|
||||
super.testNioBufferExposeOnlyRegion();
|
||||
}
|
||||
|
||||
@Test
|
||||
@Override
|
||||
public void testLittleEndianWithExpand() {
|
||||
// ignore for SlicedByteBuf
|
||||
}
|
||||
}
|
||||
|
Loading…
Reference in New Issue
Block a user