BGPCEP-763: Fix Pcep Deadlocked thread

improve synchronization

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

diff --git a/pcep/impl/src/main/java/org/opendaylight/protocol/pcep/impl/PCEPSessionImpl.java b/pcep/impl/src/main/java/org/opendaylight/protocol/pcep/impl/PCEPSessionImpl.java
index 8fe8525b957e4921d772a1b99dd7739a576a036b..87eff40e6e57a2928388fd766bd4980eb9295f59 100644 (file)
--- a/pcep/impl/src/main/java/org/opendaylight/protocol/pcep/impl/PCEPSessionImpl.java
+++ b/pcep/impl/src/main/java/org/opendaylight/protocol/pcep/impl/PCEPSessionImpl.java
@@ -403,9 +403,9 @@ public class PCEPSessionImpl extends SimpleChannelInboundHandler<Message> implem
     }
 
     @Override
-    public final void channelInactive(final ChannelHandlerContext ctx) {
+    public synchronized final void channelInactive(final ChannelHandlerContext ctx) {
         LOG.debug("Channel {} inactive.", ctx.channel());
-        this.endOfInput();
+        endOfInput();
 
         try {
             super.channelInactive(ctx);
@@ -415,18 +415,18 @@ public class PCEPSessionImpl extends SimpleChannelInboundHandler<Message> implem
     }
 
     @Override
-    protected final void channelRead0(final ChannelHandlerContext ctx, final Message msg) {
+    protected synchronized final void channelRead0(final ChannelHandlerContext ctx, final Message msg) {
         LOG.debug("Message was received: {}", msg);
-        this.handleMessage(msg);
+        handleMessage(msg);
     }
 
     @Override
-    public final void handlerAdded(final ChannelHandlerContext ctx) {
+    public synchronized final void handlerAdded(final ChannelHandlerContext ctx) {
         this.sessionUp();
     }
 
     @Override
-    public void exceptionCaught(final ChannelHandlerContext ctx, final Throwable cause) {
+    public  synchronized void exceptionCaught(final ChannelHandlerContext ctx, final Throwable cause) {
         handleException(cause);
     }
 

diff --git a/pcep/topology/topology-provider/src/main/java/org/opendaylight/bgpcep/pcep/topology/provider/AbstractTopologySessionListener.java b/pcep/topology/topology-provider/src/main/java/org/opendaylight/bgpcep/pcep/topology/provider/AbstractTopologySessionListener.java
index fbbc337a83a421a2db843e09a6d050404eb7f37b..cdb6212d461bc6ab8fd30d246d9e60752f315a7d 100755 (executable)
--- a/pcep/topology/topology-provider/src/main/java/org/opendaylight/bgpcep/pcep/topology/provider/AbstractTopologySessionListener.java
+++ b/pcep/topology/topology-provider/src/main/java/org/opendaylight/bgpcep/pcep/topology/provider/AbstractTopologySessionListener.java
@@ -285,15 +285,19 @@ public abstract class AbstractTopologySessionListener<S, L> implements TopologyS
         Futures.addCallback(ctx.trans.submit(), new FutureCallback<Void>() {
             @Override
             public void onSuccess(final Void result) {
-                LOG.trace("Internal state for session {} updated successfully", psession);
-                ctx.notifyRequests();
+                synchronized (AbstractTopologySessionListener.this) {
+                    LOG.trace("Internal state for session {} updated successfully", psession);
+                    ctx.notifyRequests();
+                }
             }
 
             @Override
             public void onFailure(final Throwable throwable) {
-                LOG.error("Failed to update internal state for session {}, closing it", psession, throwable);
-                ctx.notifyRequests();
-                psession.close(TerminationReason.UNKNOWN);
+                synchronized (AbstractTopologySessionListener.this) {
+                    LOG.error("Failed to update internal state for session {}, closing it", psession, throwable);
+                    ctx.notifyRequests();
+                    psession.close(TerminationReason.UNKNOWN);
+                }
             }
         }, MoreExecutors.directExecutor());
     }

diff --git a/pcep/topology/topology-provider/src/main/java/org/opendaylight/bgpcep/pcep/topology/provider/ServerSessionManager.java b/pcep/topology/topology-provider/src/main/java/org/opendaylight/bgpcep/pcep/topology/provider/ServerSessionManager.java
index 56f8f6c014434068f62b54c4ac99d57dab2a19fa..448b0ac5c94719e3ea8eec61ee72eb526956311f 100755 (executable)
--- a/pcep/topology/topology-provider/src/main/java/org/opendaylight/bgpcep/pcep/topology/provider/ServerSessionManager.java
+++ b/pcep/topology/topology-provider/src/main/java/org/opendaylight/bgpcep/pcep/topology/provider/ServerSessionManager.java
@@ -223,9 +223,13 @@ final class ServerSessionManager implements PCEPSessionListenerFactory, Topology
             LOG.error("Session Manager has already been closed.");
             return Futures.immediateFuture(null);
         }
-        this.nodes.values().iterator().forEachRemaining(TopologySessionListener::close);
+        for (final TopologySessionListener node : this.nodes.values()) {
+            node.close();
+        }
         this.nodes.clear();
-        this.state.values().iterator().forEachRemaining(TopologyNodeState::close);
+        for (final TopologyNodeState topologyNodeState : this.state.values()) {
+            topologyNodeState.close();
+        }
         this.state.clear();
 
         final WriteTransaction t = this.dependenciesProvider.getDataBroker().newWriteOnlyTransaction();

diff --git a/pcep/topology/topology-provider/src/main/java/org/opendaylight/bgpcep/pcep/topology/provider/config/PCEPTopologyDeployerImpl.java b/pcep/topology/topology-provider/src/main/java/org/opendaylight/bgpcep/pcep/topology/provider/config/PCEPTopologyDeployerImpl.java
index b8b2e1063486a518291fcc74eb887c80583a6183..34ca7ae3b56123a98a2e9ea5eddaa92e6b9cb189 100644 (file)
--- a/pcep/topology/topology-provider/src/main/java/org/opendaylight/bgpcep/pcep/topology/provider/config/PCEPTopologyDeployerImpl.java
+++ b/pcep/topology/topology-provider/src/main/java/org/opendaylight/bgpcep/pcep/topology/provider/config/PCEPTopologyDeployerImpl.java
@@ -8,7 +8,6 @@
 package org.opendaylight.bgpcep.pcep.topology.provider.config;
 
 import static java.util.Objects.requireNonNull;
-import static org.opendaylight.bgpcep.pcep.topology.provider.config.PCEPTopologyProviderUtil.closeTopology;
 import static org.opendaylight.bgpcep.pcep.topology.provider.config.PCEPTopologyProviderUtil.filterPcepTopologies;
 import static org.opendaylight.controller.md.sal.common.api.data.LogicalDatastoreType.CONFIGURATION;
 
@@ -16,6 +15,7 @@ import java.util.Collection;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
+import java.util.concurrent.TimeUnit;
 import java.util.stream.Collectors;
 import javax.annotation.concurrent.GuardedBy;
 import org.opendaylight.bgpcep.programming.spi.InstructionScheduler;
@@ -40,6 +40,7 @@ public class PCEPTopologyDeployerImpl implements ClusteredDataTreeChangeListener
 
     private static final Logger LOG = LoggerFactory.getLogger(PCEPTopologyDeployerImpl.class);
 
+    private static final long TIMEOUT_NS = TimeUnit.SECONDS.toNanos(5);
     private final BlueprintContainer container;
     private final InstanceIdentifier<NetworkTopology> networTopology;
     private final DataBroker dataBroker;
@@ -131,9 +132,25 @@ public class PCEPTopologyDeployerImpl implements ClusteredDataTreeChangeListener
             this.listenerRegistration.close();
             this.listenerRegistration = null;
         }
-        this.pcepTopologyServices.entrySet().iterator()
-                .forEachRemaining(entry -> closeTopology(entry.getValue(), entry.getKey()));
+        for (Map.Entry<TopologyId, PCEPTopologyProviderBean> entry:this.pcepTopologyServices.entrySet()) {
+            closeTopology(entry.getValue(), entry.getKey());
+        }
         this.pcepTopologyServices.clear();
         LOG.info("PCEP Topology Deployer closed");
     }
+
+
+    @SuppressWarnings("checkstyle:IllegalCatch")
+    private synchronized void closeTopology(final PCEPTopologyProviderBean topology, final TopologyId topologyId) {
+        if (topology == null) {
+            return;
+        }
+        LOG.info("Removing Topology {}", topologyId);
+        try {
+            topology.closeServiceInstance().get(TIMEOUT_NS, TimeUnit.NANOSECONDS);
+            topology.close();
+        } catch (final Exception e) {
+            LOG.error("Topology {} instance failed to close service instance", topologyId, e);
+        }
+    }
 }
