Repository: netty/netty
Issue: EmbeddedChannel constructor throws IllegalStateException
Labels: defect

On Netty 4.0.29, when I try to construct an `EmbeddedChannel`, I get an `IllegalStateException`:

```
java.lang.IllegalStateException: channel not registered to an event loop
    at io.netty.channel.AbstractChannel.eventLoop(AbstractChannel.java:111)
    at io.netty.channel.AbstractChannelHandlerContext.executor(AbstractChannelHandlerContext.java:103)
    at io.netty.channel.AbstractChannelHandlerContext.fireExceptionCaught(AbstractChannelHandlerContext.java:222)
    at io.netty.channel.DefaultChannelPipeline.fireExceptionCaught(DefaultChannelPipeline.java:834)
    at io.netty.channel.DefaultChannelPipeline.callHandlerAdded0(DefaultChannelPipeline.java:504)
    at io.netty.channel.DefaultChannelPipeline.callHandlerAdded(DefaultChannelPipeline.java:482)
    at io.netty.channel.DefaultChannelPipeline.addLast0(DefaultChannelPipeline.java:146)
    at io.netty.channel.DefaultChannelPipeline.addLast(DefaultChannelPipeline.java:129)
    at io.netty.channel.DefaultChannelPipeline.addLast(DefaultChannelPipeline.java:257)
    at io.netty.channel.DefaultChannelPipeline.addLast(DefaultChannelPipeline.java:244)
    at io.netty.channel.embedded.EmbeddedChannel.<init>(EmbeddedChannel.java:79)
```

I'm not sure what the problem is, it first registers the event loop, this is usually an asynchronous operation so adding the handler straight after would usually be a race condition, but the implementation of the `EmbeddedEventLoop` register looks like it should be synchronous, so it looks like it should work with no problems.


Issue comments:
I misread the code, I thought it was adding the handlers after registering with the event loop, but it's actually adding the handlers before registering with the event loop, which is clearly not allowed.

@jroper can you show me the code you use ?

Sorry, I just noticed... the problem is that my `handlerAdded` callback is throwing an exception, and firing that exception is failing due to no event loop.  Up to you if you think this should be handled better, if not, just close this issue.

@jroper it's a bug... Let me fix it.

Fixed by https://github.com/netty/netty/pull/3935


Pull request review comments:
Can you do this with real javadocs style ?

``` java
/**
 * ...
 * ...
 */
```

same as above 

also could you open an openjdk issue and link it ?

@normanmaurer Sure but what does this buy us?

Add CRLF after `**` 

Add CRLF after `**`

That you can click the link ;)

