BGPCEP-760: Fix Dead lock

when connecting multiple peers at
the same time multiple routes are announced.
Fix by improve sync under Peer export groups.

Change-Id: I588db1bfc3cfca80308837084ac5088b8fc89cea
Signed-off-by: Claudio D. Gasparini <claudio.gasparini@pantheon.tech>

diff --git a/bgp/rib-impl/src/main/java/org/opendaylight/protocol/bgp/rib/impl/ExportPolicyPeerTrackerImpl.java b/bgp/rib-impl/src/main/java/org/opendaylight/protocol/bgp/rib/impl/ExportPolicyPeerTrackerImpl.java
index 2d200406352f340ef83f5acfa4674a5a50c3c7dc..5fec91ad6d34819fbac4ab3becc5bcb2fccba5cd 100644 (file)
--- a/bgp/rib-impl/src/main/java/org/opendaylight/protocol/bgp/rib/impl/ExportPolicyPeerTrackerImpl.java
+++ b/bgp/rib-impl/src/main/java/org/opendaylight/protocol/bgp/rib/impl/ExportPolicyPeerTrackerImpl.java
@@ -13,6 +13,7 @@ import java.util.EnumMap;
 import java.util.HashMap;
 import java.util.Map;
 import java.util.Optional;
+import java.util.function.BiConsumer;
 import javax.annotation.concurrent.GuardedBy;
 import javax.annotation.concurrent.ThreadSafe;
 import org.opendaylight.protocol.bgp.rib.impl.spi.PeerExportGroupRegistry;
@@ -26,17 +27,18 @@ import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.bgp.rib.
 import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.bgp.rib.rev171207.SimpleRoutingPolicy;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.bgp.rib.rev171207.rib.TablesKey;
 import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier;
