From f3a2c22738c53af5770183ff8a60a49ae21f62eb Mon Sep 17 00:00:00 2001
From: Trustin Lee <t@motd.kr>
Date: Fri, 17 Oct 2014 16:04:37 +0900
Subject: [PATCH] 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
---
 .../socket/SocketFileRegionTest.java          |  9 ++++-
 .../channel/nio/AbstractNioByteChannel.java   | 38 ++++++++++---------
 2 files changed, 29 insertions(+), 18 deletions(-)

diff --git a/testsuite/src/test/java/io/netty/testsuite/transport/socket/SocketFileRegionTest.java b/testsuite/src/test/java/io/netty/testsuite/transport/socket/SocketFileRegionTest.java
index 2b01959081..bcee780e0f 100644
--- a/testsuite/src/test/java/io/netty/testsuite/transport/socket/SocketFileRegionTest.java
+++ b/testsuite/src/test/java/io/netty/testsuite/transport/socket/SocketFileRegionTest.java
@@ -134,16 +134,23 @@ public class SocketFileRegionTest extends AbstractSocketTest {
         Channel cc = cb.connect().sync().channel();
         FileRegion region = new DefaultFileRegion(
                 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
+        //     https://github.com/netty/netty/issues/2964
         if (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()));
         } else {
             assertNotEquals(cc.voidPromise(), cc.write(Unpooled.wrappedBuffer(data, 0, bufferSize)));
+            assertNotEquals(cc.voidPromise(), cc.write(emptyRegion));
             assertNotEquals(cc.voidPromise(), cc.writeAndFlush(region));
         }
+
         while (sh.counter < data.length) {
             if (sh.exception.get() != null) {
                 break;
diff --git a/transport/src/main/java/io/netty/channel/nio/AbstractNioByteChannel.java b/transport/src/main/java/io/netty/channel/nio/AbstractNioByteChannel.java
index 977614ca66..ccf8f54a82 100644
--- a/transport/src/main/java/io/netty/channel/nio/AbstractNioByteChannel.java
+++ b/transport/src/main/java/io/netty/channel/nio/AbstractNioByteChannel.java
@@ -219,27 +219,31 @@ public abstract class AbstractNioByteChannel extends AbstractNioChannel {
                 }
             } else if (msg instanceof FileRegion) {
                 FileRegion region = (FileRegion) msg;
+                boolean done = region.transfered() >= region.count();
                 boolean setOpWrite = false;
-                boolean done = false;
-                long flushedAmount = 0;
-                if (writeSpinCount == -1) {
-                    writeSpinCount = config().getWriteSpinCount();
-                }
-                for (int i = writeSpinCount - 1; i >= 0; i --) {
-                    long localFlushedAmount = doWriteFileRegion(region);
-                    if (localFlushedAmount == 0) {
-                        setOpWrite = true;
-                        break;
+
+                if (!done) {
+                    long flushedAmount = 0;
+                    if (writeSpinCount == -1) {
+                        writeSpinCount = config().getWriteSpinCount();
                     }
 
-                    flushedAmount += localFlushedAmount;
-                    if (region.transfered() >= region.count()) {
-                        done = true;
-                        break;
-                    }
-                }
+                    for (int i = writeSpinCount - 1; i >= 0; i--) {
+                        long localFlushedAmount = doWriteFileRegion(region);
+                        if (localFlushedAmount == 0) {
+                            setOpWrite = true;
+                            break;
+                        }
 
-                in.progress(flushedAmount);
+                        flushedAmount += localFlushedAmount;
+                        if (region.transfered() >= region.count()) {
+                            done = true;
+                            break;
+                        }
+                    }
+
+                    in.progress(flushedAmount);
+                }
 
                 if (done) {
                     in.remove();