Bug 6797 - Fix deadlock on cached schema-changed notifications

Add an atomic state in NetconfDeviceCommunicator to improve the state
transition of close operation, to avoid deadlock while multiple threads
are doing the close.

Change-Id: I2f2c41b86b724eba78a74bfb96c0b499dc7f79a3
Signed-off-by: ykq <yin.kangqian@zte.com.cn>

diff --git a/netconf/sal-netconf-connector/src/main/java/org/opendaylight/netconf/sal/connect/netconf/listener/NetconfDeviceCommunicator.java b/netconf/sal-netconf-connector/src/main/java/org/opendaylight/netconf/sal/connect/netconf/listener/NetconfDeviceCommunicator.java
index 4fbd6f624145eac163c8bdb2cc61b3384782907d..93ac945f2d2c9c9cc8479796ada6a27906bab102 100644 (file)
--- a/netconf/sal-netconf-connector/src/main/java/org/opendaylight/netconf/sal/connect/netconf/listener/NetconfDeviceCommunicator.java
+++ b/netconf/sal-netconf-connector/src/main/java/org/opendaylight/netconf/sal/connect/netconf/listener/NetconfDeviceCommunicator.java
@@ -21,6 +21,7 @@ import java.util.Iterator;
 import java.util.List;
 import java.util.Queue;
 import java.util.concurrent.Semaphore;
+import java.util.concurrent.atomic.AtomicBoolean;
 import java.util.concurrent.locks.Lock;
 import java.util.concurrent.locks.ReentrantLock;
 import org.opendaylight.controller.config.util.xml.XmlElement;
@@ -63,6 +64,17 @@ public class NetconfDeviceCommunicator implements NetconfClientSessionListener,
     private Future<?> initFuture;
     private SettableFuture<NetconfDeviceCapabilities> firstConnectionFuture;
 
+    // isSessionClosing indicates a close operation on the session is issued and
+    // tearDown will surely be called later to finish the close.
+    // Used to allow only one thread to enter tearDown and other threads should
+    // NOT enter it simultaneously and should end its close operation without
+    // calling tearDown to release the locks they hold to avoid deadlock.
+    private volatile AtomicBoolean isSessionClosing = new AtomicBoolean(false);
+
+    public Boolean isSessionClosing() {
+        return isSessionClosing.get();
+    }
+
     public NetconfDeviceCommunicator(final RemoteDeviceId id, final RemoteDevice<NetconfSessionPreferences, NetconfMessage, NetconfDeviceCommunicator> remoteDevice,
             final UserPreferences NetconfSessionPreferences, final int rpcMessageLimit) {
         this(id, remoteDevice, Optional.of(NetconfSessionPreferences), rpcMessageLimit);
@@ -148,12 +160,16 @@ public class NetconfDeviceCommunicator implements NetconfClientSessionListener,
     }
 
     public void disconnect() {
-        if(session != null) {
+        // If session is already in closing, no need to close it again
+        if(session != null && isSessionClosing.compareAndSet(false, true)) {
             session.close();
         }
     }
 
     private void tearDown( String reason ) {
+        if (!isSessionClosing()) {
+            LOG.warn("It's curious that no one to close the session but tearDown is called!");
+        }
         LOG.debug("Tearing down {}", reason);
         List<UncancellableFuture<RpcResult<NetconfMessage>>> futuresToCancel = Lists.newArrayList();
         sessionLock.lock();
@@ -193,6 +209,8 @@ public class NetconfDeviceCommunicator implements NetconfClientSessionListener,
                 future.set( createErrorRpcResult( RpcError.ErrorType.TRANSPORT, reason ) );
             }
         }
+
+        isSessionClosing.set(false);
     }
 
     private RpcResult<NetconfMessage> createSessionDownRpcResult() {
@@ -207,12 +225,16 @@ public class NetconfDeviceCommunicator implements NetconfClientSessionListener,
 
     @Override
     public void onSessionDown(final NetconfClientSession session, final Exception e) {
-        LOG.warn("{}: Session went down", id, e);
-        tearDown( null );
+        // If session is already in closing, no need to call tearDown again.
+        if (isSessionClosing.compareAndSet(false, true)) {
+            LOG.warn("{}: Session went down", id, e);
+            tearDown( null );
+        }
     }
 
     @Override
     public void onSessionTerminated(final NetconfClientSession session, final NetconfTerminationReason reason) {
+        // onSessionTerminated is called directly by disconnect, no need to compare and set isSessionClosing.
         LOG.warn("{}: Session terminated {}", id, reason);
         tearDown( reason.getErrorMessage() );
     }