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

[#2761] ChannelOutboundBuffer can cause data-corruption because of caching ByteBuffers

Browse Source 
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.
This commit is contained in:
Norman Maurer 2014-08-13 11:06:03 +02:00
parent ba58f949e8
commit c7be4e4937
1 changed files with 3 additions and 25 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

28
transport/src/main/java/io/netty/channel/ChannelOutboundBuffer.java
View File

@ -352,31 +352,17 @@ public final class ChannelOutboundBuffer {
if (readableBytes > 0) { if (readableBytes > 0) {
nioBufferSize += readableBytes; nioBufferSize += readableBytes;
int count = entry.count; int count = buf.nioBufferCount();
if (count == -1) {
//noinspection ConstantValueVariableUse
entry.count = count = buf.nioBufferCount();
}
int neededSpace = nioBufferCount + count; int neededSpace = nioBufferCount + count;
if (neededSpace > nioBuffers.length) { if (neededSpace > nioBuffers.length) {
nioBuffers = expandNioBufferArray(nioBuffers, neededSpace, nioBufferCount); nioBuffers = expandNioBufferArray(nioBuffers, neededSpace, nioBufferCount);
NIO_BUFFERS.set(threadLocalMap, nioBuffers); NIO_BUFFERS.set(threadLocalMap, nioBuffers);
} }
if (count == 1) { if (count == 1) {
ByteBuffer nioBuf = entry.buf; ByteBuffer nioBuf = buf.internalNioBuffer(readerIndex, readableBytes);
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);
}
nioBuffers[nioBufferCount ++] = nioBuf; nioBuffers[nioBufferCount ++] = nioBuf;
} else { } else {
ByteBuffer[] nioBufs = entry.bufs; ByteBuffer[] nioBufs = buf.nioBuffers(readerIndex, readableBytes);
if (nioBufs == null) {
// cached ByteBuffers as they may be expensive to create in terms
// of Object allocation
entry.bufs = nioBufs = buf.nioBuffers();
}
nioBufferCount = fillBufferArray(nioBufs, nioBuffers, nioBufferCount); nioBufferCount = fillBufferArray(nioBufs, nioBuffers, nioBufferCount);
} }
} }
@ -586,13 +572,10 @@ public final class ChannelOutboundBuffer {
private final Handle handle; private final Handle handle;
Entry next; Entry next;
Object msg; Object msg;
ByteBuffer[] bufs;
ByteBuffer buf;
ChannelPromise promise; ChannelPromise promise;
long progress; long progress;
long total; long total;
int pendingSize; int pendingSize;
int count = -1;
boolean cancelled; boolean cancelled;
private Entry(Handle handle) { private Entry(Handle handle) {
@ -620,8 +603,6 @@ public final class ChannelOutboundBuffer {
pendingSize = 0; pendingSize = 0;
total = 0; total = 0;
progress = 0; progress = 0;
bufs = null;
buf = null;
return pSize; return pSize;
} }
return 0; return 0;
@ -629,14 +610,11 @@ public final class ChannelOutboundBuffer {
void recycle() { void recycle() {
next = null; next = null;
bufs = null;
buf = null;
msg = null; msg = null;
promise = null; promise = null;
progress = 0; progress = 0;
total = 0; total = 0;
pendingSize = 0; pendingSize = 0;
count = -1;
cancelled = false; cancelled = false;
RECYCLER.recycle(this, handle); RECYCLER.recycle(this, handle);
} }