Repository: netty/netty
Issue: ChannelInitializer prevents propagation of channelRegistered event
Labels: defect

If I understand Netty's event handling correctly, in general it doesn't (shouldn't?) matter whether a channel handler was added to the pipeline via `addFirst` or `addLast`, the rules of event propagation are the same.  This is not the case though when the pipeline is modified within `ChannelInitializer::initChannel`.  `ChannelInitializer` is implemented as a handler, it's part of the pipeline when `initChannel` is invoked and it removes itself afterwards.  With the current implementation of `ChannelInitializer::channelRegistered`, handlers added via `addFirst` do not receive the channelRegistered event.

Tested with current HEAD of Netty's v4.1 branch.

% java -version
java version "1.8.0_51"
Java(TM) SE Runtime Environment (build 1.8.0_51-b16)
Java HotSpot(TM) 64-Bit Server VM (build 25.51-b03, mixed mode)
% uname -a
Linux marvin 3.16.0-4-amd64 #1 SMP Debian 3.16.7-ckt20-1+deb8u3 (2016-01-17) x86_64 GNU/Linux


Issue comments:
Fixed by https://github.com/netty/netty/pull/4995


Pull request review comments:
actually I think the remove should still happen after the `initChannel(...)`

ensure you close the server and client channel.

ensure you close the server and client channel.

Also you can share most of the test-code and just use a different ChannelInitializer for each of them.


Patch that resolved the issue:
diff --git a/transport/src/main/java/io/netty/channel/ChannelInitializer.java b/transport/src/main/java/io/netty/channel/ChannelInitializer.java
index 5db2c291805..a23b0712271 100644
--- a/transport/src/main/java/io/netty/channel/ChannelInitializer.java
+++ b/transport/src/main/java/io/netty/channel/ChannelInitializer.java
@@ -67,7 +67,7 @@ public abstract class ChannelInitializer<C extends Channel> extends ChannelInbou
     public final void channelRegistered(ChannelHandlerContext ctx) throws Exception {
         initChannel((C) ctx.channel());
         ctx.pipeline().remove(this);
-        ctx.fireChannelRegistered();
+        ctx.pipeline().fireChannelRegistered();
     }
 
     /**


Test patch:
diff --git a/transport/src/test/java/io/netty/channel/ChannelInitializerTest.java b/transport/src/test/java/io/netty/channel/ChannelInitializerTest.java
new file mode 100644
index 00000000000..03d6f014e82
--- /dev/null
+++ b/transport/src/test/java/io/netty/channel/ChannelInitializerTest.java
@@ -0,0 +1,107 @@
+/*
+ * Copyright 2016 The Netty Project
+ *
+ * The Netty Project licenses this file to you under the Apache License,
+ * version 2.0 (the "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at:
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ */
+package io.netty.channel;
+
+import io.netty.bootstrap.Bootstrap;
+import io.netty.bootstrap.ServerBootstrap;
+import io.netty.channel.local.LocalAddress;
+import io.netty.channel.local.LocalChannel;
+import io.netty.channel.local.LocalServerChannel;
+import org.junit.After;
+import org.junit.Before;
+import org.junit.Test;
+
+import java.util.concurrent.TimeUnit;
+import java.util.concurrent.atomic.AtomicInteger;
+
+import static org.junit.Assert.assertEquals;
+
+public class ChannelInitializerTest {
+    private static final int TIMEOUT_MILLIS = 1000;
+    private static final LocalAddress SERVER_ADDRESS = new LocalAddress("addr");
+    private EventLoopGroup group;
+    private ServerBootstrap server;
+    private Bootstrap client;
+    private InspectableHandler testHandler;
+
+    @Before
+    public void setUp() {
+        group = new DefaultEventLoopGroup(1);
+        server = new ServerBootstrap()
+                .group(group)
+                .channel(LocalServerChannel.class)
+                .localAddress(SERVER_ADDRESS);
+        client = new Bootstrap()
+                .group(group)
+                .channel(LocalChannel.class)
+                .handler(new ChannelInboundHandlerAdapter());
+        testHandler = new InspectableHandler();
+    }
+
+    @After
+    public void tearDown() {
+        group.shutdownGracefully(0, TIMEOUT_MILLIS, TimeUnit.MILLISECONDS).syncUninterruptibly();
+    }
+
+    @Test(timeout = TIMEOUT_MILLIS)
+    public void firstHandlerInPipelineShouldReceiveChannelRegisteredEvent() {
+        testChannelRegisteredEventPropagation(new ChannelInitializer<LocalChannel>() {
+            @Override
+            public void initChannel(LocalChannel channel) {
+                channel.pipeline().addFirst(testHandler);
+            }
+        });
+    }
+
+    @Test(timeout = TIMEOUT_MILLIS)
+    public void lastHandlerInPipelineShouldReceiveChannelRegisteredEvent() {
+        testChannelRegisteredEventPropagation(new ChannelInitializer<LocalChannel>() {
+            @Override
+            public void initChannel(LocalChannel channel) {
+                channel.pipeline().addLast(testHandler);
+            }
+        });
+    }
+
+    private void testChannelRegisteredEventPropagation(ChannelInitializer<LocalChannel> init) {
+        Channel clientChannel = null, serverChannel = null;
+        try {
+            server.childHandler(init);
+            serverChannel = server.bind().syncUninterruptibly().channel();
+            clientChannel = client.connect(SERVER_ADDRESS).syncUninterruptibly().channel();
+            assertEquals(1, testHandler.channelRegisteredCount.get());
+        } finally {
+            closeChannel(clientChannel);
+            closeChannel(serverChannel);
+        }
+    }
+
+    private static void closeChannel(Channel c) {
+        if (c != null) {
+            c.close().syncUninterruptibly();
+        }
+    }
+
+    private static final class InspectableHandler extends ChannelDuplexHandler {
+        final AtomicInteger channelRegisteredCount = new AtomicInteger(0);
+
+        @Override
+        public void channelRegistered(ChannelHandlerContext ctx) {
+            channelRegisteredCount.incrementAndGet();
+            ctx.fireChannelRegistered();
+        }
+    }
+}