From: Andrej Mak <andrej.mak@pantheon.tech>
Date: Tue, 28 Feb 2017 09:14:50 +0000 (+0100)
Subject: Bug 7812: NPE when NetconfDeviceSalProvider.close
X-Git-Tag: release/carbon~38
X-Git-Url: https://git.opendaylight.org/gerrit/gitweb?a=commitdiff_plain;h=5118ef40f4ac4cf64a2613c3b01cb90752b72071;p=netconf.git

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 ad72eeaa46..3d74d07d30 100644
--- 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 3fe01bbbfe..c0f066ffdc 100644
--- 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 7fd6b96957..b49b3fd3fb 100644
--- 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