+import org.opendaylight.yangtools.yang.data.api.schema.ContainerNode;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
 /**
  * There is one ExportPolicyPeerTracker per table
- *  - peerTables: keep track of registered peers, the ones which support this table.
- *  - peerTables: flag indicates whether the structure of the peer has been created, and therefore it can start
- *  to be updated.
- *  - peerAddPathTables: keeps track of peer which supports Additional Path for this table and which Add Path
- *  configuration they are using.
- *  - groups: Contains peers grouped by peerRole and therefore sharing the same export policy.
+ * - peerTables: keep track of registered peers, the ones which support this table.
+ * - peerTables: flag indicates whether the structure of the peer has been created, and therefore it can start
+ * to be updated.
+ * - peerAddPathTables: keeps track of peer which supports Additional Path for this table and which Add Path
+ * configuration they are using.
+ * - groups: Contains peers grouped by peerRole and therefore sharing the same export policy.
  */
 @ThreadSafe
 final class ExportPolicyPeerTrackerImpl implements ExportPolicyPeerTracker {
@@ -139,4 +141,64 @@ final class ExportPolicyPeerTrackerImpl implements ExportPolicyPeerTracker {
     public synchronized boolean isTableStructureInitialized(final PeerId peerId) {
         return this.peerTables.get(peerId);
     }
+
+    final class PeerExportGroupImpl implements PeerExportGroupRegistry {
+        @GuardedBy("this")
+        private final Map<PeerId, PeerExporTuple> peers = new HashMap<>();
+        private final AbstractExportPolicy policy;
+
+        public PeerExportGroupImpl(final AbstractExportPolicy policy) {
+            this.policy = requireNonNull(policy);
+        }
+
+        @Override
+        public ContainerNode effectiveAttributes(final PeerRole role, final ContainerNode attributes) {
+            return attributes == null || role == null ? null : this.policy.effectiveAttributes(role, attributes);
+        }
+
+        @Override
+        public boolean containsPeer(final PeerId routePeerId) {
+            synchronized (this.peers) {
+                return this.peers.containsKey(routePeerId);
+            }
+        }
+
+        @Override
+        public void forEach(final BiConsumer<PeerId, YangInstanceIdentifier> action) {
+            synchronized (ExportPolicyPeerTrackerImpl.this) {
+                synchronized (this.peers) {
+                    for (final Map.Entry<PeerId, PeerExporTuple> pid : this.peers.entrySet()) {
+                        action.accept(pid.getKey(), pid.getValue().getYii());
+                    }
+                }
+            }
+        }
+
+        @Override
+        public AbstractRegistration registerPeer(final PeerId peerId, final PeerExporTuple peerExporTuple) {
+            synchronized (ExportPolicyPeerTrackerImpl.this) {
+                synchronized (this.peers) {
+                    this.peers.put(peerId, peerExporTuple);
+                }
+
+                return new AbstractRegistration() {
+                    @Override
+                    protected void removeRegistration() {
+                        synchronized (ExportPolicyPeerTrackerImpl.this) {
+                            synchronized (PeerExportGroupImpl.this.peers) {
+                                PeerExportGroupImpl.this.peers.remove(peerId);
+                            }
+                        }
+                    }
+                };
+            }
+        }
+
+        @Override
+        public boolean isEmpty() {
+            synchronized (this.peers) {
+                return this.peers.isEmpty();
+            }
+        }
+    }
 }
\ No newline at end of file

diff --git a/bgp/rib-impl/src/main/java/org/opendaylight/protocol/bgp/rib/impl/PeerExportGroupImpl.java b/bgp/rib-impl/src/main/java/org/opendaylight/protocol/bgp/rib/impl/PeerExportGroupImpl.java
deleted file mode 100644 (file)
index 251f220..0000000
--- a/bgp/rib-impl/src/main/java/org/opendaylight/protocol/bgp/rib/impl/PeerExportGroupImpl.java
+++ /dev/null
@@ -1,75 +0,0 @@
-/*
- * Copyright (c) 2015 Cisco Systems, Inc. and others.  All rights reserved.
- *
- * This program and the accompanying materials are made available under the
- * terms of the Eclipse Public License v1.0 which accompanies this distribution,
- * and is available at http://www.eclipse.org/legal/epl-v10.html
- */
-package org.opendaylight.protocol.bgp.rib.impl;
-
-import static java.util.Objects.requireNonNull;
-
-import java.util.HashMap;
-import java.util.Map;
-import java.util.function.BiConsumer;
-import javax.annotation.concurrent.GuardedBy;
-import org.opendaylight.protocol.bgp.rib.impl.spi.PeerExportGroupRegistry;
-import org.opendaylight.protocol.concepts.AbstractRegistration;
-import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.bgp.rib.rev171207.PeerId;
-import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.bgp.rib.rev171207.PeerRole;
-import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier;
-import org.opendaylight.yangtools.yang.data.api.schema.ContainerNode;
-
-final class PeerExportGroupImpl implements PeerExportGroupRegistry {
-    @GuardedBy("this")
-    private final Map<PeerId, PeerExporTuple> peers = new HashMap<>();
-    private final AbstractExportPolicy policy;
-
-    public PeerExportGroupImpl(final AbstractExportPolicy policy) {
-        this.policy = requireNonNull(policy);
-    }
-
-    @Override
-    public ContainerNode effectiveAttributes(final PeerRole role, final ContainerNode attributes) {
-        return attributes == null || role == null ? null : this.policy.effectiveAttributes(role, attributes);
-    }
-
-    @Override
-    public boolean containsPeer(final PeerId routePeerId) {
-        synchronized (this.peers) {
-            return this.peers.containsKey(routePeerId);
-        }
-    }
-
-    @Override
-    public void forEach(final BiConsumer<PeerId, YangInstanceIdentifier> action) {
-        synchronized (this.peers) {
-            for (final Map.Entry<PeerId, PeerExporTuple> pid : this.peers.entrySet()) {
-                action.accept(pid.getKey(), pid.getValue().getYii());
-            }
-        }
-    }
-
-    @Override
-    public AbstractRegistration registerPeer(final PeerId peerId, final PeerExporTuple peerExporTuple) {
-        synchronized (this.peers) {
-            this.peers.put(peerId, peerExporTuple);
-        }
-
-        return new AbstractRegistration() {
-            @Override
-            protected void removeRegistration() {
-                synchronized (PeerExportGroupImpl.this.peers) {
-                    PeerExportGroupImpl.this.peers.remove(peerId);
-                }
-            }
-        };
-    }
-
-    @Override
-    public boolean isEmpty() {
-        synchronized (this.peers) {
-            return this.peers.isEmpty();
-        }
-    }
-}
\ No newline at end of file