Introduce lifecycle to runtime beans registrator in config-manager 14/32314/3
authorMaros Marsalek <mmarsale@cisco.com>
Sat, 9 Jan 2016 13:44:59 +0000 (14:44 +0100)
committerGerrit Code Review <gerrit@opendaylight.org>
Wed, 13 Jan 2016 09:44:42 +0000 (09:44 +0000)
Root runtime bean registrator was not properly closed or reused
during reconfiguration.

Change-Id: I537f7af5957496001f51663ded206a4ab04e5401
Signed-off-by: Maros Marsalek <mmarsale@cisco.com>
opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/ConfigRegistryImpl.java
opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/ModuleInternalInfo.java
opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/dependencyresolver/DestroyedModule.java
opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/dependencyresolver/ModuleInternalTransactionalInfo.java
opendaylight/config/config-manager/src/test/java/org/opendaylight/controller/config/manager/testingservices/scheduledthreadpool/TestingScheduledThreadPoolModule.java

index 737e2ae..45dac31 100644 (file)
@@ -252,23 +252,35 @@ public class ConfigRegistryImpl implements AutoCloseable, ConfigRegistryImplMXBe
         }
 
         // set RuntimeBeanRegistrators on beans implementing
-        // RuntimeBeanRegistratorAwareModule, each module
-        // should have exactly one runtime jmx registrator.
+        // RuntimeBeanRegistratorAwareModule
         Map<ModuleIdentifier, RootRuntimeBeanRegistratorImpl> runtimeRegistrators = new HashMap<>();
         for (ModuleInternalTransactionalInfo entry : commitInfo.getCommitted()
                 .values()) {
-            // FIXME creating new Runtime bean registrator for each new instance might cause leaks if client
-            // code doesn't close registration (and thus underlying registrator) in createInstance
-            RootRuntimeBeanRegistratorImpl runtimeBeanRegistrator = baseJMXRegistrator
-                    .createRuntimeBeanRegistrator(entry.getIdentifier());
             // set runtime jmx registrator if required
             Module module = entry.getProxiedModule();
+            RootRuntimeBeanRegistratorImpl runtimeBeanRegistrator = null;
+
             if (module instanceof RuntimeBeanRegistratorAwareModule) {
-                ((RuntimeBeanRegistratorAwareModule) module)
-                        .setRuntimeBeanRegistrator(runtimeBeanRegistrator);
+
+                if(entry.hasOldModule()) {
+
+                    if(module.canReuse(entry.getOldInternalInfo().getReadableModule().getModule())) {
+                        runtimeBeanRegistrator = entry.getOldInternalInfo().getRuntimeBeanRegistrator();
+                        ((RuntimeBeanRegistratorAwareModule) module).setRuntimeBeanRegistrator(runtimeBeanRegistrator);
+                    } else {
+                        runtimeBeanRegistrator = baseJMXRegistrator.createRuntimeBeanRegistrator(entry.getIdentifier());
+                        entry.getOldInternalInfo().getRuntimeBeanRegistrator().close();
+                        ((RuntimeBeanRegistratorAwareModule) module).setRuntimeBeanRegistrator(runtimeBeanRegistrator);
+                    }
+                } else {
+                    runtimeBeanRegistrator = baseJMXRegistrator.createRuntimeBeanRegistrator(entry.getIdentifier());
+                    ((RuntimeBeanRegistratorAwareModule) module).setRuntimeBeanRegistrator(runtimeBeanRegistrator);
+                }
             }
             // save it to info so it is accessible afterwards
-            runtimeRegistrators.put(entry.getIdentifier(), runtimeBeanRegistrator);
+            if(runtimeBeanRegistrator != null) {
+                runtimeRegistrators.put(entry.getIdentifier(), runtimeBeanRegistrator);
+            }
         }
 
         // can register runtime beans
index abef4d2..267e504 100644 (file)
@@ -44,7 +44,7 @@ public class ModuleInternalInfo implements Comparable<ModuleInternalInfo>,
     public ModuleInternalInfo(ModuleIdentifier name,
             @Nullable DynamicReadableWrapper readableModule,
             OsgiRegistration osgiRegistration,
-            RootRuntimeBeanRegistratorImpl runtimeBeanRegistrator,
+            @Nullable RootRuntimeBeanRegistratorImpl runtimeBeanRegistrator,
             ModuleJMXRegistrator moduleJMXRegistrator, int orderingIdx,
             boolean isDefaultBean, ModuleFactory moduleFactory, BundleContext bundleContext) {
 
@@ -52,10 +52,6 @@ public class ModuleInternalInfo implements Comparable<ModuleInternalInfo>,
             throw new IllegalArgumentException(
                     "Parameter 'osgiRegistration' is missing");
         }
