[#2580] [#2587] Fix buffer corruption regression when ByteBuf.order(LITTLE_ENDIAN) is used

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:
Norman Maurer 2014-06-20 18:19:45 +02:00
parent 3d3ec4753d
commit 371f8066d2
5 changed files with 28 additions and 6 deletions

View File

@ -384,6 +384,6 @@ final class PooledUnsafeDirectByteBuf extends PooledByteBuf<ByteBuffer> {
@Override
protected SwappedByteBuf newSwappedByteBuf() {
return new UnsafeDirectSwappedByteBuf(this, memoryAddress);
return new UnsafeDirectSwappedByteBuf(this);
}
}

View File

@ -517,6 +517,6 @@ public class UnpooledUnsafeDirectByteBuf extends AbstractReferenceCountedByteBuf
@Override
protected SwappedByteBuf newSwappedByteBuf() {
return new UnsafeDirectSwappedByteBuf(this, memoryAddress);
return new UnsafeDirectSwappedByteBuf(this);
}
}

View File

@ -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

View File

@ -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);

View File

@ -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
}
}