*/
package org.opendaylight.controller.config.manager.impl;
+import com.google.common.base.Throwables;
import com.google.common.collect.Maps;
import java.util.ArrayList;
import java.util.Collection;
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;
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;
@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.
*/
- @GuardedBy("this")
private final ConfigHolder currentConfig = new ConfigHolder();
/**
* 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<transactionName, transactionController> and purges it each time
* its content is requested.
*/
- @GuardedBy("this")
private final TransactionsHolder transactionsHolder = new TransactionsHolder();
private final BaseJMXRegistrator baseJMXRegistrator;
// 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<ModuleFactory> 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) {
+ MBeanServer configMBeanServer, BindingContextProvider bindingContextProvider) {
this(resolver, configMBeanServer,
- new BaseJMXRegistrator(configMBeanServer), codecRegistry);
+ new BaseJMXRegistrator(configMBeanServer), bindingContextProvider);
}
// constructor
public ConfigRegistryImpl(ModuleFactoriesResolver resolver,
MBeanServer configMBeanServer,
- BaseJMXRegistrator baseJMXRegistrator, CodecRegistry codecRegistry) {
+ BaseJMXRegistrator baseJMXRegistrator, 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
* 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
*/
- public synchronized ObjectName beginConfig(boolean blankTransaction) {
- return beginConfigInternal(blankTransaction).getControllerObjectName();
+ public ObjectName beginConfig(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(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(boolean blankTransaction) {
versionCounter++;
final String transactionName = "ConfigTransaction-" + version + "-" + versionCounter;
// 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()));
readableSRRegistry, txLookupRegistry, allCurrentFactories);
ConfigTransactionControllerInternal transactionController = new ConfigTransactionControllerImpl(
- txLookupRegistry, version, codecRegistry,
+ txLookupRegistry, version, bindingContextProvider,
versionCounter, allCurrentFactories, transactionsMBeanServer,
configMBeanServer, blankTransaction, writableRegistry);
try {
/**
* {@inheritDoc}
+ * @throws ConflictingVersionException
*/
@Override
- public synchronized CommitStatus commitConfig(ObjectName transactionControllerON)
+ public CommitStatus commitConfig(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(ObjectName transactionControllerON)
throws ConflictingVersionException, ValidationException {
final String transactionName = ObjectNameUtil
.getTransactionName(transactionControllerON);
// non recoverable from here:
try {
return secondPhaseCommit(configTransactionController, commitInfo, configTransactionControllerEntry.getValue());
- } catch (Error | RuntimeException t) { // some libs throw Errors: e.g.
+ // some libs throw Errors: e.g.
// javax.xml.ws.spi.FactoryFinder$ConfigurationError
+ } catch (Throwable t) {
isHealthy = false;
LOG.error("Configuration Transaction failed on 2PC, server is unhealthy", t);
- if (t instanceof RuntimeException) {
- throw (RuntimeException) t;
- } else {
- throw (Error) t;
- }
+ throw Throwables.propagate(t);
}
}
+ @GuardedBy("configTransactionLock")
private CommitStatus secondPhaseCommit(ConfigTransactionControllerInternal configTransactionController,
CommitInfo commitInfo, ConfigTransactionLookupRegistry txLookupRegistry) {
// (hopefully) runtime beans
for (DestroyedModule toBeDestroyed : commitInfo
.getDestroyedFromPreviousTransactions()) {
- toBeDestroyed.close(); // closes instance (which should close
+ // 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<ModuleIdentifier, RootRuntimeBeanRegistratorImpl> 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();
- }
// 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
// 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
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);
* {@inheritDoc}
*/
@Override
- public synchronized List<ObjectName> getOpenConfigs() {
+ public List<ObjectName> getOpenConfigs() {
Map<String, Entry<ConfigTransactionControllerInternal, ConfigTransactionLookupRegistry>> transactions = transactionsHolder
.getCurrentTransactions();
List<ObjectName> result = new ArrayList<>(transactions.size());
* here.
*/
@Override
- public synchronized void close() {
+ public void close() {
+ if(!closed.compareAndSet(false, true)) {
+ return;
+ }
+
// abort transactions
Map<String, Entry<ConfigTransactionControllerInternal, ConfigTransactionLookupRegistry>> transactions = transactionsHolder
.getCurrentTransactions();
configTransactionControllerEntry.getValue().close();
configTransactionController.abortConfig();
} catch (RuntimeException e) {
- LOG.warn("Ignoring exception while aborting {}",
- configTransactionController, e);
+ LOG.debug("Ignoring exception while aborting {}", configTransactionController, e);
}
}
- // destroy all live objects one after another in order of the dependency
- // hierarchy
- List<DestroyedModule> destroyedModules = currentConfig
- .getModulesToBeDestroyed();
+ // destroy all live objects one after another in order of the dependency hierarchy, from top to bottom
+ List<DestroyedModule> destroyedModules = currentConfig.getModulesToBeDestroyed();
+
+ LOG.info("ConfigRegistry closing - destroying {} modules", destroyedModules.size());
+
for (DestroyedModule destroyedModule : destroyedModules) {
destroyedModule.close();
}
MBeanServerFactory.releaseMBeanServer(registryMBeanServer);
MBeanServerFactory.releaseMBeanServer(transactionsMBeanServer);
+ LOG.info("ConfigRegistry closed");
}
/**
// service reference functionality:
@Override
- public synchronized ObjectName lookupConfigBeanByServiceInterfaceName(String serviceInterfaceQName, String refName) {
- return readableSRRegistry.lookupConfigBeanByServiceInterfaceName(serviceInterfaceQName, refName);
+ public ObjectName lookupConfigBeanByServiceInterfaceName(String serviceInterfaceQName, String refName) {
+ synchronized(readableSRRegistryLock) {
+ return readableSRRegistry.lookupConfigBeanByServiceInterfaceName(serviceInterfaceQName, refName);
+ }
}
@Override
- public synchronized Map<String, Map<String, ObjectName>> getServiceMapping() {
- return readableSRRegistry.getServiceMapping();
+ public Map<String, Map<String, ObjectName>> getServiceMapping() {
+ synchronized(readableSRRegistryLock) {
+ return readableSRRegistry.getServiceMapping();
+ }
}
@Override
- public synchronized Map<String, ObjectName> lookupServiceReferencesByServiceInterfaceName(String serviceInterfaceQName) {
- return readableSRRegistry.lookupServiceReferencesByServiceInterfaceName(serviceInterfaceQName);
+ public Map<String, ObjectName> lookupServiceReferencesByServiceInterfaceName(String serviceInterfaceQName) {
+ synchronized(readableSRRegistryLock) {
+ return readableSRRegistry.lookupServiceReferencesByServiceInterfaceName(serviceInterfaceQName);
+ }
}
@Override
- public synchronized Set<String> lookupServiceInterfaceNames(ObjectName objectName) throws InstanceNotFoundException {
- return readableSRRegistry.lookupServiceInterfaceNames(objectName);
+ public Set<String> lookupServiceInterfaceNames(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(String namespace, String localName) {
+ synchronized(readableSRRegistryLock) {
+ return readableSRRegistry.getServiceInterfaceName(namespace, localName);
+ }
}
@Override
public void checkServiceReferenceExists(ObjectName objectName) throws InstanceNotFoundException {
- readableSRRegistry.checkServiceReferenceExists(objectName);
+ synchronized(readableSRRegistryLock) {
+ readableSRRegistry.checkServiceReferenceExists(objectName);
+ }
}
@Override
public ObjectName getServiceReference(String serviceInterfaceQName, String refName) throws InstanceNotFoundException {
- return readableSRRegistry.getServiceReference(serviceInterfaceQName, refName);
+ synchronized(readableSRRegistryLock) {
+ return readableSRRegistry.getServiceReference(serviceInterfaceQName, refName);
+ }
}
@Override
/**
* Holds currently running modules
*/
-@NotThreadSafe
class ConfigHolder {
- private final Map<ModuleIdentifier, ModuleInternalInfo> currentConfig = new HashMap<>();
+ private final ConcurrentMap<ModuleIdentifier, ModuleInternalInfo> currentConfig = new ConcurrentHashMap<>();
/**
* Add all modules to the internal map. Also add service instance to OSGi
}
private void add(ModuleInternalInfo configInfo) {
- ModuleInternalInfo oldValue = currentConfig.put(configInfo.getIdentifier(),
- configInfo);
+ ModuleInternalInfo oldValue = currentConfig.putIfAbsent(configInfo.getIdentifier(), configInfo);
if (oldValue != null) {
throw new IllegalStateException(
"Cannot overwrite module with same name:"
* Holds Map<transactionName, transactionController> and purges it each time its
* content is requested.
*/
-@NotThreadSafe
class TransactionsHolder {
/**
* This map keeps transaction names and
* MBeanServer transforms mbeans into another representation. Map is cleaned
* every time current transactions are requested.
*/
- @GuardedBy("ConfigRegistryImpl.this")
- private final Map<String /* transactionName */,
- Entry<ConfigTransactionControllerInternal, ConfigTransactionLookupRegistry>> transactions = new HashMap<>();
+ private final ConcurrentMap<String /* transactionName */,
+ Entry<ConfigTransactionControllerInternal, ConfigTransactionLookupRegistry>> transactions = new ConcurrentHashMap<>();
- /**
- * Can only be called from within synchronized method.
- */
public void add(String transactionName,
ConfigTransactionControllerInternal transactionController, ConfigTransactionLookupRegistry txLookupRegistry) {
- Object oldValue = transactions.put(transactionName,
+ Object oldValue = transactions.putIfAbsent(transactionName,
Maps.immutableEntry(transactionController, txLookupRegistry));
if (oldValue != null) {
throw new IllegalStateException(
}
/**
- * Purges closed transactions from transactions map. Can only be called from
- * within synchronized method. Calling this method more than once within the
+ * 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.