Use lock in topology node writer

Prevents race conditions from akka callbacks.
Prevent NPE's when deleting node that's not connected.

Change-Id: I0e46083fe98c88cae7dc836c98f9eb4e22026579
Signed-off-by: Tomas Cere <tcere@cisco.com>

diff --git a/opendaylight/netconf/abstract-topology/src/main/java/org/opendaylight/netconf/topology/util/NodeRoleChangeStrategy.java b/opendaylight/netconf/abstract-topology/src/main/java/org/opendaylight/netconf/topology/util/NodeRoleChangeStrategy.java
index 3cafd35b55f73391d35ac712c65cbc1eba9c85fa..4783404b54149555ddab4affde1fed29dfe95342 100644 (file)
--- a/opendaylight/netconf/abstract-topology/src/main/java/org/opendaylight/netconf/topology/util/NodeRoleChangeStrategy.java
+++ b/opendaylight/netconf/abstract-topology/src/main/java/org/opendaylight/netconf/topology/util/NodeRoleChangeStrategy.java
@@ -61,10 +61,14 @@ public class NodeRoleChangeStrategy implements RoleChangeStrategy, EntityOwnersh
     @Override
     public void unregisterRoleCandidate() {
         LOG.debug("Unregistering role candidate");
-        candidateRegistration.close();
-        candidateRegistration = null;
-        ownershipListenerRegistration.close();
-        ownershipListenerRegistration = null;
+        if (candidateRegistration != null) {
+            candidateRegistration.close();
+            candidateRegistration = null;
+        }
+        if (ownershipListenerRegistration != null) {
+            ownershipListenerRegistration.close();
+            ownershipListenerRegistration = null;
+        }
     }
 
     @Override

diff --git a/opendaylight/netconf/netconf-topology/src/main/java/org/opendaylight/netconf/topology/impl/TopologyNodeWriter.java b/opendaylight/netconf/netconf-topology/src/main/java/org/opendaylight/netconf/topology/impl/TopologyNodeWriter.java
index 563ce977b290cd34fa757c557019c15f34396998..54ef71f2b626ece1afadd55e6c5e8993a4324a67 100644 (file)
--- a/opendaylight/netconf/netconf-topology/src/main/java/org/opendaylight/netconf/topology/impl/TopologyNodeWriter.java
+++ b/opendaylight/netconf/netconf-topology/src/main/java/org/opendaylight/netconf/topology/impl/TopologyNodeWriter.java
@@ -12,6 +12,7 @@ import com.google.common.base.Preconditions;
 import com.google.common.util.concurrent.CheckedFuture;
 import com.google.common.util.concurrent.FutureCallback;
 import com.google.common.util.concurrent.Futures;
+import java.util.concurrent.locks.ReentrantLock;
 import javax.annotation.Nonnull;
 import org.opendaylight.controller.md.sal.binding.api.BindingTransactionChain;
 import org.opendaylight.controller.md.sal.binding.api.DataBroker;