-        if (runtimeBeanRegistrator == null) {
-            throw new IllegalArgumentException(
-                    "Parameter 'runtimeBeanRegistrator' is missing");
-        }
         this.readableModule = readableModule;
         this.osgiRegistration = osgiRegistration;
         this.runtimeBeanRegistrator = runtimeBeanRegistrator;
@@ -114,7 +110,7 @@ public class ModuleInternalInfo implements Comparable<ModuleInternalInfo>,
     public DestroyedModule toDestroyedModule() {
         return new DestroyedModule(getIdentifier(),
                 getReadableModule().getInstance(), getModuleJMXRegistrator(),
-                getOsgiRegistration(), getOrderingIdx());
+                getOsgiRegistration(), getOrderingIdx(), runtimeBeanRegistrator);
     }
 
     @Override
index 05c1007..dae6cb2 100644 (file)
@@ -9,6 +9,7 @@ package org.opendaylight.controller.config.manager.impl.dependencyresolver;
 
 import org.opendaylight.controller.config.api.ModuleIdentifier;
 import org.opendaylight.controller.config.manager.impl.jmx.ModuleJMXRegistrator;
+import org.opendaylight.controller.config.manager.impl.jmx.RootRuntimeBeanRegistratorImpl;
 import org.opendaylight.controller.config.manager.impl.osgi.BeanToOsgiServiceManager.OsgiRegistration;
 import org.opendaylight.yangtools.concepts.Identifiable;
 import org.slf4j.Logger;
@@ -30,15 +31,18 @@ public class DestroyedModule implements AutoCloseable,
     private final ModuleJMXRegistrator oldJMXRegistrator;
     private final OsgiRegistration osgiRegistration;
     private final int orderingIdx;
+    private RootRuntimeBeanRegistratorImpl runtimeBeanRegistrator;
 
     public DestroyedModule(ModuleIdentifier identifier, AutoCloseable instance,
-            ModuleJMXRegistrator oldJMXRegistrator,
-            OsgiRegistration osgiRegistration, int orderingIdx) {
+                           ModuleJMXRegistrator oldJMXRegistrator,
+                           OsgiRegistration osgiRegistration, int orderingIdx,
+                           final RootRuntimeBeanRegistratorImpl runtimeBeanRegistrator) {
         this.identifier = identifier;
         this.instance = instance;
         this.oldJMXRegistrator = oldJMXRegistrator;
         this.osgiRegistration = osgiRegistration;
         this.orderingIdx = orderingIdx;
+        this.runtimeBeanRegistrator = runtimeBeanRegistrator;
     }
 
     @Override
@@ -54,6 +58,13 @@ public class DestroyedModule implements AutoCloseable,
         } catch (Exception e) {
             LOG.error("Error while closing jmx registrator of {}", identifier, e);
         }
+        try {
+            if (runtimeBeanRegistrator != null) {
+                runtimeBeanRegistrator.close();
+            }
+        } catch (Exception e) {
+            LOG.error("Error while closing runtime bean jmx registrator of {}", identifier, e);
+        }
         try {
             osgiRegistration.close();
         } catch (Exception e) {
index fc9b645..771cddd 100644 (file)
@@ -58,7 +58,7 @@ public class ModuleInternalTransactionalInfo implements Identifiable<ModuleIdent
         return new DestroyedModule(name, oldModule.getInstance(),
                 maybeOldInternalInfo.getModuleJMXRegistrator(),
                 maybeOldInternalInfo.getOsgiRegistration(),
-                maybeOldInternalInfo.getOrderingIdx());
+                maybeOldInternalInfo.getOrderingIdx(), maybeOldInternalInfo.getRuntimeBeanRegistrator());
     }
 
 
index e35509d..8c6cd03 100644 (file)
@@ -63,7 +63,8 @@ public class TestingScheduledThreadPoolModule implements Module,
 
     @Override
     public boolean canReuse(final Module oldModule) {
-        return false;
+        return getClass().isInstance(oldModule) && getThreadCount() ==
+                ((TestingScheduledThreadPoolModule) oldModule).getThreadCount();
     }
 
     @Override