From 2c504c4419a919ea70a80671cd4e4857ce156324 Mon Sep 17 00:00:00 2001 From: Robert Varga Date: Sat, 6 Jun 2015 03:17:38 +0200 Subject: [PATCH] BUG-3363: code optimization and cleanup - 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 (cherry picked from commit 06068449350ddee54597a38db39c678f93fe2ab3) --- .../impl/registry/flow/DeviceFlowRegistryImpl.java | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) 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 ddf48078f7..116891296a 100644 --- 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 flowRegistry = new HashMap<>(); + @GuardedBy("marks") private final Collection 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 getAllFlowDescriptors() { return Collections.unmodifiableMap(flowRegistry); -- 2.36.6