BUG-4514: clean children in InternalJMXRegistrator 08/31808/11
authorRobert Varga <rovarga@cisco.com>
Tue, 22 Dec 2015 18:44:27 +0000 (19:44 +0100)
committerGerrit Code Review <gerrit@opendaylight.org>
Fri, 8 Jan 2016 09:11:21 +0000 (09:11 +0000)
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 <rovarga@cisco.com>
opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/jmx/BaseJMXRegistrator.java
opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/jmx/ConfigRegistryJMXRegistrator.java
opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/jmx/InternalJMXRegistration.java [new file with mode: 0644]
opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/jmx/InternalJMXRegistrator.java
opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/jmx/ModuleJMXRegistrator.java
opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/jmx/RootRuntimeBeanRegistratorImpl.java
opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/jmx/ServiceReferenceRegistrator.java
opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/jmx/TransactionJMXRegistrator.java
opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/jmx/TransactionModuleJMXRegistrator.java
opendaylight/config/config-manager/src/test/java/org/opendaylight/controller/config/manager/impl/AbstractConfigTest.java

index 603c0f5..a4c69a0 100644 (file)
@@ -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<ObjectName> queryNames(ObjectName name, QueryExp query) {
+    public Set<ObjectName> queryNames(final ObjectName name, final QueryExp query) {
         return internalJMXRegistrator.queryNames(name, query);
     }
 
index 52be4de..d782764 100644 (file)
@@ -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 (file)
index 0000000..4f4f990
--- /dev/null
@@ -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);
+    }
+}
index 85ad8c5..71656c2 100644 (file)
@@ -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<ObjectName> registeredObjectNames = new HashSet<>();
+    private final Set<ObjectName> registeredObjectNames = new HashSet<>(1);
     @GuardedBy("this")
-    private final List<InternalJMXRegistrator> children = new ArrayList<>();
+    private final List<Nested> 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> T newMBeanProxy(ObjectName objectName, Class<T> interfaceClass) {
-        return JMX.newMBeanProxy(configMBeanServer, objectName, interfaceClass);
+    public final <T> T newMBeanProxy(final ObjectName objectName, final Class<T> interfaceClass) {
+        return JMX.newMBeanProxy(getMBeanServer(), objectName, interfaceClass);
     }
 
-    public <T> T newMBeanProxy(ObjectName objectName, Class<T> interfaceClass,
-                               boolean notificationBroadcaster) {
-        return JMX.newMBeanProxy(configMBeanServer, objectName, interfaceClass,
-                notificationBroadcaster);
+    public final <T> T newMBeanProxy(final ObjectName objectName, final Class<T> interfaceClass,
+            final boolean notificationBroadcaster) {
+        return JMX.newMBeanProxy(getMBeanServer(), objectName, interfaceClass, notificationBroadcaster);
     }
 
-    public <T> T newMXBeanProxy(ObjectName objectName, Class<T> interfaceClass) {
-        return JMX
-                .newMXBeanProxy(configMBeanServer, objectName, interfaceClass);
+    public final <T> T newMXBeanProxy(final ObjectName objectName, final Class<T> interfaceClass) {
+        return JMX.newMXBeanProxy(getMBeanServer(), objectName, interfaceClass);
     }
 
-    public <T> T newMXBeanProxy(ObjectName objectName, Class<T> interfaceClass,
-                                boolean notificationBroadcaster) {
-        return JMX.newMXBeanProxy(configMBeanServer, objectName,
-                interfaceClass, notificationBroadcaster);
+    public final <T> T newMXBeanProxy(final ObjectName objectName, final Class<T> interfaceClass,
+            final boolean notificationBroadcaster) {
+        return JMX.newMXBeanProxy(getMBeanServer(), objectName, interfaceClass, notificationBroadcaster);
     }
 
-    public Set<ObjectName> getRegisteredObjectNames() {
+    public final Set<ObjectName> getRegisteredObjectNames() {
         return Collections.unmodifiableSet(registeredObjectNames);
     }
 
-    public Set<ObjectName> queryNames(ObjectName name, QueryExp query) {
-        Set<ObjectName> result = configMBeanServer.queryNames(name, query);
+    public final Set<ObjectName> 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<ObjectName> getSameNames(Set<ObjectName> superSet) {
-        Set<ObjectName> 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<ObjectName> getSameNames(final Set<ObjectName> superSet) {
+        final Set<ObjectName> result = new HashSet<>(superSet);
         result.retainAll(registeredObjectNames);
+
         for (InternalJMXRegistrator child : children) {
             result.addAll(child.getSameNames(superSet));
         }
         return result;
     }
-
 }
index 6af500f..70ed695 100644 (file)
@@ -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) {
index d9ee3d3..8bccef3 100644 (file)
@@ -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);
 
index ed366f6..f1ceb7e 100644 (file)
@@ -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 {
 
index 9813186..cf2c553 100644 (file)
@@ -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<ObjectName> queryNames(ObjectName name, QueryExp query) {
+    public Set<ObjectName> queryNames(final ObjectName name, final QueryExp query) {
         return childJMXRegistrator.queryNames(name, query);
     }
 
index fbdf47e..b5c5991 100644 (file)
@@ -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;
index 07891b8..38ff0d9 100644 (file)
@@ -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<RegistrationHolder> registrations = new LinkedList<>();
         @Override
-        public void handleServiceRegistration(Class<?> clazz, Object serviceInstance, Dictionary<String, ?> props) {
-
+        public void handleServiceRegistration(final Class<?> clazz, final Object serviceInstance, final Dictionary<String, ?> props) {
             registrations.add(new RegistrationHolder(clazz, serviceInstance, props));
         }
 
@@ -100,33 +95,27 @@ public abstract class AbstractConfigTest extends
             protected final Object instance;
             protected final Dictionary<String, ?> props;
 
-            public RegistrationHolder(Class<?> clazz, Object instance, Dictionary<String, ?> props) {
+            public RegistrationHolder(final Class<?> clazz, final Object instance, final Dictionary<String, ?> 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<? extends Module> configBeanClass, String implementationName) {
+    protected ClassBasedModuleFactory createClassBasedCBF(final Class<? extends Module> 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<String, ?> props);
-
     }
 
     private class RegisterServiceAnswer implements Answer<ServiceRegistration<?>> {
-
         @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<String, ?> props = (Dictionary<String, ?>) 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<String, ?> props) {
+        public void invokeServiceHandler(final Object serviceInstance, final String className, final Dictionary<String, ?> 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<String, ?> props) {
+        private void invokeServiceHandler(final Object serviceInstance, final Class<?> serviceType, final Dictionary<String, ?> 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> 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();
         }
     }
-
 }