You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@brooklyn.apache.org by al...@apache.org on 2015/07/29 17:35:29 UTC

[1/4] incubator-brooklyn git commit: Graceful exit on REST shutdown call

Repository: incubator-brooklyn
Updated Branches:
  refs/heads/master 05b95f377 -> 1d5480687


Graceful exit on REST shutdown call

Instead of calling System.exit, exit the main thread.


Project: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/repo
Commit: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/commit/b83ef4da
Tree: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/tree/b83ef4da
Diff: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/diff/b83ef4da

Branch: refs/heads/master
Commit: b83ef4dab63407fbb8d0f2f02800e9b85723ef5b
Parents: efaceed
Author: Svetoslav Neykov <sv...@cloudsoftcorp.com>
Authored: Thu Jul 23 18:54:30 2015 +0300
Committer: Svetoslav Neykov <sv...@cloudsoftcorp.com>
Committed: Wed Jul 29 17:04:14 2015 +0300

----------------------------------------------------------------------
 usage/cli/src/main/java/brooklyn/cli/Main.java  | 105 +++++++++++++------
 .../cli/src/test/java/brooklyn/cli/CliTest.java |   5 +-
 usage/launcher/pom.xml                          |   7 ++
 .../brooklyn/launcher/BrooklynLauncher.java     |  39 ++++---
 .../brooklyn/launcher/BrooklynWebServer.java    |  33 ++++--
 .../brooklyn/rest/resources/ServerResource.java |  26 +++--
 .../brooklyn/rest/util/ShutdownHandler.java     |  23 ++++
 .../rest/util/ShutdownHandlerProvider.java      |  30 ++++++
 .../brooklyn/rest/BrooklynRestApiLauncher.java  |   9 +-
 .../rest/resources/ServerResourceTest.java      |  33 ++++--
 .../rest/testing/BrooklynRestApiTest.java       |  31 ++++--
 .../rest/testing/BrooklynRestResourceTest.java  |   1 +
 .../brooklyn/rest/util/TestShutdownHandler.java |  39 +++++++
 13 files changed, 295 insertions(+), 86 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/b83ef4da/usage/cli/src/main/java/brooklyn/cli/Main.java
----------------------------------------------------------------------
diff --git a/usage/cli/src/main/java/brooklyn/cli/Main.java b/usage/cli/src/main/java/brooklyn/cli/Main.java
index 80d2b05..2379bdd 100644
--- a/usage/cli/src/main/java/brooklyn/cli/Main.java
+++ b/usage/cli/src/main/java/brooklyn/cli/Main.java
@@ -19,12 +19,6 @@
 package brooklyn.cli;
 
 import static com.google.common.base.Preconditions.checkNotNull;
-import groovy.lang.GroovyClassLoader;
-import groovy.lang.GroovyShell;
-import io.airlift.command.Cli;
-import io.airlift.command.Cli.CliBuilder;
-import io.airlift.command.Command;
-import io.airlift.command.Option;
 
 import java.io.Console;
 import java.io.IOException;
@@ -33,10 +27,23 @@ import java.lang.reflect.Constructor;
 import java.lang.reflect.InvocationTargetException;
 import java.util.Arrays;
 import java.util.Collection;
+import java.util.concurrent.Callable;
+import java.util.concurrent.CountDownLatch;
+import java.util.concurrent.ExecutionException;
+import java.util.concurrent.TimeoutException;
+import java.util.concurrent.atomic.AtomicBoolean;
 
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
+import com.google.common.annotations.Beta;
+import com.google.common.annotations.VisibleForTesting;
+import com.google.common.base.Function;
+import com.google.common.base.Objects.ToStringHelper;
+import com.google.common.base.Stopwatch;
+import com.google.common.collect.ImmutableList;
+import com.google.common.collect.Iterables;
+
 import brooklyn.BrooklynVersion;
 import brooklyn.basic.BrooklynTypes;
 import brooklyn.catalog.BrooklynCatalog;
@@ -69,9 +76,11 @@ import brooklyn.launcher.BrooklynLauncher;
 import brooklyn.launcher.BrooklynServerDetails;
 import brooklyn.launcher.config.StopWhichAppsOnShutdown;
 import brooklyn.management.ManagementContext;
+import brooklyn.management.Task;
 import brooklyn.management.ha.HighAvailabilityMode;
 import brooklyn.management.ha.OsgiManager;
 import brooklyn.rest.security.PasswordHasher;
+import brooklyn.rest.util.ShutdownHandler;
 import brooklyn.util.ResourceUtils;
 import brooklyn.util.exceptions.Exceptions;
 import brooklyn.util.exceptions.FatalConfigurationRuntimeException;
@@ -84,14 +93,12 @@ import brooklyn.util.text.Identifiers;
 import brooklyn.util.text.StringEscapes.JavaStringEscapes;
 import brooklyn.util.text.Strings;
 import brooklyn.util.time.Duration;
