From d4e097a03ce5cb23b2418a0bc8fdc182d00da4d0 Mon Sep 17 00:00:00 2001 From: Tomas Olvecky Date: Tue, 22 Apr 2014 12:32:19 +0200 Subject: [PATCH] Resolve Bug:807 - Keep ModuleFactory references shutdown purposes. Retain ModuleFactory instances even if they disappear from OSGi SR, so that the already committed instances can be closed cleanly. Also remove shutdown customization (stopping config-manager as first bundle) from shutdown-impl. Change-Id: I8e90a7d593b7e4cc84c7a9a7d3f8bb804daf5f41 Signed-off-by: Tomas Olvecky --- .../manager/impl/ConfigRegistryImpl.java | 21 +++-- .../impl/ConfigTransactionControllerImpl.java | 82 +++++++++++-------- .../manager/impl/ModuleInternalInfo.java | 16 +++- ...hableServiceReferenceWritableRegistry.java | 6 +- .../impl/ServiceReferenceRegistryImpl.java | 48 +++++------ .../DependencyResolverManager.java | 28 +++---- .../ModuleInternalTransactionalInfo.java | 9 +- ...ierarchicalConfigMBeanFactoriesHolder.java | 14 ++-- .../impl/osgi/BeanToOsgiServiceManager.java | 23 +++--- .../manager/impl/util/InterfacesHelper.java | 9 +- .../DependencyResolverManagerTest.java | 20 ++--- .../threadpool/TestingFixedThreadPool.java | 1 + .../threadpool/test/ShutdownTest.java | 63 ++++++++++++++ .../test/SimpleConfigurationTest.java | 12 +-- .../shutdown/impl/ShutdownServiceImpl.java | 18 ---- .../yang/shutdown/impl/ShutdownTest.java | 31 ++++--- 16 files changed, 248 insertions(+), 153 deletions(-) create mode 100644 opendaylight/config/config-manager/src/test/java/org/opendaylight/controller/config/manager/testingservices/threadpool/test/ShutdownTest.java 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 fbf18f7134..8f85972d05 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 @@ -159,9 +159,20 @@ public class ConfigRegistryImpl implements AutoCloseable, ConfigRegistryImplMXBe } }; - Map> allCurrentFactories = Collections.unmodifiableMap( + Map> allCurrentFactories = new HashMap<>( resolver.getAllFactories()); + // add all factories that disappeared from SR but are still committed + for (ModuleInternalInfo moduleInternalInfo : currentConfig.getEntries()) { + String name = moduleInternalInfo.getModuleFactory().getImplementationName(); + if (allCurrentFactories.containsKey(name) == false) { + logger.trace("Factory {} not found in SR, using reference from previous commit", name); + allCurrentFactories.put(name, + Maps.immutableEntry(moduleInternalInfo.getModuleFactory(), moduleInternalInfo.getBundleContext())); + } + } + allCurrentFactories = Collections.unmodifiableMap(allCurrentFactories); + // closed by transaction controller ConfigTransactionLookupRegistry txLookupRegistry = new ConfigTransactionLookupRegistry(new TransactionIdentifier( transactionName), factory, allCurrentFactories); @@ -216,16 +227,14 @@ public class ConfigRegistryImpl implements AutoCloseable, ConfigRegistryImplMXBe // non recoverable from here: try { return secondPhaseCommit(configTransactionController, commitInfo, configTransactionControllerEntry.getValue()); - } catch (Throwable t) { // some libs throw Errors: e.g. + } catch (Error | RuntimeException t) { // some libs throw Errors: e.g. // javax.xml.ws.spi.FactoryFinder$ConfigurationError isHealthy = false; logger.error("Configuration Transaction failed on 2PC, server is unhealthy", t); if (t instanceof RuntimeException) { throw (RuntimeException) t; - } else if (t instanceof Error) { - throw (Error) t; } else { - throw new RuntimeException(t); + throw (Error) t; } } } @@ -365,7 +374,7 @@ public class ConfigRegistryImpl implements AutoCloseable, ConfigRegistryImplMXBe ModuleInternalInfo newInfo = new ModuleInternalInfo( entry.getIdentifier(), newReadableConfigBean, osgiRegistration, runtimeBeanRegistrator, newModuleJMXRegistrator, - orderingIdx, entry.isDefaultBean()); + orderingIdx, entry.isDefaultBean(), entry.getModuleFactory(), entry.getBundleContext()); newConfigEntries.put(realModule, newInfo); orderingIdx++; diff --git a/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/ConfigTransactionControllerImpl.java b/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/ConfigTransactionControllerImpl.java index c549f52017..3e23120182 100644 --- a/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/ConfigTransactionControllerImpl.java +++ b/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/ConfigTransactionControllerImpl.java @@ -7,6 +7,23 @@ */ package org.opendaylight.controller.config.manager.impl; +import static java.lang.String.format; + +import java.util.ArrayList; +import java.util.Collection; +import java.util.HashSet; +import java.util.List; +import java.util.Map; +import java.util.Map.Entry; +import java.util.Set; +import java.util.concurrent.atomic.AtomicBoolean; +import javax.annotation.Nullable; +import javax.annotation.concurrent.GuardedBy; +import javax.management.DynamicMBean; +import javax.management.InstanceAlreadyExistsException; +import javax.management.InstanceNotFoundException; +import javax.management.MBeanServer; +import javax.management.ObjectName; import org.opendaylight.controller.config.api.DependencyResolver; import org.opendaylight.controller.config.api.ModuleIdentifier; import org.opendaylight.controller.config.api.ValidationException; @@ -27,25 +44,7 @@ import org.osgi.framework.BundleContext; import org.slf4j.Logger; import org.slf4j.LoggerFactory; -import javax.annotation.Nullable; -import javax.annotation.concurrent.GuardedBy; -import javax.management.DynamicMBean; -import javax.management.InstanceAlreadyExistsException; -import javax.management.InstanceNotFoundException; -import javax.management.MBeanServer; -import javax.management.ObjectName; -import java.util.ArrayList; -import java.util.Collection; -import java.util.HashSet; -import java.util.List; -import java.util.Map; -import java.util.Map.Entry; -import java.util.Set; -import java.util.concurrent.atomic.AtomicBoolean; - import static com.google.common.base.Preconditions.checkNotNull; -import static java.lang.String.format; - /** * This is a JMX bean representing current transaction. It contains * transaction identifier, unique version and parent version for @@ -138,14 +137,16 @@ class ConfigTransactionControllerImpl implements } // add default modules for (ModuleFactory moduleFactory : toBeAdded) { + BundleContext bundleContext = getModuleFactoryBundleContext(moduleFactory.getImplementationName()); Set defaultModules = moduleFactory.getDefaultModules(dependencyResolverManager, - getModuleFactoryBundleContext(moduleFactory.getImplementationName())); + bundleContext); for (Module module : defaultModules) { // ensure default module to be registered to jmx even if its module factory does not use dependencyResolverFactory DependencyResolver dependencyResolver = dependencyResolverManager.getOrCreate(module.getIdentifier()); try { boolean defaultBean = true; - putConfigBeanToJMXAndInternalMaps(module.getIdentifier(), module, moduleFactory, null, dependencyResolver, defaultBean); + putConfigBeanToJMXAndInternalMaps(module.getIdentifier(), module, moduleFactory, null, + dependencyResolver, defaultBean, bundleContext); } catch (InstanceAlreadyExistsException e) { throw new IllegalStateException(e); } @@ -162,21 +163,26 @@ class ConfigTransactionControllerImpl implements } - private synchronized void copyExistingModule( - ModuleInternalInfo oldConfigBeanInfo) - throws InstanceAlreadyExistsException { + private synchronized void copyExistingModule(ModuleInternalInfo oldConfigBeanInfo) throws InstanceAlreadyExistsException { + transactionStatus.checkNotCommitStarted(); transactionStatus.checkNotAborted(); ModuleIdentifier moduleIdentifier = oldConfigBeanInfo.getIdentifier(); dependencyResolverManager.assertNotExists(moduleIdentifier); - ModuleFactory moduleFactory = factoriesHolder - .findByModuleName(moduleIdentifier.getFactoryName()); + ModuleFactory moduleFactory; + BundleContext bc; + try { + moduleFactory = factoriesHolder.findByModuleName(moduleIdentifier.getFactoryName()); + bc = getModuleFactoryBundleContext(moduleFactory.getImplementationName()); + } catch (InstanceNotFoundException e) { + throw new IllegalStateException(e); + } Module module; DependencyResolver dependencyResolver = dependencyResolverManager.getOrCreate(moduleIdentifier); try { - BundleContext bc = getModuleFactoryBundleContext(moduleFactory.getImplementationName()); + module = moduleFactory.createModule( moduleIdentifier.getInstanceName(), dependencyResolver, oldConfigBeanInfo.getReadableModule(), bc); @@ -186,7 +192,7 @@ class ConfigTransactionControllerImpl implements oldConfigBeanInfo, moduleFactory), e); } putConfigBeanToJMXAndInternalMaps(moduleIdentifier, module, moduleFactory, oldConfigBeanInfo, dependencyResolver, - oldConfigBeanInfo.isDefaultBean()); + oldConfigBeanInfo.isDefaultBean(), bc); } @Override @@ -199,19 +205,26 @@ class ConfigTransactionControllerImpl implements dependencyResolverManager.assertNotExists(moduleIdentifier); // find factory - ModuleFactory moduleFactory = factoriesHolder.findByModuleName(factoryName); + ModuleFactory moduleFactory; + try { + moduleFactory = factoriesHolder.findByModuleName(factoryName); + } catch (InstanceNotFoundException e) { + throw new IllegalArgumentException(e); + } DependencyResolver dependencyResolver = dependencyResolverManager.getOrCreate(moduleIdentifier); + BundleContext bundleContext = getModuleFactoryBundleContext(moduleFactory.getImplementationName()); Module module = moduleFactory.createModule(instanceName, dependencyResolver, - getModuleFactoryBundleContext(moduleFactory.getImplementationName())); + bundleContext); boolean defaultBean = false; return putConfigBeanToJMXAndInternalMaps(moduleIdentifier, module, - moduleFactory, null, dependencyResolver, defaultBean); + moduleFactory, null, dependencyResolver, defaultBean, bundleContext); } private synchronized ObjectName putConfigBeanToJMXAndInternalMaps( ModuleIdentifier moduleIdentifier, Module module, ModuleFactory moduleFactory, - @Nullable ModuleInternalInfo maybeOldConfigBeanInfo, DependencyResolver dependencyResolver, boolean isDefaultBean) + @Nullable ModuleInternalInfo maybeOldConfigBeanInfo, DependencyResolver dependencyResolver, + boolean isDefaultBean, BundleContext bundleContext) throws InstanceAlreadyExistsException { logger.debug("Adding module {} to transaction {}", moduleIdentifier, this); @@ -236,7 +249,7 @@ class ConfigTransactionControllerImpl implements dependencyResolverManager.put( moduleIdentifier, module, moduleFactory, - maybeOldConfigBeanInfo, transactionModuleJMXRegistration, isDefaultBean); + maybeOldConfigBeanInfo, transactionModuleJMXRegistration, isDefaultBean, bundleContext); return writableON; } @@ -262,14 +275,15 @@ class ConfigTransactionControllerImpl implements logger.debug("Destroying module {} in transaction {}", moduleIdentifier, this); transactionStatus.checkNotAborted(); + ModuleInternalTransactionalInfo found = dependencyResolverManager.findModuleInternalTransactionalInfo(moduleIdentifier); if (blankTransaction == false) { - ModuleInternalTransactionalInfo found = - dependencyResolverManager.findModuleInternalTransactionalInfo(moduleIdentifier); + if (found.isDefaultBean()) { logger.warn("Warning: removing default bean. This will be forbidden in next version of config-subsystem"); } } // first remove refNames, it checks for objectname existence + try { writableSRRegistry.removeServiceReferences( ObjectNameUtil.createTransactionModuleON(getTransactionName(), moduleIdentifier)); 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 fd6262cb8c..76299e67a2 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 @@ -15,7 +15,9 @@ import org.opendaylight.controller.config.manager.impl.dynamicmbean.DynamicReada 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.controller.config.spi.ModuleFactory; import org.opendaylight.yangtools.concepts.Identifiable; +import org.osgi.framework.BundleContext; /** * Provides metadata about Module from controller to registry. @@ -37,13 +39,15 @@ public class ModuleInternalInfo implements Comparable, private final ModuleJMXRegistrator moduleJMXRegistrator; private final int orderingIdx; private final boolean isDefaultBean; + private final ModuleFactory moduleFactory; + private final BundleContext bundleContext; public ModuleInternalInfo(ModuleIdentifier name, @Nullable DynamicReadableWrapper readableModule, OsgiRegistration osgiRegistration, RootRuntimeBeanRegistratorImpl runtimeBeanRegistrator, ModuleJMXRegistrator moduleJMXRegistrator, int orderingIdx, - boolean isDefaultBean) { + boolean isDefaultBean, ModuleFactory moduleFactory, BundleContext bundleContext) { if (osgiRegistration == null) { throw new IllegalArgumentException( @@ -60,6 +64,8 @@ public class ModuleInternalInfo implements Comparable, this.moduleJMXRegistrator = moduleJMXRegistrator; this.orderingIdx = orderingIdx; this.isDefaultBean = isDefaultBean; + this.moduleFactory = moduleFactory; + this.bundleContext = bundleContext; } public DynamicReadableWrapper getReadableModule() { @@ -120,4 +126,12 @@ public class ModuleInternalInfo implements Comparable, public boolean isDefaultBean() { return isDefaultBean; } + + public ModuleFactory getModuleFactory() { + return moduleFactory; + } + + public BundleContext getBundleContext() { + return bundleContext; + } } diff --git a/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/SearchableServiceReferenceWritableRegistry.java b/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/SearchableServiceReferenceWritableRegistry.java index 24ddf6362e..7a41a3bfa2 100644 --- a/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/SearchableServiceReferenceWritableRegistry.java +++ b/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/SearchableServiceReferenceWritableRegistry.java @@ -8,18 +8,18 @@ package org.opendaylight.controller.config.manager.impl; +import java.util.Map; import org.opendaylight.controller.config.api.ModuleIdentifier; import org.opendaylight.controller.config.api.ServiceReferenceWritableRegistry; import org.opendaylight.controller.config.api.annotations.ServiceInterfaceAnnotation; -import java.util.Map; - public interface SearchableServiceReferenceWritableRegistry extends ServiceReferenceWritableRegistry { /** * Return mapping between service ref names and service interface annotation for given * module. + * * @throws java.lang.IllegalArgumentException if any of service qNames is not found - * @throws java.lang.NullPointerException if parameter is null + * @throws java.lang.NullPointerException if parameter is null */ Map findServiceInterfaces(ModuleIdentifier moduleIdentifier); diff --git a/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/ServiceReferenceRegistryImpl.java b/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/ServiceReferenceRegistryImpl.java index ed04779d9e..ceea994154 100644 --- a/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/ServiceReferenceRegistryImpl.java +++ b/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/ServiceReferenceRegistryImpl.java @@ -7,6 +7,17 @@ */ package org.opendaylight.controller.config.manager.impl; +import static com.google.common.base.Preconditions.checkNotNull; + +import java.util.Collections; +import java.util.HashMap; +import java.util.HashSet; +import java.util.Map; +import java.util.Map.Entry; +import java.util.Set; +import javax.management.InstanceAlreadyExistsException; +import javax.management.InstanceNotFoundException; +import javax.management.ObjectName; import org.opendaylight.controller.config.api.LookupRegistry; import org.opendaylight.controller.config.api.ModuleIdentifier; import org.opendaylight.controller.config.api.ServiceReferenceReadableRegistry; @@ -26,18 +37,6 @@ import org.osgi.framework.BundleContext; import org.slf4j.Logger; import org.slf4j.LoggerFactory; -import javax.management.InstanceAlreadyExistsException; -import javax.management.InstanceNotFoundException; -import javax.management.ObjectName; -import java.util.Collections; -import java.util.HashMap; -import java.util.HashSet; -import java.util.Map; -import java.util.Map.Entry; -import java.util.Set; - -import static com.google.common.base.Preconditions.checkNotNull; - public class ServiceReferenceRegistryImpl implements CloseableServiceReferenceReadableRegistry, SearchableServiceReferenceWritableRegistry { private static final Logger logger = LoggerFactory.getLogger(ServiceReferenceRegistryImpl.class); @@ -211,10 +210,7 @@ public class ServiceReferenceRegistryImpl implements CloseableServiceReferenceRe throw new IllegalArgumentException("Possible error in code: Mismatch between supplied and actual name of " + entry); } Set siAnnotations = InterfacesHelper.getServiceInterfaceAnnotations(entry.getValue()); - Set qNames = new HashSet<>(); - for (ServiceInterfaceAnnotation sia: siAnnotations) { - qNames.add(sia.value()); - } + Set qNames = InterfacesHelper.getQNames(siAnnotations); allAnnotations.addAll(siAnnotations); allQNames.addAll(qNames); modifiableFactoryNamesToQNames.put(entry.getKey(), Collections.unmodifiableSet(qNames)); @@ -501,21 +497,28 @@ public class ServiceReferenceRegistryImpl implements CloseableServiceReferenceRe @Override public synchronized boolean removeServiceReferences(ObjectName moduleObjectName) throws InstanceNotFoundException { + lookupRegistry.checkConfigBeanExists(moduleObjectName); + String factoryName = ObjectNameUtil.getFactoryName(moduleObjectName); + // check that service interface name exist + Set serviceInterfaceQNames = factoryNamesToQNames.get(factoryName); + return removeServiceReferences(moduleObjectName, serviceInterfaceQNames); + } + + + private boolean removeServiceReferences(ObjectName moduleObjectName, Set qNames) throws InstanceNotFoundException { + ObjectNameUtil.checkType(moduleObjectName, ObjectNameUtil.TYPE_MODULE); assertWritable(); - Set serviceReferencesLinkingTo = findServiceReferencesLinkingTo(moduleObjectName); + Set serviceReferencesLinkingTo = findServiceReferencesLinkingTo(moduleObjectName, qNames); for (ServiceReference sr : serviceReferencesLinkingTo) { removeServiceReference(sr); } return serviceReferencesLinkingTo.isEmpty() == false; } - private synchronized Set findServiceReferencesLinkingTo(ObjectName moduleObjectName) throws InstanceNotFoundException { - lookupRegistry.checkConfigBeanExists(moduleObjectName); + private Set findServiceReferencesLinkingTo(ObjectName moduleObjectName, Set serviceInterfaceQNames) { String factoryName = ObjectNameUtil.getFactoryName(moduleObjectName); - // check that service interface name exist - Set serviceInterfaceQNames = factoryNamesToQNames.get(factoryName); if (serviceInterfaceQNames == null) { - logger.error("Possible error in code: cannot find factoryName {} in {}, object name {}", factoryName, factoryNamesToQNames, moduleObjectName); + logger.warn("Possible error in code: cannot find factoryName {} in {}, object name {}", factoryName, factoryNamesToQNames, moduleObjectName); throw new IllegalStateException("Possible error in code: cannot find annotations of existing factory " + factoryName); } String instanceName = ObjectNameUtil.getInstanceName(moduleObjectName); @@ -529,7 +532,6 @@ public class ServiceReferenceRegistryImpl implements CloseableServiceReferenceRe return result; } - @Override public String toString() { return "ServiceReferenceRegistryImpl{" + diff --git a/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/dependencyresolver/DependencyResolverManager.java b/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/dependencyresolver/DependencyResolverManager.java index b99bf8330e..0c1531728f 100644 --- a/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/dependencyresolver/DependencyResolverManager.java +++ b/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/dependencyresolver/DependencyResolverManager.java @@ -7,8 +7,19 @@ */ package org.opendaylight.controller.config.manager.impl.dependencyresolver; +import static com.google.common.base.Preconditions.checkState; + import com.google.common.reflect.AbstractInvocationHandler; import com.google.common.reflect.Reflection; +import java.lang.reflect.InvocationTargetException; +import java.lang.reflect.Method; +import java.util.ArrayList; +import java.util.Collections; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import javax.annotation.concurrent.GuardedBy; +import javax.management.InstanceAlreadyExistsException; import org.opendaylight.controller.config.api.DependencyResolver; import org.opendaylight.controller.config.api.DependencyResolverFactory; import org.opendaylight.controller.config.api.JmxAttribute; @@ -24,18 +35,7 @@ import org.opendaylight.controller.config.manager.impl.jmx.TransactionModuleJMXR import org.opendaylight.controller.config.spi.Module; import org.opendaylight.controller.config.spi.ModuleFactory; import org.opendaylight.yangtools.yang.data.impl.codec.CodecRegistry; - -import javax.annotation.concurrent.GuardedBy; -import javax.management.InstanceAlreadyExistsException; -import java.lang.reflect.InvocationTargetException; -import java.lang.reflect.Method; -import java.util.ArrayList; -import java.util.Collections; -import java.util.HashMap; -import java.util.List; -import java.util.Map; - -import static com.google.common.base.Preconditions.checkState; +import org.osgi.framework.BundleContext; /** * Holds information about modules being created and destroyed within this @@ -117,7 +117,7 @@ public class DependencyResolverManager implements DependencyResolverFactory, Aut ModuleFactory moduleFactory, ModuleInternalInfo maybeOldInternalInfo, TransactionModuleJMXRegistration transactionModuleJMXRegistration, - boolean isDefaultBean) { + boolean isDefaultBean, BundleContext bundleContext) { transactionStatus.checkNotCommitted(); Class moduleClass = Module.class; @@ -159,7 +159,7 @@ public class DependencyResolverManager implements DependencyResolverFactory, Aut ModuleInternalTransactionalInfo moduleInternalTransactionalInfo = new ModuleInternalTransactionalInfo( moduleIdentifier, proxiedModule, moduleFactory, - maybeOldInternalInfo, transactionModuleJMXRegistration, isDefaultBean, module); + maybeOldInternalInfo, transactionModuleJMXRegistration, isDefaultBean, module, bundleContext); modulesHolder.put(moduleInternalTransactionalInfo); } 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 e9f573a05d..13424a60eb 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 @@ -16,6 +16,7 @@ import org.opendaylight.controller.config.spi.ModuleFactory; import org.opendaylight.yangtools.concepts.Identifiable; import javax.annotation.Nullable; +import org.osgi.framework.BundleContext; public class ModuleInternalTransactionalInfo implements Identifiable { private final ModuleIdentifier name; @@ -25,12 +26,13 @@ public class ModuleInternalTransactionalInfo implements Identifiable result = moduleNamesToConfigBeanFactories.get(moduleName); if (result == null) { - throw new IllegalArgumentException( + throw new InstanceNotFoundException( "ModuleFactory not found with module name: " + moduleName); } return result.getKey(); @@ -73,7 +72,4 @@ public class HierarchicalConfigMBeanFactoriesHolder { return moduleFactories; } - public Map> getModuleNamesToConfigBeanFactories() { - return moduleNamesToConfigBeanFactories; - } } diff --git a/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/osgi/BeanToOsgiServiceManager.java b/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/osgi/BeanToOsgiServiceManager.java index 01afe223bb..fbc7eb6cbe 100644 --- a/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/osgi/BeanToOsgiServiceManager.java +++ b/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/osgi/BeanToOsgiServiceManager.java @@ -7,22 +7,21 @@ */ package org.opendaylight.controller.config.manager.impl.osgi; -import org.opendaylight.controller.config.api.ModuleIdentifier; -import org.opendaylight.controller.config.api.annotations.ServiceInterfaceAnnotation; -import org.osgi.framework.BundleContext; -import org.osgi.framework.ServiceRegistration; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; +import static com.google.common.base.Preconditions.checkState; -import javax.annotation.concurrent.GuardedBy; import java.util.Dictionary; import java.util.HashSet; import java.util.Hashtable; import java.util.Map; import java.util.Map.Entry; import java.util.Set; - -import static com.google.common.base.Preconditions.checkState; +import javax.annotation.concurrent.GuardedBy; +import org.opendaylight.controller.config.api.ModuleIdentifier; +import org.opendaylight.controller.config.api.annotations.ServiceInterfaceAnnotation; +import org.osgi.framework.BundleContext; +import org.osgi.framework.ServiceRegistration; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; /** * Registers instantiated beans as OSGi services and unregisters these services @@ -87,7 +86,11 @@ public class BeanToOsgiServiceManager { @Override public synchronized void close() { for (ServiceRegistration serviceRegistration : serviceRegistrations) { - serviceRegistration.unregister(); + try { + serviceRegistration.unregister(); + } catch(IllegalStateException e) { + logger.trace("Cannot unregister {}", serviceRegistration, e); + } } serviceRegistrations.clear(); } diff --git a/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/util/InterfacesHelper.java b/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/util/InterfacesHelper.java index 033f7222fc..a6e9b1ba96 100644 --- a/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/util/InterfacesHelper.java +++ b/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/util/InterfacesHelper.java @@ -120,6 +120,13 @@ public class InterfacesHelper { return result; } + public static Set getQNames(Set siAnnotations) { + Set qNames = new HashSet<>(); + for (ServiceInterfaceAnnotation sia: siAnnotations) { + qNames.add(sia.value()); + } + return Collections.unmodifiableSet(qNames); + } public static Set getServiceInterfaceAnnotations(ModuleFactory factory) { Set> implementedServiceIntefaces = Collections.unmodifiableSet(factory.getImplementedServiceIntefaces()); @@ -136,7 +143,7 @@ public class InterfacesHelper { result.add(annotation); } } - return result; + return Collections.unmodifiableSet(result); } static Set> getAllAbstractServiceInterfaceClasses( diff --git a/opendaylight/config/config-manager/src/test/java/org/opendaylight/controller/config/manager/impl/dependencyresolver/DependencyResolverManagerTest.java b/opendaylight/config/config-manager/src/test/java/org/opendaylight/controller/config/manager/impl/dependencyresolver/DependencyResolverManagerTest.java index 123e52f675..d5d3823ef0 100644 --- a/opendaylight/config/config-manager/src/test/java/org/opendaylight/controller/config/manager/impl/dependencyresolver/DependencyResolverManagerTest.java +++ b/opendaylight/config/config-manager/src/test/java/org/opendaylight/controller/config/manager/impl/dependencyresolver/DependencyResolverManagerTest.java @@ -7,6 +7,14 @@ */ package org.opendaylight.controller.config.manager.impl.dependencyresolver; +import static org.junit.Assert.assertEquals; +import static org.mockito.Mockito.doNothing; +import static org.mockito.Mockito.doReturn; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.reset; + +import java.util.Arrays; +import java.util.List; import org.junit.Before; import org.junit.Test; import org.opendaylight.controller.config.api.JmxAttribute; @@ -19,15 +27,7 @@ import org.opendaylight.controller.config.manager.impl.TransactionStatus; import org.opendaylight.controller.config.manager.impl.jmx.TransactionModuleJMXRegistrator.TransactionModuleJMXRegistration; import org.opendaylight.controller.config.spi.Module; import org.opendaylight.controller.config.spi.ModuleFactory; - -import java.util.Arrays; -import java.util.List; - -import static org.junit.Assert.assertEquals; -import static org.mockito.Mockito.doNothing; -import static org.mockito.Mockito.doReturn; -import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.reset; +import org.osgi.framework.BundleContext; public class DependencyResolverManagerTest { @@ -101,7 +101,7 @@ public class DependencyResolverManagerTest { moduleFactory, maybeOldInternalInfo, transactionModuleJMXRegistration, - isDefaultBean); + isDefaultBean, mock(BundleContext.class)); } private static Module mockedModule() { diff --git a/opendaylight/config/config-manager/src/test/java/org/opendaylight/controller/config/manager/testingservices/threadpool/TestingFixedThreadPool.java b/opendaylight/config/config-manager/src/test/java/org/opendaylight/controller/config/manager/testingservices/threadpool/TestingFixedThreadPool.java index d5434d2ed5..25125fcd82 100644 --- a/opendaylight/config/config-manager/src/test/java/org/opendaylight/controller/config/manager/testingservices/threadpool/TestingFixedThreadPool.java +++ b/opendaylight/config/config-manager/src/test/java/org/opendaylight/controller/config/manager/testingservices/threadpool/TestingFixedThreadPool.java @@ -53,6 +53,7 @@ public class TestingFixedThreadPool implements TestingThreadPoolIfc, Closeable, @Override public void close() throws IOException { executorService.shutdown(); + allExecutors.remove(executorService); } diff --git a/opendaylight/config/config-manager/src/test/java/org/opendaylight/controller/config/manager/testingservices/threadpool/test/ShutdownTest.java b/opendaylight/config/config-manager/src/test/java/org/opendaylight/controller/config/manager/testingservices/threadpool/test/ShutdownTest.java new file mode 100644 index 0000000000..e047a1ecca --- /dev/null +++ b/opendaylight/config/config-manager/src/test/java/org/opendaylight/controller/config/manager/testingservices/threadpool/test/ShutdownTest.java @@ -0,0 +1,63 @@ +/* + * Copyright (c) 2014 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.testingservices.threadpool.test; + +import static org.junit.Assert.assertEquals; +import static org.mockito.Mockito.doReturn; + +import com.google.common.collect.ImmutableMap; +import com.google.common.collect.Maps; +import java.util.Collections; +import java.util.Map; +import java.util.Map.Entry; +import org.junit.Before; +import org.junit.Test; +import org.mockito.Mock; +import org.mockito.MockitoAnnotations; +import org.opendaylight.controller.config.manager.impl.AbstractConfigTest; +import org.opendaylight.controller.config.manager.impl.factoriesresolver.ModuleFactoriesResolver; +import org.opendaylight.controller.config.manager.testingservices.threadpool.TestingFixedThreadPool; +import org.opendaylight.controller.config.manager.testingservices.threadpool.TestingFixedThreadPoolModuleFactory; +import org.opendaylight.controller.config.spi.ModuleFactory; +import org.opendaylight.controller.config.util.ConfigTransactionJMXClient; +import org.osgi.framework.BundleContext; + +public class ShutdownTest extends AbstractConfigTest { + private final TestingFixedThreadPoolModuleFactory testingFixedThreadPoolModuleFactory = new TestingFixedThreadPoolModuleFactory(); + + @Mock + ModuleFactoriesResolver mockedResolver; + + @Before + public void setUp() { + MockitoAnnotations.initMocks(this); + Map> allFactories = ImmutableMap.of( + testingFixedThreadPoolModuleFactory.getImplementationName(), + Maps.immutableEntry(testingFixedThreadPoolModuleFactory, mockedContext)); + doReturn(allFactories).when(mockedResolver).getAllFactories(); + super.initConfigTransactionManagerImpl(mockedResolver); + } + + + @Test + public void testCreateAndDestroyBeanInSameTransaction() throws Exception { + { + ConfigTransactionJMXClient transaction = configRegistryClient.createTransaction(); + SimpleConfigurationTest.createFixedThreadPool(transaction); + transaction.commit(); + } + assertEquals(1, TestingFixedThreadPool.allExecutors.size()); + doReturn(Collections.emptyMap()).when(mockedResolver).getAllFactories(); + { + ConfigTransactionJMXClient transaction = configRegistryClient.createTransaction(); + transaction.commit(); + } + assertEquals(1, TestingFixedThreadPool.allExecutors.size()); + } +} diff --git a/opendaylight/config/config-manager/src/test/java/org/opendaylight/controller/config/manager/testingservices/threadpool/test/SimpleConfigurationTest.java b/opendaylight/config/config-manager/src/test/java/org/opendaylight/controller/config/manager/testingservices/threadpool/test/SimpleConfigurationTest.java index 547d421c92..97d1c63ed2 100644 --- a/opendaylight/config/config-manager/src/test/java/org/opendaylight/controller/config/manager/testingservices/threadpool/test/SimpleConfigurationTest.java +++ b/opendaylight/config/config-manager/src/test/java/org/opendaylight/controller/config/manager/testingservices/threadpool/test/SimpleConfigurationTest.java @@ -56,7 +56,7 @@ import static org.junit.Assert.fail; * dependencies. */ public class SimpleConfigurationTest extends AbstractConfigTest { - private final int numberOfThreads = 5; + private static final int numberOfThreads = 5; private final int numberOfThreads2 = 10; private static final String fixed1 = "fixed1"; private static final List emptyONs = Collections @@ -96,7 +96,7 @@ public class SimpleConfigurationTest extends AbstractConfigTest { return fixed1names; } - private ObjectName createFixedThreadPool( + static ObjectName createFixedThreadPool( ConfigTransactionJMXClient transaction) throws InstanceAlreadyExistsException, InstanceNotFoundException { transaction.assertVersion(0, 1); @@ -246,8 +246,7 @@ public class SimpleConfigurationTest extends AbstractConfigTest { // 4, check assertEquals(2, configRegistryClient.getVersion()); - assertEquals(1, TestingFixedThreadPool.allExecutors.size()); - assertTrue(TestingFixedThreadPool.allExecutors.get(0).isShutdown()); + assertEquals(0, TestingFixedThreadPool.allExecutors.size()); // dynamic config should be removed from platform try { @@ -278,7 +277,7 @@ public class SimpleConfigurationTest extends AbstractConfigTest { // commit transaction.commit(); // check that first threadpool is closed - checkThreadPools(2, numberOfThreads2); + checkThreadPools(1, numberOfThreads2); } private void checkThreadPools(int expectedTotalNumberOfExecutors, @@ -308,7 +307,7 @@ public class SimpleConfigurationTest extends AbstractConfigTest { // commit CommitStatus commitStatus = transaction.commit(); // check that new threadpool is created and old one is closed - checkThreadPools(2, numberOfThreads); + checkThreadPools(1, numberOfThreads); CommitStatus expected = new CommitStatus(emptyONs, emptyONs, fixed1List); assertEquals(expected, commitStatus); } @@ -337,6 +336,7 @@ public class SimpleConfigurationTest extends AbstractConfigTest { platformMBeanServer.getMBeanInfo(transaction.getObjectName()); fail(); }catch(InstanceNotFoundException e){ + assertEquals("org.opendaylight.controller:TransactionName=ConfigTransaction-0-1,type=ConfigTransaction", e.getMessage()); } } diff --git a/opendaylight/config/shutdown-impl/src/main/java/org/opendaylight/controller/config/yang/shutdown/impl/ShutdownServiceImpl.java b/opendaylight/config/shutdown-impl/src/main/java/org/opendaylight/controller/config/yang/shutdown/impl/ShutdownServiceImpl.java index f9622192fe..4abbd3b36f 100644 --- a/opendaylight/config/shutdown-impl/src/main/java/org/opendaylight/controller/config/yang/shutdown/impl/ShutdownServiceImpl.java +++ b/opendaylight/config/shutdown-impl/src/main/java/org/opendaylight/controller/config/yang/shutdown/impl/ShutdownServiceImpl.java @@ -82,7 +82,6 @@ class Impl implements ShutdownService { class StopSystemBundleThread extends Thread { private static final Logger logger = LoggerFactory.getLogger(StopSystemBundleThread.class); - public static final String CONFIG_MANAGER_SYMBOLIC_NAME = "org.opendaylight.controller.config-manager"; private final Bundle systemBundle; StopSystemBundleThread(Bundle systemBundle) { @@ -95,13 +94,6 @@ class StopSystemBundleThread extends Thread { try { // wait so that JMX response is received Thread.sleep(1000); - // first try to stop config-manager - Bundle configManager = findConfigManager(); - if (configManager != null){ - logger.debug("Stopping config-manager"); - configManager.stop(); - Thread.sleep(1000); - } logger.debug("Stopping system bundle"); systemBundle.stop(); } catch (BundleException e) { @@ -110,16 +102,6 @@ class StopSystemBundleThread extends Thread { logger.warn("Shutdown process interrupted", e); } } - - private Bundle findConfigManager() { - for(Bundle bundle: systemBundle.getBundleContext().getBundles()){ - if (CONFIG_MANAGER_SYMBOLIC_NAME.equals(bundle.getSymbolicName())) { - return bundle; - } - } - return null; - } - } class CallSystemExitThread extends Thread { diff --git a/opendaylight/config/shutdown-impl/src/test/java/org/opendaylight/controller/config/yang/shutdown/impl/ShutdownTest.java b/opendaylight/config/shutdown-impl/src/test/java/org/opendaylight/controller/config/yang/shutdown/impl/ShutdownTest.java index d4a3160b89..358586f634 100644 --- a/opendaylight/config/shutdown-impl/src/test/java/org/opendaylight/controller/config/yang/shutdown/impl/ShutdownTest.java +++ b/opendaylight/config/shutdown-impl/src/test/java/org/opendaylight/controller/config/yang/shutdown/impl/ShutdownTest.java @@ -7,6 +7,17 @@ */ package org.opendaylight.controller.config.yang.shutdown.impl; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.fail; +import static org.mockito.Mockito.doNothing; +import static org.mockito.Mockito.doReturn; +import static org.mockito.Mockito.verify; +import static org.opendaylight.controller.config.yang.shutdown.impl.ShutdownModuleFactory.NAME; + +import java.util.Collections; +import javax.management.InstanceNotFoundException; +import javax.management.JMX; +import javax.management.ObjectName; import org.junit.Before; import org.junit.Test; import org.mockito.Mock; @@ -20,22 +31,10 @@ import org.opendaylight.controller.config.manager.impl.factoriesresolver.ModuleF import org.opendaylight.controller.config.util.ConfigTransactionJMXClient; import org.osgi.framework.Bundle; -import javax.management.InstanceNotFoundException; -import javax.management.JMX; -import javax.management.ObjectName; -import java.util.Collections; - -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.fail; -import static org.mockito.Mockito.doNothing; -import static org.mockito.Mockito.doReturn; -import static org.mockito.Mockito.verify; -import static org.opendaylight.controller.config.yang.shutdown.impl.ShutdownModuleFactory.NAME; - public class ShutdownTest extends AbstractConfigTest { private final ShutdownModuleFactory factory = new ShutdownModuleFactory(); @Mock - private Bundle mockedSysBundle, mockedConfigManager; + private Bundle mockedSysBundle; @Before @@ -46,11 +45,10 @@ public class ShutdownTest extends AbstractConfigTest { doReturn(mockedSysBundle).when(mockedContext).getBundle(0); mockedContext.getBundle(0); doNothing().when(mockedSysBundle).stop(); - doNothing().when(mockedConfigManager).stop(); doReturn(mockedContext).when(mockedSysBundle).getBundleContext(); - doReturn(new Bundle[]{mockedSysBundle, mockedConfigManager}).when(mockedContext).getBundles(); + doReturn(new Bundle[]{mockedSysBundle}).when(mockedContext).getBundles(); doReturn("system bundle").when(mockedSysBundle).getSymbolicName(); - doReturn(StopSystemBundleThread.CONFIG_MANAGER_SYMBOLIC_NAME).when(mockedConfigManager).getSymbolicName(); + ConfigTransactionJMXClient transaction = configRegistryClient.createTransaction(); @@ -129,7 +127,6 @@ public class ShutdownTest extends AbstractConfigTest { private void assertStopped() throws Exception { Thread.sleep(3000); // happens on another thread - verify(mockedConfigManager).stop(); verify(mockedSysBundle).stop(); } } -- 2.36.6