ARTEMIS-2111 ManagementContext can leak (cherry picked from commit 744838faaf651fa6b433bba7b93058af5b85b313)
Project: http://git-wip-us.apache.org/repos/asf/activemq-artemis/repo Commit: http://git-wip-us.apache.org/repos/asf/activemq-artemis/commit/70a70f4c Tree: http://git-wip-us.apache.org/repos/asf/activemq-artemis/tree/70a70f4c Diff: http://git-wip-us.apache.org/repos/asf/activemq-artemis/diff/70a70f4c Branch: refs/heads/2.6.x Commit: 70a70f4c2f23fe76a0da2f10d35e15d6fdb5e8c1 Parents: 01c50e1 Author: Justin Bertram <[email protected]> Authored: Fri Oct 5 13:39:07 2018 -0500 Committer: Clebert Suconic <[email protected]> Committed: Thu Oct 11 15:04:15 2018 -0400 ---------------------------------------------------------------------- artemis-cli/pom.xml | 6 ++ .../activemq/artemis/cli/commands/Run.java | 59 +++++++++++++------- .../apache/activemq/cli/test/ArtemisTest.java | 19 +++++++ .../server/management/ManagementContext.java | 15 +++-- 4 files changed, 73 insertions(+), 26 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/activemq-artemis/blob/70a70f4c/artemis-cli/pom.xml ---------------------------------------------------------------------- diff --git a/artemis-cli/pom.xml b/artemis-cli/pom.xml index fa386ef..a8b7a92 100644 --- a/artemis-cli/pom.xml +++ b/artemis-cli/pom.xml @@ -146,6 +146,12 @@ <scope>test</scope> <type>test-jar</type> </dependency> + <dependency> + <groupId>org.apache.activemq</groupId> + <artifactId>artemis-junit</artifactId> + <version>2.7.0-SNAPSHOT</version> + <scope>test</scope> + </dependency> </dependencies> <build> http://git-wip-us.apache.org/repos/asf/activemq-artemis/blob/70a70f4c/artemis-cli/src/main/java/org/apache/activemq/artemis/cli/commands/Run.java ---------------------------------------------------------------------- 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 12c00db..759815c 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 @@ -22,12 +22,14 @@ import java.util.TimerTask; import io.airlift.airline.Command; import io.airlift.airline.Option; +import org.apache.activemq.artemis.api.core.Pair; import org.apache.activemq.artemis.cli.Artemis; import org.apache.activemq.artemis.cli.commands.tools.LockAbstract; import org.apache.activemq.artemis.cli.factory.BrokerFactory; import org.apache.activemq.artemis.cli.factory.jmx.ManagementFactory; import org.apache.activemq.artemis.cli.factory.security.SecurityManagerFactory; import org.apache.activemq.artemis.components.ExternalComponent; +import org.apache.activemq.artemis.core.server.ActivateCallback; import org.apache.activemq.artemis.core.server.management.ManagementContext; import org.apache.activemq.artemis.dto.BrokerDTO; import org.apache.activemq.artemis.dto.ComponentDTO; @@ -65,34 +67,49 @@ public class Run extends LockAbstract { public Object execute(ActionContext context) throws Exception { super.execute(context); - ManagementContextDTO managementDTO = getManagementDTO(); - managementContext = ManagementFactory.create(managementDTO); + try { + ManagementContextDTO managementDTO = getManagementDTO(); + managementContext = ManagementFactory.create(managementDTO); - Artemis.printBanner(); + Artemis.printBanner(); - BrokerDTO broker = getBrokerDTO(); + BrokerDTO broker = getBrokerDTO(); - addShutdownHook(broker.server.getConfigurationFile().getParentFile()); + addShutdownHook(broker.server.getConfigurationFile().getParentFile()); - ActiveMQSecurityManager security = SecurityManagerFactory.create(broker.security); + ActiveMQSecurityManager security = SecurityManagerFactory.create(broker.security); - server = BrokerFactory.createServer(broker.server, security); + server = BrokerFactory.createServer(broker.server, security); - managementContext.start(); - server.start(); + managementContext.start(); + server.start(); + server.getServer().registerActivateCallback(new ActivateCallback() { + @Override + public void deActivate() { + try { + managementContext.stop(); + } catch (Exception e) { + e.printStackTrace(); + } + } + }); - if (broker.web != null) { - broker.components.add(broker.web); - } + if (broker.web != null) { + broker.components.add(broker.web); + } - for (ComponentDTO componentDTO : broker.components) { - Class clazz = this.getClass().getClassLoader().loadClass(componentDTO.componentClassName); - ExternalComponent component = (ExternalComponent) clazz.newInstance(); - component.configure(componentDTO, getBrokerInstance(), getBrokerHome()); - component.start(); - server.getServer().addExternalComponent(component); + for (ComponentDTO componentDTO : broker.components) { + Class clazz = this.getClass().getClassLoader().loadClass(componentDTO.componentClassName); + ExternalComponent component = (ExternalComponent) clazz.newInstance(); + component.configure(componentDTO, getBrokerInstance(), getBrokerHome()); + component.start(); + server.getServer().addExternalComponent(component); + } + } catch (Throwable t) { + t.printStackTrace(); + stop(); } - return null; + return new Pair<>(managementContext, server.getServer()); } /** @@ -155,7 +172,9 @@ public class Run extends LockAbstract { protected void stop() { try { - server.stop(true); + if (server != null) { + server.stop(true); + } if (managementContext != null) { managementContext.stop(); } http://git-wip-us.apache.org/repos/asf/activemq-artemis/blob/70a70f4c/artemis-cli/src/test/java/org/apache/activemq/cli/test/ArtemisTest.java ---------------------------------------------------------------------- diff --git a/artemis-cli/src/test/java/org/apache/activemq/cli/test/ArtemisTest.java b/artemis-cli/src/test/java/org/apache/activemq/cli/test/ArtemisTest.java index 9045888..45b02b7 100644 --- a/artemis-cli/src/test/java/org/apache/activemq/cli/test/ArtemisTest.java +++ b/artemis-cli/src/test/java/org/apache/activemq/cli/test/ArtemisTest.java @@ -36,6 +36,7 @@ import java.util.ArrayList; import java.util.List; import java.util.concurrent.TimeUnit; +import org.apache.activemq.artemis.api.core.Pair; import org.apache.activemq.artemis.api.core.SimpleString; import org.apache.activemq.artemis.api.core.client.ClientSession; import org.apache.activemq.artemis.api.core.client.ClientSessionFactory; @@ -55,10 +56,13 @@ import org.apache.activemq.artemis.cli.commands.util.SyncCalculation; import org.apache.activemq.artemis.core.client.impl.ServerLocatorImpl; import org.apache.activemq.artemis.core.config.FileDeploymentManager; import org.apache.activemq.artemis.core.config.impl.FileConfiguration; +import org.apache.activemq.artemis.core.server.ActiveMQServer; import org.apache.activemq.artemis.core.server.JournalType; +import org.apache.activemq.artemis.core.server.management.ManagementContext; import org.apache.activemq.artemis.jlibaio.LibaioContext; import org.apache.activemq.artemis.jms.client.ActiveMQConnectionFactory; import org.apache.activemq.artemis.jms.client.ActiveMQDestination; +import org.apache.activemq.artemis.junit.Wait; import org.apache.activemq.artemis.utils.DefaultSensitiveStringCodec; import org.apache.activemq.artemis.utils.HashProcessor; import org.apache.activemq.artemis.utils.PasswordMaskingUtil; @@ -234,6 +238,21 @@ public class ArtemisTest extends CliTestBase { } @Test + public void testStopManagementContext() throws Exception { + Run.setEmbedded(true); + File instance1 = new File(temporaryFolder.getRoot(), "instance_user"); + System.setProperty("java.security.auth.login.config", instance1.getAbsolutePath() + "/etc/login.config"); + Artemis.main("create", instance1.getAbsolutePath(), "--silent", "--no-autotune", "--no-web", "--no-amqp-acceptor", "--no-mqtt-acceptor", "--no-stomp-acceptor", "--no-hornetq-acceptor"); + System.setProperty("artemis.instance", instance1.getAbsolutePath()); + Object result = Artemis.internalExecute("run"); + ManagementContext managementContext = ((Pair<ManagementContext, ActiveMQServer>)result).getA(); + ActiveMQServer activeMQServer = ((Pair<ManagementContext, ActiveMQServer>)result).getB(); + activeMQServer.stop(); + assertTrue(Wait.waitFor(() -> managementContext.isStarted() == false, 5000, 200)); + stopServer(); + } + + @Test public void testUserCommand() throws Exception { Run.setEmbedded(true); File instance1 = new File(temporaryFolder.getRoot(), "instance_user"); http://git-wip-us.apache.org/repos/asf/activemq-artemis/blob/70a70f4c/artemis-server/src/main/java/org/apache/activemq/artemis/core/server/management/ManagementContext.java ---------------------------------------------------------------------- diff --git a/artemis-server/src/main/java/org/apache/activemq/artemis/core/server/management/ManagementContext.java b/artemis-server/src/main/java/org/apache/activemq/artemis/core/server/management/ManagementContext.java index 3ff834e..175007c 100644 --- a/artemis-server/src/main/java/org/apache/activemq/artemis/core/server/management/ManagementContext.java +++ b/artemis-server/src/main/java/org/apache/activemq/artemis/core/server/management/ManagementContext.java @@ -17,11 +17,13 @@ package org.apache.activemq.artemis.core.server.management; +import java.util.concurrent.atomic.AtomicBoolean; + import org.apache.activemq.artemis.core.config.JMXConnectorConfiguration; import org.apache.activemq.artemis.core.server.ActiveMQComponent; public class ManagementContext implements ActiveMQComponent { - private boolean isStarted = false; + private AtomicBoolean isStarted = new AtomicBoolean(false); private JMXAccessControlList accessControlList; private JMXConnectorConfiguration jmxConnectorConfiguration; private ManagementConnector mBeanServer; @@ -40,20 +42,21 @@ public class ManagementContext implements ActiveMQComponent { mBeanServer = new ManagementConnector(jmxConnectorConfiguration); mBeanServer.start(); } - isStarted = true; + isStarted.set(true); } @Override public void stop() throws Exception { - isStarted = false; - if (mBeanServer != null) { - mBeanServer.stop(); + if (isStarted.getAndSet(false)) { + if (mBeanServer != null) { + mBeanServer.stop(); + } } } @Override public boolean isStarted() { - return isStarted; + return isStarted.get(); } public void setAccessControlList(JMXAccessControlList accessControlList) {
