Skip to content
Permalink
Browse files
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)
  • Loading branch information
ahgittin committed Jan 20, 2016
1 parent 8303fea commit 2a432cf38a2fb0571e4f76604c3a532b1f89c2c6
Showing 7 changed files with 143 additions and 41 deletions.
@@ -30,6 +30,7 @@
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 static int terminateAll() {
return closed;
}

private AtomicBoolean terminated = new AtomicBoolean(false);
private String managementPlaneId;
private String managementNodeId;
private BasicExecutionManager execution;
@@ -316,15 +318,26 @@ public synchronized ExecutionManager getExecutionManager() {

@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
@@ -218,6 +218,7 @@ public void run() {
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);
}
@@ -48,8 +48,8 @@
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 @@ enum StartMode {
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 Server start() {
((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 @@ private ContextHandler servletContextHandler(ManagementContext managementContext
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 @@ private ContextHandler servletContextHandler(ManagementContext managementContext
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 @@ private ContextHandler webXmlContextHandler(ManagementContext mgmt) {
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 static Server startServer(ManagementContext mgmt, ContextHandler context,
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 @@ private void installAsServletFilter(ServletContextHandler context, List<Class<?

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));
}

@@ -47,7 +47,7 @@
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 @@ protected synchronized void useLocalScannedCatalog() {
useLocalScannedCatalog = true;
}

private TestShutdownHandler createShutdownHandler() {
return new TestShutdownHandler();
private NoOpRecordingShutdownHandler createShutdownHandler() {
return new NoOpRecordingShutdownHandler();
}

protected synchronized ManagementContext getManagementContext() {
@@ -20,7 +20,7 @@

import org.apache.brooklyn.rest.util.ShutdownHandler;

public class TestShutdownHandler implements ShutdownHandler {
public class NoOpRecordingShutdownHandler implements ShutdownHandler {
private volatile boolean isRequested;

@Override
@@ -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;
}

}
@@ -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.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.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 Void apply(CatalogInitialization catInit) {
}

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;
}

0 comments on commit 2a432cf

Please sign in to comment.