X-Git-Url: https://git.opendaylight.org/gerrit/gitweb?p=controller.git;a=blobdiff_plain;f=opendaylight%2Fconfig%2Fconfig-manager%2Fsrc%2Fmain%2Fjava%2Forg%2Fopendaylight%2Fcontroller%2Fconfig%2Fmanager%2Fimpl%2Fjmx%2FInternalJMXRegistrator.java;h=7d0bee25333b84148bfce23e1c09380e169b2149;hp=98f0908dc710cf40fbb93b88004d411d03f594fd;hb=b712eb01354ddb5878008e2a2e8f03fb19b92555;hpb=808313e2a87d8dd037a0566574d0acc34687149c diff --git a/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/jmx/InternalJMXRegistrator.java b/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/jmx/InternalJMXRegistrator.java index 98f0908dc7..7d0bee2533 100644 --- a/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/jmx/InternalJMXRegistrator.java +++ b/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/jmx/InternalJMXRegistrator.java @@ -7,9 +7,12 @@ */ package org.opendaylight.controller.config.manager.impl.jmx; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; - +import com.google.common.base.Preconditions; +import java.util.ArrayList; +import java.util.Collections; +import java.util.HashSet; +import java.util.List; +import java.util.Set; import javax.annotation.concurrent.GuardedBy; import javax.management.InstanceAlreadyExistsException; import javax.management.InstanceNotFoundException; @@ -19,69 +22,85 @@ import javax.management.MBeanServer; import javax.management.NotCompliantMBeanException; import javax.management.ObjectName; import javax.management.QueryExp; -import java.io.Closeable; -import java.util.ArrayList; -import java.util.Collections; -import java.util.HashSet; -import java.util.List; -import java.util.Set; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +abstract class InternalJMXRegistrator implements AutoCloseable { + private static final class Root extends InternalJMXRegistrator { + private final MBeanServer mbeanServer; -public class InternalJMXRegistrator implements Closeable { - private static final Logger logger = LoggerFactory - .getLogger(InternalJMXRegistrator.class); - private final MBeanServer configMBeanServer; + Root(final MBeanServer mbeanServer) { + this.mbeanServer = Preconditions.checkNotNull(mbeanServer); + } - public InternalJMXRegistrator(MBeanServer configMBeanServer) { - this.configMBeanServer = configMBeanServer; + @Override + MBeanServer getMBeanServer() { + return mbeanServer; + } } - static class InternalJMXRegistration implements AutoCloseable { - private final InternalJMXRegistrator internalJMXRegistrator; - private final ObjectName on; + private static final class Nested extends InternalJMXRegistrator { + private final InternalJMXRegistrator parent; - InternalJMXRegistration(InternalJMXRegistrator internalJMXRegistrator, - ObjectName on) { - this.internalJMXRegistrator = internalJMXRegistrator; - this.on = on; + Nested(final InternalJMXRegistrator parent) { + this.parent = Preconditions.checkNotNull(parent); + } + + @Override + MBeanServer getMBeanServer() { + return parent.getMBeanServer(); } @Override public void close() { - internalJMXRegistrator.unregisterMBean(on); + try { + super.close(); + } finally { + parent.removeChild(this); + } } } + private static final Logger LOG = LoggerFactory.getLogger(InternalJMXRegistrator.class); @GuardedBy("this") - private final Set registeredObjectNames = new HashSet<>(); - private final List children = new ArrayList<>(); + private final Set registeredObjectNames = new HashSet<>(1); + @GuardedBy("this") + private final List children = new ArrayList<>(); + + public static InternalJMXRegistrator create(final MBeanServer configMBeanServer) { + return new Root(configMBeanServer); + } - public synchronized InternalJMXRegistration registerMBean(Object object, - ObjectName on) throws InstanceAlreadyExistsException { + public final synchronized InternalJMXRegistration registerMBean(final Object object, final ObjectName on) + throws InstanceAlreadyExistsException { try { - configMBeanServer.registerMBean(object, on); - } catch (MBeanRegistrationException | NotCompliantMBeanException e) { - throw new IllegalStateException(e); + getMBeanServer().registerMBean(object, on); + } catch (final NotCompliantMBeanException e) { + throw new IllegalArgumentException("Object does not comply to JMX", e); + } catch (final MBeanRegistrationException e) { + throw new IllegalStateException("Failed to register " + on, e); } + registeredObjectNames.add(on); return new InternalJMXRegistration(this, on); } - private synchronized void unregisterMBean(ObjectName on) { + final synchronized void unregisterMBean(final ObjectName on) { // first check that on was registered using this instance boolean removed = registeredObjectNames.remove(on); - if (!removed) { - throw new IllegalStateException("Cannot unregister - ObjectName not found in 'registeredObjectNames': " + on); - } + Preconditions.checkState(removed, "Cannot unregister - ObjectName not found in 'registeredObjectNames': %s", on); + try { - configMBeanServer.unregisterMBean(on); - } catch (InstanceNotFoundException | MBeanRegistrationException e) { - throw new IllegalStateException(e); + getMBeanServer().unregisterMBean(on); + } catch (final InstanceNotFoundException e) { + LOG.warn("MBean {} not found on server", on, e); + } catch (final MBeanRegistrationException e) { + throw new IllegalStateException("Failed to unregister MBean " + on, e); } } - public InternalJMXRegistrator createChild() { - InternalJMXRegistrator child = new InternalJMXRegistrator( - configMBeanServer); + public final synchronized InternalJMXRegistrator createChild() { + final Nested child = new Nested(this); children.add(child); return child; } @@ -90,60 +109,70 @@ public class InternalJMXRegistrator implements Closeable { * Allow close to be called multiple times. */ @Override - public synchronized void close() { - // close children - for (InternalJMXRegistrator child : children) { - child.close(); - } - // close registered ONs - for (ObjectName on : registeredObjectNames) { - try { - configMBeanServer.unregisterMBean(on); - } catch (Exception e) { - logger.warn("Ignoring error while unregistering {}", on, e); - } - } - registeredObjectNames.clear(); + public void close() { + internalClose(); } - public T newMBeanProxy(ObjectName objectName, Class interfaceClass) { - return JMX.newMBeanProxy(configMBeanServer, objectName, interfaceClass); + public final T newMBeanProxy(final ObjectName objectName, final Class interfaceClass) { + return JMX.newMBeanProxy(getMBeanServer(), objectName, interfaceClass); } - public T newMBeanProxy(ObjectName objectName, Class interfaceClass, - boolean notificationBroadcaster) { - return JMX.newMBeanProxy(configMBeanServer, objectName, interfaceClass, - notificationBroadcaster); + public final T newMBeanProxy(final ObjectName objectName, final Class interfaceClass, + final boolean notificationBroadcaster) { + return JMX.newMBeanProxy(getMBeanServer(), objectName, interfaceClass, notificationBroadcaster); } - public T newMXBeanProxy(ObjectName objectName, Class interfaceClass) { - return JMX - .newMXBeanProxy(configMBeanServer, objectName, interfaceClass); + public final T newMXBeanProxy(final ObjectName objectName, final Class interfaceClass) { + return JMX.newMXBeanProxy(getMBeanServer(), objectName, interfaceClass); } - public T newMXBeanProxy(ObjectName objectName, Class interfaceClass, - boolean notificationBroadcaster) { - return JMX.newMXBeanProxy(configMBeanServer, objectName, - interfaceClass, notificationBroadcaster); + public final T newMXBeanProxy(final ObjectName objectName, final Class interfaceClass, + final boolean notificationBroadcaster) { + return JMX.newMXBeanProxy(getMBeanServer(), objectName, interfaceClass, notificationBroadcaster); } - public Set getRegisteredObjectNames() { + public final Set getRegisteredObjectNames() { return Collections.unmodifiableSet(registeredObjectNames); } - public Set queryNames(ObjectName name, QueryExp query) { - Set result = configMBeanServer.queryNames(name, query); + public final Set queryNames(final ObjectName name, final QueryExp query) { // keep only those that were registered using this instance - return getSameNames(result); + return getSameNames(getMBeanServer().queryNames(name, query)); + } + + abstract MBeanServer getMBeanServer(); + + synchronized void removeChild(final InternalJMXRegistrator child) { + children.remove(child); } - private Set getSameNames(Set superSet) { - Set result = new HashSet<>(superSet); + private synchronized void internalClose() { + // close all children + for (InternalJMXRegistrator child : children) { + // This bypasses the call to removeChild(), preventing a potential deadlock when children are being closed + // concurrently + child.internalClose(); + } + children.clear(); + + // close registered ONs + for (ObjectName on : registeredObjectNames) { + try { + getMBeanServer().unregisterMBean(on); + } catch (final Exception e) { + LOG.warn("Ignoring error while unregistering {}", on, e); + } + } + registeredObjectNames.clear(); + } + + private synchronized Set getSameNames(final Set superSet) { + final Set result = new HashSet<>(superSet); result.retainAll(registeredObjectNames); + for (InternalJMXRegistrator child : children) { result.addAll(child.getSameNames(superSet)); } return result; } - }