background the dev-mode web server shutdown,

and Entities.destroyAll is more graceful on concurrent shutdown
apache · Jan 20, 2016 · db44cd7 · db44cd7
1 parent 2354c48
commit db44cd7
Show file tree

Hide file tree

Showing 3 changed files with 33 additions and 20 deletions.
diff --git a/brooklyn-server/core/src/main/java/org/apache/brooklyn/core/entity/Entities.java b/brooklyn-server/core/src/main/java/org/apache/brooklyn/core/entity/Entities.java
@@ -805,10 +805,15 @@ public void run() {
                 ((ManagementContextInternal)mgmt).terminate();
             }
             if (error.get() != null) throw Exceptions.propagate(error.get());
-        } catch (InterruptedException e) {
-            throw Exceptions.propagate(e);
-        } catch (ExecutionException e) {
-            throw Exceptions.propagate(e);
+        } catch (Exception e) {
+            if (!mgmt.isRunning()) {
+                // we've checked this above so it would only happen if a different thread stopped it;
+                // this does happen sometimes e.g. in CliTest where the server shutdown occurs concurrently
+                log.debug("Destroying apps gave an error, but mgmt context was concurrently stopped so not really a problem; swallowing (unless fatal): "+e);
+                Exceptions.propagateIfFatal(e);
+            } else {
+                throw Exceptions.propagate(e);
+            }
         } finally {
             executor.shutdownNow();
         }

diff --git a/...ver/rest/rest-server/src/main/java/org/apache/brooklyn/rest/resources/ServerResource.java b/...ver/rest/rest-server/src/main/java/org/apache/brooklyn/rest/resources/ServerResource.java
@@ -218,7 +218,7 @@ public void run() {
                         if (shutdownHandler != null) {
                             shutdownHandler.onShutdownRequest();
                         } else {
-                            // should always be set as it is required by jersey injection?
+                            // should normally be set, as @Context is required by jersey injection
                             log.warn("ShutdownHandler not set, exiting process");
                             System.exit(0);
                         }

diff --git a/...est-server/src/test/java/org/apache/brooklyn/rest/util/ServerStoppingShutdownHandler.java b/...est-server/src/test/java/org/apache/brooklyn/rest/util/ServerStoppingShutdownHandler.java
@@ -20,6 +20,8 @@
 
 import org.apache.brooklyn.api.mgmt.ManagementContext;
 import org.apache.brooklyn.core.mgmt.internal.ManagementContextInternal;
+import org.apache.brooklyn.util.time.Duration;
+import org.apache.brooklyn.util.time.Time;
 import org.eclipse.jetty.server.Server;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -40,23 +42,29 @@ public ServerStoppingShutdownHandler(ManagementContext mgmt) {
 
     @Override
     public void onShutdownRequest() {
-        log.info("Shutting down (when running in rest-api dev mode)...");
+        log.info("Shutting down server (when running in rest-api dev mode, using background thread)");
 
-        // essentially same as BrooklynLauncher.terminate() but cut down as this is only used in dev mode
-
-        if (server!=null) {
-            try {
-                server.stop();
-                server.join();
-            } catch (Exception e) {
-                log.debug("Stopping server gave an error (not usually a concern): "+e);
-                /* NPE may be thrown e.g. if threadpool not started */
-            }
-        }
+        // essentially same as BrooklynLauncher.terminate() but cut down ...
+        // NB: this is only used in dev mode use of BrooklynJavascriptGuiLauncher
+        new Thread(new Runnable() {
+            public void run() {
+                Time.sleep(Duration.millis(250));
+                log.debug("Shutting down server in background thread, closing "+server+" and "+mgmt);
+                if (server!=null) {
+                    try {
+                        server.stop();
+                        server.join();
+                    } catch (Exception e) {
+                        log.debug("Stopping server gave an error (not usually a concern): "+e);
+                        /* NPE may be thrown e.g. if threadpool not started */
+                    }
+                }
 
-        if (mgmt instanceof ManagementContextInternal) {
-            ((ManagementContextInternal)mgmt).terminate();
-        }
+                if (mgmt instanceof ManagementContextInternal) {
+                    ((ManagementContextInternal)mgmt).terminate();
+                }
+            }
+        }).start();
     }
 
     /** Expect this to be injeted; typically it is not known when this is created, but we need it to trigger shutdown. */