AMQ-4008: Fixed MDC leak during shutting down AMQ broker, such as reported by Apache Tomcat.

git-svn-id: https://svn.apache.org/repos/asf/activemq/trunk@1379376 13f79535-47bb-0310-9956-ffa450edef68
This commit is contained in:
Claus Ibsen 2012-08-31 09:10:23 +00:00
parent d6025b8a09
commit eea9fca04d
2 changed files with 40 additions and 4 deletions

View File

@ -506,7 +506,7 @@ public class BrokerService implements Service {
// as its way too easy to not be completely sure if start() has been // as its way too easy to not be completely sure if start() has been
// called or not with the gazillion of different configuration // called or not with the gazillion of different configuration
// mechanisms // mechanisms
// throw new IllegalStateException("Allready started."); // throw new IllegalStateException("Already started.");
return; return;
} }
@ -518,7 +518,14 @@ public class BrokerService implements Service {
} }
processHelperProperties(); processHelperProperties();
if (isUseJmx()) { 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 // in jvm master slave, lets not publish over existing broker till we get the lock
final BrokerRegistry brokerRegistry = BrokerRegistry.getInstance(); final BrokerRegistry brokerRegistry = BrokerRegistry.getInstance();
@ -678,7 +685,7 @@ public class BrokerService implements Service {
stopAllConnectors(stopper); stopAllConnectors(stopper);
// remove any VMTransports connected // remove any VMTransports connected
// this has to be done after services are stopped, // 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()); BrokerRegistry.getInstance().unbind(getBrokerName());
VMTransportFactory.stopped(getBrokerName()); VMTransportFactory.stopped(getBrokerName());
if (broker != null) { if (broker != null) {
@ -2290,6 +2297,7 @@ public class BrokerService implements Service {
} }
protected void startManagementContext() throws Exception { protected void startManagementContext() throws Exception {
getManagementContext().setBrokerName(brokerName);
getManagementContext().start(); getManagementContext().start();
adminView = new BrokerView(this, null); adminView = new BrokerView(this, null);
ObjectName objectName = getBrokerObjectName(); ObjectName objectName = getBrokerObjectName();

View File

@ -19,6 +19,7 @@ package org.apache.activemq.broker.jmx;
import org.apache.activemq.Service; import org.apache.activemq.Service;
import org.slf4j.Logger; import org.slf4j.Logger;
import org.slf4j.LoggerFactory; import org.slf4j.LoggerFactory;
import org.slf4j.MDC;
import javax.management.*; import javax.management.*;
import javax.management.remote.JMXConnectorServer; import javax.management.remote.JMXConnectorServer;
@ -69,6 +70,7 @@ public class ManagementContext implements Service {
private ServerSocket registrySocket; private ServerSocket registrySocket;
private final List<ObjectName> registeredMBeanNames = new CopyOnWriteArrayList<ObjectName>(); private final List<ObjectName> registeredMBeanNames = new CopyOnWriteArrayList<ObjectName>();
private boolean allowRemoteAddressInMBeanNames = true; private boolean allowRemoteAddressInMBeanNames = true;
private String brokerName;
public ManagementContext() { public ManagementContext() {
this(null); this(null);
@ -90,21 +92,32 @@ public class ManagementContext implements Service {
Thread t = new Thread("JMX connector") { Thread t = new Thread("JMX connector") {
@Override @Override
public void run() { 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 { try {
JMXConnectorServer server = connectorServer; JMXConnectorServer server = connectorServer;
if (started.get() && server != null) { if (started.get() && server != null) {
LOG.debug("Starting JMXConnectorServer..."); LOG.debug("Starting JMXConnectorServer...");
connectorStarting.set(true); connectorStarting.set(true);
try { try {
// need to remove MDC as we must not inherit MDC in child threads causing leaks
MDC.remove("activemq.broker");
server.start(); server.start();
} finally { } finally {
connectorStarting.set(false); if (brokerName != null) {
MDC.put("activemq.broker", brokerName);
}
connectorStarting.set(false);
} }
LOG.info("JMX consoles can connect to " + server.getAddress()); LOG.info("JMX consoles can connect to " + server.getAddress());
} }
} catch (IOException e) { } catch (IOException e) {
LOG.warn("Failed to start jmx connector: " + e.getMessage()); LOG.warn("Failed to start jmx connector: " + e.getMessage());
LOG.debug("Reason for failed jms connector start", e); LOG.debug("Reason for failed jms connector start", e);
} finally {
MDC.remove("activemq.broker");
} }
} }
}; };
@ -159,6 +172,21 @@ public class ManagementContext implements Service {
} }
} }
/**
* 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. * @return Returns the jmxDomainName.
*/ */