\ No newline at end of file

diff --git a/pcep/topology/topology-provider/src/main/java/org/opendaylight/bgpcep/pcep/topology/provider/config/PCEPTopologyProviderUtil.java b/pcep/topology/topology-provider/src/main/java/org/opendaylight/bgpcep/pcep/topology/provider/config/PCEPTopologyProviderUtil.java
index b3198f253c4e38d97ce9e0b21be3ee433344125b..4bbcee35263b4d654c346a9dc088aaf4b8f70bc6 100644 (file)
--- a/pcep/topology/topology-provider/src/main/java/org/opendaylight/bgpcep/pcep/topology/provider/config/PCEPTopologyProviderUtil.java
+++ b/pcep/topology/topology-provider/src/main/java/org/opendaylight/bgpcep/pcep/topology/provider/config/PCEPTopologyProviderUtil.java
@@ -12,7 +12,6 @@ import java.net.InetAddress;
 import java.net.InetSocketAddress;
 import java.nio.charset.StandardCharsets;
 import java.util.Objects;
-import java.util.concurrent.TimeUnit;
 import javax.annotation.Nonnull;
 import javax.annotation.Nullable;
 import org.opendaylight.protocol.concepts.KeyMapping;
@@ -24,7 +23,6 @@ import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.controll
 import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.topology.pcep.config.rev171025.PcepNodeConfig;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.topology.pcep.rev171025.TopologyTypes1;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.topology.pcep.sync.optimizations.config.rev171025.PcepNodeSyncConfig;
