Bug 7812: NPE when NetconfDeviceSalProvider.close

Clustered connector close logic could be called twice on master.
Add boolean guard field to prevent this.

Add null check to netconf device sal provider close method
to prevent NPE.

Change-Id: Ib84be162826726169fb254a933781bb39dce604a
Signed-off-by: Andrej Mak <andrej.mak@pantheon.tech>

diff --git a/netconf/netconf-topology-singleton/src/main/java/org/opendaylight/netconf/topology/singleton/impl/NetconfTopologyContext.java b/netconf/netconf-topology-singleton/src/main/java/org/opendaylight/netconf/topology/singleton/impl/NetconfTopologyContext.java
index ad72eeaa4603e10ba7278e6185a6f4b24d12a951..3d74d07d308efbf27b47c4731d4ef40842041829 100644 (file)
--- a/netconf/netconf-topology-singleton/src/main/java/org/opendaylight/netconf/topology/singleton/impl/NetconfTopologyContext.java
+++ b/netconf/netconf-topology-singleton/src/main/java/org/opendaylight/netconf/topology/singleton/impl/NetconfTopologyContext.java
@@ -43,6 +43,7 @@ class NetconfTopologyContext implements ClusterSingletonService {
     private RemoteDeviceConnector remoteDeviceConnector;
     private NetconfNodeManager netconfNodeManager;
     private boolean finalClose = false;
+    private boolean closed = false;
     private boolean isMaster;
 
     private ActorRef masterActorRef;
@@ -95,13 +96,7 @@ class NetconfTopologyContext implements ClusterSingletonService {
             // in case that master changes role to slave, new NodeDeviceManager must be created and listener registered
             netconfNodeManager = createNodeDeviceManager();
         }
-        if (masterActorRef != null) {
-            netconfTopologyDeviceSetup.getActorSystem().stop(masterActorRef);
-            masterActorRef = null;
-        }
-        if (remoteDeviceConnector != null) {
-            remoteDeviceConnector.stopRemoteDeviceConnection();
-        }
+        stopDeviceConnectorAndActor();
 
         return Futures.immediateCheckedFuture(null);
     }
@@ -127,15 +122,8 @@ class NetconfTopologyContext implements ClusterSingletonService {
         if (netconfNodeManager != null) {
             netconfNodeManager.close();
         }
+        stopDeviceConnectorAndActor();
 
-        if (remoteDeviceConnector != null) {
-            remoteDeviceConnector.stopRemoteDeviceConnection();
-        }
-
-        if (masterActorRef != null) {
-            netconfTopologyDeviceSetup.getActorSystem().stop(masterActorRef);
-            masterActorRef = null;
-        }
     }
 
     /**
@@ -171,4 +159,19 @@ class NetconfTopologyContext implements ClusterSingletonService {
             }, netconfTopologyDeviceSetup.getActorSystem().dispatcher());
         }
     }
+
+    private synchronized void stopDeviceConnectorAndActor() {
+        if (closed) {
+            return;
+        }
+        if (remoteDeviceConnector != null) {
+            remoteDeviceConnector.stopRemoteDeviceConnection();
+        }
+
+        if (masterActorRef != null) {
+            netconfTopologyDeviceSetup.getActorSystem().stop(masterActorRef);
+            masterActorRef = null;
+        }
+        closed = true;
+    }
 }

diff --git a/netconf/sal-netconf-connector/src/main/java/org/opendaylight/netconf/sal/connect/netconf/sal/NetconfDeviceSalProvider.java b/netconf/sal-netconf-connector/src/main/java/org/opendaylight/netconf/sal/connect/netconf/sal/NetconfDeviceSalProvider.java
index 3fe01bbbfeb8992b14212289a7847375959e7fc9..c0f066ffdc07f8a27dc099b7d15eeca65d0606d8 100644 (file)
--- a/netconf/sal-netconf-connector/src/main/java/org/opendaylight/netconf/sal/connect/netconf/sal/NetconfDeviceSalProvider.java
+++ b/netconf/sal-netconf-connector/src/main/java/org/opendaylight/netconf/sal/connect/netconf/sal/NetconfDeviceSalProvider.java
@@ -110,9 +110,13 @@ public class NetconfDeviceSalProvider implements AutoCloseable, Provider, Bindin
 
     public void close() throws Exception {
         mountInstance.close();
-        topologyDatastoreAdapter.close();
+        if (topologyDatastoreAdapter != null) {
+            topologyDatastoreAdapter.close();
+        }
         topologyDatastoreAdapter = null;
-        txChain.close();
+        if (txChain != null) {
+            txChain.close();
+        }
     }
 
     public static final class MountInstance implements AutoCloseable {

diff --git a/netconf/sal-netconf-connector/src/test/java/org/opendaylight/netconf/sal/connect/netconf/sal/NetconfDeviceSalProviderTest.java b/netconf/sal-netconf-connector/src/test/java/org/opendaylight/netconf/sal/connect/netconf/sal/NetconfDeviceSalProviderTest.java
index 7fd6b9695783907299fea373b148012116d7af27..b49b3fd3fbe93091133a8db3ced5b983479059ba 100644 (file)
--- a/netconf/sal-netconf-connector/src/test/java/org/opendaylight/netconf/sal/connect/netconf/sal/NetconfDeviceSalProviderTest.java
+++ b/netconf/sal-netconf-connector/src/test/java/org/opendaylight/netconf/sal/connect/netconf/sal/NetconfDeviceSalProviderTest.java
@@ -94,4 +94,12 @@ public class NetconfDeviceSalProviderTest {
         verify(chain).close();
     }
 
+    @Test
+    public void closeWithoutNPE() throws Exception {
+        provider.onSessionInitiated(session);
+        provider.onSessionInitiated(context);
+        provider.close();
+        provider.close();
+        verify(chain, times(2)).close();
+    }
 }
\ No newline at end of file