Introduce lifecycle to runtime beans registrator in config-manager

Root runtime bean registrator was not properly closed or reused
during reconfiguration.

Change-Id: I537f7af5957496001f51663ded206a4ab04e5401
Signed-off-by: Maros Marsalek <mmarsale@cisco.com>

diff --git a/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/ConfigRegistryImpl.java b/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/ConfigRegistryImpl.java
index 737e2ae334fdaeab12990237f5d8024561fc1e73..45dac318fa06caa539b2daeb52f0e3c8bb31cfba 100644 (file)
--- a/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/ConfigRegistryImpl.java
+++ b/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/ConfigRegistryImpl.java
@@ -252,23 +252,35 @@ public class ConfigRegistryImpl implements AutoCloseable, ConfigRegistryImplMXBe
         }
 
         // set RuntimeBeanRegistrators on beans implementing
         }
 
         // 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()) {
         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();
             // set runtime jmx registrator if required
             Module module = entry.getProxiedModule();

+            RootRuntimeBeanRegistratorImpl runtimeBeanRegistrator = null;
+

             if (module instanceof RuntimeBeanRegistratorAwareModule) {
             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
             }
             // 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
         }
 
         // can register runtime beans

diff --git a/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/ModuleInternalInfo.java b/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/ModuleInternalInfo.java
index abef4d2fc4a775a26e920932c9e5f7e96622c11a..267e504f859fe3bd024bf5eb2d098fdb4904e935 100644 (file)
--- a/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/ModuleInternalInfo.java
+++ b/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/ModuleInternalInfo.java
@@ -44,7 +44,7 @@ public class ModuleInternalInfo implements Comparable<ModuleInternalInfo>,
     public ModuleInternalInfo(ModuleIdentifier name,
             @Nullable DynamicReadableWrapper readableModule,
             OsgiRegistration osgiRegistration,
     public ModuleInternalInfo(ModuleIdentifier name,
             @Nullable DynamicReadableWrapper readableModule,
             OsgiRegistration osgiRegistration,

-            RootRuntimeBeanRegistratorImpl runtimeBeanRegistrator,
+            @Nullable RootRuntimeBeanRegistratorImpl runtimeBeanRegistrator,

             ModuleJMXRegistrator moduleJMXRegistrator, int orderingIdx,
             boolean isDefaultBean, ModuleFactory moduleFactory, BundleContext bundleContext) {
 
             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");
         }
             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;
         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(),
     public DestroyedModule toDestroyedModule() {
         return new DestroyedModule(getIdentifier(),
                 getReadableModule().getInstance(), getModuleJMXRegistrator(),

-                getOsgiRegistration(), getOrderingIdx());
+                getOsgiRegistration(), getOrderingIdx(), runtimeBeanRegistrator);

     }
 
     @Override
     }
 
     @Override

diff --git a/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/dependencyresolver/DestroyedModule.java b/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/dependencyresolver/DestroyedModule.java
index 05c10077c46f07e25b36a2811b836c1725a1493b..dae6cb27e4b9777e77497a805565a5c57284791c 100644 (file)
--- a/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/dependencyresolver/DestroyedModule.java
+++ b/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/dependencyresolver/DestroyedModule.java
@@ -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.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;
 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 final ModuleJMXRegistrator oldJMXRegistrator;
     private final OsgiRegistration osgiRegistration;
     private final int orderingIdx;

+    private RootRuntimeBeanRegistratorImpl runtimeBeanRegistrator;

 
     public DestroyedModule(ModuleIdentifier identifier, AutoCloseable instance,
 
     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.identifier = identifier;
         this.instance = instance;
         this.oldJMXRegistrator = oldJMXRegistrator;
         this.osgiRegistration = osgiRegistration;
         this.orderingIdx = orderingIdx;

+        this.runtimeBeanRegistrator = runtimeBeanRegistrator;

     }
 
     @Override
     }
 
     @Override


@@ -54,6 +58,13 @@ public class DestroyedModule implements AutoCloseable,
         } catch (Exception e) {
             LOG.error("Error while closing jmx registrator of {}", identifier, e);
         }
         } 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) {
         try {
             osgiRegistration.close();
         } catch (Exception e) {

diff --git a/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/dependencyresolver/ModuleInternalTransactionalInfo.java b/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/dependencyresolver/ModuleInternalTransactionalInfo.java
index fc9b645ea2b0220a301858edfef47d0474994c10..771cddd442ef6210b234472e4d29305e35839a0a 100644 (file)
--- a/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/dependencyresolver/ModuleInternalTransactionalInfo.java
+++ b/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/dependencyresolver/ModuleInternalTransactionalInfo.java
@@ -58,7 +58,7 @@ public class ModuleInternalTransactionalInfo implements Identifiable<ModuleIdent
         return new DestroyedModule(name, oldModule.getInstance(),
                 maybeOldInternalInfo.getModuleJMXRegistrator(),
                 maybeOldInternalInfo.getOsgiRegistration(),
         return new DestroyedModule(name, oldModule.getInstance(),
                 maybeOldInternalInfo.getModuleJMXRegistrator(),
                 maybeOldInternalInfo.getOsgiRegistration(),

-                maybeOldInternalInfo.getOrderingIdx());
+                maybeOldInternalInfo.getOrderingIdx(), maybeOldInternalInfo.getRuntimeBeanRegistrator());

     }
 
 
     }
 
 

diff --git a/opendaylight/config/config-manager/src/test/java/org/opendaylight/controller/config/manager/testingservices/scheduledthreadpool/TestingScheduledThreadPoolModule.java b/opendaylight/config/config-manager/src/test/java/org/opendaylight/controller/config/manager/testingservices/scheduledthreadpool/TestingScheduledThreadPoolModule.java
index e35509d09025854e6f205b65f0ba05d6778cd754..8c6cd03cde8c919a65934de336825e74509b9a6b 100644 (file)
--- a/opendaylight/config/config-manager/src/test/java/org/opendaylight/controller/config/manager/testingservices/scheduledthreadpool/TestingScheduledThreadPoolModule.java
+++ b/opendaylight/config/config-manager/src/test/java/org/opendaylight/controller/config/manager/testingservices/scheduledthreadpool/TestingScheduledThreadPoolModule.java
@@ -63,7 +63,8 @@ public class TestingScheduledThreadPoolModule implements Module,
 
     @Override
     public boolean canReuse(final Module oldModule) {
 
     @Override
     public boolean canReuse(final Module oldModule) {

-        return false;
+        return getClass().isInstance(oldModule) && getThreadCount() ==
+                ((TestingScheduledThreadPoolModule) oldModule).getThreadCount();

     }
 
     @Override
     }
 
     @Override