From 8d6d6be39a9ae40c5d0b737bf820b102936c256a Mon Sep 17 00:00:00 2001 From: Juergen Hoeller Date: Wed, 11 Dec 2013 16:31:26 +0100 Subject: [PATCH] MBean registration happens in a fully synchronized fashion for consistent results Issue: SPR-11002 --- .../jmx/support/MBeanRegistrationSupport.java | 105 +++++++++++------- 1 file changed, 62 insertions(+), 43 deletions(-) diff --git a/spring-context/src/main/java/org/springframework/jmx/support/MBeanRegistrationSupport.java b/spring-context/src/main/java/org/springframework/jmx/support/MBeanRegistrationSupport.java index 38339d4b7f..eb208aa272 100644 --- a/spring-context/src/main/java/org/springframework/jmx/support/MBeanRegistrationSupport.java +++ b/spring-context/src/main/java/org/springframework/jmx/support/MBeanRegistrationSupport.java @@ -16,7 +16,6 @@ package org.springframework.jmx.support; -import java.util.Collections; import java.util.LinkedHashSet; import java.util.Set; import javax.management.InstanceAlreadyExistsException; @@ -113,7 +112,7 @@ public class MBeanRegistrationSupport { /** * The beans that have been registered by this exporter. */ - private final Set registeredBeans = Collections.synchronizedSet(new LinkedHashSet()); + private final Set registeredBeans = new LinkedHashSet(); /** * The policy used when registering an MBean and finding that it already exists. @@ -188,40 +187,45 @@ public class MBeanRegistrationSupport { * @throws JMException if the registration failed */ protected void doRegister(Object mbean, ObjectName objectName) throws JMException { - ObjectInstance registeredBean = null; - try { - registeredBean = this.server.registerMBean(mbean, objectName); - } - catch (InstanceAlreadyExistsException ex) { - if (this.registrationPolicy == RegistrationPolicy.IGNORE_EXISTING) { - if (logger.isDebugEnabled()) { - logger.debug("Ignoring existing MBean at [" + objectName + "]"); - } + ObjectName actualObjectName; + + synchronized (this.registeredBeans) { + ObjectInstance registeredBean = null; + try { + registeredBean = this.server.registerMBean(mbean, objectName); } - else if (this.registrationPolicy == RegistrationPolicy.REPLACE_EXISTING) { - try { + catch (InstanceAlreadyExistsException ex) { + if (this.registrationPolicy == RegistrationPolicy.IGNORE_EXISTING) { if (logger.isDebugEnabled()) { - logger.debug("Replacing existing MBean at [" + objectName + "]"); + logger.debug("Ignoring existing MBean at [" + objectName + "]"); + } + } + else if (this.registrationPolicy == RegistrationPolicy.REPLACE_EXISTING) { + try { + if (logger.isDebugEnabled()) { + logger.debug("Replacing existing MBean at [" + objectName + "]"); + } + this.server.unregisterMBean(objectName); + registeredBean = this.server.registerMBean(mbean, objectName); + } + catch (InstanceNotFoundException ex2) { + logger.error("Unable to replace existing MBean at [" + objectName + "]", ex2); + throw ex; } - this.server.unregisterMBean(objectName); - registeredBean = this.server.registerMBean(mbean, objectName); } - catch (InstanceNotFoundException ex2) { - logger.error("Unable to replace existing MBean at [" + objectName + "]", ex2); + else { throw ex; } } - else { - throw ex; + + // Track registration and notify listeners. + actualObjectName = (registeredBean != null ? registeredBean.getObjectName() : null); + if (actualObjectName == null) { + actualObjectName = objectName; } + this.registeredBeans.add(actualObjectName); } - // Track registration and notify listeners. - ObjectName actualObjectName = (registeredBean != null ? registeredBean.getObjectName() : null); - if (actualObjectName == null) { - actualObjectName = objectName; - } - this.registeredBeans.add(actualObjectName); onRegister(actualObjectName, mbean); } @@ -229,7 +233,11 @@ public class MBeanRegistrationSupport { * Unregisters all beans that have been registered by an instance of this class. */ protected void unregisterBeans() { - for (ObjectName objectName : new LinkedHashSet(this.registeredBeans)) { + Set snapshot; + synchronized (this.registeredBeans) { + snapshot = new LinkedHashSet(this.registeredBeans); + } + for (ObjectName objectName : snapshot) { doUnregister(objectName); } } @@ -239,32 +247,43 @@ public class MBeanRegistrationSupport { * @param objectName the suggested ObjectName for the MBean */ protected void doUnregister(ObjectName objectName) { - try { - // MBean might already have been unregistered by an external process. - if (this.server.isRegistered(objectName)) { - this.server.unregisterMBean(objectName); - onUnregister(objectName); - } - else { - if (logger.isWarnEnabled()) { - logger.warn("Could not unregister MBean [" + objectName + "] as said MBean " + - "is not registered (perhaps already unregistered by an external process)"); + boolean actuallyUnregistered = false; + + synchronized (this.registeredBeans) { + if (this.registeredBeans.remove(objectName)) { + try { + // MBean might already have been unregistered by an external process + if (this.server.isRegistered(objectName)) { + this.server.unregisterMBean(objectName); + actuallyUnregistered = true; + } + else { + if (logger.isWarnEnabled()) { + logger.warn("Could not unregister MBean [" + objectName + "] as said MBean " + + "is not registered (perhaps already unregistered by an external process)"); + } + } + } + catch (JMException ex) { + if (logger.isErrorEnabled()) { + logger.error("Could not unregister MBean [" + objectName + "]", ex); + } } } } - catch (JMException ex) { - if (logger.isErrorEnabled()) { - logger.error("Could not unregister MBean [" + objectName + "]", ex); - } + + if (actuallyUnregistered) { + onUnregister(objectName); } - this.registeredBeans.remove(objectName); } /** * Return the {@link ObjectName ObjectNames} of all registered beans. */ protected final ObjectName[] getRegisteredObjectNames() { - return this.registeredBeans.toArray(new ObjectName[this.registeredBeans.size()]); + synchronized (this.registeredBeans) { + return this.registeredBeans.toArray(new ObjectName[this.registeredBeans.size()]); + } }