Fix race condition when registering services

Dependency Manager is able to notify every component when a new
service tracked is added/removed, this kind of operation is usually
done by allocating a synchronized set or an concurrent map to store
the new service reference or remove it.
This works fine, just it's necessary to be aware that the services may
be learnt by the Component before the "init" method of the component
it's called. In fact the init method of a Dependency Manager component
is called after all the required dependency have been met so that
means that the services may have already been registered. Now if a
component allocate the data structure needed to hold the service
reference during the init time, it can run in the condition where the
services already registered would be wiped out.

The fix is to make sure all the Dependency Manager component init
method don't allocate the data structure needed to hold the services,
but they should be created or during constructor time or during field
initialization. The second approach is prefered here.

Change-Id: I9057351830c09736c5ad7b6777330ef14b39fe9c
Signed-off-by: Giovanni Meo <gmeo@cisco.com>

diff --git a/opendaylight/forwardingrulesmanager/implementation/src/main/java/org/opendaylight/controller/forwardingrulesmanager/internal/ForwardingRulesManager.java b/opendaylight/forwardingrulesmanager/implementation/src/main/java/org/opendaylight/controller/forwardingrulesmanager/internal/ForwardingRulesManager.java
index cade266..836c127 100644 (file)
--- a/opendaylight/forwardingrulesmanager/implementation/src/main/java/org/opendaylight/controller/forwardingrulesmanager/internal/ForwardingRulesManager.java
+++ b/opendaylight/forwardingrulesmanager/implementation/src/main/java/org/opendaylight/controller/forwardingrulesmanager/internal/ForwardingRulesManager.java
@@ -131,7 +131,8 @@ public class ForwardingRulesManager implements IForwardingRulesManager, PortGrou
     private ConcurrentMap<FlowEntry, FlowEntry> inactiveFlows;
 
     private IContainer container;
-    private Set<IForwardingRulesManagerAware> frmAware;
+    private Set<IForwardingRulesManagerAware> frmAware =
+        Collections.synchronizedSet(new HashSet<IForwardingRulesManagerAware>());
     private PortGroupProvider portGroupProvider;
     private IFlowProgrammerService programmer;
     private IClusterContainerServices clusterContainerService = null;
