Fix race conditions between config-manager and persister.
[controller.git] / opendaylight / config / config-manager / src / main / java / org / opendaylight / controller / config / manager / impl / ConfigTransactionControllerImpl.java
index 5e8fac6048fceca1e51326fd3f7b00beeaf11edf..a9ab664fd6d44cd19a4b38c02248a96d938b41b9 100644 (file)
@@ -7,20 +7,6 @@
  */
 package org.opendaylight.controller.config.manager.impl;
 
-import static java.lang.String.format;
-
-import java.util.*;
-import java.util.Map.Entry;
-import java.util.concurrent.atomic.AtomicBoolean;
-
-import javax.annotation.Nullable;
-import javax.annotation.concurrent.GuardedBy;
-import javax.management.DynamicMBean;
-import javax.management.InstanceAlreadyExistsException;
-import javax.management.InstanceNotFoundException;
-import javax.management.MBeanServer;
-import javax.management.ObjectName;
-
 import org.opendaylight.controller.config.api.DependencyResolver;
 import org.opendaylight.controller.config.api.ModuleIdentifier;
 import org.opendaylight.controller.config.api.ValidationException;
@@ -32,14 +18,33 @@ 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.TransactionJMXRegistrator;
 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.jmx.TransactionModuleJMXRegistrator.TransactionModuleJMXRegistration;
 import org.opendaylight.controller.config.manager.impl.util.LookupBeansUtil;
 import org.opendaylight.controller.config.spi.Module;
 import org.opendaylight.controller.config.spi.ModuleFactory;
+import org.opendaylight.yangtools.concepts.Identifiable;
+import org.osgi.framework.BundleContext;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
+import javax.annotation.Nullable;
+import javax.annotation.concurrent.GuardedBy;
+import javax.management.DynamicMBean;
+import javax.management.InstanceAlreadyExistsException;
+import javax.management.InstanceNotFoundException;
+import javax.management.MBeanServer;
+import javax.management.ObjectName;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Map;
+import java.util.Map.Entry;
+import java.util.Set;
+import java.util.ArrayList;
+import java.util.Collection;
+import java.util.concurrent.atomic.AtomicBoolean;
+
+import static java.lang.String.format;
+
 /**
  * This is a JMX bean representing current transaction. It contains
  * {@link #transactionIdentifier}, unique version and parent version for
@@ -47,7 +52,8 @@ import org.slf4j.LoggerFactory;
  */
 class ConfigTransactionControllerImpl implements
         ConfigTransactionControllerInternal,
