Fix race condition when registering services 25/825/1
authorGiovanni Meo <gmeo@cisco.com>
Wed, 7 Aug 2013 09:35:29 +0000 (11:35 +0200)
committerGiovanni Meo <gmeo@cisco.com>
Wed, 7 Aug 2013 09:35:29 +0000 (11:35 +0200)
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>
opendaylight/forwardingrulesmanager/implementation/src/main/java/org/opendaylight/controller/forwardingrulesmanager/internal/ForwardingRulesManager.java
opendaylight/protocol_plugins/openflow/src/main/java/org/opendaylight/controller/protocol_plugin/openflow/internal/OFStatisticsManager.java
opendaylight/protocol_plugins/openflow/src/main/java/org/opendaylight/controller/protocol_plugin/openflow/internal/ReadService.java
opendaylight/protocol_plugins/openflow/src/main/java/org/opendaylight/controller/protocol_plugin/openflow/internal/ReadServiceFilter.java
opendaylight/sal/implementation/src/main/java/org/opendaylight/controller/sal/implementation/internal/ReadService.java

index cade2666883c66b4f8ca3e838b9b60524903ce88..836c12749f0b94c6cce567910f60b5f0af797d81 100644 (file)
@@ -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();
     }
 
     /**
index 635a8bce7c3ef847b7c20ff1ee293e140087d05d..3ab38cc41f6db686b273864801c238f3cb36eb11 100644 (file)
@@ -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();
     }
 
     /**
index 84f7dd9282ca89cc67eb160a6465712c2b489d8b..215216e25f00a8fb3f9538d59902d9daa5e86357 100644 (file)
@@ -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();
     }
 
     /**
index 2c8708f20ee46295972a6b18b280df436f009dfc..b877eb0ee793626d2990e24855052653466aa4ba 100644 (file)
@@ -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();
     }
 
     /**
index 869dc81f8db8993ddf097556c793ff5017a8a64b..bdd546055c22ce29228d587b0a7cc266a7a700e6 100644 (file)
@@ -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();
     }
 
     /**