From: Giovanni Meo Date: Wed, 7 Aug 2013 09:35:29 +0000 (+0200) Subject: Fix race condition when registering services X-Git-Tag: releasepom-0.1.0~218^2 X-Git-Url: https://git.opendaylight.org/gerrit/gitweb?p=controller.git;a=commitdiff_plain;h=0d1d688ac55acc55c3909c52c2cdb940cfb3764f 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 --- 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 cade266688..836c12749f 100644 --- 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 inactiveFlows; private IContainer container; - private Set frmAware; + private Set frmAware = + Collections.synchronizedSet(new HashSet()); 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()); 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 635a8bce7c..3ab38cc41f 100644 --- 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 switchSupportsVendorExtStats; // Per port sampled (every portStatsPeriod) transmit rate private Map> txRates; - private Set statisticsListeners; + private Set statisticsListeners = new CopyOnWriteArraySet(); /** * The object containing the latest factoredSamples tx rate samples for a @@ -198,7 +198,6 @@ IInventoryShimExternalListener, CommandProvider { switchPortStatsUpdated = new LinkedBlockingQueue(INITIAL_SIZE); switchSupportsVendorExtStats = new ConcurrentHashMap(INITIAL_SIZE); txRates = new HashMap>(INITIAL_SIZE); - statisticsListeners = new CopyOnWriteArraySet(); 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 84f7dd9282..215216e25f 100644 --- 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 pluginOutReadServices; + private Set pluginOutReadServices = new CopyOnWriteArraySet(); private String containerName; private IPluginOutConnectionService connectionOutService; @@ -51,9 +51,7 @@ public class ReadService implements IPluginInReadService, IReadFilterInternalLis @SuppressWarnings("unchecked") void init(Component c) { Dictionary props = c.getServiceProperties(); - containerName = (props != null) ? (String) props.get("containerName") - : null; - pluginOutReadServices = new CopyOnWriteArraySet(); + 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 2c8708f20e..b877eb0ee7 100644 --- 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> containerToNode; private ConcurrentMap> containerToNt; private ConcurrentMap> containerFlows; - private ConcurrentMap readFilterInternalListeners; + private ConcurrentMap readFilterInternalListeners = + new ConcurrentHashMap(); public void setController(IController core) { this.controller = core; @@ -119,7 +120,6 @@ public class ReadServiceFilter implements IReadServiceFilter, IContainerListener containerToNt = new ConcurrentHashMap>(); containerToNode = new ConcurrentHashMap>(); containerFlows = new ConcurrentHashMap>(); - readFilterInternalListeners = new ConcurrentHashMap(); } /** @@ -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 869dc81f8d..bdd546055c 100644 --- 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 pluginReader; - private Set readerListeners; + private ConcurrentHashMap pluginReader = + new ConcurrentHashMap(); + private Set readerListeners = + new CopyOnWriteArraySet(); /** * 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(); - readerListeners = new CopyOnWriteArraySet(); } /** @@ -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(); } /**