@@ -2138,7 +2139,6 @@ public class ForwardingRulesManager implements IForwardingRulesManager, PortGrou
      *
      */
     void init() {
-        frmAware = Collections.synchronizedSet(new HashSet<IForwardingRulesManagerAware>());
         frmFileName = GlobalConstants.STARTUPHOME.toString() + "frm_staticflows_" + this.getContainerName() + ".conf";
         portGroupFileName = GlobalConstants.STARTUPHOME.toString() + "portgroup_" + this.getContainerName() + ".conf";
 
@@ -2207,6 +2207,7 @@ public class ForwardingRulesManager implements IForwardingRulesManager, PortGrou
      *
      */
     void destroy() {
+        frmAware.clear();
     }
 
     /**

diff --git a/opendaylight/protocol_plugins/openflow/src/main/java/org/opendaylight/controller/protocol_plugin/openflow/internal/OFStatisticsManager.java b/opendaylight/protocol_plugins/openflow/src/main/java/org/opendaylight/controller/protocol_plugin/openflow/internal/OFStatisticsManager.java
index 635a8bc..3ab38cc 100644 (file)
--- a/opendaylight/protocol_plugins/openflow/src/main/java/org/opendaylight/controller/protocol_plugin/openflow/internal/OFStatisticsManager.java
+++ b/opendaylight/protocol_plugins/openflow/src/main/java/org/opendaylight/controller/protocol_plugin/openflow/internal/OFStatisticsManager.java
@@ -98,7 +98,7 @@ IInventoryShimExternalListener, CommandProvider {
     private ConcurrentMap<Long, Boolean> switchSupportsVendorExtStats;
     // Per port sampled (every portStatsPeriod) transmit rate
     private Map<Long, Map<Short, TxRates>> txRates;
-    private Set<IOFStatisticsListener> statisticsListeners;
+    private Set<IOFStatisticsListener> statisticsListeners = new CopyOnWriteArraySet<IOFStatisticsListener>();
 
     /**
      * The object containing the latest factoredSamples tx rate samples for a
@@ -198,7 +198,6 @@ IInventoryShimExternalListener, CommandProvider {
         switchPortStatsUpdated = new LinkedBlockingQueue<Long>(INITIAL_SIZE);
         switchSupportsVendorExtStats = new ConcurrentHashMap<Long, Boolean>(INITIAL_SIZE);
         txRates = new HashMap<Long, Map<Short, TxRates>>(INITIAL_SIZE);
-        statisticsListeners = new CopyOnWriteArraySet<IOFStatisticsListener>();
 
         configStatsPollIntervals();
 
@@ -252,6 +251,7 @@ IInventoryShimExternalListener, CommandProvider {
      *
      */
     void destroy() {
+        statisticsListeners.clear();
     }
 
     /**

diff --git a/opendaylight/protocol_plugins/openflow/src/main/java/org/opendaylight/controller/protocol_plugin/openflow/internal/ReadService.java b/opendaylight/protocol_plugins/openflow/src/main/java/org/opendaylight/controller/protocol_plugin/openflow/internal/ReadService.java
index 84f7dd9..215216e 100644 (file)
--- a/opendaylight/protocol_plugins/openflow/src/main/java/org/opendaylight/controller/protocol_plugin/openflow/internal/ReadService.java
+++ b/opendaylight/protocol_plugins/openflow/src/main/java/org/opendaylight/controller/protocol_plugin/openflow/internal/ReadService.java
@@ -39,7 +39,7 @@ public class ReadService implements IPluginInReadService, IReadFilterInternalLis
     private static final Logger logger = LoggerFactory
             .getLogger(ReadService.class);
     private IReadServiceFilter filter;
-    private Set<IPluginOutReadService> pluginOutReadServices;
+    private Set<IPluginOutReadService> pluginOutReadServices = new CopyOnWriteArraySet<IPluginOutReadService>();
     private String containerName;
     private IPluginOutConnectionService connectionOutService;
 
@@ -51,9 +51,7 @@ public class ReadService implements IPluginInReadService, IReadFilterInternalLis
     @SuppressWarnings("unchecked")
     void init(Component c) {
         Dictionary<Object, Object> props = c.getServiceProperties();
-        containerName = (props != null) ? (String) props.get("containerName")
-                : null;
-        pluginOutReadServices = new CopyOnWriteArraySet<IPluginOutReadService>();
+        containerName = (props != null) ? (String) props.get("containerName") : null;
     }
 
     /**
@@ -63,6 +61,7 @@ public class ReadService implements IPluginInReadService, IReadFilterInternalLis
      *
      */
     void destroy() {
+        pluginOutReadServices.clear();
     }
 
     /**

diff --git a/opendaylight/protocol_plugins/openflow/src/main/java/org/opendaylight/controller/protocol_plugin/openflow/internal/ReadServiceFilter.java b/opendaylight/protocol_plugins/openflow/src/main/java/org/opendaylight/controller/protocol_plugin/openflow/internal/ReadServiceFilter.java
index 2c8708f..b877eb0 100644 (file)
--- a/opendaylight/protocol_plugins/openflow/src/main/java/org/opendaylight/controller/protocol_plugin/openflow/internal/ReadServiceFilter.java
+++ b/opendaylight/protocol_plugins/openflow/src/main/java/org/opendaylight/controller/protocol_plugin/openflow/internal/ReadServiceFilter.java
@@ -64,7 +64,8 @@ public class ReadServiceFilter implements IReadServiceFilter, IContainerListener
     private ConcurrentMap<String, Set<Node>> containerToNode;
     private ConcurrentMap<String, Set<NodeTable>> containerToNt;
     private ConcurrentMap<String, Set<ContainerFlow>> containerFlows;
-    private ConcurrentMap<String, IReadFilterInternalListener> readFilterInternalListeners;
+    private ConcurrentMap<String, IReadFilterInternalListener> readFilterInternalListeners =
+        new ConcurrentHashMap<String, IReadFilterInternalListener>();
 
     public void setController(IController core) {
         this.controller = core;
@@ -119,7 +120,6 @@ public class ReadServiceFilter implements IReadServiceFilter, IContainerListener
         containerToNt = new ConcurrentHashMap<String, Set<NodeTable>>();
         containerToNode = new ConcurrentHashMap<String, Set<Node>>();
         containerFlows = new ConcurrentHashMap<String, Set<ContainerFlow>>();
-        readFilterInternalListeners = new ConcurrentHashMap<String, IReadFilterInternalListener>();
     }
 
     /**
@@ -129,6 +129,7 @@ public class ReadServiceFilter implements IReadServiceFilter, IContainerListener
      *
      */
     void destroy() {
+        readFilterInternalListeners.clear();
     }
 
     /**

diff --git a/opendaylight/sal/implementation/src/main/java/org/opendaylight/controller/sal/implementation/internal/ReadService.java b/opendaylight/sal/implementation/src/main/java/org/opendaylight/controller/sal/implementation/internal/ReadService.java
index 869dc81..bdd5460 100644 (file)
--- a/opendaylight/sal/implementation/src/main/java/org/opendaylight/controller/sal/implementation/internal/ReadService.java
+++ b/opendaylight/sal/implementation/src/main/java/org/opendaylight/controller/sal/implementation/internal/ReadService.java
@@ -59,8 +59,10 @@ import org.slf4j.LoggerFactory;
 public class ReadService implements IReadService, CommandProvider, IPluginOutReadService {
 
     protected static final Logger logger = LoggerFactory.getLogger(ReadService.class);
-    private ConcurrentHashMap<String, IPluginInReadService> pluginReader;
-    private Set<IReadServiceListener> readerListeners;
+    private ConcurrentHashMap<String, IPluginInReadService> pluginReader =
+        new ConcurrentHashMap<String, IPluginInReadService>();
+    private Set<IReadServiceListener> readerListeners =
+        new CopyOnWriteArraySet<IReadServiceListener>();
 
     /**
      * Function called by the dependency manager when all the required
@@ -68,8 +70,6 @@ public class ReadService implements IReadService, CommandProvider, IPluginOutRea
      *
      */
     void init() {
-        pluginReader = new ConcurrentHashMap<String, IPluginInReadService>();
-        readerListeners = new CopyOnWriteArraySet<IReadServiceListener>();
     }
 
     /**
@@ -82,6 +82,7 @@ public class ReadService implements IReadService, CommandProvider, IPluginOutRea
         // In case of plugin disactivating make sure we clear the
         // dependencies
         this.pluginReader.clear();
+        this.readerListeners.clear();
     }
 
     /**