Repository: netty/netty
Issue: Netty Client concurrent issue when close FixedChannelPool

I use Netty as A tcp Client, use FixedChannelPool as channelPool.



when Close client，some time will cause follow exception



### Expected behavior

no exception



### Actual behavior



```text
[xxxxxx]2018-04-03 14:34:56 04 [io.netty.util.concurrent.DefaultPromise.rejectedExecution]-[ERROR] Failed to submit a listener notification task. Event loop shut down?
 java.util.concurrent.RejectedExecutionException: event executor terminated

        at io.netty.util.concurrent.SingleThreadEventExecutor.reject(SingleThreadEventExecutor.java:821)
        at io.netty.util.concurrent.SingleThreadEventExecutor.offerTask(SingleThreadEventExecutor.java:327)
        at io.netty.util.concurrent.SingleThreadEventExecutor.addTask(SingleThreadEventExecutor.java:320)
        at io.netty.util.concurrent.SingleThreadEventExecutor.execute(SingleThreadEventExecutor.java:746)
        at io.netty.util.concurrent.DefaultPromise.safeExecute(DefaultPromise.java:760)
        at io.netty.util.concurrent.DefaultPromise.notifyListeners(DefaultPromise.java:428)
        at io.netty.util.concurrent.DefaultPromise.setFailure(DefaultPromise.java:113)
        at io.netty.channel.DefaultChannelPromise.setFailure(DefaultChannelPromise.java:87)
        at io.netty.channel.AbstractChannelHandlerContext.safeExecute(AbstractChannelHandlerContext.java:1010)
        at io.netty.channel.AbstractChannelHandlerContext.close(AbstractChannelHandlerContext.java:610)
        at io.netty.channel.AbstractChannelHandlerContext.close(AbstractChannelHandlerContext.java:465)
        at io.netty.channel.DefaultChannelPipeline.close(DefaultChannelPipeline.java:972)
        at io.netty.channel.AbstractChannel.close(AbstractChannel.java:234)
        at io.netty.channel.pool.SimpleChannelPool.close(SimpleChannelPool.java:397)
        at io.netty.channel.pool.FixedChannelPool.access$1201(FixedChannelPool.java:38)
        at io.netty.channel.pool.FixedChannelPool$5.run(FixedChannelPool.java:458)
        at io.netty.util.concurrent.AbstractEventExecutor.safeExecute(AbstractEventExecutor.java:163)
        at io.netty.util.concurrent.SingleThreadEventExecutor.runAllTasks(SingleThreadEventExecutor.java:403)
        at io.netty.channel.nio.NioEventLoop.run(NioEventLoop.java:463
at io.netty.util.concurrent.SingleThreadEventExecutor$5.run(SingleThreadEventExecutor.java:858)
        at io.netty.util.concurrent.DefaultThreadFactory$DefaultRunnableDecorator.run(DefaultThreadFactory.java:138)
        at java.lang.Thread.run(Thread.java:745)
```


### Steps to reproduce

my client code

```java
public void init() {
    ioGroup = new NioEventLoopGroup(ioThreadCnt,
            new DefaultThreadFactory("mainstay-ioNioEventLoopGroup" + "-" + getUrl().getGroup()));
    
    bootstrap.group(ioGroup).channel(SocketChannel);
    bootstrap.option(ChannelOption.ALLOCATOR, PooledByteBufAllocator.DEFAULT);
    bootstrap.option(ChannelOption.TCP_NODELAY, true);
    bootstrap.option(ChannelOption.SO_KEEPALIVE, true);
    bootstrap.option(ChannelOption.CONNECT_TIMEOUT_MILLIS, reqConnTimeout);
    bootstrap.option(ChannelOption.WRITE_BUFFER_WATER_MARK, new WriteBufferWaterMark(8 * 
        1024, 32 * 1024));
    bootstrap.remoteAddress(new InetSocketAddress(remoteIp, port));

    fixedChannelPool = new FixedChannelPool(bootstrap, nettyClientChannelPoolHandler, channelHealthChecker,
        FixedChannelPool.AcquireTimeoutAction.FAIL, 2000, 1000, 100,
        false,
        false);

}

public void close() {
    fixedChannelPool.close();
    if (ioGroup != null) {
        io.netty.util.concurrent.Future shutownfuture = ioGroup.shutdownGracefully(4, 32, TimeUnit.SECONDS);
        try {
          shutownfuture.get(64L, TimeUnit.SECONDS);
        } catch (Throwable e) {
          LOG.error("wait shutdown gracefully more than 64 second!", e);
        }
      }
}

```

