Prevent deadlock between PCEP session mgmt and stats thread 85/92085/2
authorAjay Lele <ajayslele@gmail.com>
Tue, 26 May 2020 17:59:57 +0000 (10:59 -0700)
committerRobert Varga <nite@hq.sk>
Thu, 5 Nov 2020 16:58:58 +0000 (16:58 +0000)
Deadlock happens between 2 threads trying to acquire lock on
Stateful07TopologySessionListener and TopologyStatsProviderImpl
instance in reverse order. In AbstractTopologySessionListener#getDelegatedLspsCount(),
locking was added to avoid ConcurrentModificationException for
lspData map. Patch achieves same result without having to acquire
the lock by using thread-safe ConcurrentHashMap for lspData.

JIRA: BGPCEP-901
Change-Id: I74b5070c2e1c8075f5df9504d6eef4de541597fd
Signed-off-by: Ajay Lele <ajayslele@gmail.com>
Signed-off-by: Deepthi V V <dvv@luminanetworks.com>
(cherry picked from commit b481a55c539be1987b303cbfab9fb190b56828dc)

pcep/topology/topology-provider/src/main/java/org/opendaylight/bgpcep/pcep/topology/provider/AbstractTopologySessionListener.java

index ff0ffc9255bf7dc9ce805708f880b55873cc64bd..79b41532efde65340229f4723033d7798f1efcc1 100644 (file)
@@ -9,7 +9,6 @@ package org.opendaylight.bgpcep.pcep.topology.provider;
 
 import static java.util.Objects.requireNonNull;
 
-import com.google.common.collect.ImmutableList;
 import com.google.common.collect.Iterables;
 import com.google.common.util.concurrent.FluentFuture;
 import com.google.common.util.concurrent.FutureCallback;
@@ -29,9 +28,9 @@ import java.util.Objects;
 import java.util.Optional;
 import java.util.Timer;
 import java.util.TimerTask;
+import java.util.concurrent.ConcurrentHashMap;
 import java.util.concurrent.TimeUnit;
 import java.util.concurrent.atomic.AtomicBoolean;
-import java.util.stream.Stream;
 import org.checkerframework.checker.lock.qual.GuardedBy;
 import org.checkerframework.checker.lock.qual.Holding;
 import org.opendaylight.bgpcep.pcep.topology.provider.session.stats.SessionStateImpl;
@@ -104,7 +103,7 @@ public abstract class AbstractTopologySessionListener<S, L> implements TopologyS
     @GuardedBy("this")
     private final Map<S, PCEPRequest> requests = new HashMap<>();
     @GuardedBy("this")
-    private final Map<String, ReportedLsp> lspData = new HashMap<>();
+    private final Map<String, ReportedLsp> lspData = new ConcurrentHashMap<>();
     private final ServerSessionManager serverSessionManager;
     private InstanceIdentifier<PathComputationClient> pccIdentifier;
     @GuardedBy("this")
@@ -598,12 +597,7 @@ public abstract class AbstractTopologySessionListener<S, L> implements TopologyS
 
     @Override
     public int getDelegatedLspsCount() {
-        final Stream<ReportedLsp> stream;
-        synchronized (this) {
-            stream = ImmutableList.copyOf(this.lspData.values()).stream();
-        }
-
-        return Math.toIntExact(stream
+        return Math.toIntExact(this.lspData.values().stream()
             .map(ReportedLsp::getPath).filter(pathList -> pathList != null && !pathList.isEmpty())
             // pick the first path, as delegate status should be same in each path
             .map(pathList -> pathList.values().iterator().next())