Try a little harder to shut down everything properly

crockpotveggies · Jul 17, 2012 · a3d62bc · a3d62bc
1 parent a482f1d
commit a3d62bc
Show file tree

Hide file tree

Showing 2 changed files with 27 additions and 12 deletions.
diff --git a/HISTORY.md b/HISTORY.md
@@ -3,6 +3,7 @@ Git master
 
 [Full changelog](https://github.com/webbit/webbit/compare/v0.4.14...master)
 
+* Trying a little harder to shut down all threads on `WebServer#stop()` (Aslak Hellesøy)
 * Write a close frame before closing the socket when a WebSocket connection is closed (Aslak Hellesøy)
 
 0.4.14 (2012-07-13)

diff --git a/src/main/java/org/webbitserver/netty/NettyWebServer.java b/src/main/java/org/webbitserver/netty/NettyWebServer.java
@@ -245,24 +245,18 @@ public WebServer call() throws Exception {
                 if (bootstrap != null) {
                     bootstrap.releaseExternalResources();
                 }
-                
+
                 // shut down all services & give them a chance to terminate
                 for (ExecutorService executorService : executorServices) {
-                    executorService.shutdown();
+                    shutdownAndAwaitTermination(executorService);
                 }
-                
+
                 bootstrap = null;
 
                 if (channel != null) {
                     channel.getCloseFuture().await();
                 }
-
-                // try best-effort to leave no resources running
-                for (ExecutorService executorService : executorServices) {
-                    boolean shutdown = executorService.awaitTermination(5, TimeUnit.SECONDS);
-                    // fail in tests only 
-                    assert shutdown : "Could not shut down ExecutorService - took more than 5 seconds to terminate";    
-                }
+
                 return NettyWebServer.this;
             }
         });
@@ -273,6 +267,26 @@ public WebServer call() throws Exception {
         return future;
     }
 
+    // See JavaDoc for ExecutorService
+    private void shutdownAndAwaitTermination(ExecutorService executorService) {
+        executorService.shutdown(); // Disable new tasks from being submitted
+        try {
+            // Wait a while for existing tasks to terminate
+            if (!executorService.awaitTermination(5, TimeUnit.SECONDS)) {
+                executorService.shutdownNow(); // Cancel currently executing tasks
+                // Wait a while for tasks to respond to being cancelled
+                if (!executorService.awaitTermination(5, TimeUnit.SECONDS)) {
+                    System.err.println("ExecutorService did not terminate");
+                }
+            }
+        } catch (InterruptedException ie) {
+            // (Re-)Cancel if current thread also interrupted
+            executorService.shutdownNow();
+            // Preserve interrupt status
+            Thread.currentThread().interrupt();
+        }
+    }
+
     @Override
     public NettyWebServer uncaughtExceptionHandler(Thread.UncaughtExceptionHandler exceptionHandler) {
         this.exceptionHandler = exceptionHandler;
@@ -322,7 +336,7 @@ private static URI localUri(int port) {
             return URI.create("http://" + InetAddress.getLocalHost()
                     .getHostName() + (port == 80 ? "" : (":" + port)) + "/");
         } catch (UnknownHostException e) {
-          throw new RuntimeException("can not create URI from localhost hostname - use constructor to pass an explicit URI", e);
+            throw new RuntimeException("can not create URI from localhost hostname - use constructor to pass an explicit URI", e);
         }
     }
 
@@ -333,5 +347,5 @@ protected long timestamp() {
     protected Object nextId() {
         return nextId++;
     }
-    
+
 }