You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@brooklyn.apache.org by he...@apache.org on 2016/01/21 02:23:28 UTC

[1/6] incubator-brooklyn git commit: prevent deadlock if requests coming in during startup

Repository: incubator-brooklyn
Updated Branches:
  refs/heads/master 8303fea86 -> 511886099


prevent deadlock if requests coming in during startup


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

Branch: refs/heads/master
Commit: 2354c4867bc8de3e80bef200bf51424696a33dfc
Parents: 2a432cf
Author: Alex Heneveld <al...@cloudsoftcorp.com>
Authored: Wed Jan 20 18:13:35 2016 +0000
Committer: Alex Heneveld <al...@cloudsoftcorp.com>
Committed: Wed Jan 20 21:07:12 2016 +0000

----------------------------------------------------------------------
 .../core/mgmt/internal/AbstractManagementContext.java    | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/2354c486/brooklyn-server/core/src/main/java/org/apache/brooklyn/core/mgmt/internal/AbstractManagementContext.java
----------------------------------------------------------------------
diff --git a/brooklyn-server/core/src/main/java/org/apache/brooklyn/core/mgmt/internal/AbstractManagementContext.java b/brooklyn-server/core/src/main/java/org/apache/brooklyn/core/mgmt/internal/AbstractManagementContext.java
index d41e059..5d07ed6 100644
--- a/brooklyn-server/core/src/main/java/org/apache/brooklyn/core/mgmt/internal/AbstractManagementContext.java
+++ b/brooklyn-server/core/src/main/java/org/apache/brooklyn/core/mgmt/internal/AbstractManagementContext.java
@@ -372,10 +372,15 @@ public abstract class AbstractManagementContext implements ManagementContextInte
         return configMap;
     }
 
+    private Object locationRegistrySemaphore = new Object();
+    
     @Override