-
-import com.google.common.annotations.Beta;
-import com.google.common.annotations.VisibleForTesting;
-import com.google.common.base.Function;
-import com.google.common.base.Objects.ToStringHelper;
-import com.google.common.base.Stopwatch;
-import com.google.common.collect.ImmutableList;
-import com.google.common.collect.Iterables;
+import groovy.lang.GroovyClassLoader;
+import groovy.lang.GroovyShell;
+import io.airlift.command.Cli;
+import io.airlift.command.Cli.CliBuilder;
+import io.airlift.command.Command;
+import io.airlift.command.Option;
 
 /**
  * This class is the primary CLI for brooklyn.
@@ -384,6 +391,7 @@ public class Main extends AbstractMain {
         public Void call() throws Exception {
             // Configure launcher
             BrooklynLauncher launcher;
+            AppShutdownHandler shutdownHandler = new AppShutdownHandler();
             failIfArguments();
             try {
                 if (log.isDebugEnabled()) log.debug("Invoked launch command {}", this);
@@ -440,6 +448,7 @@ public class Main extends AbstractMain {
                 launcher.highAvailabilityMode(highAvailabilityMode);
 
                 launcher.stopWhichAppsOnShutdown(stopWhichAppsOnShutdownMode);
+                launcher.shutdownHandler(shutdownHandler);
                 
                 computeAndSetApp(launcher, utils, loader);
                 
@@ -478,8 +487,10 @@ public class Main extends AbstractMain {
             }
             
             if (!exitAndLeaveAppsRunningAfterStarting) {
-                waitAfterLaunch(ctx);
+                waitAfterLaunch(ctx, shutdownHandler);
             }
+
+            // will call mgmt.terminate() in BrooklynShutdownHookJob
             return null;
         }
 
@@ -687,31 +698,37 @@ public class Main extends AbstractMain {
             }
         }
         
-        protected void waitAfterLaunch(ManagementContext ctx) throws IOException {
+        protected void waitAfterLaunch(ManagementContext ctx, AppShutdownHandler shutdownHandler) throws IOException {
             if (stopOnKeyPress) {
                 // Wait for the user to type a key
                 log.info("Server started. Press return to stop.");
-                stdin.read();
+                // Read in another thread so we can use timeout on the wait.
+                Task<Void> readTask = ctx.getExecutionManager().submit(new Callable<Void>() {
+                    @Override
+                    public Void call() throws Exception {
+                        stdin.read();
+                        return null;
+                    }
+                });
+                while (!shutdownHandler.isRequested()) {
+                    try {
+                        readTask.get(Duration.ONE_SECOND);
+                        break;
+                    } catch (TimeoutException e) {
+                        //check if there's a shutdown request
+                    } catch (InterruptedException e) {
+                        Thread.currentThread().interrupt();
+                        throw Exceptions.propagate(e);
+                    } catch (ExecutionException e) {
+                        throw Exceptions.propagate(e);
+                    }
+                }
+                log.info("Shutting down applications.");
                 stopAllApps(ctx.getApplications());
             } else {
                 // Block forever so that Brooklyn doesn't exit (until someone does cntrl-c or kill)
                 log.info("Launched Brooklyn; will now block until shutdown command received via GUI/API (recommended) or process interrupt.");
-                waitUntilInterrupted();
-            }
-        }
-
-        protected void waitUntilInterrupted() {
-            Object mutex = new Object();
-            synchronized (mutex) {
-                try {
-                    while (!Thread.currentThread().isInterrupted()) {
-                        mutex.wait();
-                        log.debug("Spurious wake in brooklyn Main while waiting for interrupt, how about that!");
-                    }
-                } catch (InterruptedException e) {
-                    Thread.currentThread().interrupt();
-                    return; // exit gracefully
-                }
+                shutdownHandler.waitOnShutdownRequest();
             }
         }
 
@@ -964,4 +981,26 @@ public class Main extends AbstractMain {
     protected Class<? extends BrooklynCommand> cliDefaultInfoCommand() {
         return DefaultInfoCommand.class;
     }
+    
+    public static class AppShutdownHandler implements ShutdownHandler {
+        private CountDownLatch lock = new CountDownLatch(1);
+
+        @Override
+        public void onShutdownRequest() {
+            lock.countDown();
+        }
+        
+        public boolean isRequested() {
+            return lock.getCount() == 0;
+        }
+
+        public void waitOnShutdownRequest() {
+            try {
+                lock.await();
+            } catch (InterruptedException e) {
+                Thread.currentThread().interrupt();
+                return; // exit gracefully
+            }
+        }
+    }
 }

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/b83ef4da/usage/cli/src/test/java/brooklyn/cli/CliTest.java
----------------------------------------------------------------------
diff --git a/usage/cli/src/test/java/brooklyn/cli/CliTest.java b/usage/cli/src/test/java/brooklyn/cli/CliTest.java
index a6c6b13..13b27c1 100644
--- a/usage/cli/src/test/java/brooklyn/cli/CliTest.java
+++ b/usage/cli/src/test/java/brooklyn/cli/CliTest.java
@@ -52,6 +52,7 @@ import org.testng.annotations.Test;
 import brooklyn.cli.AbstractMain.BrooklynCommand;
 import brooklyn.cli.AbstractMain.BrooklynCommandCollectingArgs;
 import brooklyn.cli.AbstractMain.HelpCommand;
+import brooklyn.cli.Main.AppShutdownHandler;
 import brooklyn.cli.Main.GeneratePasswordCommand;
 import brooklyn.cli.Main.LaunchCommand;
 import brooklyn.entity.Entity;
@@ -214,10 +215,10 @@ public class CliTest {
     
     @Test
     public void testWaitsForInterrupt() throws Exception {
-        final LaunchCommand launchCommand = new Main.LaunchCommand();
+        final AppShutdownHandler listener = new AppShutdownHandler();
         Thread t = new Thread(new Runnable() {
             @Override public void run() {
-                launchCommand.waitUntilInterrupted();
+                listener.waitOnShutdownRequest();
             }});
         
         t.start();

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/b83ef4da/usage/launcher/pom.xml
----------------------------------------------------------------------
diff --git a/usage/launcher/pom.xml b/usage/launcher/pom.xml
index 9da43e7..37b03d1 100644
--- a/usage/launcher/pom.xml
+++ b/usage/launcher/pom.xml
@@ -194,6 +194,13 @@
         </dependency>
         <dependency>
             <groupId>org.apache.brooklyn</groupId>
+            <artifactId>brooklyn-rest-server</artifactId>
+            <version>${project.version}</version>
+            <classifier>tests</classifier>
+            <scope>test</scope>
+        </dependency>
+        <dependency>
+            <groupId>org.apache.brooklyn</groupId>
             <artifactId>brooklyn-software-messaging</artifactId>
             <version>${project.version}</version>
             <scope>test</scope>

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/b83ef4da/usage/launcher/src/main/java/brooklyn/launcher/BrooklynLauncher.java
----------------------------------------------------------------------
diff --git a/usage/launcher/src/main/java/brooklyn/launcher/BrooklynLauncher.java b/usage/launcher/src/main/java/brooklyn/launcher/BrooklynLauncher.java
index 7b1d437..a6db5f7 100644
--- a/usage/launcher/src/main/java/brooklyn/launcher/BrooklynLauncher.java
+++ b/usage/launcher/src/main/java/brooklyn/launcher/BrooklynLauncher.java
@@ -19,11 +19,6 @@
 package brooklyn.launcher;
 
 import static com.google.common.base.Preconditions.checkNotNull;
-import io.brooklyn.camp.CampPlatform;
-import io.brooklyn.camp.brooklyn.BrooklynCampPlatformLauncherNoServer;
-import io.brooklyn.camp.brooklyn.spi.creation.BrooklynAssemblyTemplateInstantiator;
-import io.brooklyn.camp.spi.AssemblyTemplate;
-import io.brooklyn.camp.spi.instantiate.AssemblyTemplateInstantiator;
 
 import java.io.Closeable;
 import java.io.File;
@@ -41,6 +36,14 @@ import javax.annotation.Nullable;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
+import com.google.common.annotations.Beta;
+import com.google.common.base.Function;
+import com.google.common.base.Splitter;
+import com.google.common.base.Stopwatch;
+import com.google.common.collect.ImmutableList;
+import com.google.common.collect.Lists;
+import com.google.common.collect.Maps;
+
 import brooklyn.catalog.internal.CatalogInitialization;
 import brooklyn.config.BrooklynProperties;
 import brooklyn.config.BrooklynServerConfig;
@@ -86,6 +89,7 @@ import brooklyn.mementos.BrooklynMementoRawData;
 import brooklyn.rest.BrooklynWebConfig;
 import brooklyn.rest.filter.BrooklynPropertiesSecurityFilter;
 import brooklyn.rest.security.provider.BrooklynUserWithRandomPasswordSecurityProvider;
+import brooklyn.rest.util.ShutdownHandler;
 import brooklyn.util.exceptions.Exceptions;
 import brooklyn.util.exceptions.FatalConfigurationRuntimeException;
 import brooklyn.util.exceptions.FatalRuntimeException;
@@ -98,14 +102,11 @@ import brooklyn.util.stream.Streams;
 import brooklyn.util.text.Strings;
 import brooklyn.util.time.Duration;
 import brooklyn.util.time.Time;
-
-import com.google.common.annotations.Beta;
-import com.google.common.base.Function;
-import com.google.common.base.Splitter;
-import com.google.common.base.Stopwatch;
-import com.google.common.collect.ImmutableList;
-import com.google.common.collect.Lists;
-import com.google.common.collect.Maps;
+import io.brooklyn.camp.CampPlatform;
+import io.brooklyn.camp.brooklyn.BrooklynCampPlatformLauncherNoServer;
+import io.brooklyn.camp.brooklyn.spi.creation.BrooklynAssemblyTemplateInstantiator;
+import io.brooklyn.camp.spi.AssemblyTemplate;
+import io.brooklyn.camp.spi.instantiate.AssemblyTemplateInstantiator;
 
 /**
  * Example usage is:
@@ -156,6 +157,7 @@ public class BrooklynLauncher {
     private boolean ignoreAppErrors = true;
     
     private StopWhichAppsOnShutdown stopWhichAppsOnShutdown = StopWhichAppsOnShutdown.THESE_IF_NOT_PERSISTED;
+    private ShutdownHandler shutdownHandler;
     
     private Function<ManagementContext,Void> customizeManagement = null;
     private CatalogInitialization catalogInitialization = null;
@@ -492,6 +494,14 @@ public class BrooklynLauncher {
     }
 
     /**
+     * A listener to call when the user requests a shutdown (i.e. through the REST API)
+     */
+    public BrooklynLauncher shutdownHandler(ShutdownHandler shutdownHandler) {
+        this.shutdownHandler = shutdownHandler;
+        return this;
+    }
+
+    /**
      * @param destinationDir Directory for state to be copied to
      * @param destinationLocation Optional location if target for copied state is a blob store.
      */
@@ -772,6 +782,7 @@ public class BrooklynLauncher {
             webServer.setPublicAddress(publicAddress);
             if (port!=null) webServer.setPort(port);
             if (useHttps!=null) webServer.setHttpsEnabled(useHttps);
+            webServer.setShutdownHandler(shutdownHandler);
             webServer.putAttributes(brooklynProperties);
             if (skipSecurityFilter != Boolean.TRUE) {
                 webServer.setSecurityFilter(BrooklynPropertiesSecurityFilter.class);
@@ -1048,5 +1059,5 @@ public class BrooklynLauncher {
             }
         }
     }
-    
+
 }

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/b83ef4da/usage/launcher/src/main/java/brooklyn/launcher/BrooklynWebServer.java
----------------------------------------------------------------------
diff --git a/usage/launcher/src/main/java/brooklyn/launcher/BrooklynWebServer.java b/usage/launcher/src/main/java/brooklyn/launcher/BrooklynWebServer.java
index ad8bd7a..9b46624 100644
--- a/usage/launcher/src/main/java/brooklyn/launcher/BrooklynWebServer.java
+++ b/usage/launcher/src/main/java/brooklyn/launcher/BrooklynWebServer.java
@@ -34,6 +34,7 @@ import java.util.LinkedHashMap;
 import java.util.List;
 import java.util.Map;
 
+import javax.annotation.Nullable;
 import javax.servlet.DispatcherType;
 
 import org.eclipse.jetty.server.Connector;
@@ -48,6 +49,16 @@ import org.eclipse.jetty.webapp.WebAppContext;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
+import com.google.common.annotations.VisibleForTesting;
+import com.google.common.base.Splitter;
+import com.google.common.base.Throwables;
+import com.google.common.collect.ImmutableList;
+import com.google.common.collect.Maps;
+import com.sun.jersey.api.container.filter.GZIPContentEncodingFilter;
+import com.sun.jersey.api.core.DefaultResourceConfig;
+import com.sun.jersey.api.core.ResourceConfig;
+import com.sun.jersey.spi.container.servlet.ServletContainer;
+
 import brooklyn.BrooklynVersion;
 import brooklyn.config.BrooklynServerPaths;
 import brooklyn.config.BrooklynServiceAttributes;
@@ -68,6 +79,8 @@ import brooklyn.rest.filter.LoggingFilter;
 import brooklyn.rest.filter.NoCacheFilter;
 import brooklyn.rest.filter.RequestTaggingFilter;
 import brooklyn.rest.util.ManagementContextProvider;
+import brooklyn.rest.util.ShutdownHandler;
+import brooklyn.rest.util.ShutdownHandlerProvider;
 import brooklyn.util.BrooklynNetworkUtils;
 import brooklyn.util.ResourceUtils;
 import brooklyn.util.collections.MutableMap;