-import org.opendaylight.yang.gen.v1.urn.tbd.params.xml.ns.yang.network.topology.rev131021.TopologyId;
 import org.opendaylight.yang.gen.v1.urn.tbd.params.xml.ns.yang.network.topology.rev131021.network.topology.Topology;
 import org.opendaylight.yang.gen.v1.urn.tbd.params.xml.ns.yang.network.topology.rev131021.network.topology.topology.TopologyTypes;
 import org.slf4j.Logger;
@@ -34,8 +32,6 @@ final class PCEPTopologyProviderUtil {
 
     private static final Logger LOG = LoggerFactory.getLogger(PCEPTopologyProviderUtil.class);
 
-    private static final long TIMEOUT_NS = TimeUnit.SECONDS.toNanos(5);
-
     private PCEPTopologyProviderUtil() {
         throw new UnsupportedOperationException();
     }
@@ -73,19 +69,6 @@ final class PCEPTopologyProviderUtil {
         return aug != null && aug.getTopologyPcep() != null;
     }
 
-    @SuppressWarnings("checkstyle:IllegalCatch")
-    static void closeTopology(@Nullable final PCEPTopologyProviderBean topology, @Nonnull final TopologyId topologyId) {
-        if (topology == null) {
-            return;
-        }
-        LOG.info("Removing Topology {}", topologyId);
-        try {
-            topology.closeServiceInstance().get(TIMEOUT_NS, TimeUnit.NANOSECONDS);
-            topology.close();
-        } catch (final Exception e) {
-            LOG.error("Topology {} instance failed to close service instance", topologyId, e);
-        }
-    }
 
     static SpeakerIdMapping contructSpeakersId(final Topology topology) {
         final SpeakerIdMapping ret = SpeakerIdMapping.getSpeakerIdMap();

diff --git a/pcep/topology/topology-stats/src/main/java/org/opendaylight/bgpcep/pcep/topology/stats/provider/TopologyStatsProviderImpl.java b/pcep/topology/topology-stats/src/main/java/org/opendaylight/bgpcep/pcep/topology/stats/provider/TopologyStatsProviderImpl.java
index 458adced06733fa09ce3bcd0193e581230998622..173691c02811dd5223f607a9428f12971c59267a 100644 (file)
--- a/pcep/topology/topology-stats/src/main/java/org/opendaylight/bgpcep/pcep/topology/stats/provider/TopologyStatsProviderImpl.java
+++ b/pcep/topology/topology-stats/src/main/java/org/opendaylight/bgpcep/pcep/topology/stats/provider/TopologyStatsProviderImpl.java
@@ -89,8 +89,8 @@ public final class TopologyStatsProviderImpl implements TransactionChainListener
         final WriteTransaction wTx = this.transactionChain.newWriteOnlyTransaction();
         for (final KeyedInstanceIdentifier<Node, NodeKey> statId : this.statsMap.keySet()) {
             wTx.delete(LogicalDatastoreType.OPERATIONAL, statId);
-            wTx.submit().get();
         }
+        wTx.submit().get();
         this.statsMap.clear();
         this.transactionChain.close();
     }