ARTEMIS-2111 ManagementContext can leak

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>

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,6 +67,7 @@ public class Run extends LockAbstract {
    public Object execute(ActionContext context) throws Exception {
       super.execute(context);
 
+      try {
          ManagementContextDTO managementDTO = getManagementDTO();
          managementContext = ManagementFactory.create(managementDTO);
 
@@ -80,6 +83,16 @@ public class Run extends LockAbstract {
 
          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);
@@ -92,7 +105,11 @@ public class Run extends LockAbstract {
             component.start();
             server.getServer().addExternalComponent(component);
          }
-      return null;
+      } catch (Throwable t) {
+         t.printStackTrace();
+         stop();
+      }
+      return new Pair<>(managementContext, server.getServer());
    }
 
    /**
@@ -155,7 +172,9 @@ public class Run extends LockAbstract {
 
    protected void stop() {
       try {
+         if (server != null) {
             server.stop(true);
+         }
          if (managementContext != null) {
             managementContext.stop();
          }

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;
@@ -233,6 +237,21 @@ public class ArtemisTest extends CliTestBase {
       assertEquals("password2", trustPass);
    }
 
+   @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);

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 (isStarted.getAndSet(false)) {
          if (mBeanServer != null) {
             mBeanServer.stop();
          }
       }
+   }
 
    @Override
    public boolean isStarted() {
-      return isStarted;
+      return isStarted.get();
    }
 
    public void setAccessControlList(JMXAccessControlList accessControlList) {