-        ConfigTransactionControllerImplMXBean {
+        ConfigTransactionControllerImplMXBean,
+        Identifiable<TransactionIdentifier>{
     private static final Logger logger = LoggerFactory.getLogger(ConfigTransactionControllerImpl.class);
 
     private final TransactionIdentifier transactionIdentifier;
@@ -59,7 +65,7 @@ class ConfigTransactionControllerImpl implements
     private final DependencyResolverManager dependencyResolverManager;
     private final TransactionStatus transactionStatus;
     private final MBeanServer transactionsMBeanServer;
-    private final List<ModuleFactory> currentlyRegisteredFactories;
+    private final Map<String, Map.Entry<ModuleFactory, BundleContext>> currentlyRegisteredFactories;
 
     /**
      * Disables ability of {@link DynamicWritableWrapper} to change attributes
@@ -72,11 +78,13 @@ class ConfigTransactionControllerImpl implements
             configBeanModificationDisabled);
     private final MBeanServer configMBeanServer;
 
+    private final BundleContext bundleContext;
+
     public ConfigTransactionControllerImpl(String transactionName,
-            TransactionJMXRegistrator transactionRegistrator,
-            long parentVersion, long currentVersion,
-            List<ModuleFactory> currentlyRegisteredFactories,
-            MBeanServer transactionsMBeanServer, MBeanServer configMBeanServer) {
+                                           TransactionJMXRegistrator transactionRegistrator,
+                                           long parentVersion, long currentVersion,
+                                           Map<String, Map.Entry<ModuleFactory, BundleContext>> currentlyRegisteredFactories,
+                                           MBeanServer transactionsMBeanServer, MBeanServer configMBeanServer, BundleContext bundleContext) {
 
         this.transactionIdentifier = new TransactionIdentifier(transactionName);
         this.controllerON = ObjectNameUtil
@@ -92,6 +100,7 @@ class ConfigTransactionControllerImpl implements
         this.dependencyResolverManager = new DependencyResolverManager(transactionName, transactionStatus);
         this.transactionsMBeanServer = transactionsMBeanServer;
         this.configMBeanServer = configMBeanServer;
+        this.bundleContext = bundleContext;
     }
 
     @Override
@@ -112,11 +121,11 @@ class ConfigTransactionControllerImpl implements
         transactionStatus.checkNotAborted();
 
         Set<ModuleFactory> oldSet = new HashSet<>(lastListOfFactories);
-        Set<ModuleFactory> newSet = new HashSet<>(currentlyRegisteredFactories);
+        Set<ModuleFactory> newSet = new HashSet<>(factoriesHolder.getModuleFactories());
 
         List<ModuleFactory> toBeAdded = new ArrayList<>();
         List<ModuleFactory> toBeRemoved = new ArrayList<>();
-        for(ModuleFactory moduleFactory: currentlyRegisteredFactories) {
+        for(ModuleFactory moduleFactory: factoriesHolder.getModuleFactories()) {
             if (oldSet.contains(moduleFactory) == false){
                 toBeAdded.add(moduleFactory);
             }
@@ -128,10 +137,13 @@ class ConfigTransactionControllerImpl implements
         }
         // add default modules
         for (ModuleFactory moduleFactory : toBeAdded) {
-            Set<? extends Module> defaultModules = moduleFactory.getDefaultModules(dependencyResolverManager);
+            Set<? extends Module> defaultModules = moduleFactory.getDefaultModules(dependencyResolverManager,
+                    getModuleFactoryBundleContext(moduleFactory.getImplementationName()));
             for (Module module : defaultModules) {
+                // ensure default module to be registered to jmx even if its module factory does not use dependencyResolverFactory
+                DependencyResolver dependencyResolver = dependencyResolverManager.getOrCreate(module.getIdentifier());
                 try {
-                    putConfigBeanToJMXAndInternalMaps(module.getName(), module, moduleFactory, null);
+                    putConfigBeanToJMXAndInternalMaps(module.getIdentifier(), module, moduleFactory, null, dependencyResolver);
                 } catch (InstanceAlreadyExistsException e) {
                     throw new IllegalStateException(e);
                 }
@@ -163,15 +175,16 @@ class ConfigTransactionControllerImpl implements
         DependencyResolver dependencyResolver = dependencyResolverManager
                 .getOrCreate(moduleIdentifier);
         try {
+            BundleContext bc = getModuleFactoryBundleContext(moduleFactory.getImplementationName());
             module = moduleFactory.createModule(
                     moduleIdentifier.getInstanceName(), dependencyResolver,
-                    oldConfigBeanInfo.getReadableModule());
+                    oldConfigBeanInfo.getReadableModule(), bc);
         } catch (Exception e) {
             throw new IllegalStateException(format(
                     "Error while copying old configuration from %s to %s",
                     oldConfigBeanInfo, moduleFactory), e);
         }
-        putConfigBeanToJMXAndInternalMaps(moduleIdentifier, module, moduleFactory, oldConfigBeanInfo);
+        putConfigBeanToJMXAndInternalMaps(moduleIdentifier, module, moduleFactory, oldConfigBeanInfo, dependencyResolver);
     }
 
     @Override
@@ -186,20 +199,26 @@ class ConfigTransactionControllerImpl implements
         // find factory
         ModuleFactory moduleFactory = factoriesHolder.findByModuleName(factoryName);
         DependencyResolver dependencyResolver = dependencyResolverManager.getOrCreate(moduleIdentifier);
-        Module module = moduleFactory.createModule(instanceName, dependencyResolver);
+        Module module = moduleFactory.createModule(instanceName, dependencyResolver,
+                getModuleFactoryBundleContext(moduleFactory.getImplementationName()));
         return putConfigBeanToJMXAndInternalMaps(moduleIdentifier, module,
-                moduleFactory, null);
+                moduleFactory, null, dependencyResolver);
     }
 
     private synchronized ObjectName putConfigBeanToJMXAndInternalMaps(
             ModuleIdentifier moduleIdentifier, Module module,
             ModuleFactory moduleFactory,
-            @Nullable ModuleInternalInfo maybeOldConfigBeanInfo)
+            @Nullable ModuleInternalInfo maybeOldConfigBeanInfo, DependencyResolver dependencyResolver)
             throws InstanceAlreadyExistsException {
+
         logger.debug("Adding module {} to transaction {}", moduleIdentifier, this);
-        if (moduleIdentifier.equals(module.getName())==false) {
+        if (moduleIdentifier.equals(module.getIdentifier())==false) {
             throw new IllegalStateException("Incorrect name reported by module. Expected "
-             + moduleIdentifier + ", got " + module.getName());
+             + moduleIdentifier + ", got " + module.getIdentifier());
+        }
+        if (dependencyResolver.getIdentifier().equals(moduleIdentifier) == false ) {
+            throw new IllegalStateException("Incorrect name reported by dependency resolver. Expected "
+                    + moduleIdentifier + ", got " + dependencyResolver.getIdentifier());
         }
         DynamicMBean writableDynamicWrapper = new DynamicWritableWrapper(
                 module, moduleIdentifier, transactionIdentifier,
@@ -337,7 +356,7 @@ class ConfigTransactionControllerImpl implements
             ModuleIdentifier name = entry.getKey();
             try {
                 logger.debug("About to commit {} in transaction {}",
-                        transactionIdentifier, name);
+                        name, transactionIdentifier);
                 module.getInstance();
             } catch (Exception e) {
                 logger.error("Commit failed on {} in transaction {}", name,
@@ -451,6 +470,20 @@ class ConfigTransactionControllerImpl implements
 
     @Override
     public List<ModuleFactory> getCurrentlyRegisteredFactories() {
-        return currentlyRegisteredFactories;
+        return new ArrayList<>(factoriesHolder.getModuleFactories());
+    }
+
+    @Override
+    public TransactionIdentifier getIdentifier() {
+        return transactionIdentifier;
+    }
+
+    @Override
+    public BundleContext getModuleFactoryBundleContext(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.");
+        }
+        return factoryBundleContextEntry.getValue();
     }
 }