[#1120] Make sure releaseExternal() can not cause a deadlock if the ExecutorService is shared

src/main/java/org/jboss/netty/channel/socket/nio/AbstractNioBossPool.java

@@ -88,7 +88,7 @@ public abstract class AbstractNioBossPool<E extends Boss>
 
     public void releaseExternalResources() {
         shutdown();
-        ExecutorUtil.terminate(bossExecutor);
+        ExecutorUtil.shutdownNow(bossExecutor);
     }
 
     public void shutdown() {

src/main/java/org/jboss/netty/channel/socket/nio/AbstractNioWorkerPool.java

@@ -112,7 +112,7 @@ public abstract class AbstractNioWorkerPool<E extends AbstractNioWorker>
 
     public void releaseExternalResources() {
         shutdown();
-        ExecutorUtil.terminate(workerExecutor);
+        ExecutorUtil.shutdownNow(workerExecutor);
     }
 
     public void shutdown() {

src/main/java/org/jboss/netty/channel/socket/oio/OioClientSocketChannelFactory.java

@@ -122,12 +122,12 @@ public class OioClientSocketChannelFactory implements ClientSocketChannelFactory
 
     public void shutdown() {
         if (shutdownExecutor) {
-            ExecutorUtil.terminate(workerExecutor);
+            ExecutorUtil.shutdownNow(workerExecutor);
         }
     }
 
     public void releaseExternalResources() {
         shutdown();
-        ExecutorUtil.terminate(workerExecutor);
+        ExecutorUtil.shutdownNow(workerExecutor);
     }
 }

src/main/java/org/jboss/netty/channel/socket/oio/OioDatagramChannelFactory.java

@@ -121,12 +121,12 @@ public class OioDatagramChannelFactory implements DatagramChannelFactory {
 
     public void shutdown() {
         if (shutdownExecutor) {
-            ExecutorUtil.terminate(workerExecutor);
+            ExecutorUtil.shutdownNow(workerExecutor);
         }
     }
 
     public void releaseExternalResources() {
         shutdown();
-        ExecutorUtil.terminate(workerExecutor);
+        ExecutorUtil.shutdownNow(workerExecutor);
     }
 }

src/main/java/org/jboss/netty/channel/socket/oio/OioServerSocketChannelFactory.java

@@ -145,12 +145,12 @@ public class OioServerSocketChannelFactory implements ServerSocketChannelFactory
 
     public void shutdown() {
         if (shutdownExecutor) {
-            ExecutorUtil.terminate(workerExecutor);
+            ExecutorUtil.shutdownNow(workerExecutor);
         }
     }
 
     public void releaseExternalResources() {
         shutdown();
-        ExecutorUtil.terminate(workerExecutor);
+        ExecutorUtil.shutdownNow(workerExecutor);
     }
 }

src/main/java/org/jboss/netty/util/internal/ExecutorUtil.java

@@ -27,6 +27,30 @@ import java.util.concurrent.TimeUnit;
  */
 public final class ExecutorUtil {
 
+    /**
+     * Try to call {@link ExecutorService#shutdownNow()}
+     */
+    public static void shutdownNow(Executor executor) {
+        if (executor instanceof ExecutorService) {
+            ExecutorService es = (ExecutorService) executor;
+            try {
+                es.shutdownNow();
+            } catch (SecurityException ex) {
+                // Running in a restricted environment - fall back.
+                try {
+                    es.shutdown();
+                } catch (SecurityException ex2) {
+                    // Running in a more restricted environment.
+                    // Can't shut down this executor - skip to the next.
+                } catch (NullPointerException ex2) {
+                    // Some JDK throws NPE here, but shouldn't.
+                }
+            } catch (NullPointerException ex) {
+                // Some JDK throws NPE here, but shouldn't.
+            }
+        }
+    }
+
     /**
      * Returns {@code true} if and only if the specified {@code executor}
      * is an {@link ExecutorService} and is shut down.  Please note that this
@@ -88,22 +112,7 @@ public final class ExecutorUtil {
 
             ExecutorService es = (ExecutorService) e;
             for (;;) {
-                try {
+                shutdownNow(es);
-                    es.shutdownNow();
-                } catch (SecurityException ex) {
-                    // Running in a restricted environment - fall back.
-                    try {
-                        es.shutdown();
-                    } catch (SecurityException ex2) {
-                        // Running in a more restricted environment.
-                        // Can't shut down this executor - skip to the next.
-                        break;
-                    } catch (NullPointerException ex2) {
-                        // Some JDK throws NPE here, but shouldn't.
-                    }
-                } catch (NullPointerException ex) {
-                    // Some JDK throws NPE here, but shouldn't.
-                }
 
                 try {
                     if (es.awaitTermination(100, TimeUnit.MILLISECONDS)) {

src/test/java/org/jboss/netty/bootstrap/BootstrapTest.java

@@ -24,11 +24,15 @@ import java.util.LinkedHashMap;
 import java.util.Map;
 import java.util.Map.Entry;
 import java.util.NoSuchElementException;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Executors;
 
 import org.jboss.netty.channel.ChannelFactory;
 import org.jboss.netty.channel.ChannelHandler;
 import org.jboss.netty.channel.ChannelPipeline;
 import org.jboss.netty.channel.ChannelPipelineFactory;
+import org.jboss.netty.channel.socket.nio.NioClientSocketChannelFactory;
+import org.jboss.netty.channel.socket.nio.NioServerSocketChannelFactory;
 import org.jboss.netty.util.DummyHandler;
 import org.junit.Test;
 
@@ -271,4 +275,20 @@ public class BootstrapTest {
     protected Bootstrap newBootstrap() {
         return new Bootstrap();
     }
+
+    @Test
+    public void testReleaseSharedNotDeadlock() {
+        // create bootstraps
+        final ExecutorService pool = Executors.newFixedThreadPool(2);
+        final ClientBootstrap client = new ClientBootstrap(
+                new NioClientSocketChannelFactory(pool,
+                        Executors.newCachedThreadPool()));
+        final ServerBootstrap server = new ServerBootstrap(
+                new NioServerSocketChannelFactory(pool,
+                        Executors.newCachedThreadPool()));
+
+        // release resources
+        client.releaseExternalResources();
+        server.releaseExternalResources();
+    }
 }