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 d63413754e
commit f3a2c22738
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,12 +219,15 @@ 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;
if (!done) {
long flushedAmount = 0; long flushedAmount = 0;
if (writeSpinCount == -1) { if (writeSpinCount == -1) {
writeSpinCount = config().getWriteSpinCount(); writeSpinCount = config().getWriteSpinCount();
} }
for (int i = writeSpinCount - 1; i >= 0; i--) { for (int i = writeSpinCount - 1; i >= 0; i--) {
long localFlushedAmount = doWriteFileRegion(region); long localFlushedAmount = doWriteFileRegion(region);
if (localFlushedAmount == 0) { if (localFlushedAmount == 0) {
@ -240,6 +243,7 @@ public abstract class AbstractNioByteChannel extends AbstractNioChannel {
} }
in.progress(flushedAmount); in.progress(flushedAmount);
}
if (done) { if (done) {
in.remove(); in.remove();