From 01ad372373854f644f648e914ed24eaec91fd783 Mon Sep 17 00:00:00 2001 From: Ajay Lele Date: Tue, 24 Mar 2020 15:43:48 -0700 Subject: [PATCH] Handle race-conditions in BGP shutdown code JIRA: BGPCEP-900 Signed-off-by: Ajay Lele Change-Id: Id50c83cd226f6786c9830b3b10d7e0eddda9653e --- .../protocol/bgp/state/StateProviderImpl.java | 5 +++-- .../impl/config/BGPClusterSingletonService.java | 15 ++++++++++++--- 2 files changed, 15 insertions(+), 5 deletions(-) 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 b2c0db760a..30eebed46b 100644 --- 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 @@ -172,8 +172,9 @@ public final class StateProviderImpl implements TransactionChainListener, AutoCl this.scheduleTask.cancel(true); if (!this.instanceIdentifiersCache.keySet().isEmpty()) { final WriteTransaction wTx = this.transactionChain.newWriteOnlyTransaction(); - this.instanceIdentifiersCache.keySet().iterator() - .forEachRemaining(ribId -> removeStoredOperationalState(ribId, wTx)); + this.instanceIdentifiersCache.values() + .forEach(bgpIID -> wTx.delete(LogicalDatastoreType.OPERATIONAL, bgpIID)); + this.instanceIdentifiersCache.clear(); wTx.commit().addCallback(new FutureCallback() { @Override public void onSuccess(final CommitInfo result) { diff --git a/bgp/rib-impl/src/main/java/org/opendaylight/protocol/bgp/rib/impl/config/BGPClusterSingletonService.java b/bgp/rib-impl/src/main/java/org/opendaylight/protocol/bgp/rib/impl/config/BGPClusterSingletonService.java index 7665fb10ed..563d97562c 100644 --- a/bgp/rib-impl/src/main/java/org/opendaylight/protocol/bgp/rib/impl/config/BGPClusterSingletonService.java +++ b/bgp/rib-impl/src/main/java/org/opendaylight/protocol/bgp/rib/impl/config/BGPClusterSingletonService.java @@ -71,6 +71,7 @@ public final class BGPClusterSingletonService implements ClusterSingletonService private final ServiceGroupIdentifier serviceGroupIdentifier; private final AtomicBoolean instantiated = new AtomicBoolean(false); private final PeerGroupConfigLoader peerGroupLoader; + @GuardedBy("this") private RibImpl ribImpl; @@ -116,8 +117,15 @@ public final class BGPClusterSingletonService implements ClusterSingletonService Futures.addCallback(futureResult, new FutureCallback>() { @Override public void onSuccess(final List result) { - done.setFuture(Futures.transform(BGPClusterSingletonService.this.ribImpl.closeServiceInstance(), - input -> null, MoreExecutors.directExecutor())); + synchronized (BGPClusterSingletonService.this) { + if (BGPClusterSingletonService.this.ribImpl != null) { + done.setFuture(Futures.transform(BGPClusterSingletonService.this.ribImpl.closeServiceInstance(), + input -> null, MoreExecutors.directExecutor())); + } else { + done.setFuture(Futures.transform(CommitInfo.emptyFluentFuture(), + input -> null, MoreExecutors.directExecutor())); + } + } } @Override @@ -179,6 +187,7 @@ public final class BGPClusterSingletonService implements ClusterSingletonService LOG.debug("RIB instance created: {}", this.ribImpl); } + @Holding("this") @SuppressWarnings("checkstyle:illegalCatch") private void closeRibService() { try { @@ -221,7 +230,7 @@ public final class BGPClusterSingletonService implements ClusterSingletonService } @Override - public void close() { + public synchronized void close() { LOG.info("BGPClusterSingletonService {} close", this.serviceGroupIdentifier.getName()); this.peers.values().iterator().forEachRemaining(PeerBean::close); this.ribImpl.close(); -- 2.36.6