Bug 9205: NPE received while receiving BGP peers

Change-Id: I05383a406b03e5dc460e5ae013da82280920981e
Signed-off-by: Ajay Lele <ajayslele@gmail.com>
Signed-off-by: Claudio D. Gasparini <claudio.gasparini@pantheon.tech>

diff --git a/bgp/openconfig-state/src/main/java/org/opendaylight/protocol/bgp/state/StateProviderImpl.java b/bgp/openconfig-state/src/main/java/org/opendaylight/protocol/bgp/state/StateProviderImpl.java
index 79b3876def3b4f420d2e6afd068d488e42f21b93..c90a2283f68a94977c8357efeed37d3181f0930c 100644 (file)
--- a/bgp/openconfig-state/src/main/java/org/opendaylight/protocol/bgp/state/StateProviderImpl.java
+++ b/bgp/openconfig-state/src/main/java/org/opendaylight/protocol/bgp/state/StateProviderImpl.java
@@ -175,8 +175,9 @@ public final class StateProviderImpl implements TransactionChainListener, Cluste
     public synchronized ListenableFuture<Void> closeServiceInstance() {
         this.scheduleTask.cancel(true);
         final WriteTransaction wTx = this.transactionChain.newWriteOnlyTransaction();
-        this.instanceIdentifiersCache.keySet().forEach(ribId -> removeStoredOperationalState(ribId, wTx));
-        final CheckedFuture<Void, TransactionCommitFailedException> futureDelete = wTx.submit();
+        this.instanceIdentifiersCache.keySet().iterator()
+            .forEachRemaining(ribId -> removeStoredOperationalState(ribId, wTx));
+        final ListenableFuture<Void> futureDelete = wTx.submit();
         this.transactionChain.close();
         return futureDelete;
     }

