From 37b0822a7a60079ccaaf261e8ee4eb6a3636c1a0 Mon Sep 17 00:00:00 2001 From: Maros Marsalek Date: Sat, 9 Jan 2016 14:44:59 +0100 Subject: [PATCH] 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 --- .../manager/impl/ConfigRegistryImpl.java | 30 +++++++++++++------ .../manager/impl/ModuleInternalInfo.java | 8 ++--- .../dependencyresolver/DestroyedModule.java | 15 ++++++++-- .../ModuleInternalTransactionalInfo.java | 2 +- .../TestingScheduledThreadPoolModule.java | 3 +- 5 files changed, 39 insertions(+), 19 deletions(-) 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 737e2ae334..45dac318fa 100644 --- 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 - // RuntimeBeanRegistratorAwareModule, each module - // should have exactly one runtime jmx registrator. + // RuntimeBeanRegistratorAwareModule Map 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 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 abef4d2fc4..267e504f85 100644 --- 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, 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, 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, public DestroyedModule toDestroyedModule() { return new DestroyedModule(getIdentifier(), getReadableModule().getInstance(), getModuleJMXRegistrator(), - getOsgiRegistration(), getOrderingIdx()); + getOsgiRegistration(), getOrderingIdx(), runtimeBeanRegistrator); } @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 05c10077c4..dae6cb27e4 100644 --- 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.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) { 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 fc9b645ea2..771cddd442 100644 --- 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