diff --git a/artemis-cli/src/main/java/org/apache/activemq/artemis/cli/commands/Run.java b/artemis-cli/src/main/java/org/apache/activemq/artemis/cli/commands/Run.java index 5efbef4c7b..1811365175 100644 --- a/artemis-cli/src/main/java/org/apache/activemq/artemis/cli/commands/Run.java +++ b/artemis-cli/src/main/java/org/apache/activemq/artemis/cli/commands/Run.java @@ -134,7 +134,7 @@ public class Run extends LockAbstract { if (file.exists()) { try { try { - server.stop(true); + server.exit(); } catch (Exception e) { e.printStackTrace(); } @@ -155,7 +155,7 @@ public class Run extends LockAbstract { @Override public void run() { try { - server.stop(true); + server.exit(); } catch (Exception e) { e.printStackTrace(); } diff --git a/artemis-cli/src/main/java/org/apache/activemq/artemis/integration/FileBroker.java b/artemis-cli/src/main/java/org/apache/activemq/artemis/integration/FileBroker.java index 2776769425..03df935483 100644 --- a/artemis-cli/src/main/java/org/apache/activemq/artemis/integration/FileBroker.java +++ b/artemis-cli/src/main/java/org/apache/activemq/artemis/integration/FileBroker.java @@ -118,15 +118,19 @@ public class FileBroker implements Broker { } @Override - public void stop(boolean isShutdown) throws Exception { + public void exit() throws Exception { + stop(true); + } + + private void stop(boolean isShutdown) throws Exception { if (!started) { return; } ActiveMQComponent[] mqComponents = new ActiveMQComponent[components.size()]; components.values().toArray(mqComponents); for (int i = mqComponents.length - 1; i >= 0; i--) { - if (mqComponents[i] instanceof ServiceComponent) { - ((ServiceComponent)mqComponents[i]).stop(isShutdown); + if (mqComponents[i] instanceof ServiceComponent && isShutdown) { + ((ServiceComponent) mqComponents[i]).exit(); } else { mqComponents[i].stop(); } diff --git a/artemis-commons/src/main/java/org/apache/activemq/artemis/core/server/ServiceComponent.java b/artemis-commons/src/main/java/org/apache/activemq/artemis/core/server/ServiceComponent.java index 2fb0b6f309..6c347dd327 100644 --- a/artemis-commons/src/main/java/org/apache/activemq/artemis/core/server/ServiceComponent.java +++ b/artemis-commons/src/main/java/org/apache/activemq/artemis/core/server/ServiceComponent.java @@ -21,5 +21,6 @@ package org.apache.activemq.artemis.core.server; */ public interface ServiceComponent extends ActiveMQComponent { - void stop(boolean isShutdown) throws Exception; + //called by shutdown hooks before exit the VM + void exit() throws Exception; } diff --git a/artemis-server/src/main/java/org/apache/activemq/artemis/core/server/ActiveMQServer.java b/artemis-server/src/main/java/org/apache/activemq/artemis/core/server/ActiveMQServer.java index bc020100fa..5798142867 100644 --- a/artemis-server/src/main/java/org/apache/activemq/artemis/core/server/ActiveMQServer.java +++ b/artemis-server/src/main/java/org/apache/activemq/artemis/core/server/ActiveMQServer.java @@ -64,7 +64,7 @@ import org.apache.activemq.artemis.utils.ExecutorFactory; *

* This is not part of our public API. */ -public interface ActiveMQServer extends ActiveMQComponent { +public interface ActiveMQServer extends ServiceComponent { /** * Sets the server identity. diff --git a/artemis-server/src/main/java/org/apache/activemq/artemis/core/server/impl/ActiveMQServerImpl.java b/artemis-server/src/main/java/org/apache/activemq/artemis/core/server/impl/ActiveMQServerImpl.java index 0332137847..aa1ebf398e 100644 --- a/artemis-server/src/main/java/org/apache/activemq/artemis/core/server/impl/ActiveMQServerImpl.java +++ b/artemis-server/src/main/java/org/apache/activemq/artemis/core/server/impl/ActiveMQServerImpl.java @@ -130,6 +130,7 @@ import org.apache.activemq.artemis.core.server.QueueQueryResult; import org.apache.activemq.artemis.api.core.RoutingType; import org.apache.activemq.artemis.core.server.SecuritySettingPlugin; import org.apache.activemq.artemis.core.server.ServerSession; +import org.apache.activemq.artemis.core.server.ServiceComponent; import org.apache.activemq.artemis.core.server.ServiceRegistry; import org.apache.activemq.artemis.core.server.cluster.BackupManager; import org.apache.activemq.artemis.core.server.cluster.ClusterManager; @@ -335,7 +336,7 @@ public class ActiveMQServerImpl implements ActiveMQServer { @Override public void stop() throws Exception { - internalStop(); + internalStop(false); } @Override @@ -667,19 +668,24 @@ public class ActiveMQServerImpl implements ActiveMQServer { thread.start(); } + @Override + public void exit() throws Exception { + internalStop(true); + } + @Override public final void stop() throws Exception { + internalStop(false); + } + + private void internalStop(boolean isExit) throws Exception { try { - internalStop(); + stop(false, isExit); } finally { networkHealthCheck.stop(); } } - private void internalStop() throws Exception { - stop(false); - } - @Override public void addActivationParam(String key, Object val) { activationParams.put(key, val); @@ -810,7 +816,11 @@ public class ActiveMQServerImpl implements ActiveMQServer { @Override public final void stop(boolean failoverOnServerShutdown) throws Exception { - stop(failoverOnServerShutdown, false, false); + stop(failoverOnServerShutdown, false, false, false); + } + + public final void stop(boolean failoverOnServerShutdown, boolean isExit) throws Exception { + stop(failoverOnServerShutdown, false, false, isExit); } @Override @@ -830,12 +840,16 @@ public class ActiveMQServerImpl implements ActiveMQServer { } } + void stop(boolean failoverOnServerShutdown, final boolean criticalIOError, boolean restarting) { + this.stop(failoverOnServerShutdown, criticalIOError, restarting, false); + } + /** * Stops the server * * @param criticalIOError whether we have encountered an IO error with the journal etc */ - void stop(boolean failoverOnServerShutdown, final boolean criticalIOError, boolean restarting) { + void stop(boolean failoverOnServerShutdown, final boolean criticalIOError, boolean restarting, boolean isExit) { synchronized (this) { if (state == SERVER_STATE.STOPPED || state == SERVER_STATE.STOPPING) { @@ -1024,7 +1038,11 @@ public class ActiveMQServerImpl implements ActiveMQServer { for (ActiveMQComponent externalComponent : externalComponents) { try { - externalComponent.stop(); + if (isExit && externalComponent instanceof ServiceComponent) { + ((ServiceComponent)externalComponent).exit(); + } else { + externalComponent.stop(); + } } catch (Exception e) { ActiveMQServerLogger.LOGGER.errorStoppingComponent(e, externalComponent.getClass().getName()); } diff --git a/artemis-server/src/test/java/org/apache/activemq/artemis/core/server/impl/EmbeddedServerTest.java b/artemis-server/src/test/java/org/apache/activemq/artemis/core/server/impl/EmbeddedServerTest.java index 4b882cf3bb..e47440dcbe 100644 --- a/artemis-server/src/test/java/org/apache/activemq/artemis/core/server/impl/EmbeddedServerTest.java +++ b/artemis-server/src/test/java/org/apache/activemq/artemis/core/server/impl/EmbeddedServerTest.java @@ -24,13 +24,18 @@ import org.apache.activemq.artemis.api.core.TransportConfiguration; import org.apache.activemq.artemis.core.config.Configuration; import org.apache.activemq.artemis.core.config.impl.ConfigurationImpl; import org.apache.activemq.artemis.core.remoting.impl.invm.InVMAcceptorFactory; +import org.apache.activemq.artemis.core.server.ActiveMQComponent; import org.apache.activemq.artemis.core.server.ActiveMQServer; import org.apache.activemq.artemis.core.server.ActiveMQServers; +import org.apache.activemq.artemis.core.server.ServiceComponent; import org.junit.After; import org.junit.Assert; import org.junit.Before; import org.junit.Test; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; + public class EmbeddedServerTest { private static final String SERVER_LOCK_NAME = "server.lock"; @@ -64,6 +69,74 @@ public class EmbeddedServerTest { public void testNoLockFileWithPersistenceFalse() { Path journalDir = Paths.get(SERVER_JOURNAL_DIR, SERVER_LOCK_NAME); boolean lockExists = Files.exists(journalDir); - Assert.assertFalse(lockExists); + assertFalse(lockExists); + } + + @Test + //make sure the correct stop/exit API is called. + public void testExternalComponentStop() throws Exception { + FakeExternalComponent normalComponent = new FakeExternalComponent(); + FakeExternalServiceComponent serviceComponent = new FakeExternalServiceComponent(); + + server.addExternalComponent(normalComponent); + server.addExternalComponent(serviceComponent); + + server.stop(); + assertTrue(normalComponent.stopCalled); + + assertTrue(serviceComponent.stopCalled); + assertFalse(serviceComponent.exitCalled); + + normalComponent.resetFlags(); + serviceComponent.resetFlags(); + + server.start(); + server.exit(); + assertTrue(normalComponent.stopCalled); + + assertFalse(serviceComponent.stopCalled); + assertTrue(serviceComponent.exitCalled); + } + + private class FakeExternalComponent implements ActiveMQComponent { + + volatile boolean startCalled; + volatile boolean stopCalled; + + @Override + public void start() throws Exception { + startCalled = true; + } + + @Override + public void stop() throws Exception { + stopCalled = true; + } + + @Override + public boolean isStarted() { + return startCalled; + } + + public void resetFlags() { + startCalled = false; + stopCalled = false; + } + } + + private class FakeExternalServiceComponent extends FakeExternalComponent implements ServiceComponent { + + volatile boolean exitCalled; + + @Override + public void exit() throws Exception { + exitCalled = true; + } + + @Override + public void resetFlags() { + super.resetFlags(); + exitCalled = false; + } } } diff --git a/artemis-web/src/main/java/org/apache/activemq/artemis/component/WebServerComponent.java b/artemis-web/src/main/java/org/apache/activemq/artemis/component/WebServerComponent.java index a6df2720c8..6dce708dc8 100644 --- a/artemis-web/src/main/java/org/apache/activemq/artemis/component/WebServerComponent.java +++ b/artemis-web/src/main/java/org/apache/activemq/artemis/component/WebServerComponent.java @@ -150,6 +150,7 @@ public class WebServerComponent implements ExternalComponent { //somehow when broker is stopped and restarted quickly //this tmpdir won't get deleted sometimes boolean fileDeleted = TimeUtils.waitOnBoolean(false, 5000, tmpdir::exists); + if (!fileDeleted) { //because the execution order of shutdown hooks are //not determined, so it's possible that the deleteOnExit @@ -190,6 +191,10 @@ public class WebServerComponent implements ExternalComponent { } @Override + public void exit() throws Exception { + stop(true); + } + public void stop(boolean isShutdown) throws Exception { if (isShutdown) { internalStop();