You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@activemq.apache.org by da...@apache.org on 2012/08/31 11:10:24 UTC
svn commit: r1379376 - in
/activemq/trunk/activemq-core/src/main/java/org/apache/activemq/broker:
BrokerService.java jmx/ManagementContext.java
Author: davsclaus
Date: Fri Aug 31 09:10:23 2012
New Revision: 1379376
URL: http://svn.apache.org/viewvc?rev=1379376&view=rev
Log:
AMQ-4008: Fixed MDC leak during shutting down AMQ broker, such as reported by Apache Tomcat.
Modified:
activemq/trunk/activemq-core/src/main/java/org/apache/activemq/broker/BrokerService.java
activemq/trunk/activemq-core/src/main/java/org/apache/activemq/broker/jmx/ManagementContext.java
Modified: activemq/trunk/activemq-core/src/main/java/org/apache/activemq/broker/BrokerService.java
URL: http://svn.apache.org/viewvc/activemq/trunk/activemq-core/src/main/java/org/apache/activemq/broker/BrokerService.java?rev=1379376&r1=1379375&r2=1379376&view=diff
==============================================================================
--- activemq/trunk/activemq-core/src/main/java/org/apache/activemq/broker/BrokerService.java (original)
+++ activemq/trunk/activemq-core/src/main/java/org/apache/activemq/broker/BrokerService.java Fri Aug 31 09:10:23 2012
@@ -506,7 +506,7 @@ public class BrokerService implements Se
// as its way too easy to not be completely sure if start() has been
// called or not with the gazillion of different configuration
// mechanisms
- // throw new IllegalStateException("Allready started.");
+ // throw new IllegalStateException("Already started.");
return;
}
@@ -518,7 +518,14 @@ public class BrokerService implements Se
}
processHelperProperties();
if (isUseJmx()) {
- startManagementContext();
+ // need to remove MDC during starting JMX, as that would otherwise causes leaks, as spawned threads inheirt the MDC and
+ // we cannot cleanup clear that during shutdown of the broker.
+ MDC.remove("activemq.broker");
+ try {
+ startManagementContext();
+ } finally {
+ MDC.put("activemq.broker", brokerName);
+ }
}
// in jvm master slave, lets not publish over existing broker till we get the lock
final BrokerRegistry brokerRegistry = BrokerRegistry.getInstance();
@@ -678,7 +685,7 @@ public class BrokerService implements Se
stopAllConnectors(stopper);
// remove any VMTransports connected
// this has to be done after services are stopped,
- // to avoid timimg issue with discovery (spinning up a new instance)
+ // to avoid timing issue with discovery (spinning up a new instance)
BrokerRegistry.getInstance().unbind(getBrokerName());
VMTransportFactory.stopped(getBrokerName());
if (broker != null) {
@@ -2290,6 +2297,7 @@ public class BrokerService implements Se
}
protected void startManagementContext() throws Exception {
+ getManagementContext().setBrokerName(brokerName);
getManagementContext().start();
adminView = new BrokerView(this, null);
ObjectName objectName = getBrokerObjectName();
Modified: activemq/trunk/activemq-core/src/main/java/org/apache/activemq/broker/jmx/ManagementContext.java
URL: http://svn.apache.org/viewvc/activemq/trunk/activemq-core/src/main/java/org/apache/activemq/broker/jmx/ManagementContext.java?rev=1379376&r1=1379375&r2=1379376&view=diff
==============================================================================
--- activemq/trunk/activemq-core/src/main/java/org/apache/activemq/broker/jmx/ManagementContext.java (original)
+++ activemq/trunk/activemq-core/src/main/java/org/apache/activemq/broker/jmx/ManagementContext.java Fri Aug 31 09:10:23 2012
@@ -19,6 +19,7 @@ package org.apache.activemq.broker.jmx;
import org.apache.activemq.Service;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
+import org.slf4j.MDC;
import javax.management.*;
import javax.management.remote.JMXConnectorServer;
@@ -69,6 +70,7 @@ public class ManagementContext implement
private ServerSocket registrySocket;
private final List<ObjectName> registeredMBeanNames = new CopyOnWriteArrayList<ObjectName>();
private boolean allowRemoteAddressInMBeanNames = true;
+ private String brokerName;
public ManagementContext() {
this(null);
@@ -90,21 +92,32 @@ public class ManagementContext implement
Thread t = new Thread("JMX connector") {
@Override
public void run() {
+ // ensure we use MDC logging with the broker name, so people can see the logs if MDC was in use
+ if (brokerName != null) {
+ MDC.put("activemq.broker", brokerName);
+ }
try {
JMXConnectorServer server = connectorServer;
if (started.get() && server != null) {
LOG.debug("Starting JMXConnectorServer...");
connectorStarting.set(true);
try {
+ // need to remove MDC as we must not inherit MDC in child threads causing leaks
+ MDC.remove("activemq.broker");
server.start();
} finally {
- connectorStarting.set(false);
+ if (brokerName != null) {
+ MDC.put("activemq.broker", brokerName);
+ }
+ connectorStarting.set(false);
}
LOG.info("JMX consoles can connect to " + server.getAddress());
}
} catch (IOException e) {
LOG.warn("Failed to start jmx connector: " + e.getMessage());
LOG.debug("Reason for failed jms connector start", e);
+ } finally {
+ MDC.remove("activemq.broker");
}
}
};
@@ -160,6 +173,21 @@ public class ManagementContext implement
}
/**
+ * Gets the broker name this context is used by, may be <tt>null</tt>
+ * if the broker name was not set.
+ */
+ public String getBrokerName() {
+ return brokerName;
+ }
+
+ /**
+ * Sets the broker name this context is being used by.
+ */
+ public void setBrokerName(String brokerName) {
+ this.brokerName = brokerName;
+ }
+
+ /**
* @return Returns the jmxDomainName.
*/
public String getJmxDomainName() {