Fix unsynchronized LSP counting 05/82905/1
authorRobert Varga <robert.varga@pantheon.tech>
Wed, 3 Jul 2019 08:40:26 +0000 (10:40 +0200)
committerRobert Varga <robert.varga@pantheon.tech>
Wed, 3 Jul 2019 08:47:07 +0000 (10:47 +0200)
Trying to copy values out of lspData without holding the lock
can result it ConcurrentModificationException and was broken
by the fix for BGPCEP-845.

Restore the lock for the duration of the copy and count the items
outside of the lock.

JIRA: BGPCEP-875
Change-Id: Ic6e839cac51a73c0812aabb58e96cb2f4959c4d6
Signed-off-by: Robert Varga <robert.varga@pantheon.tech>
pcep/topology/topology-provider/src/main/java/org/opendaylight/bgpcep/pcep/topology/provider/AbstractTopologySessionListener.java

index 212e1783d450596bd6188751915b80de36b9784b..f4026048853321e35de5644a56ba6db56110ae61 100755 (executable)
@@ -30,6 +30,7 @@ import java.util.Timer;
 import java.util.TimerTask;
 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;
@@ -437,8 +438,8 @@ public abstract class AbstractTopologySessionListener<S, L> implements TopologyS
         this.lspData.put(name, rl);
     }
 
-    private List<Path> makeBeforeBreak(final ReportedLspBuilder rlb, final ReportedLsp previous, final String name,
-            final boolean remove) {
+    private static List<Path> makeBeforeBreak(final ReportedLspBuilder rlb, final ReportedLsp previous,
+            final String name, final boolean remove) {
         // just one path should be reported
         Preconditions.checkState(rlb.getPath().size() == 1);
         final org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.rsvp.rev150820.LspId reportedLspId =
@@ -594,14 +595,19 @@ public abstract class AbstractTopologySessionListener<S, L> implements TopologyS
 
     @Override
     public int getDelegatedLspsCount() {
-        return Math.toIntExact(ImmutableList.copyOf(this.lspData.values()).stream()
-                .map(ReportedLsp::getPath).filter(Objects::nonNull).filter(pathList -> !pathList.isEmpty())
-                // pick the first path, as delegate status should be same in each path
-                .map(pathList -> pathList.get(0))
-                .map(path -> path.augmentation(Path1.class)).filter(Objects::nonNull)
-                .map(LspObject::getLsp).filter(Objects::nonNull)
-                .filter(Lsp::isDelegate)
-                .count());
+        final Stream<ReportedLsp> stream;
+        synchronized (this) {
+            stream = ImmutableList.copyOf(this.lspData.values()).stream();
+        }
+
+        return Math.toIntExact(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.get(0))
+            .map(path -> path.augmentation(Path1.class)).filter(Objects::nonNull)
+            .map(LspObject::getLsp).filter(Objects::nonNull)
+            .filter(Lsp::isDelegate)
+            .count());
     }
 
     @Override