String performance and maintenability
[controller.git] / opendaylight / config / config-manager / src / main / java / org / opendaylight / controller / config / manager / impl / ConfigTransactionControllerImpl.java
index 39eef8741ba840309b8293e19f14af437652e82e..d5d82dd1cb581563c11104035b07f9760f87182c 100644 (file)
@@ -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,
@@ -7,9 +7,8 @@
  */
 package org.opendaylight.controller.config.manager.impl;
 
-import static com.google.common.base.Preconditions.checkNotNull;
-import static java.lang.String.format;
-
+import com.google.common.base.Preconditions;
+import com.google.common.collect.Lists;
 import java.util.ArrayList;
 import java.util.Collection;
 import java.util.HashSet;
@@ -26,8 +25,10 @@ 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.ModuleFactoryNotFoundException;
 import org.opendaylight.controller.config.api.ModuleIdentifier;
 import org.opendaylight.controller.config.api.ValidationException;
+import org.opendaylight.controller.config.api.annotations.ServiceInterfaceAnnotation;
 import org.opendaylight.controller.config.api.jmx.ObjectNameUtil;
 import org.opendaylight.controller.config.manager.impl.dependencyresolver.DependencyResolverManager;
 import org.opendaylight.controller.config.manager.impl.dependencyresolver.ModuleInternalTransactionalInfo;
@@ -37,28 +38,29 @@ import org.opendaylight.controller.config.manager.impl.dynamicmbean.ReadOnlyAtom
 import org.opendaylight.controller.config.manager.impl.factoriesresolver.HierarchicalConfigMBeanFactoriesHolder;
 import org.opendaylight.controller.config.manager.impl.jmx.TransactionModuleJMXRegistrator;
 import org.opendaylight.controller.config.manager.impl.jmx.TransactionModuleJMXRegistrator.TransactionModuleJMXRegistration;
+import org.opendaylight.controller.config.manager.impl.osgi.mapping.BindingContextProvider;
+import org.opendaylight.controller.config.manager.impl.util.InterfacesHelper;
+import org.opendaylight.controller.config.spi.AbstractModule;
 import org.opendaylight.controller.config.spi.Module;
 import org.opendaylight.controller.config.spi.ModuleFactory;
 import org.opendaylight.yangtools.concepts.Identifiable;
-import org.opendaylight.yangtools.yang.data.impl.codec.CodecRegistry;
 import org.osgi.framework.BundleContext;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
+
 /**
- * This is a JMX bean representing current transaction. It contains
- * transaction identifier, unique version and parent version for
- * optimistic locking.
+ * This is a JMX bean representing current transaction. It contains transaction
+ * identifier, unique version and parent version for optimistic locking.
  */
