You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@activemq.apache.org by cl...@apache.org on 2018/10/10 22:43:18 UTC
[2/2] activemq-artemis git commit: ARTEMIS-2111 ManagementContext can
leak
ARTEMIS-2111 ManagementContext can leak
Project: http://git-wip-us.apache.org/repos/asf/activemq-artemis/repo
Commit: http://git-wip-us.apache.org/repos/asf/activemq-artemis/commit/744838fa
Tree: http://git-wip-us.apache.org/repos/asf/activemq-artemis/tree/744838fa
Diff: http://git-wip-us.apache.org/repos/asf/activemq-artemis/diff/744838fa
Branch: refs/heads/master
Commit: 744838faaf651fa6b433bba7b93058af5b85b313
Parents: 81a5dd3
Author: Justin Bertram <jb...@apache.org>
Authored: Fri Oct 5 13:39:07 2018 -0500
Committer: Clebert Suconic <cl...@apache.org>
Committed: Wed Oct 10 18:43:10 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/744838fa/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/744838fa/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/744838fa/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/744838fa/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) {