From f2e18b6c80c5044747ca6de2b182805e3d9c301d Mon Sep 17 00:00:00 2001 From: Robert Varga Date: Tue, 22 Dec 2015 19:44:27 +0100 Subject: [PATCH] BUG-4514: clean children in InternalJMXRegistrator Retaining children once they have been closed can lead to a leak, make sure we call back to parent to remove ourselves from its list. This includes a refactor to hide InternalJMXRegistrator, which becomes abstract and has two subclasses: Root and Nested. This allows us to use proper synchronization between closing of the child and parent registrators. Also saves a bit of memory. Also clean up {Module,Transaction}JMXRegistrator constructors to not do a createChild(), as that fails to cleanup immediate children of the Root, leading to empty InternalJMXRegistrators being collected in Root's child list. Change-Id: I9a4708b67777ca6033e5a83c586b3f78692dff2a Signed-off-by: Robert Varga --- .../manager/impl/jmx/BaseJMXRegistrator.java | 20 +-- .../jmx/ConfigRegistryJMXRegistrator.java | 14 +- .../impl/jmx/InternalJMXRegistration.java | 26 +++ .../impl/jmx/InternalJMXRegistrator.java | 160 +++++++++++------- .../impl/jmx/ModuleJMXRegistrator.java | 10 +- .../jmx/RootRuntimeBeanRegistratorImpl.java | 15 +- .../impl/jmx/ServiceReferenceRegistrator.java | 1 - .../impl/jmx/TransactionJMXRegistrator.java | 14 +- .../jmx/TransactionModuleJMXRegistrator.java | 1 - .../manager/impl/AbstractConfigTest.java | 73 +++----- 10 files changed, 182 insertions(+), 152 deletions(-) create mode 100644 opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/jmx/InternalJMXRegistration.java diff --git a/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/jmx/BaseJMXRegistrator.java b/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/jmx/BaseJMXRegistrator.java index 603c0f58a6..a4c69a0904 100644 --- a/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/jmx/BaseJMXRegistrator.java +++ b/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/jmx/BaseJMXRegistrator.java @@ -17,31 +17,27 @@ public class BaseJMXRegistrator implements AutoCloseable, NestableJMXRegistrator private final InternalJMXRegistrator internalJMXRegistrator; - public BaseJMXRegistrator(MBeanServer configMBeanServer) { - internalJMXRegistrator = new InternalJMXRegistrator(configMBeanServer); + public BaseJMXRegistrator(final MBeanServer configMBeanServer) { + internalJMXRegistrator = InternalJMXRegistrator.create(configMBeanServer); } - public BaseJMXRegistrator(InternalJMXRegistrator internalJMXRegistrator) { + public BaseJMXRegistrator(final InternalJMXRegistrator internalJMXRegistrator) { this.internalJMXRegistrator = internalJMXRegistrator; } - public TransactionJMXRegistrator createTransactionJMXRegistrator( - String transactionName) { - return new TransactionJMXRegistrator( - internalJMXRegistrator.createChild(), transactionName); + public TransactionJMXRegistrator createTransactionJMXRegistrator(final String transactionName) { + return new TransactionJMXRegistrator(internalJMXRegistrator.createChild(), transactionName); } public ModuleJMXRegistrator createModuleJMXRegistrator() { return new ModuleJMXRegistrator(internalJMXRegistrator.createChild()); } - public RootRuntimeBeanRegistratorImpl createRuntimeBeanRegistrator( - ModuleIdentifier moduleIdentifier) { - return new RootRuntimeBeanRegistratorImpl(internalJMXRegistrator.createChild(), - moduleIdentifier); + public RootRuntimeBeanRegistratorImpl createRuntimeBeanRegistrator(final ModuleIdentifier moduleIdentifier) { + return new RootRuntimeBeanRegistratorImpl(internalJMXRegistrator.createChild(), moduleIdentifier); } - public Set queryNames(ObjectName name, QueryExp query) { + public Set queryNames(final ObjectName name, final QueryExp query) { return internalJMXRegistrator.queryNames(name, query); } diff --git a/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/jmx/ConfigRegistryJMXRegistrator.java b/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/jmx/ConfigRegistryJMXRegistrator.java index 52be4de7e7..d782764679 100644 --- a/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/jmx/ConfigRegistryJMXRegistrator.java +++ b/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/jmx/ConfigRegistryJMXRegistrator.java @@ -19,20 +19,18 @@ import org.opendaylight.controller.config.manager.impl.ConfigRegistryImplMXBean; public class ConfigRegistryJMXRegistrator implements AutoCloseable { private final InternalJMXRegistrator internalJMXRegistrator; - public ConfigRegistryJMXRegistrator(MBeanServer configMBeanServer) { - internalJMXRegistrator = new InternalJMXRegistrator(configMBeanServer); + public ConfigRegistryJMXRegistrator(final MBeanServer configMBeanServer) { + internalJMXRegistrator = InternalJMXRegistrator.create(configMBeanServer); } - public AutoCloseable registerToJMX(ConfigRegistryImplMXBean configRegistry) + public AutoCloseable registerToJMX(final ConfigRegistryImplMXBean configRegistry) throws InstanceAlreadyExistsException { - return internalJMXRegistrator.registerMBean(configRegistry, - ConfigRegistryMXBean.OBJECT_NAME); + return internalJMXRegistrator.registerMBean(configRegistry, ConfigRegistryMXBean.OBJECT_NAME); } - public AutoCloseable registerToJMXNoNotifications(ConfigRegistryImplMXBean configRegistry) + public AutoCloseable registerToJMXNoNotifications(final ConfigRegistryImplMXBean configRegistry) throws InstanceAlreadyExistsException { - return internalJMXRegistrator.registerMBean(configRegistry, - ConfigRegistryMXBean.OBJECT_NAME_NO_NOTIFICATIONS); + return internalJMXRegistrator.registerMBean(configRegistry, ConfigRegistryMXBean.OBJECT_NAME_NO_NOTIFICATIONS); } @Override diff --git a/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/jmx/InternalJMXRegistration.java b/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/jmx/InternalJMXRegistration.java new file mode 100644 index 0000000000..4f4f99027d --- /dev/null +++ b/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/jmx/InternalJMXRegistration.java @@ -0,0 +1,26 @@ +/* + * Copyright (c) 2013 Cisco Systems, Inc. and others. All rights reserved. + * + * This program and the accompanying materials are made available under the + * terms of the Eclipse Public License v1.0 which accompanies this distribution, + * and is available at http://www.eclipse.org/legal/epl-v10.html + */ +package org.opendaylight.controller.config.manager.impl.jmx; + +import com.google.common.base.Preconditions; +import javax.management.ObjectName; + +final class InternalJMXRegistration implements AutoCloseable { + private final InternalJMXRegistrator internalJMXRegistrator; + private final ObjectName on; + + InternalJMXRegistration(final InternalJMXRegistrator internalJMXRegistrator, final ObjectName on) { + this.internalJMXRegistrator = Preconditions.checkNotNull(internalJMXRegistrator); + this.on = on; + } + + @Override + public void close() { + internalJMXRegistrator.unregisterMBean(on); + } +} 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 85ad8c5134..71656c27bc 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,7 +7,7 @@ */ package org.opendaylight.controller.config.manager.impl.jmx; -import java.io.Closeable; +import com.google.common.base.Preconditions; import java.util.ArrayList; import java.util.Collections; import java.util.HashSet; @@ -25,62 +25,82 @@ import javax.management.QueryExp; import org.slf4j.Logger; import org.slf4j.LoggerFactory; -public class InternalJMXRegistrator implements Closeable { - private static final Logger LOG = LoggerFactory - .getLogger(InternalJMXRegistrator.class); - private final MBeanServer configMBeanServer; +abstract class InternalJMXRegistrator implements AutoCloseable { + private static final class Root extends InternalJMXRegistrator { + private final MBeanServer mbeanServer; - public InternalJMXRegistrator(MBeanServer configMBeanServer) { - this.configMBeanServer = configMBeanServer; + Root(final MBeanServer mbeanServer) { + this.mbeanServer = Preconditions.checkNotNull(mbeanServer); + } + + @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 Set registeredObjectNames = new HashSet<>(1); @GuardedBy("this") - private final List children = new ArrayList<>(); + 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 (NotCompliantMBeanException e) { + throw new IllegalArgumentException("Object does not comply to JMX", e); + } catch (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': {}", on); + try { - configMBeanServer.unregisterMBean(on); - } catch (InstanceNotFoundException | MBeanRegistrationException e) { - throw new IllegalStateException(e); + getMBeanServer().unregisterMBean(on); + } catch (InstanceNotFoundException e) { + LOG.warn("MBean {} not found on server", on, e); + } catch (MBeanRegistrationException e) { + throw new IllegalStateException("Failed to unregister MBean " + on, e); } } - public synchronized InternalJMXRegistrator createChild() { - InternalJMXRegistrator child = new InternalJMXRegistrator(configMBeanServer); + public final synchronized InternalJMXRegistrator createChild() { + final Nested child = new Nested(this); children.add(child); return child; } @@ -89,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) { - LOG.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 synchronized 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 (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; } - } diff --git a/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/jmx/ModuleJMXRegistrator.java b/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/jmx/ModuleJMXRegistrator.java index 6af500fbd1..70ed69543a 100644 --- a/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/jmx/ModuleJMXRegistrator.java +++ b/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/jmx/ModuleJMXRegistrator.java @@ -7,12 +7,12 @@ */ package org.opendaylight.controller.config.manager.impl.jmx; +import com.google.common.base.Preconditions; import java.io.Closeable; import javax.annotation.concurrent.ThreadSafe; import javax.management.InstanceAlreadyExistsException; import javax.management.ObjectName; import org.opendaylight.controller.config.api.jmx.ObjectNameUtil; -import org.opendaylight.controller.config.manager.impl.jmx.InternalJMXRegistrator.InternalJMXRegistration; /** * This subclass is used for registering readable module into JMX, it is also @@ -24,14 +24,14 @@ import org.opendaylight.controller.config.manager.impl.jmx.InternalJMXRegistrato public class ModuleJMXRegistrator implements Closeable { private final InternalJMXRegistrator childJMXRegistrator; - public ModuleJMXRegistrator(InternalJMXRegistrator internalJMXRegistrator) { - this.childJMXRegistrator = internalJMXRegistrator.createChild(); + ModuleJMXRegistrator(final InternalJMXRegistrator internalJMXRegistrator) { + this.childJMXRegistrator = Preconditions.checkNotNull(internalJMXRegistrator); } static class ModuleJMXRegistration implements AutoCloseable { private final InternalJMXRegistration internalJMXRegistration; - ModuleJMXRegistration(InternalJMXRegistration registration) { + ModuleJMXRegistration(final InternalJMXRegistration registration) { this.internalJMXRegistration = registration; } @@ -41,7 +41,7 @@ public class ModuleJMXRegistrator implements Closeable { } } - public ModuleJMXRegistration registerMBean(Object object, ObjectName on) + public ModuleJMXRegistration registerMBean(final Object object, final ObjectName on) throws InstanceAlreadyExistsException { ObjectNameUtil.checkType(on, ObjectNameUtil.TYPE_MODULE); if (ObjectNameUtil.getTransactionName(on) != null) { diff --git a/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/jmx/RootRuntimeBeanRegistratorImpl.java b/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/jmx/RootRuntimeBeanRegistratorImpl.java index d9ee3d3225..8bccef3b81 100644 --- a/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/jmx/RootRuntimeBeanRegistratorImpl.java +++ b/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/jmx/RootRuntimeBeanRegistratorImpl.java @@ -7,6 +7,7 @@ */ package org.opendaylight.controller.config.manager.impl.jmx; +import com.google.common.base.Preconditions; import java.util.Collections; import javax.management.InstanceAlreadyExistsException; import javax.management.ObjectName; @@ -21,10 +22,9 @@ public class RootRuntimeBeanRegistratorImpl implements private final ModuleIdentifier moduleIdentifier; private final ObjectName defaultRuntimeON; - public RootRuntimeBeanRegistratorImpl( - InternalJMXRegistrator internalJMXRegistrator, - ModuleIdentifier moduleIdentifier) { - this.internalJMXRegistrator = internalJMXRegistrator; + RootRuntimeBeanRegistratorImpl(final InternalJMXRegistrator internalJMXRegistrator, + final ModuleIdentifier moduleIdentifier) { + this.internalJMXRegistrator = Preconditions.checkNotNull(internalJMXRegistrator); this.moduleIdentifier = moduleIdentifier; defaultRuntimeON = ObjectNameUtil.createRuntimeBeanName( moduleIdentifier.getFactoryName(), @@ -33,8 +33,7 @@ public class RootRuntimeBeanRegistratorImpl implements } @Override - public HierarchicalRuntimeBeanRegistrationImpl registerRoot( - RuntimeBean mxBean) { + public HierarchicalRuntimeBeanRegistrationImpl registerRoot(final RuntimeBean mxBean) { try { internalJMXRegistrator.registerMBean(mxBean, defaultRuntimeON); } catch (InstanceAlreadyExistsException e) { @@ -49,8 +48,8 @@ public class RootRuntimeBeanRegistratorImpl implements internalJMXRegistrator.close(); } - static IllegalStateException sanitize(InstanceAlreadyExistsException e, - ModuleIdentifier moduleIdentifier, ObjectName on) { + static IllegalStateException sanitize(final InstanceAlreadyExistsException e, + final ModuleIdentifier moduleIdentifier, final ObjectName on) { throw new IllegalStateException("Could not register runtime bean in " + moduleIdentifier + " under name " + on, e); diff --git a/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/jmx/ServiceReferenceRegistrator.java b/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/jmx/ServiceReferenceRegistrator.java index ed366f6a29..f1ceb7e227 100644 --- a/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/jmx/ServiceReferenceRegistrator.java +++ b/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/jmx/ServiceReferenceRegistrator.java @@ -10,7 +10,6 @@ package org.opendaylight.controller.config.manager.impl.jmx; import javax.management.InstanceAlreadyExistsException; import javax.management.ObjectName; import org.opendaylight.controller.config.api.jmx.ObjectNameUtil; -import org.opendaylight.controller.config.manager.impl.jmx.InternalJMXRegistrator.InternalJMXRegistration; public interface ServiceReferenceRegistrator extends AutoCloseable { diff --git a/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/jmx/TransactionJMXRegistrator.java b/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/jmx/TransactionJMXRegistrator.java index 98131868d2..cf2c553f6b 100644 --- a/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/jmx/TransactionJMXRegistrator.java +++ b/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/jmx/TransactionJMXRegistrator.java @@ -7,13 +7,13 @@ */ package org.opendaylight.controller.config.manager.impl.jmx; +import com.google.common.base.Preconditions; import java.io.Closeable; import java.util.Set; import javax.management.InstanceAlreadyExistsException; import javax.management.ObjectName; import javax.management.QueryExp; import org.opendaylight.controller.config.api.jmx.ObjectNameUtil; -import org.opendaylight.controller.config.manager.impl.jmx.InternalJMXRegistrator.InternalJMXRegistration; /** * Contains constraints on passed {@link ObjectName} parameters. Only allow (un) @@ -23,16 +23,16 @@ public class TransactionJMXRegistrator implements Closeable { private final InternalJMXRegistrator childJMXRegistrator; private final String transactionName; - TransactionJMXRegistrator(InternalJMXRegistrator internalJMXRegistrator, - String transactionName) { - this.childJMXRegistrator = internalJMXRegistrator.createChild(); + TransactionJMXRegistrator(final InternalJMXRegistrator internalJMXRegistrator, + final String transactionName) { + this.childJMXRegistrator = Preconditions.checkNotNull(internalJMXRegistrator); this.transactionName = transactionName; } public static class TransactionJMXRegistration implements AutoCloseable { private final InternalJMXRegistration registration; - TransactionJMXRegistration(InternalJMXRegistration registration) { + TransactionJMXRegistration(final InternalJMXRegistration registration) { this.registration = registration; } @@ -42,7 +42,7 @@ public class TransactionJMXRegistrator implements Closeable { } } - public TransactionJMXRegistration registerMBean(Object object, ObjectName on) + public TransactionJMXRegistration registerMBean(final Object object, final ObjectName on) throws InstanceAlreadyExistsException { if (!transactionName.equals(ObjectNameUtil.getTransactionName(on))) { throw new IllegalArgumentException( @@ -54,7 +54,7 @@ public class TransactionJMXRegistrator implements Closeable { childJMXRegistrator.registerMBean(object, on)); } - public Set queryNames(ObjectName name, QueryExp query) { + public Set queryNames(final ObjectName name, final QueryExp query) { return childJMXRegistrator.queryNames(name, query); } diff --git a/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/jmx/TransactionModuleJMXRegistrator.java b/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/jmx/TransactionModuleJMXRegistrator.java index fbdf47ebe4..b5c59918f6 100644 --- a/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/jmx/TransactionModuleJMXRegistrator.java +++ b/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/jmx/TransactionModuleJMXRegistrator.java @@ -13,7 +13,6 @@ import javax.management.InstanceAlreadyExistsException; import javax.management.ObjectName; import javax.management.QueryExp; import org.opendaylight.controller.config.api.jmx.ObjectNameUtil; -import org.opendaylight.controller.config.manager.impl.jmx.InternalJMXRegistrator.InternalJMXRegistration; public class TransactionModuleJMXRegistrator implements Closeable, NestableJMXRegistrator { private final InternalJMXRegistrator currentJMXRegistrator; diff --git a/opendaylight/config/config-manager/src/test/java/org/opendaylight/controller/config/manager/impl/AbstractConfigTest.java b/opendaylight/config/config-manager/src/test/java/org/opendaylight/controller/config/manager/impl/AbstractConfigTest.java index 07891b8b91..38ff0d9f87 100644 --- a/opendaylight/config/config-manager/src/test/java/org/opendaylight/controller/config/manager/impl/AbstractConfigTest.java +++ b/opendaylight/config/config-manager/src/test/java/org/opendaylight/controller/config/manager/impl/AbstractConfigTest.java @@ -12,7 +12,6 @@ import static org.mockito.Matchers.any; import static org.mockito.Mockito.doAnswer; import static org.mockito.Mockito.doNothing; import static org.mockito.Mockito.mock; - import com.google.common.base.Preconditions; import java.io.File; import java.io.IOException; @@ -40,7 +39,6 @@ import org.opendaylight.controller.config.api.jmx.CommitStatus; import org.opendaylight.controller.config.manager.impl.factoriesresolver.ModuleFactoriesResolver; import org.opendaylight.controller.config.manager.impl.jmx.BaseJMXRegistrator; import org.opendaylight.controller.config.manager.impl.jmx.ConfigRegistryJMXRegistrator; -import org.opendaylight.controller.config.manager.impl.jmx.InternalJMXRegistrator; import org.opendaylight.controller.config.manager.impl.jmx.JMXNotifierConfigRegistry; import org.opendaylight.controller.config.manager.impl.osgi.mapping.BindingContextProvider; import org.opendaylight.controller.config.manager.testingservices.scheduledthreadpool.TestingScheduledThreadPoolImpl; @@ -63,31 +61,28 @@ import org.osgi.framework.ServiceRegistration; * {@link #initConfigTransactionManagerImpl(org.opendaylight.controller.config.manager.impl.factoriesresolver.ModuleFactoriesResolver)} * typically during setting up the each test. */ -public abstract class AbstractConfigTest extends - AbstractLockedPlatformMBeanServerTest { +public abstract class AbstractConfigTest extends AbstractLockedPlatformMBeanServerTest { protected ConfigRegistryJMXRegistrator configRegistryJMXRegistrator; protected ConfigRegistryImpl configRegistry; private JMXNotifierConfigRegistry notifyingConfigRegistry; protected ConfigRegistryJMXClient configRegistryClient; protected BaseJMXRegistrator baseJmxRegistrator; - protected InternalJMXRegistrator internalJmxRegistrator; @Mock protected BundleContext mockedContext; @Mock protected ServiceRegistration mockedServiceRegistration; + protected BundleContextServiceRegistrationHandler currentBundleContextServiceRegistrationHandler; @Before public void setUpMocks() { MockitoAnnotations.initMocks(this); } - // Default handler for OSGi service registration protected static class RecordingBundleContextServiceRegistrationHandler implements BundleContextServiceRegistrationHandler { private final List registrations = new LinkedList<>(); @Override - public void handleServiceRegistration(Class clazz, Object serviceInstance, Dictionary props) { - + public void handleServiceRegistration(final Class clazz, final Object serviceInstance, final Dictionary props) { registrations.add(new RegistrationHolder(clazz, serviceInstance, props)); } @@ -100,33 +95,27 @@ public abstract class AbstractConfigTest extends protected final Object instance; protected final Dictionary props; - public RegistrationHolder(Class clazz, Object instance, Dictionary props) { + public RegistrationHolder(final Class clazz, final Object instance, final Dictionary props) { this.clazz = clazz; this.instance = instance; this.props = props; } } - } - protected BundleContextServiceRegistrationHandler currentBundleContextServiceRegistrationHandler; - - protected BundleContextServiceRegistrationHandler getBundleContextServiceRegistrationHandler(Class serviceType) { + protected BundleContextServiceRegistrationHandler getBundleContextServiceRegistrationHandler(final Class serviceType) { return currentBundleContextServiceRegistrationHandler; } // this method should be called in @Before - protected void initConfigTransactionManagerImpl( - ModuleFactoriesResolver resolver) { + protected void initConfigTransactionManagerImpl(final ModuleFactoriesResolver resolver) { - final MBeanServer platformMBeanServer = ManagementFactory - .getPlatformMBeanServer(); + final MBeanServer platformMBeanServer = ManagementFactory.getPlatformMBeanServer(); configRegistryJMXRegistrator = new ConfigRegistryJMXRegistrator(platformMBeanServer); initBundleContext(); - internalJmxRegistrator = new InternalJMXRegistrator(platformMBeanServer); - baseJmxRegistrator = new BaseJMXRegistrator(internalJmxRegistrator); + baseJmxRegistrator = new BaseJMXRegistrator(platformMBeanServer); configRegistry = new ConfigRegistryImpl(resolver, platformMBeanServer, baseJmxRegistrator, new BindingContextProvider() { @Override @@ -183,19 +172,18 @@ public abstract class AbstractConfigTest extends transaction.commit(); } - protected void assertStatus(CommitStatus status, int expectedNewInstances, - int expectedRecreatedInstances, int expectedReusedInstances) { + protected void assertStatus(final CommitStatus status, final int expectedNewInstances, + final int expectedRecreatedInstances, final int expectedReusedInstances) { assertEquals("New instances mismatch in " + status, expectedNewInstances, status.getNewInstances().size()); - assertEquals("Recreated instances mismatch in " + status, expectedRecreatedInstances, status.getRecreatedInstances() - .size()); - assertEquals("Reused instances mismatch in " + status, expectedReusedInstances, status.getReusedInstances() - .size()); + assertEquals("Recreated instances mismatch in " + status, expectedRecreatedInstances, + status.getRecreatedInstances().size()); + assertEquals("Reused instances mismatch in " + status, expectedReusedInstances, + status.getReusedInstances().size()); } - protected void assertBeanCount(int i, String configMXBeanName) { - assertEquals(i, configRegistry.lookupConfigBeans(configMXBeanName) - .size()); + protected void assertBeanCount(final int i, final String configMXBeanName) { + assertEquals(i, configRegistry.lookupConfigBeans(configMXBeanName).size()); } /** @@ -206,8 +194,8 @@ public abstract class AbstractConfigTest extends * @param implementationName * @return */ - protected ClassBasedModuleFactory createClassBasedCBF( - Class configBeanClass, String implementationName) { + protected ClassBasedModuleFactory createClassBasedCBF(final Class configBeanClass, + final String implementationName) { return new ClassBasedModuleFactory(implementationName, configBeanClass); } @@ -216,28 +204,25 @@ public abstract class AbstractConfigTest extends } public static interface BundleContextServiceRegistrationHandler { - void handleServiceRegistration(Class clazz, Object serviceInstance, Dictionary props); - } private class RegisterServiceAnswer implements Answer> { - @Override - public ServiceRegistration answer(InvocationOnMock invocation) throws Throwable { + public ServiceRegistration answer(final InvocationOnMock invocation) throws Throwable { Object[] args = invocation.getArguments(); Preconditions.checkArgument(args.length == 3, "Unexpected arguments size (expected 3 was %s)", args.length); Object serviceTypeRaw = args[0]; Object serviceInstance = args[1]; + @SuppressWarnings("unchecked") Dictionary props = (Dictionary) args[2]; if (serviceTypeRaw instanceof Class) { Class serviceType = (Class) serviceTypeRaw; invokeServiceHandler(serviceInstance, serviceType, props); - - } else if(serviceTypeRaw instanceof String[]) { + } else if (serviceTypeRaw instanceof String[]) { for (String className : (String[]) serviceTypeRaw) { invokeServiceHandler(serviceInstance, className, props); } @@ -247,11 +232,10 @@ public abstract class AbstractConfigTest extends throw new IllegalStateException("Not handling service registration of type, Unknown type" + serviceTypeRaw); } - return mockedServiceRegistration; } - public void invokeServiceHandler(Object serviceInstance, String className, Dictionary props) { + public void invokeServiceHandler(final Object serviceInstance, final String className, final Dictionary props) { try { Class serviceType = Class.forName(className); invokeServiceHandler(serviceInstance, serviceType, props); @@ -260,7 +244,7 @@ public abstract class AbstractConfigTest extends } } - private void invokeServiceHandler(Object serviceInstance, Class serviceType, Dictionary props) { + private void invokeServiceHandler(final Object serviceInstance, final Class serviceType, final Dictionary props) { BundleContextServiceRegistrationHandler serviceRegistrationHandler = getBundleContextServiceRegistrationHandler(serviceType); if (serviceRegistrationHandler != null) { @@ -275,11 +259,11 @@ public abstract class AbstractConfigTest extends * @param innerObject jmx proxy which will be wrapped and returned */ protected T rethrowCause(final T innerObject) { - - Object proxy = Proxy.newProxyInstance(innerObject.getClass().getClassLoader(), + @SuppressWarnings("unchecked") + final T proxy = (T)Proxy.newProxyInstance(innerObject.getClass().getClassLoader(), innerObject.getClass().getInterfaces(), new InvocationHandler() { @Override - public Object invoke(Object proxy, Method method, Object[] args) throws Throwable { + public Object invoke(final Object proxy, final Method method, final Object[] args) throws Throwable { try { return method.invoke(innerObject, args); } catch (InvocationTargetException e) { @@ -292,7 +276,7 @@ public abstract class AbstractConfigTest extends } } ); - return (T) proxy; + return proxy; } /** @@ -300,7 +284,7 @@ public abstract class AbstractConfigTest extends * @param dir to be cleaned * @throws IOException */ - protected void cleanDirectory(File dir) throws IOException { + protected void cleanDirectory(final File dir) throws IOException { if (!dir.isDirectory()) { throw new IllegalStateException("dir must be a directory"); } @@ -317,5 +301,4 @@ public abstract class AbstractConfigTest extends file.delete(); } } - } -- 2.36.6