diff --git a/bgp/rib-impl/src/main/java/org/opendaylight/protocol/bgp/rib/impl/config/AppPeer.java b/bgp/rib-impl/src/main/java/org/opendaylight/protocol/bgp/rib/impl/config/AppPeer.java
index 4891504c9d80596850f386b1076515f254031f64..29ee2e1719f104ea80dbd7528aaa68b4738d4541 100644 (file)
--- a/bgp/rib-impl/src/main/java/org/opendaylight/protocol/bgp/rib/impl/config/AppPeer.java
+++ b/bgp/rib-impl/src/main/java/org/opendaylight/protocol/bgp/rib/impl/config/AppPeer.java
@@ -41,12 +41,15 @@ import org.slf4j.LoggerFactory;
 public final class AppPeer implements PeerBean, BGPPeerStateConsumer {
     private static final Logger LOG = LoggerFactory.getLogger(AppPeer.class);
     private static final QName APP_ID_QNAME = QName.create(ApplicationRib.QNAME, "id").intern();
+    @GuardedBy("this")
     private Neighbor currentConfiguration;
+    @GuardedBy("this")
     private BgpAppPeerSingletonService bgpAppPeerSingletonService;
+    @GuardedBy("this")
     private ServiceRegistration<?> serviceRegistration;
 
     @Override
-    public void start(final RIB rib, final Neighbor neighbor, final BGPTableTypeRegistryConsumer tableTypeRegistry,
+    public synchronized void start(final RIB rib, final Neighbor neighbor, final BGPTableTypeRegistryConsumer tableTypeRegistry,
         final WriteConfiguration configurationWriter) {
         Preconditions.checkState(this.bgpAppPeerSingletonService == null, "Previous peer instance was not closed.");
         this.currentConfiguration = neighbor;
@@ -55,18 +58,20 @@ public final class AppPeer implements PeerBean, BGPPeerStateConsumer {
     }
 
     @Override
-    public void restart(final RIB rib, final BGPTableTypeRegistryConsumer tableTypeRegistry) {
+    public synchronized void restart(final RIB rib, final BGPTableTypeRegistryConsumer tableTypeRegistry) {
         Preconditions.checkState(this.currentConfiguration != null);
         start(rib, this.currentConfiguration, tableTypeRegistry, null);
     }
 
     @Override
-    public void close() {
-        try {
-            this.bgpAppPeerSingletonService.close();
-            this.bgpAppPeerSingletonService = null;
-        } catch (final Exception e) {
-            LOG.warn("Failed to close application peer instance", e);
+    public synchronized void close() {
+        if (this.bgpAppPeerSingletonService != null) {
+            try {
+                this.bgpAppPeerSingletonService.close();
+                this.bgpAppPeerSingletonService = null;
+            } catch (final Exception e) {
+                LOG.warn("Failed to close application peer instance", e);
+            }
         }
         if (this.serviceRegistration != null) {
             this.serviceRegistration.unregister();
@@ -75,7 +80,7 @@ public final class AppPeer implements PeerBean, BGPPeerStateConsumer {
     }
 
     @Override
-    public ListenableFuture<Void> closeServiceInstance() {
+    public synchronized ListenableFuture<Void> closeServiceInstance() {
         if (this.bgpAppPeerSingletonService != null) {
             return this.bgpAppPeerSingletonService.closeServiceInstance();
         }
@@ -102,7 +107,7 @@ public final class AppPeer implements PeerBean, BGPPeerStateConsumer {
         return this.bgpAppPeerSingletonService.getPeerState();
     }
 
-    void setServiceRegistration(final ServiceRegistration<?> serviceRegistration) {
+    synchronized void setServiceRegistration(final ServiceRegistration<?> serviceRegistration) {
         this.serviceRegistration = serviceRegistration;
     }
 
@@ -175,4 +180,4 @@ public final class AppPeer implements PeerBean, BGPPeerStateConsumer {
             return this.applicationPeer.getPeerState();
         }
     }
-}
\ No newline at end of file
+}

diff --git a/bgp/rib-impl/src/main/java/org/opendaylight/protocol/bgp/rib/impl/config/BgpPeer.java b/bgp/rib-impl/src/main/java/org/opendaylight/protocol/bgp/rib/impl/config/BgpPeer.java
index 5c888ca47466d88098eaed0ea13fa6f4b0e83df2..1cdf75052777f6099ab6ae7b388fd4e2299e1dc4 100644 (file)
--- a/bgp/rib-impl/src/main/java/org/opendaylight/protocol/bgp/rib/impl/config/BgpPeer.java
+++ b/bgp/rib-impl/src/main/java/org/opendaylight/protocol/bgp/rib/impl/config/BgpPeer.java
@@ -72,8 +72,11 @@ public final class BgpPeer implements PeerBean, BGPPeerStateConsumer, BGPPeerRun
     private static final Logger LOG = LoggerFactory.getLogger(BgpPeer.class);
 
     private final RpcProviderRegistry rpcRegistry;
+    @GuardedBy("this")
     private ServiceRegistration<?> serviceRegistration;
+    @GuardedBy("this")
     private Neighbor currentConfiguration;
+    @GuardedBy("this")
     private BgpPeerSingletonService bgpPeerSingletonService;
 
     public BgpPeer(final RpcProviderRegistry rpcRegistry) {
@@ -81,7 +84,7 @@ public final class BgpPeer implements PeerBean, BGPPeerStateConsumer, BGPPeerRun
     }
 
     @Override
-    public void start(final RIB rib, final Neighbor neighbor, final BGPTableTypeRegistryConsumer tableTypeRegistry,
+    public synchronized void start(final RIB rib, final Neighbor neighbor, final BGPTableTypeRegistryConsumer tableTypeRegistry,
         final WriteConfiguration configurationWriter) {
         Preconditions.checkState(this.bgpPeerSingletonService == null, "Previous peer instance was not closed.");
         this.bgpPeerSingletonService = new BgpPeerSingletonService(rib, neighbor, tableTypeRegistry, configurationWriter);
@@ -89,13 +92,13 @@ public final class BgpPeer implements PeerBean, BGPPeerStateConsumer, BGPPeerRun
     }
 
     @Override
-    public void restart(final RIB rib, final BGPTableTypeRegistryConsumer tableTypeRegistry) {
+    public synchronized void restart(final RIB rib, final BGPTableTypeRegistryConsumer tableTypeRegistry) {
         Preconditions.checkState(this.currentConfiguration != null);
         start(rib, this.currentConfiguration, tableTypeRegistry, null);
     }
 
     @Override
-    public void close() {
+    public synchronized void close() {
         closeSingletonService();
         if (this.serviceRegistration != null) {
             this.serviceRegistration.unregister();
@@ -104,7 +107,7 @@ public final class BgpPeer implements PeerBean, BGPPeerStateConsumer, BGPPeerRun
     }
 
     @Override
-    public ListenableFuture<Void> closeServiceInstance() {
+    public synchronized ListenableFuture<Void> closeServiceInstance() {
         if (this.bgpPeerSingletonService != null) {
             return this.bgpPeerSingletonService.closeServiceInstance();
         }
@@ -112,11 +115,13 @@ public final class BgpPeer implements PeerBean, BGPPeerStateConsumer, BGPPeerRun
     }
 
     private void closeSingletonService() {
-        try {
-            this.bgpPeerSingletonService.close();
-            this.bgpPeerSingletonService = null;
-        } catch (final Exception e) {
-            LOG.warn("Failed to close peer instance", e);
+        if (this.bgpPeerSingletonService != null) {
+            try {
+                this.bgpPeerSingletonService.close();
+                this.bgpPeerSingletonService = null;
+            } catch (final Exception e) {
+                LOG.warn("Failed to close peer instance", e);
+            }
         }
     }
 
@@ -210,7 +215,7 @@ public final class BgpPeer implements PeerBean, BGPPeerStateConsumer, BGPPeerRun
         return this.bgpPeerSingletonService.getPeerState();
     }
 
-    void setServiceRegistration(final ServiceRegistration<?> serviceRegistration) {
+    synchronized void setServiceRegistration(final ServiceRegistration<?> serviceRegistration) {
         this.serviceRegistration = serviceRegistration;
     }