395148: avoid certain hangs in shutdown and document the different shutdown cases
diff --git a/org.eclipse.virgo.nano.core/src/main/java/org/eclipse/virgo/nano/core/internal/CoreBundleActivator.java b/org.eclipse.virgo.nano.core/src/main/java/org/eclipse/virgo/nano/core/internal/CoreBundleActivator.java
index 715875a..b20dff6 100644
--- a/org.eclipse.virgo.nano.core/src/main/java/org/eclipse/virgo/nano/core/internal/CoreBundleActivator.java
+++ b/org.eclipse.virgo.nano.core/src/main/java/org/eclipse/virgo/nano/core/internal/CoreBundleActivator.java
@@ -211,8 +211,8 @@
protected Shutdown createShutdown(BundleContext context, EventLogger eventLogger) {
Framework framework = (Framework) context.getBundle(0);
-
- ShutdownManager manager = new ShutdownManager(eventLogger, framework);
+ Runtime runtime = Runtime.getRuntime();
+ ShutdownManager manager = new ShutdownManager(eventLogger, framework, runtime);
return manager;
}
diff --git a/org.eclipse.virgo.nano.core/src/main/java/org/eclipse/virgo/nano/core/internal/ShutdownManager.java b/org.eclipse.virgo.nano.core/src/main/java/org/eclipse/virgo/nano/core/internal/ShutdownManager.java
index de21247..d49024e 100644
--- a/org.eclipse.virgo.nano.core/src/main/java/org/eclipse/virgo/nano/core/internal/ShutdownManager.java
+++ b/org.eclipse.virgo.nano.core/src/main/java/org/eclipse/virgo/nano/core/internal/ShutdownManager.java
@@ -14,6 +14,9 @@
import java.util.concurrent.TimeUnit;
import java.util.concurrent.atomic.AtomicInteger;
+import org.eclipse.virgo.medic.eventlog.EventLogger;
+import org.eclipse.virgo.nano.core.Shutdown;
+import org.eclipse.virgo.nano.diagnostics.KernelLogEvents;
import org.osgi.framework.BundleContext;
import org.osgi.framework.BundleEvent;
import org.osgi.framework.BundleException;
@@ -23,18 +26,47 @@
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
-
-import org.eclipse.virgo.nano.core.Shutdown;
-import org.eclipse.virgo.nano.diagnostics.KernelLogEvents;
-import org.eclipse.virgo.medic.eventlog.EventLogger;
-
/**
- * Standard implementation of {@link Shutdown} that performs all shutdown actions synchronously.
+ * Standard implementation of {@link Shutdown} that may be called to initiate JVM shutdown. This class also listens for
+ * <i>unsolicited</i> shutdown (that is, shutdown not initiated by this class) of the JVM or the OSGi framework and
+ * reacts accordingly.
* <p />
- * This implementation registers a {@link Runtime#addShutdownHook(Thread) shutdown hook} to perform graceful shutdown
- * when the user kills the VM.
+ * JVM shutdown may be initiated in either of two ways:
+ * <ol>
+ * <li>By this class (when a program calls one of the methods of the {@link Shutdown} interface). If graceful shutdown
+ * is requested, this class shuts down the OSGi framework and then the JVM. If immediate shutdown is requested, this
+ * class shuts down the JVM.</li>
+ * <li>Not by this class. A {@link Runtime#addShutdownHook(Thread) shutdown hook} previously registered by this class
+ * responds to JVM shutdown by shutting down the OSGi framework before allowing JVM shutdown to continue. Note that
+ * {@link System#exit} must not be called under the shutdown hook as this can block indefinitely (see the javadoc of
+ * {@link Runtime#exit}).</li>
+ * </ol>
+ * If this class attempts to shut down the OSGi framework but this takes longer than {@code SHUTDOWN_TIMOUT}, an error
+ * message is written to the event log and the JVM is halted abruptly with a non-zero exit code.
* <p />
- *
+ * When OSGi framework shutdown occurs, a synchronous bundle listener previously registered by this class unregisters
+ * the shutdown hook (unless this class initiated the OSGi framework shutdown in response to an unsolicited JVM
+ * shutdown, in which case the shutdown hook is left in place).</li>
+ * <p />
+ * This class expects some recursive invocations and avoids others:
+ * <ul>
+ * <li>A graceful shutdown request on the {@link Shutdown} interface normally results in the synchronous bundle listener
+ * being driven on OSGi framework shutdown.</li>
+ * <li>An immediate shutdown request on the {@link Shutdown} interface unregisters the shutdown hook so that it is not
+ * driven when the JVM is shut down.</li>
+ * <li>An unsolicited JVM termination prevents the shutdown hook from being unregistered.</li>
+ * </ul>
+ * <p />
+ * So, in summary, the JVM may terminate in the following ways:
+ * <ol>
+ * <li>Solicited graceful shutdown, which attempts to stop the OSGi framework and, if successful, exits the JVM.</li>
+ * <li>Unsolicited graceful shutdown, which attempts to stop the OSGi framework and, if successful, allows JVM
+ * termination to continue.</li>
+ * <li>Solicited immediate shutdown, which exits the JVM.</li>
+ * <li>Solicited halt if an attempt by this class to stop the OSGi framework fails or times out.</li>
+ * <li>Unsolicited halt, which does not involve this class.</li>
+ * </ol>
+ * <p />
* <strong>Concurrent Semantics</strong><br />
*
* Threadsafe
@@ -42,6 +74,10 @@
*/
class ShutdownManager implements Shutdown {
+ private static final int NORMAL_TERMINATION_EXIT_CODE = 0;
+
+ private static final int GRACEFUL_TERMINATION_FAILURE_EXIT_CODE = 1;
+
private static final Logger LOGGER = LoggerFactory.getLogger(ShutdownManager.class);
private static final long SHUTDOWN_TIMEOUT = TimeUnit.SECONDS.toMillis(30);
@@ -56,13 +92,17 @@
private final Framework framework;
+ private final Runtime runtime;
+
private final Thread shutdownHook = new Thread(new Runnable() {
+ @Override
public void run() {
try {
- // set to stopping so we don't try to remove the shutdown hook later
- state.set(STATE_STOPPING);
- ShutdownManager.this.shutdown();
+ // set to stopping so we don't remove the shutdown hook later
+ if (ShutdownManager.this.compareAndSetHookStopping()) {
+ ShutdownManager.this.doShutdown(false);
+ }
} catch (Throwable t) {
t.printStackTrace();
}
@@ -70,10 +110,11 @@
});
- public ShutdownManager(EventLogger eventLogger, Framework framework) {
+ public ShutdownManager(EventLogger eventLogger, Framework framework, Runtime runtime) {
this.eventLogger = eventLogger;
this.framework = framework;
- Runtime.getRuntime().addShutdownHook(this.shutdownHook);
+ this.runtime = runtime;
+ runtime.addShutdownHook(this.shutdownHook);
BundleContext bundleContext = framework.getBundleContext();
bundleContext.addBundleListener(new ShutdownLoggingListener());
}
@@ -81,7 +122,12 @@
/**
* {@inheritDoc}
*/
+ @Override
public void shutdown() {
+ doShutdown(true);
+ }
+
+ private void doShutdown(boolean solicitedShutdown) {
FrameworkEvent shutdownResponse = null;
try {
this.framework.stop();
@@ -92,38 +138,66 @@
LOGGER.error("Interrupted during shutdown.", ex);
}
- if (!isSuccessfulStopResponse(shutdownResponse)) {
- immediateShutdown();
+ if (isSuccessfulStopResponse(shutdownResponse)) {
+ // If this class initiated shutdown, shut down the JVM. Otherwise allow JVM termination to continue.
+ if (solicitedShutdown) {
+ initiateJvmTermination();
+ }
} else {
- removeShutdownHook();
+ // Escalate to JVM halt.
+ this.eventLogger.log(KernelLogEvents.SHUTDOWN_HALTED);
+ haltJvm(GRACEFUL_TERMINATION_FAILURE_EXIT_CODE);
}
}
+ private void initiateJvmTermination() {
+ removeShutdownHook();
+ exitJvm();
+ }
+
/**
* {@inheritDoc}
*/
+ @Override
public void immediateShutdown() {
- removeShutdownHook();
this.eventLogger.log(KernelLogEvents.IMMEDIATE_SHUTDOWN_INITIATED);
- exitVM();
+ initiateJvmTermination();
}
-
- protected void exitVM() {
- System.exit(0);
+
+ /**
+ * This method must not be overridden except by testcases.
+ */
+ protected void exitJvm() {
+ System.exit(NORMAL_TERMINATION_EXIT_CODE);
+ }
+
+ /**
+ * This method must not be overridden except by testcases.
+ */
+ protected void haltJvm(int status) {
+ this.runtime.halt(status);
}
private boolean isSuccessfulStopResponse(FrameworkEvent shutdownResponse) {
return shutdownResponse != null && shutdownResponse.getType() == FrameworkEvent.STOPPED;
}
- private void removeShutdownHook() {
- if(this.state.compareAndSet(STATE_RUNNING, STATE_STOPPING)) {
- Runtime.getRuntime().removeShutdownHook(this.shutdownHook);
+ /**
+ * This method must only be called by testcases.
+ */
+ final void removeShutdownHook() {
+ if (compareAndSetHookStopping()) {
+ this.runtime.removeShutdownHook(this.shutdownHook);
}
}
+ private boolean compareAndSetHookStopping() {
+ return this.state.compareAndSet(STATE_RUNNING, STATE_STOPPING);
+ }
+
private final class ShutdownLoggingListener implements SynchronousBundleListener {
+ @Override
public void bundleChanged(BundleEvent event) {
BundleContext bundleContext = ShutdownManager.this.framework.getBundleContext();
if (BundleEvent.STOPPING == event.getType() && event.getBundle() == bundleContext.getBundle()) {
diff --git a/org.eclipse.virgo.nano.core/src/main/java/org/eclipse/virgo/nano/diagnostics/KernelLogEvents.java b/org.eclipse.virgo.nano.core/src/main/java/org/eclipse/virgo/nano/diagnostics/KernelLogEvents.java
index 8e9507c..843c1e6 100644
--- a/org.eclipse.virgo.nano.core/src/main/java/org/eclipse/virgo/nano/diagnostics/KernelLogEvents.java
+++ b/org.eclipse.virgo.nano.core/src/main/java/org/eclipse/virgo/nano/diagnostics/KernelLogEvents.java
@@ -35,6 +35,7 @@
SHUTDOWN_INITIATED(10, Level.INFO), //
IMMEDIATE_SHUTDOWN_INITIATED(11, Level.INFO), //
+ SHUTDOWN_HALTED(12, Level.ERROR), //
APPLICATION_CONTEXT_DEPENDENCY_DELAYED(100, Level.WARNING), //
APPLICATION_CONTEXT_DEPENDENCY_SATISFIED(101, Level.INFO), //
diff --git a/org.eclipse.virgo.nano.core/src/main/resources/EventLogMessages.properties b/org.eclipse.virgo.nano.core/src/main/resources/EventLogMessages.properties
index 1cb7745..1405dd9 100644
--- a/org.eclipse.virgo.nano.core/src/main/resources/EventLogMessages.properties
+++ b/org.eclipse.virgo.nano.core/src/main/resources/EventLogMessages.properties
@@ -10,6 +10,7 @@
KE0010I = Shutdown initiated.
KE0011I = Immediate shutdown initiated.
+KE0012E = Shutdown failed - halting JVM.
KE0100W = Reference '{}' in bundle '{}' version '{}' is waiting for service with filter '{}'.
KE0101I = Reference '{}' in bundle '{}' version '{}' was satisfied by service with filter '{}'.
diff --git a/org.eclipse.virgo.nano.core/src/test/java/org/eclipse/virgo/nano/core/internal/ShutdownManagerTests.java b/org.eclipse.virgo.nano.core/src/test/java/org/eclipse/virgo/nano/core/internal/ShutdownManagerTests.java
index 4df2f92..230a871 100644
--- a/org.eclipse.virgo.nano.core/src/test/java/org/eclipse/virgo/nano/core/internal/ShutdownManagerTests.java
+++ b/org.eclipse.virgo.nano.core/src/test/java/org/eclipse/virgo/nano/core/internal/ShutdownManagerTests.java
@@ -16,6 +16,7 @@
import static org.easymock.EasyMock.replay;
import static org.easymock.EasyMock.verify;
import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertTrue;
import java.util.List;
@@ -34,70 +35,101 @@
import org.eclipse.virgo.test.stubs.framework.StubBundleContext;
public class ShutdownManagerTests {
-
- private final Framework framework = createMock(Framework.class);
-
- private final MockEventLogger eventLogger = new MockEventLogger();
-
- private final StubBundle bundle = new StubBundle();
-
- private final StubBundleContext bundleContext = new StubBundleContext(bundle);
-
- @Test
- public void shutdown() throws BundleException, InterruptedException {
- expect(this.framework.getBundleContext()).andReturn(this.bundleContext).anyTimes();
- this.framework.stop();
- expect(this.framework.waitForStop(30000)).andReturn(new FrameworkEvent(FrameworkEvent.STOPPED, this.bundle, null));
-
- replay(this.framework);
-
- ShutdownManager shutdownManager = new ShutdownManager(this.eventLogger, this.framework);
- shutdownManager.shutdown();
-
- verify(this.framework);
- }
-
- @Test
- public void failedShutdownDrivesImmediateShutdown() throws Exception {
- expect(this.framework.getBundleContext()).andReturn(this.bundleContext).anyTimes();
- this.framework.stop();
- expect(this.framework.waitForStop(30000)).andReturn(new FrameworkEvent(FrameworkEvent.WAIT_TIMEDOUT, this.bundle, null));
-
- replay(this.framework);
-
- UnitTestShutdownManager shutdownManager = new UnitTestShutdownManager(this.eventLogger, this.framework);
- shutdownManager.shutdown();
-
- verify(this.framework);
-
- this.eventLogger.isLogged("KE0011I");
- }
-
- @Test
- public void shutdownMessageLogged() {
- expect(this.framework.getBundleContext()).andReturn(this.bundleContext).anyTimes();
-
- replay(this.framework);
- ShutdownManager shutdownManager = new ShutdownManager(this.eventLogger, this.framework);
- verify(this.framework);
-
- List<BundleListener> bundleListeners = this.bundleContext.getBundleListeners();
-
- assertEquals(1, bundleListeners.size());
- bundleListeners.get(0).bundleChanged(new BundleEvent(BundleEvent.STOPPING, this.bundle));
-
- assertTrue(this.eventLogger.isLogged("KE0010I"));
- }
-
- private static final class UnitTestShutdownManager extends ShutdownManager {
+ private final Framework framework = createMock(Framework.class);
+
+ private final Runtime runtime = Runtime.getRuntime();
- public UnitTestShutdownManager(EventLogger eventLogger, Framework framework) {
- super(eventLogger, framework);
- }
+ private final MockEventLogger eventLogger = new MockEventLogger();
- @Override
- public void exitVM() {
- }
- }
+ private final StubBundle bundle = new StubBundle();
+
+ private final StubBundleContext bundleContext = new StubBundleContext(bundle);
+
+ @Test
+ public void shutdown() throws BundleException, InterruptedException {
+
+ expect(this.framework.getBundleContext()).andReturn(this.bundleContext).anyTimes();
+ this.framework.stop();
+ expect(this.framework.waitForStop(30000)).andReturn(new FrameworkEvent(FrameworkEvent.STOPPED, this.bundle, null));
+
+ replay(this.framework);
+
+ UnitTestShutdownManager shutdownManager = new UnitTestShutdownManager(this.eventLogger, this.framework, this.runtime);
+ shutdownManager.shutdown();
+
+ verify(this.framework);
+
+ assertTrue(shutdownManager.isExited());
+ assertFalse(shutdownManager.isHalted());
+ }
+
+ @Test
+ public void failedShutdownDrivesImmediateShutdown() throws Exception {
+ expect(this.framework.getBundleContext()).andReturn(this.bundleContext).anyTimes();
+ this.framework.stop();
+ expect(this.framework.waitForStop(30000)).andReturn(new FrameworkEvent(FrameworkEvent.WAIT_TIMEDOUT, this.bundle, null));
+
+ replay(this.framework);
+
+ UnitTestShutdownManager shutdownManager = new UnitTestShutdownManager(this.eventLogger, this.framework, this.runtime);
+ shutdownManager.shutdown();
+
+ verify(this.framework);
+
+ assertTrue(this.eventLogger.isLogged("KE0012E"));
+ assertFalse(shutdownManager.isExited());
+ assertTrue(shutdownManager.isHalted());
+
+ shutdownManager.removeShutdownHook(); // avoid error messages on the console
+ }
+
+ @Test
+ public void shutdownMessageLogged() {
+ expect(this.framework.getBundleContext()).andReturn(this.bundleContext).anyTimes();
+
+ replay(this.framework);
+ UnitTestShutdownManager shutdownManager = new UnitTestShutdownManager(this.eventLogger, this.framework, this.runtime);
+ verify(this.framework);
+
+ List<BundleListener> bundleListeners = this.bundleContext.getBundleListeners();
+
+ assertEquals(1, bundleListeners.size());
+ bundleListeners.get(0).bundleChanged(new BundleEvent(BundleEvent.STOPPING, this.bundle));
+
+ assertTrue(this.eventLogger.isLogged("KE0010I"));
+ assertFalse(shutdownManager.isExited());
+ assertFalse(shutdownManager.isHalted());
+
+ }
+
+ private static final class UnitTestShutdownManager extends ShutdownManager {
+
+ private volatile boolean exited = false;
+
+ private volatile boolean halted = false;
+
+ public UnitTestShutdownManager(EventLogger eventLogger, Framework framework, Runtime runtime) {
+ super(eventLogger, framework, runtime);
+ }
+
+ @Override
+ protected void exitJvm() {
+ this.exited = true;
+ }
+
+ @Override
+ protected void haltJvm(int status) {
+ this.halted = true;
+ }
+
+ public boolean isExited() {
+ return this.exited;
+ }
+
+ public boolean isHalted() {
+ return this.halted;
+ }
+ }
+
}
