Fix unsynchronized access to marks

We must not iterate over marks while not holding the lock, as we risk a
concurrent modification. Mkae sure we synchronize properly.

Also minimize the synchronized sections around flowRegistry. These will
be removed in the next patchset.

Change-Id: I5575fa972aea6d8055d578da2d3418c25286668d
Signed-off-by: Robert Varga <rovarga@cisco.com>

diff --git a/openflowplugin-impl/src/main/java/org/opendaylight/openflowplugin/impl/registry/flow/DeviceFlowRegistryImpl.java b/openflowplugin-impl/src/main/java/org/opendaylight/openflowplugin/impl/registry/flow/DeviceFlowRegistryImpl.java
index ddf48078f7e6e33e909bd90426cf06acb724c3f8..116891296a72fc6f1f7285163dc2f7008d4e3552 100644 (file)
--- a/openflowplugin-impl/src/main/java/org/opendaylight/openflowplugin/impl/registry/flow/DeviceFlowRegistryImpl.java
+++ b/openflowplugin-impl/src/main/java/org/opendaylight/openflowplugin/impl/registry/flow/DeviceFlowRegistryImpl.java
@@ -13,6 +13,7 @@ import java.util.Collections;
 import java.util.HashMap;
 import java.util.HashSet;
 import java.util.Map;
+import javax.annotation.concurrent.GuardedBy;
 import org.opendaylight.openflowplugin.api.openflow.registry.flow.DeviceFlowRegistry;
 import org.opendaylight.openflowplugin.api.openflow.registry.flow.FlowDescriptor;
 import org.opendaylight.openflowplugin.api.openflow.registry.flow.FlowRegistryKey;
@@ -27,7 +28,9 @@ import org.slf4j.LoggerFactory;
  */
 public class DeviceFlowRegistryImpl implements DeviceFlowRegistry {
 
+    @GuardedBy("flowRegistry")
     private final Map<FlowRegistryKey, FlowDescriptor> flowRegistry = new HashMap<>();
+    @GuardedBy("marks")
     private final Collection<FlowRegistryKey> marks = new HashSet<>();
     private static final Logger LOG = LoggerFactory.getLogger(DeviceFlowRegistryImpl.class);
 
@@ -75,18 +78,18 @@ public class DeviceFlowRegistryImpl implements DeviceFlowRegistry {
 
     @Override
     public void removeMarked() {
-        synchronized (flowRegistry) {
+        synchronized (marks) {
             for (FlowRegistryKey flowRegistryKey : marks) {
                 LOG.trace("Removing flowDescriptor for flow hash : {}", flowRegistryKey.hashCode());
-                flowRegistry.remove(flowRegistryKey);
+                synchronized (flowRegistry) {
+                    flowRegistry.remove(flowRegistryKey);
+                }
             }
-        }
-        synchronized (marks) {
+
             marks.clear();
         }
     }
 
-
     @Override
     public Map<FlowRegistryKey, FlowDescriptor> getAllFlowDescriptors() {
         return Collections.unmodifiableMap(flowRegistry);