my netty client runs well after init(), but when call `close()`，maybe cause `rejectedExecution` Exception， and this exception is not appear every time when call `close()`. 

This Exception happend probability is low ,may be lower than  5%


### my solution 

This is because, fixedChannelPool.close actually submit close Task to Threadpool， is
async.

so maybe ioGroup will be closed before. 

If i add some sleep after `fixedChannelPool.close` will also reduce exception happend probability. but may still happened.

```
fixedChannelPool.close();
Thread.sleep(1500);
// ioGroup.close ...
```

how about `fixedChannelPool.close` return a Future， or make do not make it run in executor, make it run synchronized.


### Minimal yet complete reproducer code (or URL to code)

### Netty version

4.1.14.Final

### JVM version (e.g. `java -version`)

java 1.6-1.8

### OS version (e.g. `uname -a`)

Linux node2.live 2.6.32-431.el6.x86_64 #1 SMP Fri Nov 22 03:15:09 UTC 2013 x86_64 x86_64 x86_64 GNU/Linux


Issue comments:

Pull request review comments:

Patch that resolved the issue:
diff --git a/transport/src/main/java/io/netty/channel/pool/FixedChannelPool.java b/transport/src/main/java/io/netty/channel/pool/FixedChannelPool.java
index 1c0ebcc5343..16a8a7cc9a7 100644
--- a/transport/src/main/java/io/netty/channel/pool/FixedChannelPool.java
+++ b/transport/src/main/java/io/netty/channel/pool/FixedChannelPool.java
@@ -20,6 +20,7 @@
 import io.netty.util.concurrent.EventExecutor;
 import io.netty.util.concurrent.Future;
 import io.netty.util.concurrent.FutureListener;
+import io.netty.util.concurrent.GlobalEventExecutor;
 import io.netty.util.concurrent.Promise;
 import io.netty.util.internal.ObjectUtil;
 import io.netty.util.internal.ThrowableUtil;
@@ -437,27 +438,43 @@ public void acquired() {
 
     @Override
     public void close() {
-        executor.execute(new Runnable() {
-            @Override
-            public void run() {
-                if (!closed) {
-                    closed = true;
-                    for (;;) {
-                        AcquireTask task = pendingAcquireQueue.poll();
-                        if (task == null) {
-                            break;
-                        }
-                        ScheduledFuture<?> f = task.timeoutFuture;
-                        if (f != null) {
-                            f.cancel(false);
-                        }
-                        task.promise.setFailure(new ClosedChannelException());
-                    }
-                    acquiredChannelCount = 0;
-                    pendingAcquireCount = 0;
-                    FixedChannelPool.super.close();
+        if (executor.inEventLoop()) {
+            close0();
+        } else {
+            executor.submit(new Runnable() {
+                @Override
+                public void run() {
+                    close0();
+                }
+            }).awaitUninterruptibly();
+        }
+    }
+
+    private void close0() {
+        if (!closed) {
+            closed = true;
+            for (;;) {
+                AcquireTask task = pendingAcquireQueue.poll();
+                if (task == null) {
+                    break;
+                }
+                ScheduledFuture<?> f = task.timeoutFuture;
+                if (f != null) {
+                    f.cancel(false);
                 }
+                task.promise.setFailure(new ClosedChannelException());
             }
-        });
+            acquiredChannelCount = 0;
+            pendingAcquireCount = 0;
+
+            // Ensure we dispatch this on another Thread as close0 will be called from the EventExecutor and we need
+            // to ensure we will not block in a EventExecutor.
+            GlobalEventExecutor.INSTANCE.execute(new Runnable() {
+                @Override
+                public void run() {
+                    FixedChannelPool.super.close();
+                }
+            });
+        }
     }
 }
diff --git a/transport/src/main/java/io/netty/channel/pool/SimpleChannelPool.java b/transport/src/main/java/io/netty/channel/pool/SimpleChannelPool.java
index d8dd0eae940..6fcfd4443fa 100644
--- a/transport/src/main/java/io/netty/channel/pool/SimpleChannelPool.java
+++ b/transport/src/main/java/io/netty/channel/pool/SimpleChannelPool.java
@@ -394,7 +394,8 @@ public void close() {
             if (channel == null) {
                 break;
             }
-            channel.close();
+            // Just ignore any errors that are reported back from close().
+            channel.close().awaitUninterruptibly();
         }
     }
 }


Test patch: