From c7be4e493726ab5694150da3ba4b17a70712ee99 Mon Sep 17 00:00:00 2001 From: Norman Maurer Date: Wed, 13 Aug 2014 11:06:03 +0200 Subject: [PATCH] [#2761] ChannelOutboundBuffer can cause data-corruption because of caching ByteBuffers Motivation: We cache the ByteBuffers in ChannelOutboundBuffer.nioBuffers() for the Entries in the ChannelOutboundBuffer to reduce some overhead. The problem is this can lead to data-corruption if an incomplete write happens and next time we try to do a non-gathering write. To fix this we should remove the caching which does not help a lot anyway and just make the code buggy. Modifications: Remove the caching of ByteBuffers. Result: No more data-corruption. --- .../netty/channel/ChannelOutboundBuffer.java | 28 ++----------------- 1 file changed, 3 insertions(+), 25 deletions(-) diff --git a/transport/src/main/java/io/netty/channel/ChannelOutboundBuffer.java b/transport/src/main/java/io/netty/channel/ChannelOutboundBuffer.java index cdfeb7aaf7..b74e1793f3 100644 --- a/transport/src/main/java/io/netty/channel/ChannelOutboundBuffer.java +++ b/transport/src/main/java/io/netty/channel/ChannelOutboundBuffer.java @@ -352,31 +352,17 @@ public final class ChannelOutboundBuffer { if (readableBytes > 0) { nioBufferSize += readableBytes; - int count = entry.count; - if (count == -1) { - //noinspection ConstantValueVariableUse - entry.count = count = buf.nioBufferCount(); - } + int count = buf.nioBufferCount(); int neededSpace = nioBufferCount + count; if (neededSpace > nioBuffers.length) { nioBuffers = expandNioBufferArray(nioBuffers, neededSpace, nioBufferCount); NIO_BUFFERS.set(threadLocalMap, nioBuffers); } if (count == 1) { - ByteBuffer nioBuf = entry.buf; - if (nioBuf == null) { - // cache ByteBuffer as it may need to create a new ByteBuffer instance if its a - // derived buffer - entry.buf = nioBuf = buf.internalNioBuffer(readerIndex, readableBytes); - } + ByteBuffer nioBuf = buf.internalNioBuffer(readerIndex, readableBytes); nioBuffers[nioBufferCount ++] = nioBuf; } else { - ByteBuffer[] nioBufs = entry.bufs; - if (nioBufs == null) { - // cached ByteBuffers as they may be expensive to create in terms - // of Object allocation - entry.bufs = nioBufs = buf.nioBuffers(); - } + ByteBuffer[] nioBufs = buf.nioBuffers(readerIndex, readableBytes); nioBufferCount = fillBufferArray(nioBufs, nioBuffers, nioBufferCount); } } @@ -586,13 +572,10 @@ public final class ChannelOutboundBuffer { private final Handle handle; Entry next; Object msg; - ByteBuffer[] bufs; - ByteBuffer buf; ChannelPromise promise; long progress; long total; int pendingSize; - int count = -1; boolean cancelled; private Entry(Handle handle) { @@ -620,8 +603,6 @@ public final class ChannelOutboundBuffer { pendingSize = 0; total = 0; progress = 0; - bufs = null; - buf = null; return pSize; } return 0; @@ -629,14 +610,11 @@ public final class ChannelOutboundBuffer { void recycle() { next = null; - bufs = null; - buf = null; msg = null; promise = null; progress = 0; total = 0; pendingSize = 0; - count = -1; cancelled = false; RECYCLER.recycle(this, handle); }