From 3e8a1ed6119bf73453ac1197fe8ad4cc09cce8fb Mon Sep 17 00:00:00 2001
From: Trustin Lee <t@motd.kr>
Date: Tue, 17 Dec 2013 09:51:41 +0900
Subject: [PATCH] Fix a race condition where flush() can be triggered before
 write()

.. when a handler overrides write() but not flush().
---
 .../channel/DefaultChannelHandlerContext.java     | 11 +++++++----
 .../local/LocalTransportThreadModelTest.java      | 15 +++++++++++++--
 2 files changed, 20 insertions(+), 6 deletions(-)

diff --git a/transport/src/main/java/io/netty/channel/DefaultChannelHandlerContext.java b/transport/src/main/java/io/netty/channel/DefaultChannelHandlerContext.java
index 738d5cea6b..3170740ca4 100644
--- a/transport/src/main/java/io/netty/channel/DefaultChannelHandlerContext.java
+++ b/transport/src/main/java/io/netty/channel/DefaultChannelHandlerContext.java
@@ -157,10 +157,13 @@ final class DefaultChannelHandlerContext extends DefaultAttributeMap implements
                     "write", ChannelHandlerContext.class,
                     Object.class, ChannelPromise.class).isAnnotationPresent(Skip.class)) {
                 flags |= MASK_WRITE;
-            }
-            if (handlerType.getMethod(
-                    "flush", ChannelHandlerContext.class).isAnnotationPresent(Skip.class)) {
-                flags |= MASK_FLUSH;
+
+                // flush() is skipped only when write() is also skipped to avoid the situation where
+                // flush() is handled by the event loop before write() in staged execution.
+                if (handlerType.getMethod(
+                        "flush", ChannelHandlerContext.class).isAnnotationPresent(Skip.class)) {
+                    flags |= MASK_FLUSH;
+                }
             }
         } catch (Exception e) {
             // Should never reach here.
diff --git a/transport/src/test/java/io/netty/channel/local/LocalTransportThreadModelTest.java b/transport/src/test/java/io/netty/channel/local/LocalTransportThreadModelTest.java
index 7ebc149204..890e7ca493 100644
--- a/transport/src/test/java/io/netty/channel/local/LocalTransportThreadModelTest.java
+++ b/transport/src/test/java/io/netty/channel/local/LocalTransportThreadModelTest.java
@@ -87,7 +87,7 @@ public class LocalTransportThreadModelTest {
         EventLoopGroup e2 = new DefaultEventLoopGroup(4, new DefaultThreadFactory("e2"));
         ThreadNameAuditor h1 = new ThreadNameAuditor();
         ThreadNameAuditor h2 = new ThreadNameAuditor();
-        ThreadNameAuditor h3 = new ThreadNameAuditor();
+        ThreadNameAuditor h3 = new ThreadNameAuditor(true);
 
         Channel ch = new LocalChannel(l.next());
         // With no EventExecutor specified, h1 will be always invoked by EventLoop 'l'.
@@ -362,6 +362,15 @@ public class LocalTransportThreadModelTest {
         private final Queue<String> inboundThreadNames = new ConcurrentLinkedQueue<String>();
         private final Queue<String> outboundThreadNames = new ConcurrentLinkedQueue<String>();
         private final Queue<String> removalThreadNames = new ConcurrentLinkedQueue<String>();
+        private final boolean discard;
+
+        ThreadNameAuditor() {
+            this(false);
+        }
+
+        ThreadNameAuditor(boolean discard) {
+            this.discard = discard;
+        }
 
         @Override
         public void handlerRemoved(ChannelHandlerContext ctx) throws Exception {
@@ -371,7 +380,9 @@ public class LocalTransportThreadModelTest {
         @Override
         public void channelRead(ChannelHandlerContext ctx, Object msg) throws Exception {
             inboundThreadNames.add(Thread.currentThread().getName());
-            ctx.fireChannelRead(msg);
+            if (!discard) {
+                ctx.fireChannelRead(msg);
+            }
         }
 
         @Override