Fix an infinite loop when writing a zero-length FileRegion

Related: #2964

Motivation:

Writing a zero-length FileRegion to an NIO channel will lead to an
infinite loop.

Modification:

- Do not write a zero-length FileRegion by protecting with proper 'if'.
- Update the testsuite

Result:

Another bug fixed
This commit is contained in:
Trustin Lee 2014-10-17 16:04:37 +09:00
parent 582f2e82ec
commit 0eb059bf58
2 changed files with 29 additions and 18 deletions

View File

@ -134,16 +134,23 @@ public class SocketFileRegionTest extends AbstractSocketTest {
Channel cc = cb.connect().sync().channel(); Channel cc = cb.connect().sync().channel();
FileRegion region = new DefaultFileRegion( FileRegion region = new DefaultFileRegion(
new FileInputStream(file).getChannel(), startOffset, data.length - bufferSize); new FileInputStream(file).getChannel(), startOffset, data.length - bufferSize);
// Do write ByteBuf and FileRegion to be sure that mixed writes work FileRegion emptyRegion = new DefaultFileRegion(new FileInputStream(file).getChannel(), 0, 0);
// Do write ByteBuf and then FileRegion to ensure that mixed writes work
// Also, write an empty FileRegion to test if writing an empty FileRegion does not cause any issues.
// //
// See https://github.com/netty/netty/issues/2769 // See https://github.com/netty/netty/issues/2769
// https://github.com/netty/netty/issues/2964
if (voidPromise) { if (voidPromise) {
assertEquals(cc.voidPromise(), cc.write(Unpooled.wrappedBuffer(data, 0, bufferSize), cc.voidPromise())); assertEquals(cc.voidPromise(), cc.write(Unpooled.wrappedBuffer(data, 0, bufferSize), cc.voidPromise()));
assertEquals(cc.voidPromise(), cc.write(emptyRegion, cc.voidPromise()));
assertEquals(cc.voidPromise(), cc.writeAndFlush(region, cc.voidPromise())); assertEquals(cc.voidPromise(), cc.writeAndFlush(region, cc.voidPromise()));
} else { } else {
assertNotEquals(cc.voidPromise(), cc.write(Unpooled.wrappedBuffer(data, 0, bufferSize))); assertNotEquals(cc.voidPromise(), cc.write(Unpooled.wrappedBuffer(data, 0, bufferSize)));
assertNotEquals(cc.voidPromise(), cc.write(emptyRegion));
assertNotEquals(cc.voidPromise(), cc.writeAndFlush(region)); assertNotEquals(cc.voidPromise(), cc.writeAndFlush(region));
} }
while (sh.counter < data.length) { while (sh.counter < data.length) {
if (sh.exception.get() != null) { if (sh.exception.get() != null) {
break; break;

View File

@ -219,27 +219,31 @@ public abstract class AbstractNioByteChannel extends AbstractNioChannel {
} }
} else if (msg instanceof FileRegion) { } else if (msg instanceof FileRegion) {
FileRegion region = (FileRegion) msg; FileRegion region = (FileRegion) msg;
boolean done = region.transfered() >= region.count();
boolean setOpWrite = false; boolean setOpWrite = false;
boolean done = false;
long flushedAmount = 0; if (!done) {
if (writeSpinCount == -1) { long flushedAmount = 0;
writeSpinCount = config().getWriteSpinCount(); if (writeSpinCount == -1) {
} writeSpinCount = config().getWriteSpinCount();
for (int i = writeSpinCount - 1; i >= 0; i --) {
long localFlushedAmount = doWriteFileRegion(region);
if (localFlushedAmount == 0) {
setOpWrite = true;
break;
} }
flushedAmount += localFlushedAmount; for (int i = writeSpinCount - 1; i >= 0; i--) {
if (region.transfered() >= region.count()) { long localFlushedAmount = doWriteFileRegion(region);
done = true; if (localFlushedAmount == 0) {
break; setOpWrite = true;
} break;
} }
in.progress(flushedAmount); flushedAmount += localFlushedAmount;
if (region.transfered() >= region.count()) {
done = true;
break;
}
}
in.progress(flushedAmount);
}
if (done) { if (done) {
in.remove(); in.remove();