ServerBootstrap.handler(...) will add handler before Channel is registered.

Motivation:

If you set a ChannelHandler via ServerBootstrap.handler(...) it is added to the ChannelPipeline before the Channel is registered. This will lead to and IllegalStateException if a user tries to access the EventLoop in the ChannelHandler.handlerAdded(...) method.

Modifications:

Delay the adding of the ChannelHandler until the Channel was registered.

Result:

No more IllegalStateException.

transport/src/main/java/io/netty/bootstrap/ServerBootstrap.java

@@ -27,7 +27,6 @@ import io.netty.channel.ChannelOption;
 import io.netty.channel.ChannelPipeline;
 import io.netty.channel.EventLoopGroup;
 import io.netty.channel.ServerChannel;
-import io.netty.channel.socket.SocketChannel;
 import io.netty.util.AttributeKey;
 import io.netty.util.internal.StringUtil;
 import io.netty.util.internal.logging.InternalLogger;
@@ -163,9 +162,6 @@ public class ServerBootstrap extends AbstractBootstrap<ServerBootstrap, ServerCh
         }
 
         ChannelPipeline p = channel.pipeline();
-        if (handler() != null) {
-            p.addLast(handler());
-        }
 
         final EventLoopGroup currentChildGroup = childGroup;
         final ChannelHandler currentChildHandler = childHandler;
@@ -181,7 +177,12 @@ public class ServerBootstrap extends AbstractBootstrap<ServerBootstrap, ServerCh
         p.addLast(new ChannelInitializer<Channel>() {
             @Override
             public void initChannel(Channel ch) throws Exception {
-                ch.pipeline().addLast(new ServerBootstrapAcceptor(
+                ChannelPipeline pipeline = ch.pipeline();
+                ChannelHandler handler = handler();
+                if (handler != null) {
+                    pipeline.addLast(handler);
+                }
+                pipeline.addLast(new ServerBootstrapAcceptor(
                         currentChildGroup, currentChildHandler, currentChildOptions, currentChildAttrs));
             }
         });

transport/src/test/java/io/netty/bootstrap/ServerBootstrapTest.java

@@ -0,0 +1,62 @@
+/*
+ * Copyright 2015 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.bootstrap;
+
+import io.netty.channel.ChannelHandlerAdapter;
+import io.netty.channel.ChannelHandlerContext;
+import io.netty.channel.ChannelInboundHandlerAdapter;
+import io.netty.channel.local.LocalEventLoopGroup;
+import io.netty.channel.local.LocalServerChannel;
+import org.junit.Test;
+
+import java.util.concurrent.CountDownLatch;
+import java.util.concurrent.atomic.AtomicReference;
+
+import static org.junit.Assert.assertNull;
+import static org.junit.Assert.assertTrue;
+
+public class ServerBootstrapTest {
+
+    @Test(timeout = 5000)
+    public void testHandlerRegister() throws Exception {
+        final CountDownLatch latch = new CountDownLatch(1);
+        final AtomicReference<Throwable> error = new AtomicReference<Throwable>();
+        LocalEventLoopGroup group = new LocalEventLoopGroup(1);
+        try {
+            ServerBootstrap sb = new ServerBootstrap();
+            sb.channel(LocalServerChannel.class)
+              .group(group)
+              .childHandler(new ChannelInboundHandlerAdapter())
+              .handler(new ChannelHandlerAdapter() {
+                  @Override
+                  public void handlerAdded(ChannelHandlerContext ctx) throws Exception {
+                      try {
+                          assertTrue(ctx.executor().inEventLoop());
+                      } catch (Throwable cause) {
+                          error.set(cause);
+                      } finally {
+                          latch.countDown();
+                      }
+                  }
+              });
+            sb.register().syncUninterruptibly();
+            latch.await();
+            assertNull(error.get());
+        } finally {
+            group.shutdownGracefully();
+        }
+    }
+}