From 7a4e3742941b7fbe476403161d8a25f3f524430c Mon Sep 17 00:00:00 2001
From: Trustin Lee <trustin@gmail.com>
Date: Mon, 18 Jun 2012 13:38:59 +0900
Subject: [PATCH] Fix a bug where timeout handlers sometimes generate events
 too early

---
 .../handler/timeout/IdleStateHandler.java     | 23 ++++++++++++----
 .../handler/timeout/ReadTimeoutHandler.java   | 26 ++++++++++++++-----
 2 files changed, 37 insertions(+), 12 deletions(-)

diff --git a/handler/src/main/java/io/netty/handler/timeout/IdleStateHandler.java b/handler/src/main/java/io/netty/handler/timeout/IdleStateHandler.java
index 6e3df84265..0a4d177d0b 100644
--- a/handler/src/main/java/io/netty/handler/timeout/IdleStateHandler.java
+++ b/handler/src/main/java/io/netty/handler/timeout/IdleStateHandler.java
@@ -129,7 +129,7 @@ public class IdleStateHandler extends ChannelHandlerAdapter {
     volatile ScheduledFuture<?> allIdleTimeout;
     int allIdleCount;
 
-    volatile boolean destroyed;
+    private volatile int state; // 0 - none, 1 - initialized, 2 - destroyed
 
     /**
      * Creates a new instance.
@@ -202,12 +202,12 @@ public class IdleStateHandler extends ChannelHandlerAdapter {
 
     @Override
     public void beforeAdd(ChannelHandlerContext ctx) throws Exception {
-        if (ctx.channel().isActive()) {
+        if (ctx.channel().isActive() & ctx.channel().isRegistered()) {
             // channelActvie() event has been fired already, which means this.channelActive() will
             // not be invoked. We have to initialize here instead.
             initialize(ctx);
         } else {
-            // channelActive() event has not been fired yet.  this.channelOpen() will be invoked
+            // channelActive() event has not been fired yet.  this.channelActive() will be invoked
             // and initialization will occur there.
         }
     }
@@ -217,6 +217,15 @@ public class IdleStateHandler extends ChannelHandlerAdapter {
         destroy();
     }
 
+    @Override
+    public void channelRegistered(ChannelHandlerContext ctx) throws Exception {
+        // Initialize early if channel is active already.
+        if (ctx.channel().isActive()) {
+            initialize(ctx);
+        }
+        super.channelRegistered(ctx);
+    }
+
     @Override
     public void channelActive(ChannelHandlerContext ctx) throws Exception {
         // This method will be invoked only if this handler was added
@@ -256,10 +265,14 @@ public class IdleStateHandler extends ChannelHandlerAdapter {
     private void initialize(ChannelHandlerContext ctx) {
         // Avoid the case where destroy() is called before scheduling timeouts.
         // See: https://github.com/netty/netty/issues/143
-        if (destroyed) {
+        switch (state) {
+        case 1:
+        case 2:
             return;
         }
 
+        state = 1;
+
         EventExecutor loop = ctx.executor();
 
         lastReadTime = lastWriteTime = System.currentTimeMillis();
@@ -281,7 +294,7 @@ public class IdleStateHandler extends ChannelHandlerAdapter {
     }
 
     private void destroy() {
-        destroyed = true;
+        state = 2;
 
         if (readerIdleTimeout != null) {
             readerIdleTimeout.cancel(false);
diff --git a/handler/src/main/java/io/netty/handler/timeout/ReadTimeoutHandler.java b/handler/src/main/java/io/netty/handler/timeout/ReadTimeoutHandler.java
index 99816a7c9c..eb998744da 100644
--- a/handler/src/main/java/io/netty/handler/timeout/ReadTimeoutHandler.java
+++ b/handler/src/main/java/io/netty/handler/timeout/ReadTimeoutHandler.java
@@ -74,7 +74,7 @@ public class ReadTimeoutHandler extends ChannelStateHandlerAdapter {
     private volatile ScheduledFuture<?> timeout;
     private volatile long lastReadTime;
 
-    private volatile boolean destroyed;
+    private volatile int state; // 0 - none, 1 - Initialized, 2 - Destroyed;
 
     private boolean closed;
 
@@ -110,12 +110,12 @@ public class ReadTimeoutHandler extends ChannelStateHandlerAdapter {
 
     @Override
     public void beforeAdd(ChannelHandlerContext ctx) throws Exception {
-        if (ctx.channel().isActive()) {
+        if (ctx.channel().isActive() && ctx.channel().isRegistered()) {
             // channelActvie() event has been fired already, which means this.channelActive() will
             // not be invoked. We have to initialize here instead.
             initialize(ctx);
         } else {
-            // channelActive() event has not been fired yet.  this.channelOpen() will be invoked
+            // channelActive() event has not been fired yet.  this.channelActive() will be invoked
             // and initialization will occur there.
         }
     }
@@ -126,8 +126,16 @@ public class ReadTimeoutHandler extends ChannelStateHandlerAdapter {
     }
 
     @Override
-    public void channelActive(ChannelHandlerContext ctx)
-            throws Exception {
+    public void channelRegistered(ChannelHandlerContext ctx) throws Exception {
+        // Initialize early if channel is active already.
+        if (ctx.channel().isActive()) {
+            initialize(ctx);
+        }
+        super.channelRegistered(ctx);
+    }
+
+    @Override
+    public void channelActive(ChannelHandlerContext ctx) throws Exception {
         // This method will be invoked only if this handler was added
         // before channelActive() event is fired.  If a user adds this handler
         // after the channelActive() event, initialize() will be called by beforeAdd().
@@ -150,10 +158,14 @@ public class ReadTimeoutHandler extends ChannelStateHandlerAdapter {
     private void initialize(ChannelHandlerContext ctx) {
         // Avoid the case where destroy() is called before scheduling timeouts.
         // See: https://github.com/netty/netty/issues/143
-        if (destroyed) {
+        switch (state) {
+        case 1:
+        case 2:
             return;
         }
 
+        state = 1;
+
         lastReadTime = System.currentTimeMillis();
         if (timeoutMillis > 0) {
             timeout = ctx.executor().schedule(
@@ -163,7 +175,7 @@ public class ReadTimeoutHandler extends ChannelStateHandlerAdapter {
     }
 
     private void destroy() {
-        destroyed = true;
+        state = 2;
 
         if (timeout != null) {
             timeout.cancel(false);