-    public synchronized LocationRegistry getLocationRegistry() {
-        if (locationRegistry==null) locationRegistry = new BasicLocationRegistry(this);
-        return locationRegistry;
+    public LocationRegistry getLocationRegistry() {
+        // NB: can deadlock if synched on whole LMC
+        synchronized (locationRegistrySemaphore) {
+            if (locationRegistry==null) locationRegistry = new BasicLocationRegistry(this);
+            return locationRegistry;
+        }
     }
 
     @Override


[2/6] incubator-brooklyn git commit: better shutdown

Posted by he...@apache.org.
better shutdown

call Main.terminate() in the main thread, rather than relying on shutdown hooks

fixes rest-initiated shutdown when using BrooklynJavascriptGuiLauncher
(looks like that has been broken since #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/2a432cf3
Tree: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/tree/2a432cf3
Diff: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/diff/2a432cf3

Branch: refs/heads/master
Commit: 2a432cf38a2fb0571e4f76604c3a532b1f89c2c6
Parents: 8303fea
Author: Alex Heneveld <al...@cloudsoftcorp.com>
Authored: Wed Jan 20 15:56:07 2016 +0000
Committer: Alex Heneveld <al...@cloudsoftcorp.com>
Committed: Wed Jan 20 21:07:12 2016 +0000

----------------------------------------------------------------------
 .../mgmt/internal/LocalManagementContext.java   | 29 ++++++---
 .../brooklyn/rest/resources/ServerResource.java |  1 +
 .../brooklyn/rest/BrooklynRestApiLauncher.java  | 32 ++++++++--
 .../rest/testing/BrooklynRestApiTest.java       |  8 +--
 .../rest/util/NoOpRecordingShutdownHandler.java | 39 ++++++++++++
 .../util/ServerStoppingShutdownHandler.java     | 67 ++++++++++++++++++++
 .../brooklyn/rest/util/TestShutdownHandler.java | 39 ------------
 .../main/java/org/apache/brooklyn/cli/Main.java | 45 +++++++------
 8 files changed, 181 insertions(+), 79 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/2a432cf3/brooklyn-server/core/src/main/java/org/apache/brooklyn/core/mgmt/internal/LocalManagementContext.java
----------------------------------------------------------------------
diff --git a/brooklyn-server/core/src/main/java/org/apache/brooklyn/core/mgmt/internal/LocalManagementContext.java b/brooklyn-server/core/src/main/java/org/apache/brooklyn/core/mgmt/internal/LocalManagementContext.java
index d88a500..76500bc 100644
--- a/brooklyn-server/core/src/main/java/org/apache/brooklyn/core/mgmt/internal/LocalManagementContext.java
+++ b/brooklyn-server/core/src/main/java/org/apache/brooklyn/core/mgmt/internal/LocalManagementContext.java
@@ -30,6 +30,7 @@ import java.util.Set;
 import java.util.WeakHashMap;
 import java.util.concurrent.Callable;
 import java.util.concurrent.CopyOnWriteArrayList;
+import java.util.concurrent.atomic.AtomicBoolean;
 
 import org.apache.brooklyn.api.effector.Effector;
 import org.apache.brooklyn.api.entity.Application;
@@ -116,6 +117,7 @@ public class LocalManagementContext extends AbstractManagementContext {
         return closed;
     }
 
+    private AtomicBoolean terminated = new AtomicBoolean(false);
     private String managementPlaneId;
     private String managementNodeId;
     private BasicExecutionManager execution;
@@ -316,15 +318,26 @@ public class LocalManagementContext extends AbstractManagementContext {
     
     @Override
     public void terminate() {
-        INSTANCES.remove(this);
-        super.terminate();
-        if (osgiManager!=null) {
-            osgiManager.stop();
-            osgiManager = null;
+        synchronized (terminated) {
+            if (terminated.getAndSet(true)) {
+                log.trace("Already terminated management context "+this);
+                // no harm in doing it twice, but it makes logs ugly!
+                return;
+            }
+            log.debug("Terminating management context "+this);
+            
+            INSTANCES.remove(this);
+            super.terminate();
+            if (osgiManager!=null) {
+                osgiManager.stop();
+                osgiManager = null;
+            }
+            if (usageManager != null) usageManager.terminate();
+            if (execution != null) execution.shutdownNow();
+            if (gc != null) gc.shutdownNow();
+            
+            log.debug("Terminated management context "+this);
         }
-        if (usageManager != null) usageManager.terminate();
-        if (execution != null) execution.shutdownNow();
-        if (gc != null) gc.shutdownNow();
     }
 
     @Override

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/2a432cf3/brooklyn-server/rest/rest-server/src/main/java/org/apache/brooklyn/rest/resources/ServerResource.java
----------------------------------------------------------------------
diff --git a/brooklyn-server/rest/rest-server/src/main/java/org/apache/brooklyn/rest/resources/ServerResource.java b/brooklyn-server/rest/rest-server/src/main/java/org/apache/brooklyn/rest/resources/ServerResource.java
index a9fa4dc..e624d89 100644
--- a/brooklyn-server/rest/rest-server/src/main/java/org/apache/brooklyn/rest/resources/ServerResource.java
+++ b/brooklyn-server/rest/rest-server/src/main/java/org/apache/brooklyn/rest/resources/ServerResource.java
@@ -218,6 +218,7 @@ public class ServerResource extends AbstractBrooklynRestResource implements Serv
                         if (shutdownHandler != null) {
                             shutdownHandler.onShutdownRequest();
                         } else {
+                            // should always be set as it is required by jersey injection?
                             log.warn("ShutdownHandler not set, exiting process");
                             System.exit(0);
                         }

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/2a432cf3/brooklyn-server/rest/rest-server/src/test/java/org/apache/brooklyn/rest/BrooklynRestApiLauncher.java
----------------------------------------------------------------------
diff --git a/brooklyn-server/rest/rest-server/src/test/java/org/apache/brooklyn/rest/BrooklynRestApiLauncher.java b/brooklyn-server/rest/rest-server/src/test/java/org/apache/brooklyn/rest/BrooklynRestApiLauncher.java
index f641267..92ac8f7 100644
--- a/brooklyn-server/rest/rest-server/src/test/java/org/apache/brooklyn/rest/BrooklynRestApiLauncher.java
+++ b/brooklyn-server/rest/rest-server/src/test/java/org/apache/brooklyn/rest/BrooklynRestApiLauncher.java
@@ -48,8 +48,8 @@ import org.apache.brooklyn.rest.security.provider.AnyoneSecurityProvider;
 import org.apache.brooklyn.rest.security.provider.SecurityProvider;
 import org.apache.brooklyn.rest.util.ManagementContextProvider;
 import org.apache.brooklyn.rest.util.OsgiCompat;
+import org.apache.brooklyn.rest.util.ServerStoppingShutdownHandler;
 import org.apache.brooklyn.rest.util.ShutdownHandlerProvider;
-import org.apache.brooklyn.rest.util.TestShutdownHandler;
 import org.apache.brooklyn.util.exceptions.Exceptions;
 import org.apache.brooklyn.util.guava.Maybe;
 import org.apache.brooklyn.util.net.Networking;
@@ -94,7 +94,7 @@ public class BrooklynRestApiLauncher {
     public static final String SCANNING_CATALOG_BOM_URL = "classpath://brooklyn/scanning.catalog.bom";
 
     enum StartMode {
-        FILTER, SERVLET, WEB_XML
+        FILTER, SERVLET, /** web-xml is not fully supported */ @Beta WEB_XML
     }
 
     public static final List<Class<? extends Filter>> DEFAULT_FILTERS = ImmutableList.of(
@@ -112,7 +112,7 @@ public class BrooklynRestApiLauncher {
     private ContextHandler customContext;
     private boolean deployJsgui = true;
     private boolean disableHighAvailability = true;
-    private final TestShutdownHandler shutdownListener = new TestShutdownHandler();
+    private ServerStoppingShutdownHandler shutdownListener;
 
     protected BrooklynRestApiLauncher() {}
 
@@ -215,7 +215,12 @@ public class BrooklynRestApiLauncher {
             ((BrooklynProperties) mgmt.getConfig()).put(BrooklynServerConfig.BROOKLYN_CATALOG_URL, ManagementContextInternal.EMPTY_CATALOG_URL);
         }
 
-        return startServer(mgmt, context, summary, disableHighAvailability);
+        Server server = startServer(mgmt, context, summary, disableHighAvailability);
+        if (shutdownListener!=null) {
+            // not available in some modes, eg webapp
+            shutdownListener.setServer(server);
+        }
+        return server;
     }
 
     private ContextHandler filterContextHandler(ManagementContext mgmt) {
@@ -241,7 +246,7 @@ public class BrooklynRestApiLauncher {
         ResourceConfig config = new DefaultResourceConfig();
         for (Object r: BrooklynRestApi.getAllResources())
             config.getSingletons().add(r);
-        config.getSingletons().add(new ShutdownHandlerProvider(shutdownListener));
+        addShutdownListener(config, mgmt);
 
         ServletContextHandler context = new ServletContextHandler(ServletContextHandler.SESSIONS);
         context.setAttribute(BrooklynServiceAttributes.BROOKLYN_MANAGEMENT_CONTEXT, managementContext);
@@ -253,6 +258,7 @@ public class BrooklynRestApiLauncher {
         return context;
     }
 
+    /** NB: not fully supported; use one of the other {@link StartMode}s */
     private ContextHandler webXmlContextHandler(ManagementContext mgmt) {
         // TODO add security to web.xml
         WebAppContext context;
@@ -266,12 +272,15 @@ public class BrooklynRestApiLauncher {
             throw new IllegalStateException("Cannot find WAR for REST API. Expected in target/*.war, Maven repo, or in source directories.");
         }
         context.setAttribute(BrooklynServiceAttributes.BROOKLYN_MANAGEMENT_CONTEXT, mgmt);
-
+        // TODO shutdown hook
+        
         return context;
     }
 
     /** starts a server, on all NICs if security is configured,
-     * otherwise (no security) only on loopback interface */
+     * otherwise (no security) only on loopback interface 
+     * @deprecated since 0.9.0 becoming private */
+    @Deprecated
     public static Server startServer(ManagementContext mgmt, ContextHandler context, String summary, boolean disableHighAvailability) {
         // TODO this repeats code in BrooklynLauncher / WebServer. should merge the two paths.
         boolean secure = mgmt != null && !BrooklynWebConfig.hasNoSecurityOptions(mgmt.getConfig());
@@ -292,8 +301,11 @@ public class BrooklynRestApiLauncher {
         return startServer(context, summary, bindLocation);
     }
 
+    /** @deprecated since 0.9.0 becoming private */
+    @Deprecated
     public static Server startServer(ContextHandler context, String summary, InetSocketAddress bindLocation) {
         Server server = new Server(bindLocation);
+        
         server.setHandler(context);
         try {
             server.start();
@@ -361,6 +373,12 @@ public class BrooklynRestApiLauncher {
 
         ManagementContext mgmt = OsgiCompat.getManagementContext(context);
         config.getSingletons().add(new ManagementContextProvider(mgmt));
+        addShutdownListener(config, mgmt);
+    }
+
+    protected synchronized void addShutdownListener(ResourceConfig config, ManagementContext mgmt) {
+        if (shutdownListener!=null) throw new IllegalStateException("Can only retrieve one shutdown listener");
+        shutdownListener = new ServerStoppingShutdownHandler(mgmt);
         config.getSingletons().add(new ShutdownHandlerProvider(shutdownListener));
     }
 

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/2a432cf3/brooklyn-server/rest/rest-server/src/test/java/org/apache/brooklyn/rest/testing/BrooklynRestApiTest.java
----------------------------------------------------------------------
diff --git a/brooklyn-server/rest/rest-server/src/test/java/org/apache/brooklyn/rest/testing/BrooklynRestApiTest.java b/brooklyn-server/rest/rest-server/src/test/java/org/apache/brooklyn/rest/testing/BrooklynRestApiTest.java
index dea8bca..e63a239 100644
--- a/brooklyn-server/rest/rest-server/src/test/java/org/apache/brooklyn/rest/testing/BrooklynRestApiTest.java
+++ b/brooklyn-server/rest/rest-server/src/test/java/org/apache/brooklyn/rest/testing/BrooklynRestApiTest.java
@@ -47,7 +47,7 @@ import org.apache.brooklyn.rest.util.BrooklynRestResourceUtils;
 import org.apache.brooklyn.rest.util.NullHttpServletRequestProvider;
 import org.apache.brooklyn.rest.util.NullServletConfigProvider;
 import org.apache.brooklyn.rest.util.ShutdownHandlerProvider;
-import org.apache.brooklyn.rest.util.TestShutdownHandler;
+import org.apache.brooklyn.rest.util.NoOpRecordingShutdownHandler;
 import org.apache.brooklyn.rest.util.json.BrooklynJacksonJsonProvider;
 import org.apache.brooklyn.util.exceptions.Exceptions;
 
@@ -56,7 +56,7 @@ public abstract class BrooklynRestApiTest {
     protected ManagementContext manager;
     
     protected boolean useLocalScannedCatalog = false;
-    protected TestShutdownHandler shutdownListener = createShutdownHandler();
+    protected NoOpRecordingShutdownHandler shutdownListener = createShutdownHandler();
 
     @BeforeMethod(alwaysRun = true)
     public void setUpMethod() {
@@ -69,8 +69,8 @@ public abstract class BrooklynRestApiTest {
         useLocalScannedCatalog = true;
     }
     
-    private TestShutdownHandler createShutdownHandler() {
-        return new TestShutdownHandler();
+    private NoOpRecordingShutdownHandler createShutdownHandler() {
+        return new NoOpRecordingShutdownHandler();
     }
 
     protected synchronized ManagementContext getManagementContext() {

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/2a432cf3/brooklyn-server/rest/rest-server/src/test/java/org/apache/brooklyn/rest/util/NoOpRecordingShutdownHandler.java
----------------------------------------------------------------------
diff --git a/brooklyn-server/rest/rest-server/src/test/java/org/apache/brooklyn/rest/util/NoOpRecordingShutdownHandler.java b/brooklyn-server/rest/rest-server/src/test/java/org/apache/brooklyn/rest/util/NoOpRecordingShutdownHandler.java
new file mode 100644
index 0000000..a99d3d9
--- /dev/null
+++ b/brooklyn-server/rest/rest-server/src/test/java/org/apache/brooklyn/rest/util/NoOpRecordingShutdownHandler.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 org.apache.brooklyn.rest.util;
+
+import org.apache.brooklyn.rest.util.ShutdownHandler;
+
+public class NoOpRecordingShutdownHandler implements ShutdownHandler {
+    private volatile boolean isRequested;
+
+    @Override
+    public void onShutdownRequest() {
+        isRequested = true;
+    }
+
+    public boolean isRequested() {
+        return isRequested;
+    }
+
+    public void reset() {
+        isRequested = false;
+    }
+
+}

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/2a432cf3/brooklyn-server/rest/rest-server/src/test/java/org/apache/brooklyn/rest/util/ServerStoppingShutdownHandler.java
----------------------------------------------------------------------
diff --git a/brooklyn-server/rest/rest-server/src/test/java/org/apache/brooklyn/rest/util/ServerStoppingShutdownHandler.java b/brooklyn-server/rest/rest-server/src/test/java/org/apache/brooklyn/rest/util/ServerStoppingShutdownHandler.java
new file mode 100644
index 0000000..7244378
--- /dev/null
+++ b/brooklyn-server/rest/rest-server/src/test/java/org/apache/brooklyn/rest/util/ServerStoppingShutdownHandler.java
@@ -0,0 +1,67 @@
+/*
+ * 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 org.apache.brooklyn.rest.util;
+
+import org.apache.brooklyn.api.mgmt.ManagementContext;
+import org.apache.brooklyn.core.mgmt.internal.ManagementContextInternal;
+import org.eclipse.jetty.server.Server;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/* not the cleanest way to enforce a clean shutdown, but a simple and effective way;
+ * usage is restricted to BrooklynRestApiLauncher and subclasses, to stop it inline.
+ * the main app stops the server in a more principled way using callbacks. */
+public class ServerStoppingShutdownHandler implements ShutdownHandler {
+
+    private static final Logger log = LoggerFactory.getLogger(ServerStoppingShutdownHandler.class);
+    
+    private final ManagementContext mgmt;
+    private Server server;
+    
+    public ServerStoppingShutdownHandler(ManagementContext mgmt) {
+        this.mgmt = mgmt;
+    }
+
+    @Override
+    public void onShutdownRequest() {
+        log.info("Shutting down (when running in rest-api dev mode)...");
+
+        // 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 */
+            }
+        }
+
+        if (mgmt instanceof ManagementContextInternal) {
+            ((ManagementContextInternal)mgmt).terminate();
+        }
+    }
+
+    /** Expect this to be injeted; typically it is not known when this is created, but we need it to trigger shutdown. */
+    public void setServer(Server server) {
+        this.server = server;
+    }
+
+}

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/2a432cf3/brooklyn-server/rest/rest-server/src/test/java/org/apache/brooklyn/rest/util/TestShutdownHandler.java
----------------------------------------------------------------------
diff --git a/brooklyn-server/rest/rest-server/src/test/java/org/apache/brooklyn/rest/util/TestShutdownHandler.java b/brooklyn-server/rest/rest-server/src/test/java/org/apache/brooklyn/rest/util/TestShutdownHandler.java
deleted file mode 100644
index 87fbb24..0000000
--- a/brooklyn-server/rest/rest-server/src/test/java/org/apache/brooklyn/rest/util/TestShutdownHandler.java
+++ /dev/null
@@ -1,39 +0,0 @@
-/*
- * 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 org.apache.brooklyn.rest.util;
-
-import org.apache.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;
-    }
-
-}

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/2a432cf3/brooklyn-server/server-cli/src/main/java/org/apache/brooklyn/cli/Main.java
----------------------------------------------------------------------
diff --git a/brooklyn-server/server-cli/src/main/java/org/apache/brooklyn/cli/Main.java b/brooklyn-server/server-cli/src/main/java/org/apache/brooklyn/cli/Main.java
index 1c66a04..96527db 100644
--- a/brooklyn-server/server-cli/src/main/java/org/apache/brooklyn/cli/Main.java
+++ b/brooklyn-server/server-cli/src/main/java/org/apache/brooklyn/cli/Main.java
@@ -19,6 +19,12 @@
 package org.apache.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;
@@ -32,17 +38,6 @@ import java.util.concurrent.CountDownLatch;
 import java.util.concurrent.ExecutionException;
 import java.util.concurrent.TimeoutException;
 
-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 org.apache.brooklyn.api.catalog.BrooklynCatalog;
 import org.apache.brooklyn.api.catalog.CatalogItem;
 import org.apache.brooklyn.api.catalog.CatalogItem.CatalogItemType;
@@ -89,16 +84,19 @@ import org.apache.brooklyn.util.guava.Maybe;
 import org.apache.brooklyn.util.javalang.Enums;
 import org.apache.brooklyn.util.net.Networking;
 import org.apache.brooklyn.util.text.Identifiers;
-import org.apache.brooklyn.util.text.Strings;
 import org.apache.brooklyn.util.text.StringEscapes.JavaStringEscapes;
+import org.apache.brooklyn.util.text.Strings;
 import org.apache.brooklyn.util.time.Duration;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 
-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 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;
 
 /**
  * This class is the primary CLI for brooklyn.
@@ -474,17 +472,22 @@ public class Main extends AbstractMain {
             }
             
             BrooklynServerDetails server = launcher.getServerDetails();
-            ManagementContext ctx = server.getManagementContext();
+            ManagementContext mgmt = server.getManagementContext();
             
             if (verbose) {
                 Entities.dumpInfo(launcher.getApplications());
             }
             
             if (!exitAndLeaveAppsRunningAfterStarting) {
-                waitAfterLaunch(ctx, shutdownHandler);
+                waitAfterLaunch(mgmt, shutdownHandler);
             }
 
-            // will call mgmt.terminate() in BrooklynShutdownHookJob
+            // BrooklynShutdownHookJob will invoke terminate() to do mgmt.terminate() and BrooklynWebServer.stop();
+            // and System.exit is invoked immediately after ...
+            // but seems better to do it explicitly here. 
+            // ('launcher' is local to us so the caller *cannot* do it, and no harm in doing it twice.) 
+            launcher.terminate();
+            
             return null;
         }
 


[5/6] incubator-brooklyn git commit: address minor shutdown cleanup code review comments

Posted by he...@apache.org.
address minor shutdown cleanup code review comments


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

Branch: refs/heads/master
Commit: 11a0b8ae67af596a9cf09cf2e22683b5aa81a20f
Parents: 88b76b7
Author: Alex Heneveld <al...@cloudsoftcorp.com>
Authored: Thu Jan 21 01:19:12 2016 +0000
Committer: Alex Heneveld <al...@cloudsoftcorp.com>
Committed: Thu Jan 21 01:22:22 2016 +0000

----------------------------------------------------------------------
 .../brooklyn/core/mgmt/internal/AbstractManagementContext.java     | 2 +-
 .../apache/brooklyn/rest/util/ServerStoppingShutdownHandler.java   | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/11a0b8ae/brooklyn-server/core/src/main/java/org/apache/brooklyn/core/mgmt/internal/AbstractManagementContext.java
----------------------------------------------------------------------
diff --git a/brooklyn-server/core/src/main/java/org/apache/brooklyn/core/mgmt/internal/AbstractManagementContext.java b/brooklyn-server/core/src/main/java/org/apache/brooklyn/core/mgmt/internal/AbstractManagementContext.java
index 5d07ed6..da9fcae 100644
--- a/brooklyn-server/core/src/main/java/org/apache/brooklyn/core/mgmt/internal/AbstractManagementContext.java
+++ b/brooklyn-server/core/src/main/java/org/apache/brooklyn/core/mgmt/internal/AbstractManagementContext.java
@@ -372,7 +372,7 @@ public abstract class AbstractManagementContext implements ManagementContextInte
         return configMap;
     }
 
-    private Object locationRegistrySemaphore = new Object();
+    private final Object locationRegistrySemaphore = new Object();
     
     @Override
     public LocationRegistry getLocationRegistry() {

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/11a0b8ae/brooklyn-server/rest/rest-server/src/test/java/org/apache/brooklyn/rest/util/ServerStoppingShutdownHandler.java
----------------------------------------------------------------------
diff --git a/brooklyn-server/rest/rest-server/src/test/java/org/apache/brooklyn/rest/util/ServerStoppingShutdownHandler.java b/brooklyn-server/rest/rest-server/src/test/java/org/apache/brooklyn/rest/util/ServerStoppingShutdownHandler.java
index 78dde57..fc3bbc4 100644
--- a/brooklyn-server/rest/rest-server/src/test/java/org/apache/brooklyn/rest/util/ServerStoppingShutdownHandler.java
+++ b/brooklyn-server/rest/rest-server/src/test/java/org/apache/brooklyn/rest/util/ServerStoppingShutdownHandler.java
@@ -67,7 +67,7 @@ public class ServerStoppingShutdownHandler implements ShutdownHandler {
         }).start();
     }
 
-    /** Expect this to be injeted; typically it is not known when this is created, but we need it to trigger shutdown. */
+    /** Expect this to be injected; typically it is not known when this is created, but we need it to trigger shutdown. */
     public void setServer(Server server) {
         this.server = server;
     }


[6/6] incubator-brooklyn git commit: This closes #1164

Posted by he...@apache.org.
This closes #1164


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

Branch: refs/heads/master
Commit: 5118860990e98881a522d377f3ceca6a1b5df868
Parents: 8303fea 11a0b8a
Author: Alex Heneveld <al...@cloudsoftcorp.com>
Authored: Thu Jan 21 01:22:59 2016 +0000
Committer: Alex Heneveld <al...@cloudsoftcorp.com>
Committed: Thu Jan 21 01:22:59 2016 +0000

----------------------------------------------------------------------
 .../apache/brooklyn/core/entity/Entities.java   | 13 ++--
 .../internal/AbstractManagementContext.java     | 11 ++-
 .../mgmt/internal/BrooklynShutdownHooks.java    |  4 +-
 .../mgmt/internal/LocalManagementContext.java   | 29 +++++---
 .../brooklyn/rest/resources/ServerResource.java |  1 +
 .../brooklyn/rest/BrooklynRestApiLauncher.java  | 32 +++++++--
 .../rest/testing/BrooklynRestApiTest.java       |  8 +--
 .../rest/util/NoOpRecordingShutdownHandler.java | 39 ++++++++++
 .../util/ServerStoppingShutdownHandler.java     | 75 ++++++++++++++++++++
 .../brooklyn/rest/util/TestShutdownHandler.java | 39 ----------
 .../main/java/org/apache/brooklyn/cli/Main.java | 46 ++++++------
 11 files changed, 210 insertions(+), 87 deletions(-)
----------------------------------------------------------------------



[3/6] incubator-brooklyn git commit: suppress explicit termination in main thread; the shutdown hook is sufficient and it's a bit of work to terminate properly in-thread

Posted by he...@apache.org.
suppress explicit termination in main thread;
the shutdown hook is sufficient and it's a bit of work to terminate properly in-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/88b76b71
Tree: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/tree/88b76b71
Diff: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/diff/88b76b71

Branch: refs/heads/master
Commit: 88b76b71bf7f977ee0c4185bba97e17ea0657baf
Parents: db44cd7
Author: Alex Heneveld <al...@cloudsoftcorp.com>
Authored: Wed Jan 20 20:49:13 2016 +0000
Committer: Alex Heneveld <al...@cloudsoftcorp.com>
Committed: Wed Jan 20 21:07:13 2016 +0000

----------------------------------------------------------------------
 .../core/mgmt/internal/BrooklynShutdownHooks.java        |  4 +++-
 .../src/main/java/org/apache/brooklyn/cli/Main.java      | 11 ++++++-----
 2 files changed, 9 insertions(+), 6 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/88b76b71/brooklyn-server/core/src/main/java/org/apache/brooklyn/core/mgmt/internal/BrooklynShutdownHooks.java
----------------------------------------------------------------------
diff --git a/brooklyn-server/core/src/main/java/org/apache/brooklyn/core/mgmt/internal/BrooklynShutdownHooks.java b/brooklyn-server/core/src/main/java/org/apache/brooklyn/core/mgmt/internal/BrooklynShutdownHooks.java
index 677a4f6..91ca5dc 100644
--- a/brooklyn-server/core/src/main/java/org/apache/brooklyn/core/mgmt/internal/BrooklynShutdownHooks.java
+++ b/brooklyn-server/core/src/main/java/org/apache/brooklyn/core/mgmt/internal/BrooklynShutdownHooks.java
@@ -187,7 +187,9 @@ public class BrooklynShutdownHooks {
             }
             entitiesToStop.addAll(entitiesToStopOnShutdown);
             for (ManagementContext mgmt: managementContextsToStopAppsOnShutdown) {
-                entitiesToStop.addAll(mgmt.getApplications());
+                if (mgmt.isRunning()) {
+                    entitiesToStop.addAll(mgmt.getApplications());
+                }
             }
             
             if (entitiesToStop.isEmpty()) {

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/88b76b71/brooklyn-server/server-cli/src/main/java/org/apache/brooklyn/cli/Main.java
----------------------------------------------------------------------
diff --git a/brooklyn-server/server-cli/src/main/java/org/apache/brooklyn/cli/Main.java b/brooklyn-server/server-cli/src/main/java/org/apache/brooklyn/cli/Main.java
index 96527db..03be76e 100644
--- a/brooklyn-server/server-cli/src/main/java/org/apache/brooklyn/cli/Main.java
+++ b/brooklyn-server/server-cli/src/main/java/org/apache/brooklyn/cli/Main.java
@@ -482,11 +482,12 @@ public class Main extends AbstractMain {
                 waitAfterLaunch(mgmt, shutdownHandler);
             }
 
-            // BrooklynShutdownHookJob will invoke terminate() to do mgmt.terminate() and BrooklynWebServer.stop();
-            // and System.exit is invoked immediately after ...
-            // but seems better to do it explicitly here. 
-            // ('launcher' is local to us so the caller *cannot* do it, and no harm in doing it twice.) 
-            launcher.terminate();
+            // do not shutdown servers here here -- 
+            // the BrooklynShutdownHookJob will invoke that and others on System.exit()
+            // which happens immediately after.
+            // might be nice to do it explicitly here, 
+            // but the server shutdown process has some special "shutdown apps" options
+            // so we'd want to refactor BrooklynShutdownHookJob to share code
             
             return null;
         }


[4/6] incubator-brooklyn git commit: background the dev-mode web server shutdown, and Entities.destroyAll is more graceful on concurrent shutdown

Posted by he...@apache.org.
background the dev-mode web server shutdown,
and Entities.destroyAll is more graceful on concurrent shutdown


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

Branch: refs/heads/master
Commit: db44cd77670672efcb6c3b44f81a716b76b072af
Parents: 2354c48
Author: Alex Heneveld <al...@cloudsoftcorp.com>
Authored: Wed Jan 20 18:26:56 2016 +0000
Committer: Alex Heneveld <al...@cloudsoftcorp.com>
Committed: Wed Jan 20 21:07:13 2016 +0000

----------------------------------------------------------------------
 .../apache/brooklyn/core/entity/Entities.java   | 13 ++++---
 .../brooklyn/rest/resources/ServerResource.java |  2 +-
 .../util/ServerStoppingShutdownHandler.java     | 38 ++++++++++++--------
 3 files changed, 33 insertions(+), 20 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/db44cd77/brooklyn-server/core/src/main/java/org/apache/brooklyn/core/entity/Entities.java
----------------------------------------------------------------------
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
index 9297388..9c8ebc8 100644
--- 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 class Entities {
                 ((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();
         }

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/db44cd77/brooklyn-server/rest/rest-server/src/main/java/org/apache/brooklyn/rest/resources/ServerResource.java
----------------------------------------------------------------------
diff --git a/brooklyn-server/rest/rest-server/src/main/java/org/apache/brooklyn/rest/resources/ServerResource.java b/brooklyn-server/rest/rest-server/src/main/java/org/apache/brooklyn/rest/resources/ServerResource.java
index e624d89..c029bd3 100644
--- a/brooklyn-server/rest/rest-server/src/main/java/org/apache/brooklyn/rest/resources/ServerResource.java
+++ b/brooklyn-server/rest/rest-server/src/main/java/org/apache/brooklyn/rest/resources/ServerResource.java
@@ -218,7 +218,7 @@ public class ServerResource extends AbstractBrooklynRestResource implements Serv
                         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);
                         }

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/db44cd77/brooklyn-server/rest/rest-server/src/test/java/org/apache/brooklyn/rest/util/ServerStoppingShutdownHandler.java
----------------------------------------------------------------------
diff --git a/brooklyn-server/rest/rest-server/src/test/java/org/apache/brooklyn/rest/util/ServerStoppingShutdownHandler.java b/brooklyn-server/rest/rest-server/src/test/java/org/apache/brooklyn/rest/util/ServerStoppingShutdownHandler.java
index 7244378..78dde57 100644
--- a/brooklyn-server/rest/rest-server/src/test/java/org/apache/brooklyn/rest/util/ServerStoppingShutdownHandler.java
+++ b/brooklyn-server/rest/rest-server/src/test/java/org/apache/brooklyn/rest/util/ServerStoppingShutdownHandler.java
@@ -20,6 +20,8 @@ package org.apache.brooklyn.rest.util;
 
 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 class ServerStoppingShutdownHandler implements ShutdownHandler {
 
     @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. */