-class ConfigTransactionControllerImpl implements
-        ConfigTransactionControllerInternal,
-        ConfigTransactionControllerImplMXBean,
-        Identifiable<TransactionIdentifier> {
-    private static final Logger logger = LoggerFactory.getLogger(ConfigTransactionControllerImpl.class);
+public class ConfigTransactionControllerImpl implements ConfigTransactionControllerInternal,
+        ConfigTransactionControllerImplMXBean, Identifiable<TransactionIdentifier> {
+    private static final Logger LOG = LoggerFactory.getLogger(ConfigTransactionControllerImpl.class);
 
     private final ConfigTransactionLookupRegistry txLookupRegistry;
     private final ObjectName controllerON;
 
-    private final long parentVersion, currentVersion;
+    private final long parentVersion;
+    private final long currentVersion;
     private final HierarchicalConfigMBeanFactoriesHolder factoriesHolder;
     private final DependencyResolverManager dependencyResolverManager;
     private final TransactionStatus transactionStatus;
@@ -70,8 +72,7 @@ class ConfigTransactionControllerImpl implements
      * during validation.
      */
     @GuardedBy("this")
-    private final AtomicBoolean configBeanModificationDisabled = new AtomicBoolean(
-            false);
+    private final AtomicBoolean configBeanModificationDisabled = new AtomicBoolean(false);
     private final ReadOnlyAtomicBoolean readOnlyAtomicBoolean = new ReadOnlyAtomicBooleanImpl(
             configBeanModificationDisabled);
     private final MBeanServer configMBeanServer;
@@ -81,11 +82,11 @@ class ConfigTransactionControllerImpl implements
     @GuardedBy("this")
     private final SearchableServiceReferenceWritableRegistry writableSRRegistry;
 
-    public ConfigTransactionControllerImpl(ConfigTransactionLookupRegistry txLookupRegistry,
-                                           long parentVersion, CodecRegistry codecRegistry, long currentVersion,
-                                           Map<String, Entry<ModuleFactory, BundleContext>> currentlyRegisteredFactories,
-                                           MBeanServer transactionsMBeanServer, MBeanServer configMBeanServer,
-                                           boolean blankTransaction, SearchableServiceReferenceWritableRegistry  writableSRRegistry) {
+    public ConfigTransactionControllerImpl(final ConfigTransactionLookupRegistry txLookupRegistry,
+            final long parentVersion, final BindingContextProvider bindingContextProvider, final long currentVersion,
+            final Map<String, Entry<ModuleFactory, BundleContext>> currentlyRegisteredFactories,
+            final MBeanServer transactionsMBeanServer, final MBeanServer configMBeanServer,
+            final boolean blankTransaction, final SearchableServiceReferenceWritableRegistry writableSRRegistry) {
         this.txLookupRegistry = txLookupRegistry;
         String transactionName = txLookupRegistry.getTransactionIdentifier().getName();
         this.controllerON = ObjectNameUtil.createTransactionControllerON(transactionName);
@@ -95,7 +96,7 @@ class ConfigTransactionControllerImpl implements
         this.factoriesHolder = new HierarchicalConfigMBeanFactoriesHolder(currentlyRegisteredFactories);
         this.transactionStatus = new TransactionStatus();
         this.dependencyResolverManager = new DependencyResolverManager(txLookupRegistry.getTransactionIdentifier(),
-                transactionStatus, writableSRRegistry, codecRegistry, transactionsMBeanServer);
+                transactionStatus, writableSRRegistry, bindingContextProvider, transactionsMBeanServer);
         this.transactionsMBeanServer = transactionsMBeanServer;
         this.configMBeanServer = configMBeanServer;
         this.blankTransaction = blankTransaction;
@@ -103,19 +104,20 @@ class ConfigTransactionControllerImpl implements
     }
 
     @Override
-    public void copyExistingModulesAndProcessFactoryDiff(Collection<ModuleInternalInfo> existingModules, List<ModuleFactory> lastListOfFactories) {
+    public void copyExistingModulesAndProcessFactoryDiff(final Collection<ModuleInternalInfo> existingModules,
+            final List<ModuleFactory> lastListOfFactories) {
         // copy old configuration to this server
         for (ModuleInternalInfo oldConfigInfo : existingModules) {
             try {
                 copyExistingModule(oldConfigInfo);
-            } catch (InstanceAlreadyExistsException e) {
+            } catch (final InstanceAlreadyExistsException e) {
                 throw new IllegalStateException("Error while copying " + oldConfigInfo, e);
             }
         }
         processDefaultBeans(lastListOfFactories);
     }
 
-    private synchronized void processDefaultBeans(List<ModuleFactory> lastListOfFactories) {
+    private synchronized void processDefaultBeans(final List<ModuleFactory> lastListOfFactories) {
         transactionStatus.checkNotCommitStarted();
         transactionStatus.checkNotAborted();
 
@@ -125,12 +127,12 @@ class ConfigTransactionControllerImpl implements
         List<ModuleFactory> toBeAdded = new ArrayList<>();
         List<ModuleFactory> toBeRemoved = new ArrayList<>();
         for (ModuleFactory moduleFactory : factoriesHolder.getModuleFactories()) {
-            if (oldSet.contains(moduleFactory) == false) {
+            if (!oldSet.contains(moduleFactory)) {
                 toBeAdded.add(moduleFactory);
             }
         }
         for (ModuleFactory moduleFactory : lastListOfFactories) {
-            if (newSet.contains(moduleFactory) == false) {
+            if (!newSet.contains(moduleFactory)) {
                 toBeRemoved.add(moduleFactory);
             }
         }
@@ -140,15 +142,29 @@ class ConfigTransactionControllerImpl implements
             Set<? extends Module> defaultModules = moduleFactory.getDefaultModules(dependencyResolverManager,
                     bundleContext);
             for (Module module : defaultModules) {
-                // ensure default module to be registered to jmx even if its module factory does not use dependencyResolverFactory
+                // ensure default module to be registered to jmx even if its module factory does
+                // not use dependencyResolverFactory
                 DependencyResolver dependencyResolver = dependencyResolverManager.getOrCreate(module.getIdentifier());
+                final ObjectName objectName;
                 try {
                     boolean defaultBean = true;
-                    putConfigBeanToJMXAndInternalMaps(module.getIdentifier(), module, moduleFactory, null,
+                    objectName = putConfigBeanToJMXAndInternalMaps(module.getIdentifier(), module, moduleFactory, null,
                             dependencyResolver, defaultBean, bundleContext);
-                } catch (InstanceAlreadyExistsException e) {
+                } catch (final InstanceAlreadyExistsException e) {
                     throw new IllegalStateException(e);
                 }
+                // register default module as every possible service
+                final Set<ServiceInterfaceAnnotation> serviceInterfaceAnnotations = InterfacesHelper
+                        .getServiceInterfaceAnnotations(moduleFactory);
+                for (String qname : InterfacesHelper.getQNames(serviceInterfaceAnnotations)) {
+                    try {
+                        saveServiceReference(qname, module.getIdentifier().getInstanceName(), objectName);
+                    } catch (final InstanceNotFoundException e) {
+                        throw new IllegalStateException(
+                                "Unable to register default module instance " + module + " as a service of " + qname,
+                                e);
+                    }
+                }
             }
         }
 
@@ -156,13 +172,29 @@ class ConfigTransactionControllerImpl implements
         for (ModuleFactory removedFactory : toBeRemoved) {
             List<ModuleIdentifier> modulesOfRemovedFactory = dependencyResolverManager.findAllByFactory(removedFactory);
             for (ModuleIdentifier name : modulesOfRemovedFactory) {
+                // remove service refs
+                final ModuleFactory moduleFactory = dependencyResolverManager.findModuleInternalTransactionalInfo(name)
+                        .getModuleFactory();
+                final Set<ServiceInterfaceAnnotation> serviceInterfaceAnnotations = InterfacesHelper
+                        .getServiceInterfaceAnnotations(moduleFactory);
+                for (String qname : InterfacesHelper.getQNames(serviceInterfaceAnnotations)) {
+                    try {
+                        removeServiceReference(qname, name.getInstanceName());
+                    } catch (final InstanceNotFoundException e) {
+                        throw new IllegalStateException(
+                                "Unable to UNregister default module instance " + name + " as a service of " + qname,
+                                e);
+                    }
+                }
+                // close module
                 destroyModule(name);
             }
         }
     }
 
-
-    private synchronized void copyExistingModule(ModuleInternalInfo oldConfigBeanInfo) throws InstanceAlreadyExistsException {
+    @SuppressWarnings("IllegalCatch")
+    private synchronized void copyExistingModule(final ModuleInternalInfo oldConfigBeanInfo)
+            throws InstanceAlreadyExistsException {
 
         transactionStatus.checkNotCommitStarted();
         transactionStatus.checkNotAborted();
@@ -174,7 +206,7 @@ class ConfigTransactionControllerImpl implements
         try {
             moduleFactory = factoriesHolder.findByModuleName(moduleIdentifier.getFactoryName());
             bc = getModuleFactoryBundleContext(moduleFactory.getImplementationName());
-        } catch (InstanceNotFoundException e) {
+        } catch (final ModuleFactoryNotFoundException e) {
             throw new IllegalStateException(e);
         }
 
@@ -182,21 +214,19 @@ class ConfigTransactionControllerImpl implements
         DependencyResolver dependencyResolver = dependencyResolverManager.getOrCreate(moduleIdentifier);
         try {
 
-            module = moduleFactory.createModule(
-                    moduleIdentifier.getInstanceName(), dependencyResolver,
+            module = moduleFactory.createModule(moduleIdentifier.getInstanceName(), dependencyResolver,
                     oldConfigBeanInfo.getReadableModule(), bc);
-        } catch (Exception e) {
-            throw new IllegalStateException(format(
-                    "Error while copying old configuration from %s to %s",
+        } catch (final Exception e) {
+            throw new IllegalStateException(String.format("Error while copying old configuration from %s to %s",
                     oldConfigBeanInfo, moduleFactory), e);
         }
-        putConfigBeanToJMXAndInternalMaps(moduleIdentifier, module, moduleFactory, oldConfigBeanInfo, dependencyResolver,
-                oldConfigBeanInfo.isDefaultBean(), bc);
+        putConfigBeanToJMXAndInternalMaps(moduleIdentifier, module, moduleFactory, oldConfigBeanInfo,
+                dependencyResolver, oldConfigBeanInfo.isDefaultBean(), bc);
     }
 
     @Override
-    public synchronized ObjectName createModule(String factoryName,
-                                                String instanceName) throws InstanceAlreadyExistsException {
+    public synchronized ObjectName createModule(final String factoryName, final String instanceName)
+            throws InstanceAlreadyExistsException {
 
         transactionStatus.checkNotCommitStarted();
         transactionStatus.checkNotAborted();
@@ -204,90 +234,85 @@ class ConfigTransactionControllerImpl implements
         dependencyResolverManager.assertNotExists(moduleIdentifier);
 
         // find factory
-        ModuleFactory moduleFactory;
-        try {
-            moduleFactory = factoriesHolder.findByModuleName(factoryName);
-        } catch (InstanceNotFoundException e) {
-            throw new IllegalArgumentException(e);
-        }
+        ModuleFactory moduleFactory = factoriesHolder.findByModuleName(factoryName);
+
         DependencyResolver dependencyResolver = dependencyResolverManager.getOrCreate(moduleIdentifier);
         BundleContext bundleContext = getModuleFactoryBundleContext(moduleFactory.getImplementationName());
-        Module module = moduleFactory.createModule(instanceName, dependencyResolver,
-                bundleContext);
+        Module module = moduleFactory.createModule(instanceName, dependencyResolver, bundleContext);
         boolean defaultBean = false;
-        return putConfigBeanToJMXAndInternalMaps(moduleIdentifier, module,
-                moduleFactory, null, dependencyResolver, defaultBean, bundleContext);
+        return putConfigBeanToJMXAndInternalMaps(moduleIdentifier, module, moduleFactory, null, dependencyResolver,
+                defaultBean, bundleContext);
     }
 
-    private synchronized ObjectName putConfigBeanToJMXAndInternalMaps(
-            ModuleIdentifier moduleIdentifier, Module module,
-            ModuleFactory moduleFactory,
-            @Nullable ModuleInternalInfo maybeOldConfigBeanInfo, DependencyResolver dependencyResolver,
-            boolean isDefaultBean, BundleContext bundleContext)
-            throws InstanceAlreadyExistsException {
+    @Override
+    public synchronized void reCreateModule(final ObjectName objectName) throws InstanceNotFoundException {
+        transactionStatus.checkNotCommitStarted();
+        transactionStatus.checkNotAborted();
+        checkTransactionName(objectName);
+        ObjectNameUtil.checkDomain(objectName);
+        ModuleIdentifier moduleIdentifier = ObjectNameUtil.fromON(objectName, ObjectNameUtil.TYPE_MODULE);
 
-        logger.debug("Adding module {} to transaction {}", moduleIdentifier, this);
-        if (moduleIdentifier.equals(module.getIdentifier()) == false) {
-            throw new IllegalStateException("Incorrect name reported by module. Expected "
-                    + moduleIdentifier + ", got " + module.getIdentifier());
+        ModuleInternalTransactionalInfo txInfo = dependencyResolverManager
+                .findModuleInternalTransactionalInfo(moduleIdentifier);
+        Module realModule = txInfo.getRealModule();
+        if (realModule instanceof AbstractModule) {
+            ((AbstractModule<?>) realModule).setCanReuseInstance(false);
         }
-        if (dependencyResolver.getIdentifier().equals(moduleIdentifier) == false) {
+    }
+
+    private synchronized ObjectName putConfigBeanToJMXAndInternalMaps(final ModuleIdentifier moduleIdentifier,
+            final Module module, final ModuleFactory moduleFactory,
+            @Nullable final ModuleInternalInfo maybeOldConfigBeanInfo, final DependencyResolver dependencyResolver,
+            final boolean isDefaultBean, final BundleContext bundleContext) throws InstanceAlreadyExistsException {
+
+        LOG.debug("Adding module {} to transaction {}", moduleIdentifier, this);
+        if (!moduleIdentifier.equals(module.getIdentifier())) {
+            throw new IllegalStateException("Incorrect name reported by module. Expected " + moduleIdentifier + ", got "
+                    + module.getIdentifier());
+        }
+        if (!dependencyResolver.getIdentifier().equals(moduleIdentifier)) {
             throw new IllegalStateException("Incorrect name reported by dependency resolver. Expected "
                     + moduleIdentifier + ", got " + dependencyResolver.getIdentifier());
         }
-        DynamicMBean writableDynamicWrapper = new DynamicWritableWrapper(
-                module, moduleIdentifier, getTransactionIdentifier(),
-                readOnlyAtomicBoolean, transactionsMBeanServer,
+        DynamicMBean writableDynamicWrapper = new DynamicWritableWrapper(module, moduleIdentifier,
+                getTransactionIdentifier().getName(), readOnlyAtomicBoolean, transactionsMBeanServer,
                 configMBeanServer);
 
-        ObjectName writableON = ObjectNameUtil.createTransactionModuleON(
-                getTransactionIdentifier().getName(), moduleIdentifier);
+        ObjectName writableON = ObjectNameUtil.createTransactionModuleON(getTransactionIdentifier().getName(),
+                moduleIdentifier);
         // put wrapper to jmx
         TransactionModuleJMXRegistration transactionModuleJMXRegistration = getTxModuleJMXRegistrator()
                 .registerMBean(writableDynamicWrapper, writableON);
 
-        dependencyResolverManager.put(
-                moduleIdentifier, module, moduleFactory,
-                maybeOldConfigBeanInfo, transactionModuleJMXRegistration, isDefaultBean, bundleContext);
+        dependencyResolverManager.put(moduleIdentifier, module, moduleFactory, maybeOldConfigBeanInfo,
+                transactionModuleJMXRegistration, isDefaultBean, bundleContext);
         return writableON;
     }
 
     @Override
-    public synchronized void destroyModule(ObjectName objectName) throws InstanceNotFoundException {
+    public synchronized void destroyModule(final ObjectName objectName) throws InstanceNotFoundException {
         checkTransactionName(objectName);
         ObjectNameUtil.checkDomain(objectName);
-        ModuleIdentifier moduleIdentifier = ObjectNameUtil.fromON(objectName,
-                ObjectNameUtil.TYPE_MODULE);
+        ModuleIdentifier moduleIdentifier = ObjectNameUtil.fromON(objectName, ObjectNameUtil.TYPE_MODULE);
         destroyModule(moduleIdentifier);
     }
 
-    private void checkTransactionName(ObjectName objectName) {
-        String foundTransactionName = ObjectNameUtil
-                .getTransactionName(objectName);
-        if (getTransactionIdentifier().getName().equals(foundTransactionName) == false) {
-            throw new IllegalArgumentException("Wrong transaction name "
-                    + objectName);
-        }
-    }
-
-    private synchronized void destroyModule(ModuleIdentifier moduleIdentifier) {
-        logger.debug("Destroying module {} in transaction {}", moduleIdentifier, this);
+    private synchronized void destroyModule(final ModuleIdentifier moduleIdentifier) {
+        LOG.debug("Destroying module {} in transaction {}", moduleIdentifier, this);
         transactionStatus.checkNotAborted();
 
-        ModuleInternalTransactionalInfo found = dependencyResolverManager.findModuleInternalTransactionalInfo(moduleIdentifier);
-        if (blankTransaction == false) {
-
-            if (found.isDefaultBean()) {
-                logger.warn("Warning: removing default bean. This will be forbidden in next version of config-subsystem");
-            }
+        ModuleInternalTransactionalInfo found = dependencyResolverManager
+                .findModuleInternalTransactionalInfo(moduleIdentifier);
+        if (!blankTransaction && found.isDefaultBean()) {
+            LOG.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));
-        } catch (InstanceNotFoundException e) {
-            logger.error("Possible code error: cannot find {} in {}", moduleIdentifier, writableSRRegistry);
+        } catch (final InstanceNotFoundException e) {
+            LOG.error("Possible code error: cannot find {} in {}", moduleIdentifier, writableSRRegistry);
             throw new IllegalStateException("Possible code error: cannot find " + moduleIdentifier, e);
         }
 
@@ -296,6 +321,13 @@ class ConfigTransactionControllerImpl implements
         removedTInfo.getTransactionModuleJMXRegistration().close();
     }
 
+    private void checkTransactionName(final ObjectName objectName) {
+        String foundTransactionName = ObjectNameUtil.getTransactionName(objectName);
+        if (!getTransactionIdentifier().getName().equals(foundTransactionName)) {
+            throw new IllegalArgumentException("Wrong transaction name " + objectName);
+        }
+    }
+
     @Override
     public long getParentVersion() {
         return parentVersion;
@@ -313,55 +345,52 @@ class ConfigTransactionControllerImpl implements
         }
         configBeanModificationDisabled.set(true);
         try {
-            validate_noLocks();
+            validateNoLocks();
         } finally {
             configBeanModificationDisabled.set(false);
         }
     }
 
-    private void validate_noLocks() throws ValidationException {
+    @SuppressWarnings("IllegalCatch")
+    private void validateNoLocks() throws ValidationException {
         transactionStatus.checkNotAborted();
-        logger.trace("Validating transaction {}", getTransactionIdentifier());
+        LOG.trace("Validating transaction {}", getTransactionIdentifier());
         // call validate()
         List<ValidationException> collectedExceptions = new ArrayList<>();
-        for (Entry<ModuleIdentifier, Module> entry : dependencyResolverManager
-                .getAllModules().entrySet()) {
+        for (Entry<ModuleIdentifier, Module> entry : dependencyResolverManager.getAllModules().entrySet()) {
             ModuleIdentifier name = entry.getKey();
             Module module = entry.getValue();
             try {
                 module.validate();
-            } catch (Exception e) {
-                logger.warn("Validation exception in {}", getTransactionName(),
-                        e);
-                collectedExceptions.add(ValidationException
-                        .createForSingleException(name, e));
+            } catch (final Exception e) {
+                LOG.warn("Validation exception in {}", getTransactionName(), e);
+                collectedExceptions.add(ValidationException.createForSingleException(name, e));
             }
         }
-        if (collectedExceptions.size() > 0) {
-            throw ValidationException
-                    .createFromCollectedValidationExceptions(collectedExceptions);
+        if (!collectedExceptions.isEmpty()) {
+            throw ValidationException.createFromCollectedValidationExceptions(collectedExceptions);
         }
-        logger.trace("Validated transaction {}", getTransactionIdentifier());
+        LOG.trace("Validated transaction {}", getTransactionIdentifier());
     }
 
     /**
      * If this method passes validation, it will grab
      * {@link TransactionStatus#secondPhaseCommitStarted} lock. This lock will
-     * prevent calling @{link #validateBeforeCommitAndLockTransaction},
-     * effectively only allowing to call {@link #secondPhaseCommit} after
-     * successful return of this method.
+     * prevent calling @{link #validateBeforeCommitAndLockTransaction}, effectively
+     * only allowing to call {@link #secondPhaseCommit} after successful return of
+     * this method.
      */
     @Override
-    public synchronized CommitInfo validateBeforeCommitAndLockTransaction()
-            throws ValidationException {
+    public synchronized CommitInfo validateBeforeCommitAndLockTransaction() throws ValidationException {
         transactionStatus.checkNotAborted();
         transactionStatus.checkNotCommitStarted();
         configBeanModificationDisabled.set(true);
         try {
-            validate_noLocks();
-        } catch (ValidationException e) {
-            logger.trace("Commit failed on validation");
-            configBeanModificationDisabled.set(false); // recoverable error
+            validateNoLocks();
+        } catch (final ValidationException e) {
+            LOG.trace("Commit failed on validation");
+            // recoverable error
+            configBeanModificationDisabled.set(false);
             throw e;
         }
         // errors in this state are not recoverable. modules are not mutable
@@ -377,55 +406,52 @@ class ConfigTransactionControllerImpl implements
     public synchronized List<ModuleIdentifier> secondPhaseCommit() {
         transactionStatus.checkNotAborted();
         transactionStatus.checkCommitStarted();
-        if (configBeanModificationDisabled.get() == false) {
+        if (!configBeanModificationDisabled.get()) {
             throw new IllegalStateException(
-                    "Internal error - validateBeforeCommitAndLockTransaction should be called "
-                            + "to obtain a lock");
+                    "Internal error - validateBeforeCommitAndLockTransaction should be called " + "to obtain a lock");
         }
 
-        logger.trace("Committing transaction {}", getTransactionIdentifier());
-
-        // call getInstance()
-        for (Entry<ModuleIdentifier, Module> entry : dependencyResolverManager
-                .getAllModules().entrySet()) {
-            Module module = entry.getValue();
-            ModuleIdentifier name = entry.getKey();
-            try {
-                logger.debug("About to commit {} in transaction {}",
-                        name, getTransactionIdentifier());
-                AutoCloseable instance = module.getInstance();
-                checkNotNull(instance, "Instance is null:{} in transaction {}", name, getTransactionIdentifier());
-            } catch (Exception e) {
-                logger.error("Commit failed on {} in transaction {}", name,
-                        getTransactionIdentifier(), e);
-                internalAbort();
-                throw new IllegalStateException(
-                        format("Error - getInstance() failed for %s in transaction %s",
-                                name, getTransactionIdentifier()), e);
-            }
+        LOG.trace("Committing transaction {}", getTransactionIdentifier());
+
+        Map<ModuleIdentifier, Module> allModules = dependencyResolverManager.getAllModules();
+
+        // call getInstance() on all Modules from top to bottom (from source to target
+        // of the dependency relation)
+        // The source of a dependency closes itself and calls getInstance recursively on
+        // the dependencies (in case of reconfiguration)
+        // This makes close() calls from top to bottom while createInstance() calls are
+        // performed bottom to top
+        List<ModuleIdentifier> sortedModuleIdentifiers = Lists
+                .reverse(dependencyResolverManager.getSortedModuleIdentifiers());
+        for (ModuleIdentifier moduleIdentifier : sortedModuleIdentifiers) {
+            Module module = allModules.get(moduleIdentifier);
+
+            LOG.debug("About to commit {} in transaction {}", moduleIdentifier, getTransactionIdentifier());
+            AutoCloseable instance = module.getInstance();
+            Preconditions.checkNotNull(instance, "Instance is null:%s in transaction %s", moduleIdentifier,
+                    getTransactionIdentifier());
         }
 
-        // count dependency order
-
-        logger.trace("Committed configuration {}", getTransactionIdentifier());
+        LOG.trace("Committed configuration {}", getTransactionIdentifier());
         transactionStatus.setCommitted();
 
-        return dependencyResolverManager.getSortedModuleIdentifiers();
+        return sortedModuleIdentifiers;
     }
 
     @Override
-    public synchronized void abortConfig() {
+    public void abortConfig() {
         transactionStatus.checkNotCommitStarted();
         transactionStatus.checkNotAborted();
         internalAbort();
     }
 
     private void internalAbort() {
-        logger.trace("Aborting {}", this);
+        LOG.trace("Aborting {}", this);
         transactionStatus.setAborted();
         close();
     }
 
+    @Override
     public void close() {
         dependencyResolverManager.close();
         txLookupRegistry.close();
@@ -453,7 +479,7 @@ class ConfigTransactionControllerImpl implements
      * {@inheritDoc}
      */
     @Override
-    public Set<ObjectName> lookupConfigBeans(String moduleName) {
+    public Set<ObjectName> lookupConfigBeans(final String moduleName) {
         return txLookupRegistry.lookupConfigBeans(moduleName);
     }
 
@@ -461,7 +487,15 @@ class ConfigTransactionControllerImpl implements
      * {@inheritDoc}
      */
     @Override
-    public ObjectName lookupConfigBean(String moduleName, String instanceName)
+    public Set<ObjectName> lookupConfigBeans(final String moduleName, final String instanceName) {
+        return txLookupRegistry.lookupConfigBeans(moduleName, instanceName);
+    }
+
+    /**
+     * {@inheritDoc}
+     */
+    @Override
+    public ObjectName lookupConfigBean(final String moduleName, final String instanceName)
             throws InstanceNotFoundException {
         return txLookupRegistry.lookupConfigBean(moduleName, instanceName);
     }
@@ -470,17 +504,26 @@ class ConfigTransactionControllerImpl implements
      * {@inheritDoc}
      */
     @Override
-    public Set<ObjectName> lookupConfigBeans(String moduleName, String instanceName) {
-        return txLookupRegistry.lookupConfigBeans(moduleName, instanceName);
+    public void checkConfigBeanExists(final ObjectName objectName) throws InstanceNotFoundException {
+        txLookupRegistry.checkConfigBeanExists(objectName);
     }
 
     /**
      * {@inheritDoc}
      */
     @Override
-    public void checkConfigBeanExists(ObjectName objectName) throws InstanceNotFoundException {
-        txLookupRegistry.checkConfigBeanExists(objectName);
+    public Set<ObjectName> lookupRuntimeBeans() {
+        return txLookupRegistry.lookupRuntimeBeans();
+    }
+
+    /**
+     * {@inheritDoc}
+     */
+    @Override
+    public Set<ObjectName> lookupRuntimeBeans(final String moduleName, final String instanceName) {
+        return txLookupRegistry.lookupRuntimeBeans(moduleName, instanceName);
     }
+
     // --
 
     /**
@@ -498,10 +541,7 @@ class ConfigTransactionControllerImpl implements
 
     @Override
     public String toString() {
-        StringBuilder sb = new StringBuilder();
-        sb.append("transactionName=");
-        sb.append(getTransactionName());
-        return sb.toString();
+        return "transactionName=" + getTransactionName();
     }
 
     // @VisibleForTesting
@@ -525,8 +565,9 @@ class ConfigTransactionControllerImpl implements
     }
 
     @Override
-    public BundleContext getModuleFactoryBundleContext(String factoryName) {
-        Map.Entry<ModuleFactory, BundleContext> factoryBundleContextEntry = this.currentlyRegisteredFactories.get(factoryName);
+    public BundleContext getModuleFactoryBundleContext(final String factoryName) {
+        Map.Entry<ModuleFactory, BundleContext> factoryBundleContextEntry = this.currentlyRegisteredFactories
+                .get(factoryName);
         if (factoryBundleContextEntry == null || factoryBundleContextEntry.getValue() == null) {
             throw new NullPointerException("Bundle context of " + factoryName + " ModuleFactory not found.");
         }
@@ -535,9 +576,9 @@ class ConfigTransactionControllerImpl implements
 
     // service reference functionality:
 
-
     @Override
-    public synchronized ObjectName lookupConfigBeanByServiceInterfaceName(String serviceInterfaceQName, String refName) {
+    public synchronized ObjectName lookupConfigBeanByServiceInterfaceName(final String serviceInterfaceQName,
+            final String refName) {
         return writableSRRegistry.lookupConfigBeanByServiceInterfaceName(serviceInterfaceQName, refName);
     }
 
@@ -547,27 +588,31 @@ class ConfigTransactionControllerImpl implements
     }
 
     @Override
-    public synchronized Map<String, ObjectName> lookupServiceReferencesByServiceInterfaceName(String serviceInterfaceQName) {
+    public synchronized Map<String, ObjectName> lookupServiceReferencesByServiceInterfaceName(
+            final String serviceInterfaceQName) {
         return writableSRRegistry.lookupServiceReferencesByServiceInterfaceName(serviceInterfaceQName);
     }
 
     @Override
-    public synchronized Set<String> lookupServiceInterfaceNames(ObjectName objectName) throws InstanceNotFoundException {
+    public synchronized Set<String> lookupServiceInterfaceNames(final ObjectName objectName)
+            throws InstanceNotFoundException {
         return writableSRRegistry.lookupServiceInterfaceNames(objectName);
     }
 
     @Override
-    public synchronized String getServiceInterfaceName(String namespace, String localName) {
+    public synchronized String getServiceInterfaceName(final String namespace, final String localName) {
         return writableSRRegistry.getServiceInterfaceName(namespace, localName);
     }
 
     @Override
-    public synchronized ObjectName saveServiceReference(String serviceInterfaceName, String refName, ObjectName moduleON) throws InstanceNotFoundException {
+    public synchronized ObjectName saveServiceReference(final String serviceInterfaceName, final String refName,
+            final ObjectName moduleON) throws InstanceNotFoundException {
         return writableSRRegistry.saveServiceReference(serviceInterfaceName, refName, moduleON);
     }
 
     @Override
-    public synchronized void removeServiceReference(String serviceInterfaceName, String refName) throws InstanceNotFoundException {
+    public synchronized void removeServiceReference(final String serviceInterfaceName, final String refName)
+            throws InstanceNotFoundException {
         writableSRRegistry.removeServiceReference(serviceInterfaceName, refName);
     }
 
@@ -577,12 +622,12 @@ class ConfigTransactionControllerImpl implements
     }
 
     @Override
-    public boolean removeServiceReferences(ObjectName objectName) throws InstanceNotFoundException {
+    public boolean removeServiceReferences(final ObjectName objectName) throws InstanceNotFoundException {
         return writableSRRegistry.removeServiceReferences(objectName);
     }
 
     @Override
-    public SearchableServiceReferenceWritableRegistry  getWritableRegistry() {
+    public SearchableServiceReferenceWritableRegistry getWritableRegistry() {
         return writableSRRegistry;
     }
 
@@ -597,12 +642,13 @@ class ConfigTransactionControllerImpl implements
     }
 
     @Override
-    public void checkServiceReferenceExists(ObjectName objectName) throws InstanceNotFoundException {
+    public void checkServiceReferenceExists(final ObjectName objectName) throws InstanceNotFoundException {
         writableSRRegistry.checkServiceReferenceExists(objectName);
     }
 
     @Override
-    public ObjectName getServiceReference(String serviceInterfaceQName, String refName) throws InstanceNotFoundException {
+    public ObjectName getServiceReference(final String serviceInterfaceQName, final String refName)
+            throws InstanceNotFoundException {
         return writableSRRegistry.getServiceReference(serviceInterfaceQName, refName);
     }
 }