@@ -48,6 +49,8 @@ public class TopologyNodeWriter implements NodeWriter{
     private final InstanceIdentifier<NetworkTopology> networkTopologyPath;
     private final KeyedInstanceIdentifier<Topology, TopologyKey> topologyListPath;
 
+    private final ReentrantLock lock = new ReentrantLock(true);
+
     public TopologyNodeWriter(final String topologyId, final DataBroker dataBroker) {
         this.topologyId = topologyId;
         this.txChain = Preconditions.checkNotNull(dataBroker).createTransactionChain(new TransactionChainListener() {
@@ -70,49 +73,65 @@ public class TopologyNodeWriter implements NodeWriter{
 
     @Override
     public void init(@Nonnull NodeId id, @Nonnull Node operationalDataNode) {
-        final WriteTransaction writeTx = txChain.newWriteOnlyTransaction();
-
-        createNetworkTopologyIfNotPresent(writeTx);
-        final InstanceIdentifier<Node> path = createBindingPathForTopology(new NodeKey(id), topologyId);
-
-        LOG.trace("{}: Init device state transaction {} putting if absent operational data started. Putting data on path {}",
-                id.getValue(), writeTx.getIdentifier(), path);
-        writeTx.put(LogicalDatastoreType.OPERATIONAL, path, operationalDataNode);
-        LOG.trace("{}: Init device state transaction {} putting operational data ended.",
-                id.getValue(), writeTx.getIdentifier());
-
-        commitTransaction(writeTx, "init", id);
+        lock.lock();
+        try {
+            final WriteTransaction writeTx = txChain.newWriteOnlyTransaction();
+
+            createNetworkTopologyIfNotPresent(writeTx);
+            final InstanceIdentifier<Node> path = createBindingPathForTopology(new NodeKey(id), topologyId);
+
+            LOG.trace("{}: Init device state transaction {} putting if absent operational data started. Putting data on path {}",
+                    id.getValue(), writeTx.getIdentifier(), path);
+            writeTx.put(LogicalDatastoreType.OPERATIONAL, path, operationalDataNode);
+            LOG.trace("{}: Init device state transaction {} putting operational data ended.",
+                    id.getValue(), writeTx.getIdentifier());
+
+            commitTransaction(writeTx, "init", id);
+        } finally {
+            lock.unlock();
+        }
     }
 
     @Override
     public void update(@Nonnull NodeId id, @Nonnull Node operationalDataNode) {
-        final WriteTransaction writeTx = txChain.newWriteOnlyTransaction();
+        lock.lock();
+        try {
+            final WriteTransaction writeTx = txChain.newWriteOnlyTransaction();
+
+            final InstanceIdentifier<Node> path = createBindingPathForTopology(new NodeKey(id), topologyId);
+            LOG.trace("{}: Update device state transaction {} merging operational data started. Putting data on path {}",
+                    id, writeTx.getIdentifier(), operationalDataNode);
+            writeTx.put(LogicalDatastoreType.OPERATIONAL, path, operationalDataNode);
+            LOG.trace("{}: Update device state transaction {} merging operational data ended.",
+                    id, writeTx.getIdentifier());
+
+            commitTransaction(writeTx, "update", id);
+        } finally {
+            lock.unlock();
+        }
 
-        final InstanceIdentifier<Node> path = createBindingPathForTopology(new NodeKey(id), topologyId);
-        LOG.trace("{}: Update device state transaction {} merging operational data started. Putting data on path {}",
-                id, writeTx.getIdentifier(), operationalDataNode);
-        writeTx.put(LogicalDatastoreType.OPERATIONAL, path, operationalDataNode);
-        LOG.trace("{}: Update device state transaction {} merging operational data ended.",
-                id, writeTx.getIdentifier());
-
-        commitTransaction(writeTx, "update", id);
     }
 
     @Override
     public void delete(@Nonnull NodeId id) {
-        final WriteTransaction writeTx = txChain.newWriteOnlyTransaction();
-
-        final InstanceIdentifier<Node> path = createBindingPathForTopology(new NodeKey(id), topologyId);
-
-        LOG.trace(
-                "{}: Close device state transaction {} removing all data started. Path: {}",
-                id, writeTx.getIdentifier(), path);
-        writeTx.delete(LogicalDatastoreType.OPERATIONAL, path);
-        LOG.trace(
-                "{}: Close device state transaction {} removing all data ended.",
-                id, writeTx.getIdentifier());
-
-        commitTransaction(writeTx, "close", id);
+        lock.lock();
+        try {
+            final WriteTransaction writeTx = txChain.newWriteOnlyTransaction();
+
+            final InstanceIdentifier<Node> path = createBindingPathForTopology(new NodeKey(id), topologyId);
+
+            LOG.trace(
+                    "{}: Close device state transaction {} removing all data started. Path: {}",
+                    id, writeTx.getIdentifier(), path);
+            writeTx.delete(LogicalDatastoreType.OPERATIONAL, path);
+            LOG.trace(
+                    "{}: Close device state transaction {} removing all data ended.",
+                    id, writeTx.getIdentifier());
+
+            commitTransaction(writeTx, "close", id);
+        } finally {
+            lock.unlock();
+        }
     }
 
     private void commitTransaction(final WriteTransaction transaction, final String txType, final NodeId id) {