@@ -86,16 +99,6 @@ import brooklyn.util.text.Identifiers;
 import brooklyn.util.text.Strings;
 import brooklyn.util.web.ContextHandlerCollectionHotSwappable;
 
-import com.google.common.annotations.VisibleForTesting;
-import com.google.common.base.Splitter;
-import com.google.common.base.Throwables;
-import com.google.common.collect.ImmutableList;
-import com.google.common.collect.Maps;
-import com.sun.jersey.api.container.filter.GZIPContentEncodingFilter;
-import com.sun.jersey.api.core.DefaultResourceConfig;
-import com.sun.jersey.api.core.ResourceConfig;
-import com.sun.jersey.spi.container.servlet.ServletContainer;
-
 /**
  * Starts the web-app running, connected to the given management context
  */
@@ -194,6 +197,8 @@ public class BrooklynWebServer {
     
     private Class<BrooklynPropertiesSecurityFilter> securityFilterClazz;
 
+    private ShutdownHandler shutdownHandler;
+
     public BrooklynWebServer(ManagementContext managementContext) {
         this(Maps.newLinkedHashMap(), managementContext);
     }
@@ -225,6 +230,10 @@ public class BrooklynWebServer {
         this.securityFilterClazz = filterClazz;
     }
 
+    public void setShutdownHandler(@Nullable ShutdownHandler shutdownHandler) {
+        this.shutdownHandler = shutdownHandler;
+    }
+
     public BrooklynWebServer setPort(Object port) {
         if (getActualPort()>0)
             throw new IllegalStateException("Can't set port after port has been assigned to server (using "+getActualPort()+")");
@@ -324,7 +333,7 @@ public class BrooklynWebServer {
         return this;
     }
 
-    public static void installAsServletFilter(ServletContextHandler context) {
+    public void installAsServletFilter(ServletContextHandler context) {
         ResourceConfig config = new DefaultResourceConfig();
         // load all our REST API modules, JSON, and Swagger
         for (Object r: BrooklynRestApi.getAllResources())
@@ -347,6 +356,8 @@ public class BrooklynWebServer {
 
         ManagementContext mgmt = (ManagementContext) context.getAttribute(BrooklynServiceAttributes.BROOKLYN_MANAGEMENT_CONTEXT);
         config.getSingletons().add(new ManagementContextProvider(mgmt));
+
+        config.getSingletons().add(new ShutdownHandlerProvider(shutdownHandler));
     }
 
     ContextHandlerCollectionHotSwappable handlers = new ContextHandlerCollectionHotSwappable();

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/b83ef4da/usage/rest-server/src/main/java/brooklyn/rest/resources/ServerResource.java
----------------------------------------------------------------------
diff --git a/usage/rest-server/src/main/java/brooklyn/rest/resources/ServerResource.java b/usage/rest-server/src/main/java/brooklyn/rest/resources/ServerResource.java
index 1bb3d95..a4e9800 100644
--- a/usage/rest-server/src/main/java/brooklyn/rest/resources/ServerResource.java
+++ b/usage/rest-server/src/main/java/brooklyn/rest/resources/ServerResource.java
@@ -23,7 +23,6 @@ import java.io.File;
 import java.io.IOException;
 import java.io.InputStream;
 import java.util.ArrayList;
-import java.util.Collection;
 import java.util.List;
 import java.util.Map;
 import java.util.Properties;
@@ -31,12 +30,16 @@ import java.util.concurrent.TimeUnit;
 import java.util.concurrent.TimeoutException;
 import java.util.concurrent.atomic.AtomicBoolean;
 
+import javax.ws.rs.core.Context;
 import javax.ws.rs.core.MediaType;
 import javax.ws.rs.core.Response;
 
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
+import com.google.common.base.Preconditions;
+import com.google.common.collect.FluentIterable;
+
 import brooklyn.BrooklynVersion;
 import brooklyn.config.ConfigKey;
 import brooklyn.entity.Application;
@@ -63,6 +66,7 @@ import brooklyn.rest.domain.HighAvailabilitySummary;
 import brooklyn.rest.domain.VersionSummary;
 import brooklyn.rest.transform.BrooklynFeatureTransformer;
 import brooklyn.rest.transform.HighAvailabilityTransformer;
+import brooklyn.rest.util.ShutdownHandler;
 import brooklyn.rest.util.WebResourceUtils;
 import brooklyn.util.ResourceUtils;
 import brooklyn.util.collections.MutableMap;
@@ -77,9 +81,6 @@ import brooklyn.util.time.CountdownTimer;
 import brooklyn.util.time.Duration;
 import brooklyn.util.time.Time;
 
-import com.google.common.base.Preconditions;
-import com.google.common.collect.FluentIterable;
-
 public class ServerResource extends AbstractBrooklynRestResource implements ServerApi {
 
     private static final int SHUTDOWN_TIMEOUT_CHECK_INTERVAL = 200;
@@ -88,6 +89,9 @@ public class ServerResource extends AbstractBrooklynRestResource implements Serv
 
     private static final String BUILD_SHA_1_PROPERTY = "git-sha-1";
     private static final String BUILD_BRANCH_PROPERTY = "git-branch-name";
+    
+    @Context
+    private ShutdownHandler shutdownHandler;
 
     @Override
     public void reloadBrooklynProperties() {
@@ -122,13 +126,14 @@ public class ServerResource extends AbstractBrooklynRestResource implements Serv
             delayForHttpReturn = Duration.of(delayMillis, TimeUnit.MILLISECONDS);
         }
 
-        Preconditions.checkState(delayForHttpReturn.isPositive(), "Only positive delay allowed for delayForHttpReturn");
+        Preconditions.checkState(delayForHttpReturn.nanos() >= 0, "Only positive or 0 delay allowed for delayForHttpReturn");
 
         boolean isSingleTimeout = shutdownTimeout.equals(requestTimeout);
         final AtomicBoolean completed = new AtomicBoolean();
         final AtomicBoolean hasAppErrorsOrTimeout = new AtomicBoolean();
 
         new Thread("shutdown") {
+            @Override
             public void run() {
                 boolean terminateTried = false;
                 try {
@@ -178,10 +183,15 @@ public class ServerResource extends AbstractBrooklynRestResource implements Serv
                     complete();
                 
                     if (!hasAppErrorsOrTimeout.get() || forceShutdownOnError) {
-                        //give the http request a chance to complete gracefully
+                        //give the http request a chance to complete gracefully, the server will be stopped in a shutdown hook
                         Time.sleep(delayForHttpReturn);
-                        
-                        System.exit(0);
+
+                        if (shutdownHandler != null) {
+                            shutdownHandler.onShutdownRequest();
+                        } else {
+                            log.warn("ShutdownHandler not set, exiting process");
+                            System.exit(0);
+                        }
                         
                     } else {
                         // There are app errors, don't exit the process, allowing any exception to continue throwing

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/b83ef4da/usage/rest-server/src/main/java/brooklyn/rest/util/ShutdownHandler.java
----------------------------------------------------------------------
diff --git a/usage/rest-server/src/main/java/brooklyn/rest/util/ShutdownHandler.java b/usage/rest-server/src/main/java/brooklyn/rest/util/ShutdownHandler.java
new file mode 100644
index 0000000..a09d99f
--- /dev/null
+++ b/usage/rest-server/src/main/java/brooklyn/rest/util/ShutdownHandler.java
@@ -0,0 +1,23 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package brooklyn.rest.util;
+
+public interface ShutdownHandler {
+    void onShutdownRequest();
+}

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/b83ef4da/usage/rest-server/src/main/java/brooklyn/rest/util/ShutdownHandlerProvider.java
----------------------------------------------------------------------
diff --git a/usage/rest-server/src/main/java/brooklyn/rest/util/ShutdownHandlerProvider.java b/usage/rest-server/src/main/java/brooklyn/rest/util/ShutdownHandlerProvider.java
new file mode 100644
index 0000000..09f395c
--- /dev/null
+++ b/usage/rest-server/src/main/java/brooklyn/rest/util/ShutdownHandlerProvider.java
@@ -0,0 +1,30 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package brooklyn.rest.util;
+
+import javax.annotation.Nullable;
+import javax.ws.rs.core.Context;
+
+import com.sun.jersey.spi.inject.SingletonTypeInjectableProvider;
+
+public class ShutdownHandlerProvider extends SingletonTypeInjectableProvider<Context, ShutdownHandler> {
+    public ShutdownHandlerProvider(@Nullable ShutdownHandler instance) {
+        super(ShutdownHandler.class, instance);
+    }
+}

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/b83ef4da/usage/rest-server/src/test/java/brooklyn/rest/BrooklynRestApiLauncher.java
----------------------------------------------------------------------
diff --git a/usage/rest-server/src/test/java/brooklyn/rest/BrooklynRestApiLauncher.java b/usage/rest-server/src/test/java/brooklyn/rest/BrooklynRestApiLauncher.java
index dff7925..138a4ca 100644
--- a/usage/rest-server/src/test/java/brooklyn/rest/BrooklynRestApiLauncher.java
+++ b/usage/rest-server/src/test/java/brooklyn/rest/BrooklynRestApiLauncher.java
@@ -56,6 +56,8 @@ import brooklyn.rest.filter.RequestTaggingFilter;
 import brooklyn.rest.security.provider.AnyoneSecurityProvider;
 import brooklyn.rest.security.provider.SecurityProvider;
 import brooklyn.rest.util.ManagementContextProvider;
+import brooklyn.rest.util.ShutdownHandlerProvider;
+import brooklyn.rest.util.TestShutdownHandler;
 import brooklyn.util.exceptions.Exceptions;
 import brooklyn.util.net.Networking;
 import brooklyn.util.text.WildcardGlobs;
@@ -107,6 +109,7 @@ public class BrooklynRestApiLauncher {
     private ContextHandler customContext;
     private boolean deployJsgui = true;
     private boolean disableHighAvailability = true;
+    private final TestShutdownHandler shutdownListener = new TestShutdownHandler();
 
     protected BrooklynRestApiLauncher() {}
 
@@ -235,6 +238,7 @@ public class BrooklynRestApiLauncher {
         ResourceConfig config = new DefaultResourceConfig();
         for (Object r: BrooklynRestApi.getAllResources())
             config.getSingletons().add(r);
+        config.getSingletons().add(new ShutdownHandlerProvider(shutdownListener));
 
         ServletContextHandler context = new ServletContextHandler(ServletContextHandler.SESSIONS);
         context.setAttribute(BrooklynServiceAttributes.BROOKLYN_MANAGEMENT_CONTEXT, managementContext);
@@ -326,11 +330,11 @@ public class BrooklynRestApiLauncher {
                 .start();
     }
 
-    public static void installAsServletFilter(ServletContextHandler context) {
+    public void installAsServletFilter(ServletContextHandler context) {
         installAsServletFilter(context, DEFAULT_FILTERS);
     }
 
-    private static void installAsServletFilter(ServletContextHandler context, List<Class<? extends Filter>> filters) {
+    private void installAsServletFilter(ServletContextHandler context, List<Class<? extends Filter>> filters) {
         installBrooklynFilters(context, filters);
 
         // now set up the REST servlet resources
@@ -354,6 +358,7 @@ public class BrooklynRestApiLauncher {
 
         ManagementContext mgmt = (ManagementContext) context.getAttribute(BrooklynServiceAttributes.BROOKLYN_MANAGEMENT_CONTEXT);
         config.getSingletons().add(new ManagementContextProvider(mgmt));
+        config.getSingletons().add(new ShutdownHandlerProvider(shutdownListener));
     }
 
     private static void installBrooklynFilters(ServletContextHandler context, List<Class<? extends Filter>> filters) {

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/b83ef4da/usage/rest-server/src/test/java/brooklyn/rest/resources/ServerResourceTest.java
----------------------------------------------------------------------
diff --git a/usage/rest-server/src/test/java/brooklyn/rest/resources/ServerResourceTest.java b/usage/rest-server/src/test/java/brooklyn/rest/resources/ServerResourceTest.java
index ed5c1b4..cdfd501 100644
--- a/usage/rest-server/src/test/java/brooklyn/rest/resources/ServerResourceTest.java
+++ b/usage/rest-server/src/test/java/brooklyn/rest/resources/ServerResourceTest.java
@@ -25,10 +25,17 @@ import static org.testng.Assert.assertTrue;
 
 import java.util.concurrent.atomic.AtomicInteger;
 
+import javax.ws.rs.core.MultivaluedMap;
+
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
+import org.testng.annotations.BeforeClass;
 import org.testng.annotations.Test;
 
+import com.google.common.collect.ImmutableSet;
+import com.sun.jersey.api.client.UniformInterfaceException;
+import com.sun.jersey.core.util.MultivaluedMapImpl;
+
 import brooklyn.BrooklynVersion;
 import brooklyn.config.BrooklynProperties;
 import brooklyn.management.ManagementContext;
@@ -38,13 +45,16 @@ import brooklyn.rest.domain.VersionSummary;
 import brooklyn.rest.testing.BrooklynRestResourceTest;
 import brooklyn.test.Asserts;
 
-import com.google.common.collect.ImmutableSet;
-import com.sun.jersey.api.client.UniformInterfaceException;
-
 @Test(singleThreaded = true)
 public class ServerResourceTest extends BrooklynRestResourceTest {
 
     private static final Logger log = LoggerFactory.getLogger(ServerResourceTest.class);
+    
+    @Override
+    @BeforeClass(alwaysRun = true)
+    public void setUp() throws Exception {
+        super.setUp();
+    }
 
     @Test
     public void testGetVersion() throws Exception {
@@ -89,13 +99,22 @@ public class ServerResourceTest extends BrooklynRestResourceTest {
         assertEquals(reloadCount.get(), 1);
     }
     
-    // TODO Do not run this! It does a system.exit in ServerResource.shutdown
-    @Test(enabled=false)
+    @Test
     public void testShutdown() throws Exception {
         assertTrue(getManagementContext().isRunning());
+        assertFalse(shutdownListener.isRequested());
+
+        MultivaluedMap<String, String> formData = new MultivaluedMapImpl();
+        formData.add("requestTimeout", "0");
+        formData.add("delayForHttpReturn", "0");
+        client().resource("/v1/server/shutdown").entity(formData).post();
         
-        client().resource("/v1/server/shutdown").post();
-        
+        Asserts.succeedsEventually(new Runnable() {
+            @Override
+            public void run() {
+                assertTrue(shutdownListener.isRequested());
+            }
+        });
         Asserts.succeedsEventually(new Runnable() {
             @Override public void run() {
                 assertFalse(getManagementContext().isRunning());

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/b83ef4da/usage/rest-server/src/test/java/brooklyn/rest/testing/BrooklynRestApiTest.java
----------------------------------------------------------------------
diff --git a/usage/rest-server/src/test/java/brooklyn/rest/testing/BrooklynRestApiTest.java b/usage/rest-server/src/test/java/brooklyn/rest/testing/BrooklynRestApiTest.java
index 10c15d2..7526254 100644
--- a/usage/rest-server/src/test/java/brooklyn/rest/testing/BrooklynRestApiTest.java
+++ b/usage/rest-server/src/test/java/brooklyn/rest/testing/BrooklynRestApiTest.java
@@ -18,10 +18,16 @@
  */
 package brooklyn.rest.testing;
 
-import io.brooklyn.camp.brooklyn.BrooklynCampPlatformLauncherNoServer;
-
 import org.codehaus.jackson.map.ObjectMapper;
 import org.testng.annotations.AfterClass;
+import org.testng.annotations.BeforeMethod;
+
+import com.google.common.base.Preconditions;
+import com.sun.jersey.api.client.Client;
+import com.sun.jersey.api.core.DefaultResourceConfig;
+import com.sun.jersey.test.framework.AppDescriptor;
+import com.sun.jersey.test.framework.JerseyTest;
+import com.sun.jersey.test.framework.LowLevelAppDescriptor;
 
 import brooklyn.entity.basic.Entities;
 import brooklyn.location.LocationRegistry;
@@ -34,22 +40,24 @@ import brooklyn.rest.BrooklynRestApiLauncherTest;
 import brooklyn.rest.util.BrooklynRestResourceUtils;
 import brooklyn.rest.util.NullHttpServletRequestProvider;
 import brooklyn.rest.util.NullServletConfigProvider;
+import brooklyn.rest.util.ShutdownHandlerProvider;
+import brooklyn.rest.util.TestShutdownHandler;
 import brooklyn.rest.util.json.BrooklynJacksonJsonProvider;
 import brooklyn.test.entity.LocalManagementContextForTests;
 import brooklyn.util.exceptions.Exceptions;
-
-import com.google.common.base.Preconditions;
-import com.sun.jersey.api.client.Client;
-import com.sun.jersey.api.core.DefaultResourceConfig;
-import com.sun.jersey.test.framework.AppDescriptor;
-import com.sun.jersey.test.framework.JerseyTest;
-import com.sun.jersey.test.framework.LowLevelAppDescriptor;
+import io.brooklyn.camp.brooklyn.BrooklynCampPlatformLauncherNoServer;
 
 public abstract class BrooklynRestApiTest {
 
     private ManagementContext manager;
     
     protected boolean useLocalScannedCatalog = false;
+    protected TestShutdownHandler shutdownListener = createShutdownHandler();
+
+    @BeforeMethod(alwaysRun = true)
+    public void setUpMethod() {
+        shutdownListener.reset();
+    }
     
     protected synchronized void useLocalScannedCatalog() {
         if (manager!=null && !useLocalScannedCatalog)
@@ -57,6 +65,10 @@ public abstract class BrooklynRestApiTest {
         useLocalScannedCatalog = true;
     }
     
+    private TestShutdownHandler createShutdownHandler() {
+        return new TestShutdownHandler();
+    }
+
     protected synchronized ManagementContext getManagementContext() {
         if (manager==null) {
             if (useLocalScannedCatalog) {
@@ -119,6 +131,7 @@ public abstract class BrooklynRestApiTest {
         // and the servlet config provider must be an instance; addClasses doesn't work for some reason
         addResource(new NullServletConfigProvider());
         addProvider(NullHttpServletRequestProvider.class);
+        addResource(new ShutdownHandlerProvider(shutdownListener));
     }
 
     protected final void setUpResources() {

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/b83ef4da/usage/rest-server/src/test/java/brooklyn/rest/testing/BrooklynRestResourceTest.java
----------------------------------------------------------------------
diff --git a/usage/rest-server/src/test/java/brooklyn/rest/testing/BrooklynRestResourceTest.java b/usage/rest-server/src/test/java/brooklyn/rest/testing/BrooklynRestResourceTest.java
index 42a622a..81c392e 100644
--- a/usage/rest-server/src/test/java/brooklyn/rest/testing/BrooklynRestResourceTest.java
+++ b/usage/rest-server/src/test/java/brooklyn/rest/testing/BrooklynRestResourceTest.java
@@ -60,6 +60,7 @@ public abstract class BrooklynRestResourceTest extends BrooklynRestApiTest {
         setUpJersey();
     }
 
+    @Override
     @AfterClass(alwaysRun = true)
     public void tearDown() throws Exception {
         tearDownJersey();

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/b83ef4da/usage/rest-server/src/test/java/brooklyn/rest/util/TestShutdownHandler.java
----------------------------------------------------------------------
diff --git a/usage/rest-server/src/test/java/brooklyn/rest/util/TestShutdownHandler.java b/usage/rest-server/src/test/java/brooklyn/rest/util/TestShutdownHandler.java
new file mode 100644
index 0000000..19c6d19
--- /dev/null
+++ b/usage/rest-server/src/test/java/brooklyn/rest/util/TestShutdownHandler.java
@@ -0,0 +1,39 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package brooklyn.rest.util;
+
+import brooklyn.rest.util.ShutdownHandler;
+
+public class TestShutdownHandler implements ShutdownHandler {
+    private volatile boolean isRequested;
+
+    @Override
+    public void onShutdownRequest() {
+        isRequested = true;
+    }
+
+    public boolean isRequested() {
+        return isRequested;
+    }
+
+    public void reset() {
+        isRequested = false;
+    }
+
+}



[4/4] incubator-brooklyn git commit: This closes #771

Posted by al...@apache.org.
This closes #771


Project: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/repo
Commit: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/commit/1d548068
Tree: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/tree/1d548068
Diff: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/diff/1d548068

Branch: refs/heads/master
Commit: 1d5480687767c1208ccee33bbe5321c653d04591
Parents: 05b95f3 20a52b2
Author: Aled Sage <al...@gmail.com>
Authored: Wed Jul 29 16:35:28 2015 +0100
Committer: Aled Sage <al...@gmail.com>
Committed: Wed Jul 29 16:35:28 2015 +0100

----------------------------------------------------------------------
 .../entity/basic/SoftwareProcessEntityTest.java | 257 +++++++++++++++++--
 usage/cli/src/main/java/brooklyn/cli/Main.java  | 105 +++++---
 .../cli/src/test/java/brooklyn/cli/CliTest.java |   5 +-
 usage/launcher/pom.xml                          |   7 +
 .../brooklyn/launcher/BrooklynLauncher.java     |  39 ++-
 .../brooklyn/launcher/BrooklynWebServer.java    |  33 ++-
 .../brooklyn/rest/resources/ServerResource.java |  81 +++++-
 .../brooklyn/rest/util/ShutdownHandler.java     |  23 ++
 .../rest/util/ShutdownHandlerProvider.java      |  30 +++
 .../brooklyn/rest/BrooklynRestApiLauncher.java  |   9 +-
 .../rest/resources/ServerResourceTest.java      |  73 ++++--
 .../rest/resources/ServerShutdownTest.java      | 187 ++++++++++++++
 .../rest/testing/BrooklynRestApiTest.java       |  35 ++-
 .../rest/testing/BrooklynRestResourceTest.java  |   1 +
 .../brooklyn/rest/util/TestShutdownHandler.java |  39 +++
 15 files changed, 801 insertions(+), 123 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/1d548068/usage/launcher/pom.xml
----------------------------------------------------------------------


[2/4] incubator-brooklyn git commit: Wait apps to stop before shutting down.

Posted by al...@apache.org.
Wait apps to stop before shutting down.


Project: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/repo
Commit: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/commit/79a1847a
Tree: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/tree/79a1847a
Diff: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/diff/79a1847a

Branch: refs/heads/master
Commit: 79a1847a6ca31a1c18569782d73cb8c4662e819a
Parents: b83ef4d
Author: Svetoslav Neykov <sv...@cloudsoftcorp.com>
Authored: Fri Jul 24 23:12:22 2015 +0300
Committer: Svetoslav Neykov <sv...@cloudsoftcorp.com>
Committed: Wed Jul 29 17:04:22 2015 +0300

----------------------------------------------------------------------
 .../brooklyn/rest/resources/ServerResource.java | 55 ++++++++++++++++++--
 1 file changed, 50 insertions(+), 5 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/79a1847a/usage/rest-server/src/main/java/brooklyn/rest/resources/ServerResource.java
----------------------------------------------------------------------
diff --git a/usage/rest-server/src/main/java/brooklyn/rest/resources/ServerResource.java b/usage/rest-server/src/main/java/brooklyn/rest/resources/ServerResource.java
index a4e9800..24e0e05 100644
--- a/usage/rest-server/src/main/java/brooklyn/rest/resources/ServerResource.java
+++ b/usage/rest-server/src/main/java/brooklyn/rest/resources/ServerResource.java
@@ -43,9 +43,11 @@ import com.google.common.collect.FluentIterable;
 import brooklyn.BrooklynVersion;
 import brooklyn.config.ConfigKey;
 import brooklyn.entity.Application;
+import brooklyn.entity.basic.Attributes;
 import brooklyn.entity.basic.ConfigKeys;
 import brooklyn.entity.basic.Entities;
 import brooklyn.entity.basic.EntityLocal;
+import brooklyn.entity.basic.Lifecycle;
 import brooklyn.entity.basic.StartableApplication;
 import brooklyn.entity.rebind.persister.BrooklynPersistenceUtils;
 import brooklyn.entity.rebind.persister.FileBasedObjectStore;
@@ -136,6 +138,7 @@ public class ServerResource extends AbstractBrooklynRestResource implements Serv
             @Override
             public void run() {
                 boolean terminateTried = false;
+                ManagementContext mgmt = mgmt();
                 try {
                     if (stopAppsFirst) {
                         CountdownTimer shutdownTimeoutTimer = null;
@@ -143,22 +146,50 @@ public class ServerResource extends AbstractBrooklynRestResource implements Serv
                             shutdownTimeoutTimer = shutdownTimeout.countdownTimer();
                         }
 
+                        log.debug("Stopping applications");
                         List<Task<?>> stoppers = new ArrayList<Task<?>>();
-                        for (Application app: mgmt().getApplications()) {
-                            if (app instanceof StartableApplication)
+                        int allStoppableApps = 0;
+                        for (Application app: mgmt.getApplications()) {
+                            allStoppableApps++;
+                            Lifecycle appState = app.getAttribute(Attributes.SERVICE_STATE_ACTUAL);
+                            if (app instanceof StartableApplication &&
+                                    // Don't try to stop an already stopping app. Subsequent stops will complete faster
+                                    // cancelling the first stop task.
+                                    appState != Lifecycle.STOPPING) {
                                 stoppers.add(Entities.invokeEffector((EntityLocal)app, app, StartableApplication.STOP));
+                            } else {
+                                log.debug("App " + app + " is already stopping, will not stop second time. Will wait for original stop to complete.");
+                            }
                         }
 
+                        log.debug("Waiting for " + allStoppableApps + " apps to stop, of which " + stoppers.size() + " stopped explicitly.");
                         for (Task<?> t: stoppers) {
                             if (!waitAppShutdown(shutdownTimeoutTimer, t)) {
                                 //app stop error
                                 hasAppErrorsOrTimeout.set(true);
                             }
                         }
+
+                        // Wait for apps which were already stopping when we tried to shut down.
+                        if (hasStoppableApps(mgmt)) {
+                            log.debug("Apps are still stopping, wait for proper unmanage.");
+                            while (hasStoppableApps(mgmt) && (shutdownTimeoutTimer == null || !shutdownTimeoutTimer.isExpired())) {
+                                Duration wait;
+                                if (shutdownTimeoutTimer != null) {
+                                    wait = Duration.min(shutdownTimeoutTimer.getDurationRemaining(), Duration.ONE_SECOND);
+                                } else {
+                                    wait = Duration.ONE_SECOND;
+                                }
+                                Time.sleep(wait);
+                            }
+                            if (hasStoppableApps(mgmt)) {
+                                hasAppErrorsOrTimeout.set(true);
+                            }
+                        }
                     }
 
                     terminateTried = true;
-                    ((ManagementContextInternal)mgmt()).terminate(); 
+                    ((ManagementContextInternal)mgmt).terminate(); 
 
                 } catch (Throwable e) {
                     Throwable interesting = Exceptions.getFirstInteresting(e);
@@ -176,7 +207,7 @@ public class ServerResource extends AbstractBrooklynRestResource implements Serv
                     hasAppErrorsOrTimeout.set(true);
                     
                     if (!terminateTried) {
-                        ((ManagementContextInternal)mgmt()).terminate(); 
+                        ((ManagementContextInternal)mgmt).terminate(); 
                     }
                 } finally {
 
@@ -201,6 +232,19 @@ public class ServerResource extends AbstractBrooklynRestResource implements Serv
                 }
             }
 
+            private boolean hasStoppableApps(ManagementContext mgmt) {
+                for (Application app : mgmt.getApplications()) {
+                    if (app instanceof StartableApplication) {
+                        Lifecycle state = app.getAttribute(Attributes.SERVICE_STATE_ACTUAL);
+                        if (state != Lifecycle.STOPPING && state != Lifecycle.STOPPED) {
+                            log.warn("Shutting down, expecting all apps to be in stopping state, but found application " + app + " to be in state " + state + ". Just started?");
+                        }
+                        return true;
+                    }
+                }
+                return false;
+            }
+
             private void complete() {
                 synchronized (completed) {
                     completed.set(true);
@@ -214,6 +258,7 @@ public class ServerResource extends AbstractBrooklynRestResource implements Serv
                 if (shutdownTimeoutTimer != null) {
                     waitInterval = Duration.of(SHUTDOWN_TIMEOUT_CHECK_INTERVAL, TimeUnit.MILLISECONDS);
                 }
+                // waitInterval == null - blocks indefinitely
                 while(!t.blockUntilEnded(waitInterval)) {
                     if (shutdownTimeoutTimer.isExpired()) {
                         log.warn("Timeout while waiting for applications to stop at "+t+".\n"+t.getStatusDetail(true));
@@ -237,7 +282,7 @@ public class ServerResource extends AbstractBrooklynRestResource implements Serv
                     //then better wait until the 'completed' flag is set, rather than timing out
                     //at just about the same time (i.e. always wait for the shutdownTimeout in this case).
                     //This will prevent undefined behaviour where either one of shutdownTimeout or requestTimeout
-                    //will be first to expire and the error flag won't be set predicably, it will
+                    //will be first to expire and the error flag won't be set predictably, it will
                     //toggle depending on which expires first.
                     //Note: shutdownTimeout is checked at SHUTDOWN_TIMEOUT_CHECK_INTERVAL interval, meaning it is
                     //practically rounded up to the nearest SHUTDOWN_TIMEOUT_CHECK_INTERVAL.


[3/4] incubator-brooklyn git commit: Add tests for apps double stop

Posted by al...@apache.org.
Add tests for apps double stop


Project: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/repo
Commit: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/commit/20a52b2a
Tree: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/tree/20a52b2a
Diff: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/diff/20a52b2a

Branch: refs/heads/master
Commit: 20a52b2ab1911b9c85d79614569808edc0574f70
Parents: 79a1847
Author: Svetoslav Neykov <sv...@cloudsoftcorp.com>
Authored: Mon Jul 27 16:28:25 2015 +0300
Committer: Svetoslav Neykov <sv...@cloudsoftcorp.com>
Committed: Wed Jul 29 17:04:23 2015 +0300

----------------------------------------------------------------------
 .../entity/basic/SoftwareProcessEntityTest.java | 257 +++++++++++++++++--
 .../rest/resources/ServerResourceTest.java      |  76 ++++--
 .../rest/resources/ServerShutdownTest.java      | 187 ++++++++++++++
 .../rest/testing/BrooklynRestApiTest.java       |   4 +
 4 files changed, 474 insertions(+), 50 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/20a52b2a/software/base/src/test/java/brooklyn/entity/basic/SoftwareProcessEntityTest.java
----------------------------------------------------------------------
diff --git a/software/base/src/test/java/brooklyn/entity/basic/SoftwareProcessEntityTest.java b/software/base/src/test/java/brooklyn/entity/basic/SoftwareProcessEntityTest.java
index 70bc416..b770293 100644
--- a/software/base/src/test/java/brooklyn/entity/basic/SoftwareProcessEntityTest.java
+++ b/software/base/src/test/java/brooklyn/entity/basic/SoftwareProcessEntityTest.java
@@ -18,6 +18,30 @@
  */
 package brooklyn.entity.basic;
 
+import static org.testng.Assert.assertEquals;
+import static org.testng.Assert.assertFalse;
+import static org.testng.Assert.assertTrue;
+
+import java.util.ArrayList;
+import java.util.Collection;
+import java.util.Iterator;
+import java.util.List;
+import java.util.Map;
+import java.util.concurrent.CountDownLatch;
+import java.util.concurrent.TimeUnit;
+
+import org.jclouds.util.Throwables2;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+import org.testng.Assert;
+import org.testng.annotations.BeforeMethod;
+import org.testng.annotations.Test;
+
+import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableMap;
+import com.google.common.collect.ImmutableSet;
+import com.google.common.collect.Iterables;
+
 import brooklyn.config.ConfigKey;
 import brooklyn.entity.BrooklynAppUnitTestSupport;
 import brooklyn.entity.Entity;
@@ -25,6 +49,8 @@ import brooklyn.entity.basic.SoftwareProcess.RestartSoftwareParameters;
 import brooklyn.entity.basic.SoftwareProcess.RestartSoftwareParameters.RestartMachineMode;
 import brooklyn.entity.basic.SoftwareProcess.StopSoftwareParameters;
 import brooklyn.entity.basic.SoftwareProcess.StopSoftwareParameters.StopMode;
+import brooklyn.entity.drivers.BasicEntityDriverManager;
+import brooklyn.entity.drivers.ReflectiveEntityDriverFactory;
 import brooklyn.entity.effector.Effectors;
 import brooklyn.entity.proxying.EntitySpec;
 import brooklyn.entity.proxying.ImplementedBy;
@@ -33,13 +59,20 @@ import brooklyn.entity.trait.Startable;
 import brooklyn.event.basic.PortAttributeSensorAndConfigKey;
 import brooklyn.location.Location;
 import brooklyn.location.LocationSpec;
+import brooklyn.location.MachineLocation;
 import brooklyn.location.basic.FixedListMachineProvisioningLocation;
 import brooklyn.location.basic.Locations;
+import brooklyn.location.basic.SimulatedLocation;
 import brooklyn.location.basic.SshMachineLocation;
+import brooklyn.management.EntityManager;
 import brooklyn.management.Task;
 import brooklyn.management.TaskAdaptable;
+import brooklyn.test.Asserts;
+import brooklyn.test.EntityTestUtils;
+import brooklyn.test.entity.TestApplication;
 import brooklyn.util.collections.MutableMap;
 import brooklyn.util.config.ConfigBag;
+import brooklyn.util.exceptions.Exceptions;
 import brooklyn.util.exceptions.PropagatedRuntimeException;
 import brooklyn.util.net.UserAndHostAndPort;
 import brooklyn.util.os.Os;
@@ -48,28 +81,6 @@ import brooklyn.util.task.Tasks;
 import brooklyn.util.text.Strings;
 import brooklyn.util.time.Duration;
 
-import com.google.common.collect.ImmutableList;
-import com.google.common.collect.ImmutableSet;
-import com.google.common.collect.Iterables;
-
-import org.jclouds.util.Throwables2;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
-import org.testng.Assert;
-import org.testng.annotations.BeforeMethod;
-import org.testng.annotations.Test;
-
-import java.util.ArrayList;
-import java.util.Collection;
-import java.util.Iterator;
-import java.util.List;
-import java.util.Map;
-import java.util.concurrent.TimeUnit;
-
-import static org.testng.Assert.assertEquals;
-import static org.testng.Assert.assertFalse;
-import static org.testng.Assert.assertTrue;
-
 
 public class SoftwareProcessEntityTest extends BrooklynAppUnitTestSupport {
 
@@ -413,6 +424,133 @@ public class SoftwareProcessEntityTest extends BrooklynAppUnitTestSupport {
     }
 
     @Test
+    public void testDoubleStopEntity() {
+        ReflectiveEntityDriverFactory f = ((BasicEntityDriverManager)mgmt.getEntityDriverManager()).getReflectiveDriverFactory();
+        f.addClassFullNameMapping(EmptySoftwareProcessDriver.class.getName(), MinimalEmptySoftwareProcessTestDriver.class.getName());
+
+        // Second stop on SoftwareProcess will return early, while the first stop is still in progress
+        // This causes the app to shutdown prematurely, leaking machines.
+        EntityManager emgr = mgmt.getEntityManager();
+        EntitySpec<TestApplication> appSpec = EntitySpec.create(TestApplication.class);
+        TestApplication app = emgr.createEntity(appSpec);
+        emgr.manage(app);
+        EntitySpec<?> latchEntitySpec = EntitySpec.create(EmptySoftwareProcess.class);
+        Entity entity = app.createAndManageChild(latchEntitySpec);
+
+        final ReleaseLatchLocation loc = mgmt.getLocationManager().createLocation(LocationSpec.create(ReleaseLatchLocation.class));
+        try {
+            app.start(ImmutableSet.of(loc));
+            EntityTestUtils.assertAttributeEquals(entity, Attributes.SERVICE_STATE_ACTUAL, Lifecycle.RUNNING);
+    
+            final Task<Void> firstStop = entity.invoke(Startable.STOP, ImmutableMap.<String, Object>of());
+            // Wait until first task tries to release the location, at this point the entity's reference 
+            // to the location is already cleared.
+            Asserts.succeedsEventually(new Runnable() {
+                @Override
+                public void run() {
+                    assertTrue(loc.isBlocked());
+                }
+            });
+    
+            // Subsequent stops will end quickly - no location to release,
+            // while the first one is still releasing the machine.
+            final Task<Void> secondStop = entity.invoke(Startable.STOP, ImmutableMap.<String, Object>of());;
+            Asserts.succeedsEventually(new Runnable() {
+                @Override
+                public void run() {
+                    assertTrue(secondStop.isDone());
+                }
+            });
+    
+            // Entity state is STOPPED even though first location is still releasing
+            EntityTestUtils.assertAttributeEquals(entity, Attributes.SERVICE_STATE_ACTUAL, Lifecycle.STOPPED);
+            Asserts.succeedsContinually(new Runnable() {
+                @Override
+                public void run() {
+                    assertFalse(firstStop.isDone());
+                }
+            });
+
+            loc.unblock();
+
+            // After the location is released, first task ends as well.
+            EntityTestUtils.assertAttributeEquals(entity, Attributes.SERVICE_STATE_ACTUAL, Lifecycle.STOPPED);
+            Asserts.succeedsEventually(new Runnable() {
+                @Override
+                public void run() {
+                    assertTrue(firstStop.isDone());
+                }
+            });
+
+        } finally {
+            loc.unblock();
+        }
+
+    }
+
+    @Test
+    public void testDoubleStopApp() {
+        ReflectiveEntityDriverFactory f = ((BasicEntityDriverManager)mgmt.getEntityDriverManager()).getReflectiveDriverFactory();
+        f.addClassFullNameMapping(EmptySoftwareProcessDriver.class.getName(), MinimalEmptySoftwareProcessTestDriver.class.getName());
+
+        // Second stop on SoftwareProcess will return early, while the first stop is still in progress
+        // This causes the app to shutdown prematurely, leaking machines.
+        EntityManager emgr = mgmt.getEntityManager();
+        EntitySpec<TestApplication> appSpec = EntitySpec.create(TestApplication.class);
+        final TestApplication app = emgr.createEntity(appSpec);
+        emgr.manage(app);
+        EntitySpec<?> latchEntitySpec = EntitySpec.create(EmptySoftwareProcess.class);
+        final Entity entity = app.createAndManageChild(latchEntitySpec);
+
+        final ReleaseLatchLocation loc = mgmt.getLocationManager().createLocation(LocationSpec.create(ReleaseLatchLocation.class));
+        try {
+            app.start(ImmutableSet.of(loc));
+            EntityTestUtils.assertAttributeEquals(entity, Attributes.SERVICE_STATE_ACTUAL, Lifecycle.RUNNING);
+    
+            final Task<Void> firstStop = app.invoke(Startable.STOP, ImmutableMap.<String, Object>of());
+            // Wait until first task tries to release the location, at this point the entity's reference 
+            // to the location is already cleared.
+            Asserts.succeedsEventually(new Runnable() {
+                @Override
+                public void run() {
+                    assertTrue(loc.isBlocked());
+                }
+            });
+    
+            // Subsequent stops will end quickly - no location to release,
+            // while the first one is still releasing the machine.
+            final Task<Void> secondStop = app.invoke(Startable.STOP, ImmutableMap.<String, Object>of());;
+            Asserts.succeedsEventually(new Runnable() {
+                @Override
+                public void run() {
+                    assertTrue(secondStop.isDone());
+                }
+            });
+    
+            // Since second stop succeeded the app will get unmanaged.
+            Asserts.succeedsEventually(new Runnable() {
+                @Override
+                public void run() {
+                    assertTrue(!Entities.isManaged(entity));
+                    assertTrue(!Entities.isManaged(app));
+                }
+            });
+    
+            // Unmanage will cancel the first task
+            Asserts.succeedsEventually(new Runnable() {
+                @Override
+                public void run() {
+                    assertTrue(firstStop.isDone());
+                }
+            });
+        } finally {
+            // We still haven't unblocked the location release, but entity is already unmanaged.
+            // Double STOP on an application could leak locations!!!
+            loc.unblock();
+        }
+    }
+
+    @Test
     public void testOpenPortsWithPortRangeConfig() throws Exception {
         MyService entity = app.createAndManageChild(EntitySpec.create(MyService.class)
             .configure("http.port", "9999+"));
@@ -568,4 +706,79 @@ public class SoftwareProcessEntityTest extends BrooklynAppUnitTestSupport {
             return (String)getEntity().getConfigRaw(ConfigKeys.newStringConfigKey("salt"), true).or((String)null);
         }
     }
+
+    public static class ReleaseLatchLocation extends SimulatedLocation {
+        private static final long serialVersionUID = 1L;
+        
+        private CountDownLatch lock = new CountDownLatch(1);
+        private volatile boolean isBlocked;
+
+        public void unblock() {
+            lock.countDown();
+        }
+        @Override
+        public void release(MachineLocation machine) {
+            super.release(machine);
+            try {
+                isBlocked = true;
+                lock.await();
+                isBlocked = false;
+            } catch (InterruptedException e) {
+                throw Exceptions.propagate(e);
+            }
+        }
+
+        public boolean isBlocked() {
+            return isBlocked;
+        }
+
+    }
+
+    public static class MinimalEmptySoftwareProcessTestDriver implements EmptySoftwareProcessDriver {
+
+        private EmptySoftwareProcessImpl entity;
+        private Location location;
+
+        public MinimalEmptySoftwareProcessTestDriver(EmptySoftwareProcessImpl entity, Location location) {
+            this.entity = entity;
+            this.location = location;
+        }
+
+        @Override
+        public Location getLocation() {
+            return location;
+        }
+
+        @Override
+        public EntityLocal getEntity() {
+            return entity;
+        }
+
+        @Override
+        public boolean isRunning() {
+            return true;
+        }
+
+        @Override
+        public void rebind() {
+        }
+
+        @Override
+        public void start() {
+        }
+
+        @Override
+        public void restart() {
+        }
+
+        @Override
+        public void stop() {
+        }
+
+        @Override
+        public void kill() {
+        }
+
+    }
+
 }

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/20a52b2a/usage/rest-server/src/test/java/brooklyn/rest/resources/ServerResourceTest.java
----------------------------------------------------------------------
diff --git a/usage/rest-server/src/test/java/brooklyn/rest/resources/ServerResourceTest.java b/usage/rest-server/src/test/java/brooklyn/rest/resources/ServerResourceTest.java
index cdfd501..ca5d860 100644
--- a/usage/rest-server/src/test/java/brooklyn/rest/resources/ServerResourceTest.java
+++ b/usage/rest-server/src/test/java/brooklyn/rest/resources/ServerResourceTest.java
@@ -23,6 +23,8 @@ import static org.testng.Assert.assertFalse;
 import static org.testng.Assert.assertNotNull;
 import static org.testng.Assert.assertTrue;
 
+import java.util.concurrent.CountDownLatch;
+import java.util.concurrent.atomic.AtomicBoolean;
 import java.util.concurrent.atomic.AtomicInteger;
 
 import javax.ws.rs.core.MultivaluedMap;
@@ -38,24 +40,23 @@ import com.sun.jersey.core.util.MultivaluedMapImpl;
 
 import brooklyn.BrooklynVersion;
 import brooklyn.config.BrooklynProperties;
+import brooklyn.entity.basic.EmptySoftwareProcess;
+import brooklyn.entity.basic.EmptySoftwareProcessDriver;
+import brooklyn.entity.basic.EmptySoftwareProcessImpl;
+import brooklyn.entity.proxying.ImplementedBy;
 import brooklyn.management.ManagementContext;
 import brooklyn.management.internal.ManagementContextInternal;
 import brooklyn.rest.domain.HighAvailabilitySummary;
 import brooklyn.rest.domain.VersionSummary;
 import brooklyn.rest.testing.BrooklynRestResourceTest;
 import brooklyn.test.Asserts;
+import brooklyn.util.exceptions.Exceptions;
 
 @Test(singleThreaded = true)
 public class ServerResourceTest extends BrooklynRestResourceTest {
 
     private static final Logger log = LoggerFactory.getLogger(ServerResourceTest.class);
     
-    @Override
-    @BeforeClass(alwaysRun = true)
-    public void setUp() throws Exception {
-        super.setUp();
-    }
-
     @Test
     public void testGetVersion() throws Exception {
         VersionSummary version = client().resource("/v1/server/version").get(VersionSummary.class);
@@ -98,28 +99,6 @@ public class ServerResourceTest extends BrooklynRestResourceTest {
         client().resource("/v1/server/properties/reload").post();
         assertEquals(reloadCount.get(), 1);
     }
-    
-    @Test
-    public void testShutdown() throws Exception {
-        assertTrue(getManagementContext().isRunning());
-        assertFalse(shutdownListener.isRequested());
-
-        MultivaluedMap<String, String> formData = new MultivaluedMapImpl();
-        formData.add("requestTimeout", "0");
-        formData.add("delayForHttpReturn", "0");
-        client().resource("/v1/server/shutdown").entity(formData).post();
-        
-        Asserts.succeedsEventually(new Runnable() {
-            @Override
-            public void run() {
-                assertTrue(shutdownListener.isRequested());
-            }
-        });
-        Asserts.succeedsEventually(new Runnable() {
-            @Override public void run() {
-                assertFalse(getManagementContext().isRunning());
-            }});
-    }
 
     @Test
     void testGetConfig() throws Exception {
@@ -153,4 +132,45 @@ public class ServerResourceTest extends BrooklynRestResourceTest {
             }
         }
     }
+
+    // Alternatively could reuse a blocking location, see brooklyn.entity.basic.SoftwareProcessEntityTest.ReleaseLatchLocation
+    @ImplementedBy(StopLatchEntityImpl.class)
+    public interface StopLatchEntity extends EmptySoftwareProcess {
+        public void unblock();
+        public boolean isBlocked();
+    }
+
+    public static class StopLatchEntityImpl extends EmptySoftwareProcessImpl implements StopLatchEntity {
+        private CountDownLatch lock = new CountDownLatch(1);
+        private volatile boolean isBlocked;
+
+        @Override
+        public void unblock() {
+            lock.countDown();
+        }
+
+        @Override
+        protected void postStop() {
+            super.preStop();
+            try {
+                isBlocked = true;
+                lock.await();
+                isBlocked = false;
+            } catch (InterruptedException e) {
+                throw Exceptions.propagate(e);
+            }
+        }
+
+        @Override
+        public Class<?> getDriverInterface() {
+            return EmptySoftwareProcessDriver.class;
+        }
+
+        @Override
+        public boolean isBlocked() {
+            return isBlocked;
+        }
+
+    }
+
 }

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/20a52b2a/usage/rest-server/src/test/java/brooklyn/rest/resources/ServerShutdownTest.java
----------------------------------------------------------------------
diff --git a/usage/rest-server/src/test/java/brooklyn/rest/resources/ServerShutdownTest.java b/usage/rest-server/src/test/java/brooklyn/rest/resources/ServerShutdownTest.java
new file mode 100644
index 0000000..061599e
--- /dev/null
+++ b/usage/rest-server/src/test/java/brooklyn/rest/resources/ServerShutdownTest.java
@@ -0,0 +1,187 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package brooklyn.rest.resources;
+
+import static org.testng.Assert.assertEquals;
+import static org.testng.Assert.assertFalse;
+import static org.testng.Assert.assertNull;
+import static org.testng.Assert.assertTrue;
+
+import java.util.concurrent.atomic.AtomicReference;
+
+import javax.ws.rs.core.MultivaluedMap;
+
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+import org.testng.annotations.AfterClass;
+import org.testng.annotations.AfterMethod;
+import org.testng.annotations.BeforeClass;
+import org.testng.annotations.BeforeMethod;
+import org.testng.annotations.Test;
+
+import com.google.common.collect.ImmutableMap;
+import com.google.common.collect.ImmutableSet;
+import com.sun.jersey.core.util.MultivaluedMapImpl;
+
+import brooklyn.entity.basic.Attributes;
+import brooklyn.entity.basic.Entities;
+import brooklyn.entity.basic.Lifecycle;
+import brooklyn.entity.drivers.BasicEntityDriverManager;
+import brooklyn.entity.drivers.ReflectiveEntityDriverFactory;
+import brooklyn.entity.proxying.EntitySpec;
+import brooklyn.entity.trait.Startable;
+import brooklyn.management.EntityManager;
+import brooklyn.management.Task;
+import brooklyn.rest.resources.ServerResourceTest.StopLatchEntity;
+import brooklyn.rest.testing.BrooklynRestResourceTest;
+import brooklyn.test.Asserts;
+import brooklyn.test.EntityTestUtils;
+import brooklyn.test.entity.TestApplication;
+import brooklyn.util.exceptions.Exceptions;
+
+public class ServerShutdownTest extends BrooklynRestResourceTest {
+    private static final Logger log = LoggerFactory.getLogger(ServerResourceTest.class);
+
+    // Need to initialise the ManagementContext before each test as it is destroyed.
+    @Override
+    @BeforeClass(alwaysRun = true)
+    public void setUp() throws Exception {
+    }
+
+    @Override
+    @AfterClass(alwaysRun = true)
+    public void tearDown() throws Exception {
+    }
+
+    @Override
+    @BeforeMethod(alwaysRun = true)
+    public void setUpMethod() {
+        setUpJersey();
+        super.setUpMethod();
+    }
+
+    @AfterMethod(alwaysRun = true)
+    public void tearDownMethod() {
+        tearDownJersey();
+        destroyManagementContext();
+    }
+
+    @Test
+    public void testShutdown() throws Exception {
+        assertTrue(getManagementContext().isRunning());
+        assertFalse(shutdownListener.isRequested());
+
+        MultivaluedMap<String, String> formData = new MultivaluedMapImpl();
+        formData.add("requestTimeout", "0");
+        formData.add("delayForHttpReturn", "0");
+        client().resource("/v1/server/shutdown").entity(formData).post();
+
+        Asserts.succeedsEventually(new Runnable() {
+            @Override
+            public void run() {
+                assertTrue(shutdownListener.isRequested());
+            }
+        });
+        Asserts.succeedsEventually(new Runnable() {
+            @Override public void run() {
+                assertFalse(getManagementContext().isRunning());
+            }});
+    }
+
+    @Test
+    public void testStopAppThenShutdownAndStopAppsWaitsForFirstStop() throws InterruptedException {
+        ReflectiveEntityDriverFactory f = ((BasicEntityDriverManager)getManagementContext().getEntityDriverManager()).getReflectiveDriverFactory();
+        f.addClassFullNameMapping("brooklyn.entity.basic.EmptySoftwareProcessDriver", "brooklyn.rest.resources.ServerResourceTest$EmptySoftwareProcessTestDriver");
+
+        // Second stop on SoftwareProcess could return early, while the first stop is still in progress
+        // This causes the app to shutdown prematurely, leaking machines.
+        EntityManager emgr = getManagementContext().getEntityManager();
+        EntitySpec<TestApplication> appSpec = EntitySpec.create(TestApplication.class);
+        TestApplication app = emgr.createEntity(appSpec);
+        emgr.manage(app);
+        EntitySpec<StopLatchEntity> latchEntitySpec = EntitySpec.create(StopLatchEntity.class);
+        final StopLatchEntity entity = app.createAndManageChild(latchEntitySpec);
+        app.start(ImmutableSet.of(app.newLocalhostProvisioningLocation()));
+        EntityTestUtils.assertAttributeEquals(entity, Attributes.SERVICE_STATE_ACTUAL, Lifecycle.RUNNING);
+
+        try {
+            final Task<Void> firstStop = app.invoke(Startable.STOP, ImmutableMap.<String, Object>of());
+            Asserts.succeedsEventually(new Runnable() {
+                @Override
+                public void run() {
+                    assertTrue(entity.isBlocked());
+                }
+            });
+
+            final AtomicReference<Exception> shutdownError = new AtomicReference<>();
+            // Can't use ExecutionContext as it will be stopped on shutdown
+            Thread shutdownThread = new Thread() {
+                @Override
+                public void run() {
+                    try {
+                        MultivaluedMap<String, String> formData = new MultivaluedMapImpl();
+                        formData.add("stopAppsFirst", "true");
+                        formData.add("shutdownTimeout", "0");
+                        formData.add("requestTimeout", "0");
+                        formData.add("delayForHttpReturn", "0");
+                        client().resource("/v1/server/shutdown").entity(formData).post();
+                    } catch (Exception e) {
+                        log.error("Shutdown request error", e);
+                        shutdownError.set(e);
+                        throw Exceptions.propagate(e);
+                    }
+                }
+            };
+            shutdownThread.start();
+
+            //shutdown must wait until the first stop completes (or time out)
+            Asserts.succeedsContinually(new Runnable() {
+                @Override
+                public void run() {
+                    assertFalse(firstStop.isDone());
+                    assertEquals(getManagementContext().getApplications().size(), 1);
+                    assertFalse(shutdownListener.isRequested());
+                }
+            });
+
+            // NOTE test is not fully deterministic. Depending on thread scheduling this will
+            // execute before or after ServerResource.shutdown does the app stop loop. This
+            // means that the shutdown code might not see the app at all. In any case though
+            // the test must succeed.
+            entity.unblock();
+
+            Asserts.succeedsEventually(new Runnable() {
+                @Override
+                public void run() {
+                    assertTrue(firstStop.isDone());
+                    assertTrue(shutdownListener.isRequested());
+                    assertFalse(getManagementContext().isRunning());
+                }
+            });
+
+            shutdownThread.join();
+            assertNull(shutdownError.get(), "Shutdown request error, logged above");
+        } finally {
+            // Be sure we always unblock entity stop even in the case of an exception.
+            // In the success path the entity is already unblocked above.
+            entity.unblock();
+        }
+    }
+
+}

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/20a52b2a/usage/rest-server/src/test/java/brooklyn/rest/testing/BrooklynRestApiTest.java
----------------------------------------------------------------------
diff --git a/usage/rest-server/src/test/java/brooklyn/rest/testing/BrooklynRestApiTest.java b/usage/rest-server/src/test/java/brooklyn/rest/testing/BrooklynRestApiTest.java
index 7526254..446eef0 100644
--- a/usage/rest-server/src/test/java/brooklyn/rest/testing/BrooklynRestApiTest.java
+++ b/usage/rest-server/src/test/java/brooklyn/rest/testing/BrooklynRestApiTest.java
@@ -93,6 +93,10 @@ public abstract class BrooklynRestApiTest {
     
     @AfterClass
     public void tearDown() throws Exception {
+        destroyManagementContext();
+    }
+
+    protected void destroyManagementContext() {
         if (manager!=null) {
             Entities.destroyAll(manager);
             manager = null;