> On 07 Jul 2015, at 22:54, Scott Mitchell notifications@github.com wrote:
> 
> In transport/src/main/java/io/netty/channel/socket/nio/NioSocketChannel.java https://github.com/netty/netty/pull/3951#discussion_r34089345:
> 
> > @@ -284,7 +284,19 @@ protected void doWrite(ChannelOutboundBuffer in) throws Exception {
> >                      // Only one ByteBuf so use non-gathering write
> >                      ByteBuffer nioBuffer = nioBuffers[0];
> >                      for (int i = config().getWriteSpinCount() - 1; i >= 0; i --) {
> > -                        final int localWrittenBytes = ch.write(nioBuffer);
> > -                        final int localWrittenBytes;
> > -                        try {
> > -                            localWrittenBytes = ch.write(nioBuffer);
> > -                        } catch (IllegalArgumentException e) {
> > -                            // There appears to be a JDK bug that when a write operation occurs on a direct buffer,
> >   @normanmaurer https://github.com/normanmaurer Sure but what does this buy us?
> 
> —
> Reply to this email directly or view it on GitHub https://github.com/netty/netty/pull/3951/files#r34089345.

@normanmaurer - O. eclipse only has this capability when you are declaring a method/variable/calls (places where javadocs typically are) and not in code.


Patch that resolved the issue:
diff --git a/transport/src/main/java/io/netty/channel/nio/AbstractNioChannel.java b/transport/src/main/java/io/netty/channel/nio/AbstractNioChannel.java
index ba68408c23c..df6232e2a8b 100644
--- a/transport/src/main/java/io/netty/channel/nio/AbstractNioChannel.java
+++ b/transport/src/main/java/io/netty/channel/nio/AbstractNioChannel.java
@@ -51,7 +51,7 @@ public abstract class AbstractNioChannel extends AbstractChannel {
     private static final InternalLogger logger =
             InternalLoggerFactory.getInstance(AbstractNioChannel.class);
 
-    private static final ClosedChannelException CLOSED_CHANNEL_EXCEPTION = new ClosedChannelException();
+    protected static final ClosedChannelException CLOSED_CHANNEL_EXCEPTION = new ClosedChannelException();
 
     static {
         CLOSED_CHANNEL_EXCEPTION.setStackTrace(EmptyArrays.EMPTY_STACK_TRACE);
diff --git a/transport/src/main/java/io/netty/channel/socket/nio/NioSocketChannel.java b/transport/src/main/java/io/netty/channel/socket/nio/NioSocketChannel.java
index 5d7aa121ba3..b706d453fad 100644
--- a/transport/src/main/java/io/netty/channel/socket/nio/NioSocketChannel.java
+++ b/transport/src/main/java/io/netty/channel/socket/nio/NioSocketChannel.java
@@ -284,7 +284,21 @@ protected void doWrite(ChannelOutboundBuffer in) throws Exception {
                     // Only one ByteBuf so use non-gathering write
                     ByteBuffer nioBuffer = nioBuffers[0];
                     for (int i = config().getWriteSpinCount() - 1; i >= 0; i --) {
-                        final int localWrittenBytes = ch.write(nioBuffer);
+                        final int localWrittenBytes;
+                        try {
+                            localWrittenBytes = ch.write(nioBuffer);
+                        } catch (IllegalArgumentException e) {
+                            /**
+                             * There appears to be a JDK bug that when a write operation occurs on a direct buffer,
+                             * and the associated FD is closed, the write operation is done and the position of the
+                             * ByteBuffer is incremented using a "large" number from the write on an invalid FD which
+                             * exceeds the limit of the ByteBuffer and throws an IllegalArgumentException. The higher
+                             * levels of Netty expect an IOException so we must re-throw. See
+                             * <a href="http://grepcode.com/file/repository.grepcode.com/java/root/jdk/openjdk/8u40-b25/
+                             * sun/nio/ch/IOUtil.java#96">IOUtil.java</a>.
+                             */
+                            throw CLOSED_CHANNEL_EXCEPTION;
+                        }
                         if (localWrittenBytes == 0) {
                             setOpWrite = true;
                             break;
@@ -299,7 +313,21 @@ protected void doWrite(ChannelOutboundBuffer in) throws Exception {
                     break;
                 default:
                     for (int i = config().getWriteSpinCount() - 1; i >= 0; i --) {
-                        final long localWrittenBytes = ch.write(nioBuffers, 0, nioBufferCnt);
+                        final long localWrittenBytes;
+                        try {
+                            localWrittenBytes = ch.write(nioBuffers, 0, nioBufferCnt);
+                        } catch (IllegalArgumentException e) {
+                            /**
+                             * There appears to be a JDK bug that when a write operation occurs on a direct buffer,
+                             * and the associated FD is closed, the write operation is done and the position of the
+                             * ByteBuffer is incremented using a "large" number from the write on an invalid FD which
+                             * exceeds the limit of the ByteBuffer and throws an IllegalArgumentException. The higher
+                             * levels of Netty expect an IOException so we must re-throw. See
+                             * <a href="http://grepcode.com/file/repository.grepcode.com/java/root/jdk/openjdk/8u40-b25/
+                             * sun/nio/ch/IOUtil.java#96">IOUtil.java</a>.
+                             */
+                            throw CLOSED_CHANNEL_EXCEPTION;
+                        }
                         if (localWrittenBytes == 0) {
                             setOpWrite = true;
                             break;


Test patch: