X-Git-Url: https://git.opendaylight.org/gerrit/gitweb?p=controller.git;a=blobdiff_plain;f=opendaylight%2Fconfig%2Fconfig-manager%2Fsrc%2Fmain%2Fjava%2Forg%2Fopendaylight%2Fcontroller%2Fconfig%2Fmanager%2Fimpl%2FConfigRegistryImpl.java;h=41e2b5bf60cca73c3882889c148a118807ae3108;hp=96739fb822c72ac97c97637a07d9d9620f45c81f;hb=f43b01b81319959b1907e3e04537f5169e7f33d8;hpb=d3e4d518cb889fd9464d75c78f0768c258d7c2bc 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 96739fb822..41e2b5bf60 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 @@ -1,5 +1,5 @@ /* - * Copyright (c) 2013 Cisco Systems, Inc. and others. All rights reserved. + * Copyright (c) 2013, 2017 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, @@ -18,8 +18,13 @@ import java.util.List; import java.util.Map; import java.util.Map.Entry; import java.util.Set; +import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.ConcurrentMap; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicBoolean; +import java.util.concurrent.locks.Lock; +import java.util.concurrent.locks.ReentrantLock; import javax.annotation.concurrent.GuardedBy; -import javax.annotation.concurrent.NotThreadSafe; import javax.annotation.concurrent.ThreadSafe; import javax.management.InstanceAlreadyExistsException; import javax.management.InstanceNotFoundException; @@ -33,6 +38,7 @@ import org.opendaylight.controller.config.api.ValidationException; import org.opendaylight.controller.config.api.annotations.ServiceInterfaceAnnotation; import org.opendaylight.controller.config.api.jmx.CommitStatus; import org.opendaylight.controller.config.api.jmx.ObjectNameUtil; +import org.opendaylight.controller.config.manager.impl.ConfigTransactionLookupRegistry.TransactionJMXRegistratorFactory; import org.opendaylight.controller.config.manager.impl.dependencyresolver.DestroyedModule; import org.opendaylight.controller.config.manager.impl.dependencyresolver.ModuleInternalTransactionalInfo; import org.opendaylight.controller.config.manager.impl.dynamicmbean.DynamicReadableWrapper; @@ -41,14 +47,13 @@ import org.opendaylight.controller.config.manager.impl.factoriesresolver.ModuleF import org.opendaylight.controller.config.manager.impl.jmx.BaseJMXRegistrator; 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.jmx.TransactionJMXRegistrator; import org.opendaylight.controller.config.manager.impl.osgi.BeanToOsgiServiceManager; import org.opendaylight.controller.config.manager.impl.osgi.BeanToOsgiServiceManager.OsgiRegistration; +import org.opendaylight.controller.config.manager.impl.osgi.mapping.BindingContextProvider; import org.opendaylight.controller.config.manager.impl.util.LookupBeansUtil; import org.opendaylight.controller.config.manager.impl.util.ModuleQNameUtil; import org.opendaylight.controller.config.spi.Module; import org.opendaylight.controller.config.spi.ModuleFactory; -import org.opendaylight.yangtools.yang.data.impl.codec.CodecRegistry; import org.osgi.framework.BundleContext; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -60,37 +65,33 @@ import org.slf4j.LoggerFactory; @ThreadSafe public class ConfigRegistryImpl implements AutoCloseable, ConfigRegistryImplMXBean { private static final Logger LOG = LoggerFactory.getLogger(ConfigRegistryImpl.class); + private static final ObjectName NOOP_TX_NAME = ObjectNameUtil.createTransactionControllerON("noop"); private final ModuleFactoriesResolver resolver; private final MBeanServer configMBeanServer; - private final CodecRegistry codecRegistry; + private final BindingContextProvider bindingContextProvider; - @GuardedBy("this") - private long version = 0; - @GuardedBy("this") - private long versionCounter = 0; + private volatile long version = 0; + private volatile long versionCounter = 0; /** - * Contains current configuration in form of {moduleName:{instanceName,read - * only module}} for copying state to new transaction. Each running module - * is present just once, no matter how many interfaces it exposes. + * Contains current configuration in form of {moduleName:{instanceName,read only + * module}} for copying state to new transaction. Each running module is present + * just once, no matter how many interfaces it exposes. */ - @GuardedBy("this") private final ConfigHolder currentConfig = new ConfigHolder(); /** * Will return true unless there was a transaction that succeeded during - * validation but failed in second phase of commit. In this case the server - * is unstable and its state is undefined. + * validation but failed in second phase of commit. In this case the server is + * unstable and its state is undefined. */ - @GuardedBy("this") - private boolean isHealthy = true; + private volatile boolean isHealthy = true; /** - * Holds Map and purges it each time - * its content is requested. + * Holds the map of transactions and purges it each time its content is + * requested. */ - @GuardedBy("this") private final TransactionsHolder transactionsHolder = new TransactionsHolder(); private final BaseJMXRegistrator baseJMXRegistrator; @@ -102,29 +103,35 @@ public class ConfigRegistryImpl implements AutoCloseable, ConfigRegistryImplMXBe // internal jmx server shared by all transactions private final MBeanServer transactionsMBeanServer; + private final AtomicBoolean closed = new AtomicBoolean(); + + private final Object readableSRRegistryLock = new Object(); + + private final Lock configTransactionLock = new ReentrantLock(); + // Used for finding new factory instances for default module functionality - @GuardedBy("this") + @GuardedBy("configTransactionLock") private List lastListOfFactories = Collections.emptyList(); - @GuardedBy("this") // switched in every 2ndPC - private CloseableServiceReferenceReadableRegistry readableSRRegistry = ServiceReferenceRegistryImpl.createInitialSRLookupRegistry(); + // switched in every 2ndPC + @GuardedBy("readableSRRegistryLock") + private CloseableServiceReferenceReadableRegistry readableSRRegistry = ServiceReferenceRegistryImpl + .createInitialSRLookupRegistry(); // constructor - public ConfigRegistryImpl(ModuleFactoriesResolver resolver, - MBeanServer configMBeanServer, CodecRegistry codecRegistry) { - this(resolver, configMBeanServer, - new BaseJMXRegistrator(configMBeanServer), codecRegistry); + public ConfigRegistryImpl(final ModuleFactoriesResolver resolver, final MBeanServer configMBeanServer, + final BindingContextProvider bindingContextProvider) { + this(resolver, configMBeanServer, new BaseJMXRegistrator(configMBeanServer), bindingContextProvider); } // constructor - public ConfigRegistryImpl(ModuleFactoriesResolver resolver, - MBeanServer configMBeanServer, - BaseJMXRegistrator baseJMXRegistrator, CodecRegistry codecRegistry) { + public ConfigRegistryImpl(final ModuleFactoriesResolver resolver, final MBeanServer configMBeanServer, + final BaseJMXRegistrator baseJMXRegistrator, final BindingContextProvider bindingContextProvider) { this.resolver = resolver; this.beanToOsgiServiceManager = new BeanToOsgiServiceManager(); this.configMBeanServer = configMBeanServer; this.baseJMXRegistrator = baseJMXRegistrator; - this.codecRegistry = codecRegistry; + this.bindingContextProvider = bindingContextProvider; this.registryMBeanServer = MBeanServerFactory .createMBeanServer("ConfigRegistry" + configMBeanServer.getDefaultDomain()); this.transactionsMBeanServer = MBeanServerFactory @@ -132,31 +139,75 @@ public class ConfigRegistryImpl implements AutoCloseable, ConfigRegistryImplMXBe } /** - * Create new {@link ConfigTransactionControllerImpl } + * Create new {@link ConfigTransactionControllerImpl }. */ @Override - public synchronized ObjectName beginConfig() { + public ObjectName beginConfig() { return beginConfig(false); } /** - * @param blankTransaction true if this transaction is created automatically by - * org.opendaylight.controller.config.manager.impl.osgi.BlankTransactionServiceTracker + * It returns true if this transaction is created automatically by + * org.opendaylight.controller.config.manager.impl.osgi.BlankTransactionServiceTracker. + * + * @param blankTransaction + * true if this transaction is created automatically by + * org.opendaylight.controller.config.manager.impl.osgi.BlankTransactionServiceTracker + * + * @return object name */ - public synchronized ObjectName beginConfig(boolean blankTransaction) { - return beginConfigInternal(blankTransaction).getControllerObjectName(); + public ObjectName beginConfig(final boolean blankTransaction) { + // If we're closed or in the process of closing then all modules are destroyed + // or being destroyed + // so there's no point in trying to acquire the lock and beginning an actual + // transaction. Also we want + // to avoid trying to lock as it may block the shutdown process if there is an + // outstanding transaction + // attempting to be committed. + // + // We could throw an exception here to indicate this but that's not part of the + // original API contract + // and callers may not be expecting an unchecked exception. Therefore we return + // a special transaction + // handle that signifies a "no-op". + if (closed.get()) { + return NOOP_TX_NAME; + } + + if (blankTransaction) { + try { + // For a "blank" transaction, we'll try to obtain the config lock but "blank" + // transactions + // are initiated via OSGi events so we don't want to block indefinitely or for a + // long period + // of time. + if (!configTransactionLock.tryLock(5, TimeUnit.SECONDS)) { + LOG.debug("Timed out trying to obtain configTransactionLock"); + return NOOP_TX_NAME; + } + } catch (final InterruptedException e) { + LOG.debug("Interrupted trying to obtain configTransactionLock", e); + Thread.currentThread().interrupt(); + return NOOP_TX_NAME; + } + } else { + configTransactionLock.lock(); + } + + try { + return beginConfigSafe(blankTransaction).getControllerObjectName(); + } finally { + configTransactionLock.unlock(); + } } - private synchronized ConfigTransactionControllerInternal beginConfigInternal(boolean blankTransaction) { + @GuardedBy("configTransactionLock") + private ConfigTransactionControllerInternal beginConfigSafe(final boolean blankTransaction) { versionCounter++; final String transactionName = "ConfigTransaction-" + version + "-" + versionCounter; - TransactionJMXRegistratorFactory factory = new TransactionJMXRegistratorFactory() { - @Override - public TransactionJMXRegistrator create() { - return baseJMXRegistrator.createTransactionJMXRegistrator(transactionName); - } - }; + TransactionJMXRegistratorFactory factory = () -> baseJMXRegistrator + .createTransactionJMXRegistrator(transactionName); Map> allCurrentFactories = new HashMap<>( resolver.getAllFactories()); @@ -164,27 +215,26 @@ public class ConfigRegistryImpl implements AutoCloseable, ConfigRegistryImplMXBe // 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) { + if (!allCurrentFactories.containsKey(name)) { LOG.trace("Factory {} not found in SR, using reference from previous commit", name); - allCurrentFactories.put(name, - Maps.immutableEntry(moduleInternalInfo.getModuleFactory(), moduleInternalInfo.getBundleContext())); + 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); - SearchableServiceReferenceWritableRegistry writableRegistry = ServiceReferenceRegistryImpl.createSRWritableRegistry( - readableSRRegistry, txLookupRegistry, allCurrentFactories); + ConfigTransactionLookupRegistry txLookupRegistry = new ConfigTransactionLookupRegistry( + new TransactionIdentifier(transactionName), factory, allCurrentFactories); + SearchableServiceReferenceWritableRegistry writableRegistry = ServiceReferenceRegistryImpl + .createSRWritableRegistry(readableSRRegistry, txLookupRegistry, allCurrentFactories); ConfigTransactionControllerInternal transactionController = new ConfigTransactionControllerImpl( - txLookupRegistry, version, codecRegistry, - versionCounter, allCurrentFactories, transactionsMBeanServer, - configMBeanServer, blankTransaction, writableRegistry); + txLookupRegistry, version, bindingContextProvider, versionCounter, allCurrentFactories, + transactionsMBeanServer, configMBeanServer, blankTransaction, writableRegistry); try { txLookupRegistry.registerMBean(transactionController, transactionController.getControllerObjectName()); - } catch (InstanceAlreadyExistsException e) { + } catch (final InstanceAlreadyExistsException e) { throw new IllegalStateException(e); } transactionController.copyExistingModulesAndProcessFactoryDiff(currentConfig.getEntries(), lastListOfFactories); @@ -192,88 +242,95 @@ public class ConfigRegistryImpl implements AutoCloseable, ConfigRegistryImplMXBe return transactionController; } - /** - * {@inheritDoc} - */ @Override - public synchronized CommitStatus commitConfig(ObjectName transactionControllerON) + public CommitStatus commitConfig(final ObjectName transactionControllerON) + throws ValidationException, ConflictingVersionException { + if (NOOP_TX_NAME.equals(transactionControllerON) || closed.get()) { + return new CommitStatus(Collections.emptyList(), Collections.emptyList(), Collections.emptyList()); + } + + configTransactionLock.lock(); + try { + return commitConfigSafe(transactionControllerON); + } finally { + configTransactionLock.unlock(); + } + } + + @GuardedBy("configTransactionLock") + private CommitStatus commitConfigSafe(final ObjectName transactionControllerON) throws ConflictingVersionException, ValidationException { - final String transactionName = ObjectNameUtil - .getTransactionName(transactionControllerON); - LOG.trace("About to commit {}. Current parentVersion: {}, versionCounter {}", transactionName, version, versionCounter); + final String transactionName = ObjectNameUtil.getTransactionName(transactionControllerON); + LOG.trace("About to commit {}. Current parentVersion: {}, versionCounter {}", transactionName, version, + versionCounter); // find ConfigTransactionController - Map> transactions = transactionsHolder.getCurrentTransactions(); - Entry configTransactionControllerEntry = transactions.get(transactionName); + Map> transactions = + transactionsHolder.getCurrentTransactions(); + Entry configTransactionControllerEntry = + transactions.get(transactionName); if (configTransactionControllerEntry == null) { - throw new IllegalArgumentException(String.format( - "Transaction with name '%s' not found", transactionName)); + throw new IllegalArgumentException(String.format("Transaction with name '%s' not found", transactionName)); } ConfigTransactionControllerInternal configTransactionController = configTransactionControllerEntry.getKey(); // check optimistic lock if (version != configTransactionController.getParentVersion()) { throw new ConflictingVersionException( - String.format( - "Optimistic lock failed. Expected parent version %d, was %d", - version, + String.format("Optimistic lock failed. Expected parent version %d, was %d", version, configTransactionController.getParentVersion())); } // optimistic lock ok CommitInfo commitInfo = configTransactionController.validateBeforeCommitAndLockTransaction(); - lastListOfFactories = Collections.unmodifiableList(configTransactionController.getCurrentlyRegisteredFactories()); - // non recoverable from here: - try { - return secondPhaseCommit(configTransactionController, commitInfo, configTransactionControllerEntry.getValue()); - } catch (Error | RuntimeException t) { // some libs throw Errors: e.g. - // javax.xml.ws.spi.FactoryFinder$ConfigurationError - isHealthy = false; - LOG.error("Configuration Transaction failed on 2PC, server is unhealthy", t); - if (t instanceof RuntimeException) { - throw (RuntimeException) t; - } else { - throw (Error) t; - } - } + lastListOfFactories = Collections + .unmodifiableList(configTransactionController.getCurrentlyRegisteredFactories()); + return secondPhaseCommit(configTransactionController, commitInfo, configTransactionControllerEntry.getValue()); } - private CommitStatus secondPhaseCommit(ConfigTransactionControllerInternal configTransactionController, - CommitInfo commitInfo, ConfigTransactionLookupRegistry txLookupRegistry) { + @GuardedBy("configTransactionLock") + private CommitStatus secondPhaseCommit(final ConfigTransactionControllerInternal configTransactionController, + final CommitInfo commitInfo, final ConfigTransactionLookupRegistry txLookupRegistry) { // close instances which were destroyed by the user, including // (hopefully) runtime beans - for (DestroyedModule toBeDestroyed : commitInfo - .getDestroyedFromPreviousTransactions()) { - toBeDestroyed.close(); // closes instance (which should close + for (DestroyedModule toBeDestroyed : commitInfo.getDestroyedFromPreviousTransactions()) { + // closes instance (which should close // runtime jmx registrator), // also closes osgi registration and ModuleJMXRegistrator // registration + toBeDestroyed.close(); currentConfig.remove(toBeDestroyed.getIdentifier()); } // 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()) { - RootRuntimeBeanRegistratorImpl runtimeBeanRegistrator; - if (entry.hasOldModule() == false) { - runtimeBeanRegistrator = baseJMXRegistrator - .createRuntimeBeanRegistrator(entry.getIdentifier()); - } else { - // reuse old JMX registrator - runtimeBeanRegistrator = entry.getOldInternalInfo() - .getRuntimeBeanRegistrator(); - } + for (ModuleInternalTransactionalInfo entry : commitInfo.getCommitted().values()) { // 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 @@ -291,15 +348,12 @@ public class ConfigRegistryImpl implements AutoCloseable, ConfigRegistryImplMXBe int orderingIdx = 0; for (ModuleIdentifier moduleIdentifier : orderedModuleIdentifiers) { LOG.trace("Registering {}", moduleIdentifier); - ModuleInternalTransactionalInfo entry = commitInfo.getCommitted() - .get(moduleIdentifier); + ModuleInternalTransactionalInfo entry = commitInfo.getCommitted().get(moduleIdentifier); if (entry == null) { - throw new NullPointerException("Module not found " - + moduleIdentifier); + throw new NullPointerException("Module not found " + moduleIdentifier); } - ObjectName primaryReadOnlyON = ObjectNameUtil - .createReadOnlyModuleON(moduleIdentifier); + ObjectName primaryReadOnlyON = ObjectNameUtil.createReadOnlyModuleON(moduleIdentifier); // determine if current instance was recreated or reused or is new @@ -310,8 +364,7 @@ public class ConfigRegistryImpl implements AutoCloseable, ConfigRegistryImplMXBe // runtime jmx registration - should be taken care of by module // itself // instance - is closed only if it was destroyed - ModuleJMXRegistrator newModuleJMXRegistrator = baseJMXRegistrator - .createModuleJMXRegistrator(); + ModuleJMXRegistrator newModuleJMXRegistrator = baseJMXRegistrator.createModuleJMXRegistrator(); OsgiRegistration osgiRegistration = null; AutoCloseable instance = entry.getProxiedModule().getInstance(); @@ -338,6 +391,14 @@ public class ConfigRegistryImpl implements AutoCloseable, ConfigRegistryImplMXBe // close old module jmx registrator oldInternalInfo.getModuleJMXRegistrator().close(); + + // We no longer need old internal info. Clear it out, so we do not create a + // serial leak evidenced + // by BUG-4514. The reason is that modules retain their resolver, which retains + // modules. If we retain + // the old module, we would have the complete reconfiguration history held in + // heap for no good reason. + entry.clearOldInternalInfo(); } else { // new instance: // wrap in readable dynamic mbean @@ -345,34 +406,32 @@ public class ConfigRegistryImpl implements AutoCloseable, ConfigRegistryImplMXBe } Module realModule = entry.getRealModule(); - DynamicReadableWrapper newReadableConfigBean = new DynamicReadableWrapper( - realModule, instance, moduleIdentifier, - registryMBeanServer, configMBeanServer); + DynamicReadableWrapper newReadableConfigBean = new DynamicReadableWrapper(realModule, instance, + moduleIdentifier, registryMBeanServer, configMBeanServer); // register to JMX try { newModuleJMXRegistrator.registerMBean(newReadableConfigBean, primaryReadOnlyON); - } catch (InstanceAlreadyExistsException e) { - throw new IllegalStateException("Possible code error, already registered:" + primaryReadOnlyON,e); + } catch (final InstanceAlreadyExistsException e) { + throw new IllegalStateException("Possible code error, already registered:" + primaryReadOnlyON, e); } // register services to OSGi - Map annotationMapping = configTransactionController.getWritableRegistry().findServiceInterfaces(moduleIdentifier); - BundleContext bc = configTransactionController.getModuleFactoryBundleContext( - entry.getModuleFactory().getImplementationName()); + Map annotationMapping = configTransactionController + .getWritableRegistry().findServiceInterfaces(moduleIdentifier); + BundleContext bc = configTransactionController + .getModuleFactoryBundleContext(entry.getModuleFactory().getImplementationName()); if (osgiRegistration == null) { - osgiRegistration = beanToOsgiServiceManager.registerToOsgi( - newReadableConfigBean.getInstance(), moduleIdentifier, bc, annotationMapping); + osgiRegistration = beanToOsgiServiceManager.registerToOsgi(newReadableConfigBean.getInstance(), + moduleIdentifier, bc, annotationMapping); } else { osgiRegistration.updateRegistrations(annotationMapping, bc, instance); } - RootRuntimeBeanRegistratorImpl runtimeBeanRegistrator = runtimeRegistrators - .get(entry.getIdentifier()); - ModuleInternalInfo newInfo = new ModuleInternalInfo( - entry.getIdentifier(), newReadableConfigBean, osgiRegistration, - runtimeBeanRegistrator, newModuleJMXRegistrator, - orderingIdx, entry.isDefaultBean(), entry.getModuleFactory(), entry.getBundleContext()); + RootRuntimeBeanRegistratorImpl runtimeBeanRegistrator = runtimeRegistrators.get(entry.getIdentifier()); + ModuleInternalInfo newInfo = new ModuleInternalInfo(entry.getIdentifier(), newReadableConfigBean, + osgiRegistration, runtimeBeanRegistrator, newModuleJMXRegistrator, orderingIdx, + entry.isDefaultBean(), entry.getModuleFactory(), entry.getBundleContext()); newConfigEntries.put(realModule, newInfo); orderingIdx++; @@ -383,57 +442,58 @@ public class ConfigRegistryImpl implements AutoCloseable, ConfigRegistryImplMXBe version = configTransactionController.getVersion(); // switch readable Service Reference Registry - this.readableSRRegistry.close(); - this.readableSRRegistry = ServiceReferenceRegistryImpl.createSRReadableRegistry( - configTransactionController.getWritableRegistry(), this, baseJMXRegistrator); + synchronized (readableSRRegistryLock) { + readableSRRegistry.close(); + readableSRRegistry = ServiceReferenceRegistryImpl.createSRReadableRegistry( + configTransactionController.getWritableRegistry(), this, baseJMXRegistrator); + } - return new CommitStatus(newInstances, reusedInstances, - recreatedInstances); + return new CommitStatus(newInstances, reusedInstances, recreatedInstances); } /** * {@inheritDoc} */ @Override - public synchronized List getOpenConfigs() { - Map> transactions = transactionsHolder - .getCurrentTransactions(); + public List getOpenConfigs() { + Map> transactions = + transactionsHolder.getCurrentTransactions(); List result = new ArrayList<>(transactions.size()); - for (Entry configTransactionControllerEntry : transactions - .values()) { + for (Entry + configTransactionControllerEntry : transactions.values()) { result.add(configTransactionControllerEntry.getKey().getControllerObjectName()); } return result; } /** - * Abort open transactions and unregister read only modules. Since this - * class is not responsible for registering itself under - * {@link org.opendaylight.controller.config.api.ConfigRegistry#OBJECT_NAME}, it will not unregister itself - * here. + * Abort open transactions and unregister read only modules. Since this class is + * not responsible for registering itself under + * {@link org.opendaylight.controller.config.api.ConfigRegistry#OBJECT_NAME}, it + * will not unregister itself here. */ @Override - public synchronized void close() { - // abort transactions - Map> transactions = transactionsHolder - .getCurrentTransactions(); - for (Entry configTransactionControllerEntry : transactions - .values()) { + public void close() { + if (!closed.compareAndSet(false, true)) { + return; + } + // abort transactions + Map> transactions = + transactionsHolder.getCurrentTransactions(); + for (Entry + configTransactionControllerEntry : transactions.values()) { ConfigTransactionControllerInternal configTransactionController = configTransactionControllerEntry.getKey(); - try { - configTransactionControllerEntry.getValue().close(); - configTransactionController.abortConfig(); - } catch (RuntimeException e) { - LOG.warn("Ignoring exception while aborting {}", - configTransactionController, e); - } + configTransactionControllerEntry.getValue().close(); + configTransactionController.abortConfig(); } // destroy all live objects one after another in order of the dependency - // hierarchy - List destroyedModules = currentConfig - .getModulesToBeDestroyed(); + // hierarchy, from top to bottom + List destroyedModules = currentConfig.getModulesToBeDestroyed(); + + LOG.info("ConfigRegistry closing - destroying {} modules", destroyedModules.size()); + for (DestroyedModule destroyedModule : destroyedModules) { destroyedModule.close(); } @@ -443,6 +503,7 @@ public class ConfigRegistryImpl implements AutoCloseable, ConfigRegistryImplMXBe MBeanServerFactory.releaseMBeanServer(registryMBeanServer); MBeanServerFactory.releaseMBeanServer(transactionsMBeanServer); + LOG.info("ConfigRegistry closed"); } /** @@ -458,8 +519,7 @@ public class ConfigRegistryImpl implements AutoCloseable, ConfigRegistryImplMXBe */ @Override public Set getAvailableModuleNames() { - return new HierarchicalConfigMBeanFactoriesHolder( - resolver.getAllFactories()).getModuleNames(); + return new HierarchicalConfigMBeanFactoriesHolder(resolver.getAllFactories()).getModuleNames(); } /** @@ -484,7 +544,7 @@ public class ConfigRegistryImpl implements AutoCloseable, ConfigRegistryImplMXBe * {@inheritDoc} */ @Override - public Set lookupConfigBeans(String moduleName) { + public Set lookupConfigBeans(final String moduleName) { return lookupConfigBeans(moduleName, "*"); } @@ -492,20 +552,18 @@ public class ConfigRegistryImpl implements AutoCloseable, ConfigRegistryImplMXBe * {@inheritDoc} */ @Override - public ObjectName lookupConfigBean(String moduleName, String instanceName) - throws InstanceNotFoundException { - return LookupBeansUtil.lookupConfigBean(this, moduleName, instanceName); + public Set lookupConfigBeans(final String moduleName, final String instanceName) { + ObjectName namePattern = ObjectNameUtil.createModulePattern(moduleName, instanceName); + return baseJMXRegistrator.queryNames(namePattern, null); } /** * {@inheritDoc} */ @Override - public Set lookupConfigBeans(String moduleName, - String instanceName) { - ObjectName namePattern = ObjectNameUtil.createModulePattern(moduleName, - instanceName); - return baseJMXRegistrator.queryNames(namePattern, null); + public ObjectName lookupConfigBean(final String moduleName, final String instanceName) + throws InstanceNotFoundException { + return LookupBeansUtil.lookupConfigBean(this, moduleName, instanceName); } /** @@ -520,61 +578,76 @@ public class ConfigRegistryImpl implements AutoCloseable, ConfigRegistryImplMXBe * {@inheritDoc} */ @Override - public Set lookupRuntimeBeans(String moduleName, - String instanceName) { + public Set lookupRuntimeBeans(final String moduleName, final String instanceName) { String finalModuleName = moduleName == null ? "*" : moduleName; String finalInstanceName = instanceName == null ? "*" : instanceName; - ObjectName namePattern = ObjectNameUtil.createRuntimeBeanPattern( - finalModuleName, finalInstanceName); + ObjectName namePattern = ObjectNameUtil.createRuntimeBeanPattern(finalModuleName, finalInstanceName); return baseJMXRegistrator.queryNames(namePattern, null); } @Override - public void checkConfigBeanExists(ObjectName objectName) throws InstanceNotFoundException { + public void checkConfigBeanExists(final ObjectName objectName) throws InstanceNotFoundException { ObjectNameUtil.checkDomain(objectName); ObjectNameUtil.checkType(objectName, ObjectNameUtil.TYPE_MODULE); String transactionName = ObjectNameUtil.getTransactionName(objectName); if (transactionName != null) { - throw new IllegalArgumentException("Transaction attribute not supported in registry, wrong ObjectName: " + objectName); + throw new IllegalArgumentException( + "Transaction attribute not supported in registry, wrong ObjectName: " + objectName); } // make sure exactly one match is found: - LookupBeansUtil.lookupConfigBean(this, ObjectNameUtil.getFactoryName(objectName), ObjectNameUtil.getInstanceName(objectName)); + LookupBeansUtil.lookupConfigBean(this, ObjectNameUtil.getFactoryName(objectName), + ObjectNameUtil.getInstanceName(objectName)); } // service reference functionality: @Override - public synchronized ObjectName lookupConfigBeanByServiceInterfaceName(String serviceInterfaceQName, String refName) { - return readableSRRegistry.lookupConfigBeanByServiceInterfaceName(serviceInterfaceQName, refName); + public ObjectName lookupConfigBeanByServiceInterfaceName(final String serviceInterfaceQName, final String refName) { + synchronized (readableSRRegistryLock) { + return readableSRRegistry.lookupConfigBeanByServiceInterfaceName(serviceInterfaceQName, refName); + } } @Override - public synchronized Map> getServiceMapping() { - return readableSRRegistry.getServiceMapping(); + public Map> getServiceMapping() { + synchronized (readableSRRegistryLock) { + return readableSRRegistry.getServiceMapping(); + } } @Override - public synchronized Map lookupServiceReferencesByServiceInterfaceName(String serviceInterfaceQName) { - return readableSRRegistry.lookupServiceReferencesByServiceInterfaceName(serviceInterfaceQName); + public Map lookupServiceReferencesByServiceInterfaceName(final String serviceInterfaceQName) { + synchronized (readableSRRegistryLock) { + return readableSRRegistry.lookupServiceReferencesByServiceInterfaceName(serviceInterfaceQName); + } } @Override - public synchronized Set lookupServiceInterfaceNames(ObjectName objectName) throws InstanceNotFoundException { - return readableSRRegistry.lookupServiceInterfaceNames(objectName); + public Set lookupServiceInterfaceNames(final ObjectName objectName) throws InstanceNotFoundException { + synchronized (readableSRRegistryLock) { + return readableSRRegistry.lookupServiceInterfaceNames(objectName); + } } @Override - public synchronized String getServiceInterfaceName(String namespace, String localName) { - return readableSRRegistry.getServiceInterfaceName(namespace, localName); + public String getServiceInterfaceName(final String namespace, final String localName) { + synchronized (readableSRRegistryLock) { + return readableSRRegistry.getServiceInterfaceName(namespace, localName); + } } @Override - public void checkServiceReferenceExists(ObjectName objectName) throws InstanceNotFoundException { - readableSRRegistry.checkServiceReferenceExists(objectName); + public void checkServiceReferenceExists(final ObjectName objectName) throws InstanceNotFoundException { + synchronized (readableSRRegistryLock) { + readableSRRegistry.checkServiceReferenceExists(objectName); + } } @Override - public ObjectName getServiceReference(String serviceInterfaceQName, String refName) throws InstanceNotFoundException { - return readableSRRegistry.getServiceReference(serviceInterfaceQName, refName); + public ObjectName getServiceReference(final String serviceInterfaceQName, final String refName) + throws InstanceNotFoundException { + synchronized (readableSRRegistryLock) { + return readableSRRegistry.getServiceReference(serviceInterfaceQName, refName); + } } @Override @@ -584,118 +657,103 @@ public class ConfigRegistryImpl implements AutoCloseable, ConfigRegistryImplMXBe @Override public String toString() { - return "ConfigRegistryImpl{" + - "versionCounter=" + versionCounter + - ", version=" + version + - '}'; + return "ConfigRegistryImpl{" + "versionCounter=" + versionCounter + ", version=" + version + '}'; } -} - -/** - * Holds currently running modules - */ -@NotThreadSafe -class ConfigHolder { - private final Map currentConfig = new HashMap<>(); /** - * Add all modules to the internal map. Also add service instance to OSGi - * Service Registry. + * Inner class holding transactions and purges it each time its content is + * requested. */ - public void addAll(Collection configInfos) { - if (!currentConfig.isEmpty()) { - throw new IllegalStateException( - "Error - some config entries were not removed: " - + currentConfig); - } - for (ModuleInternalInfo configInfo : configInfos) { - add(configInfo); + class TransactionsHolder { + /** + * This map keeps transaction names and + * {@link ConfigTransactionControllerInternal} instances, because platform + * MBeanServer transforms mbeans into another representation. Map is cleaned + * every time current transactions are requested. + */ + private final ConcurrentMap> transactions = new ConcurrentHashMap<>(); + + public void add(final String transactionName, final ConfigTransactionControllerInternal transactionController, + final ConfigTransactionLookupRegistry txLookupRegistry) { + Object oldValue = transactions.putIfAbsent(transactionName, + Maps.immutableEntry(transactionController, txLookupRegistry)); + if (oldValue != null) { + throw new IllegalStateException("Error: two transactions with same name"); + } } - } - private void add(ModuleInternalInfo configInfo) { - ModuleInternalInfo oldValue = currentConfig.put(configInfo.getIdentifier(), - configInfo); - if (oldValue != null) { - throw new IllegalStateException( - "Cannot overwrite module with same name:" - + configInfo.getIdentifier() + ":" + configInfo); + /** + * Purges closed transactions from transactions map. Calling this method more + * than once within the method can modify the resulting map that was obtained in + * previous calls. + * + * @return current view on transactions map. + */ + public Map> getCurrentTransactions() { + // first, remove closed transaction + for (Iterator>> + it = transactions.entrySet().iterator(); it.hasNext();) { + Entry> entry = it + .next(); + if (entry.getValue().getKey().isClosed()) { + it.remove(); + } + } + return Collections.unmodifiableMap(transactions); } } /** - * Remove entry from current config. + * Inner class that holds currently running modules. */ - public void remove(ModuleIdentifier name) { - ModuleInternalInfo removed = currentConfig.remove(name); - if (removed == null) { - throw new IllegalStateException( - "Cannot remove from ConfigHolder - name not found:" + name); + class ConfigHolder { + private final ConcurrentMap currentConfig = new ConcurrentHashMap<>(); + + /** + * Add all modules to the internal map. Also add service instance to OSGi + * Service Registry. + */ + public void addAll(final Collection configInfos) { + if (!currentConfig.isEmpty()) { + throw new IllegalStateException("Error - some config entries were not removed: " + currentConfig); + } + for (ModuleInternalInfo configInfo : configInfos) { + add(configInfo); + } } - } - - public Collection getEntries() { - return currentConfig.values(); - } - public List getModulesToBeDestroyed() { - List result = new ArrayList<>(); - for (ModuleInternalInfo moduleInternalInfo : getEntries()) { - result.add(moduleInternalInfo.toDestroyedModule()); + private void add(final ModuleInternalInfo configInfo) { + ModuleInternalInfo oldValue = currentConfig.putIfAbsent(configInfo.getIdentifier(), configInfo); + if (oldValue != null) { + throw new IllegalStateException( + "Cannot overwrite module with same name:" + configInfo.getIdentifier() + ":" + configInfo); + } } - Collections.sort(result); - return result; - } - -} - -/** - * Holds Map and purges it each time its - * content is requested. - */ -@NotThreadSafe -class TransactionsHolder { - /** - * This map keeps transaction names and - * {@link ConfigTransactionControllerInternal} instances, because platform - * MBeanServer transforms mbeans into another representation. Map is cleaned - * every time current transactions are requested. - */ - @GuardedBy("ConfigRegistryImpl.this") - private final Map> transactions = new HashMap<>(); + /** + * Remove entry from current config. + */ + public void remove(final ModuleIdentifier name) { + ModuleInternalInfo removed = currentConfig.remove(name); + if (removed == null) { + throw new IllegalStateException("Cannot remove from ConfigHolder - name not found:" + name); + } + } - /** - * Can only be called from within synchronized method. - */ - public void add(String transactionName, - ConfigTransactionControllerInternal transactionController, ConfigTransactionLookupRegistry txLookupRegistry) { - Object oldValue = transactions.put(transactionName, - Maps.immutableEntry(transactionController, txLookupRegistry)); - if (oldValue != null) { - throw new IllegalStateException( - "Error: two transactions with same name"); + public Collection getEntries() { + return currentConfig.values(); } - } - /** - * Purges closed transactions from transactions map. Can only be called from - * within synchronized method. Calling this method more than once within the - * method can modify the resulting map that was obtained in previous calls. - * - * @return current view on transactions map. - */ - public Map> getCurrentTransactions() { - // first, remove closed transaction - for (Iterator>> it = transactions - .entrySet().iterator(); it.hasNext(); ) { - Entry> entry = it - .next(); - if (entry.getValue().getKey().isClosed()) { - it.remove(); + public List getModulesToBeDestroyed() { + List result = new ArrayList<>(); + for (ModuleInternalInfo moduleInternalInfo : getEntries()) { + result.add(moduleInternalInfo.toDestroyedModule()); } + Collections.sort(result); + return result; } - return Collections.unmodifiableMap(transactions); } }