From d158a267998290fe1bfeac3da143b952bd7da18a Mon Sep 17 00:00:00 2001
From: Robert Varga <robert.varga@pantheon.tech>
Date: Fri, 19 May 2017 15:32:13 +0200
Subject: [PATCH] BUG-8511: add more explicit messages

This adds more defensive handling of connections and locking,
even if it should not strictly be necessary, as we are using
atomic operations and run on the actor thread. This makes the
transitions work even in fact of actor context leakage.

Change-Id: I26df0f208d63b861a0f3d3dc3c0f1959bbc79e90
Signed-off-by: Robert Varga <robert.varga@pantheon.tech>
(cherry picked from commit 90a377ec4fe7c1aa4df6d4fde74bfc8189e95b08)
---
 .../access/client/ClientActorBehavior.java    | 46 +++++++++++++++----
 1 file changed, 37 insertions(+), 9 deletions(-)

diff --git a/opendaylight/md-sal/cds-access-client/src/main/java/org/opendaylight/controller/cluster/access/client/ClientActorBehavior.java b/opendaylight/md-sal/cds-access-client/src/main/java/org/opendaylight/controller/cluster/access/client/ClientActorBehavior.java
index 5d61885cd5..ee63eddaa7 100644
--- a/opendaylight/md-sal/cds-access-client/src/main/java/org/opendaylight/controller/cluster/access/client/ClientActorBehavior.java
+++ b/opendaylight/md-sal/cds-access-client/src/main/java/org/opendaylight/controller/cluster/access/client/ClientActorBehavior.java
@@ -296,16 +296,35 @@ public abstract class ClientActorBehavior<T extends BackendInfo> extends
             conn.finishReplay(forwarder);
 
             // Make sure new lookups pick up the new connection
-            connections.replace(shard, conn, newConn);
-            LOG.info("{}: replaced connection {} with {}", persistenceId(), conn, newConn);
+            if (!connections.replace(shard, conn, newConn)) {
+                final AbstractClientConnection<T> existing = connections.get(conn.cookie());
+                LOG.warn("{}: old connection {} does not match existing {}, new connection {} in limbo",
+                    persistenceId(), conn, existing, newConn);
+            } else {
+                LOG.info("{}: replaced connection {} with {}", persistenceId(), conn, newConn);
+            }
         } finally {
             connectionsLock.unlockWrite(stamp);
         }
     }
 
     void removeConnection(final AbstractClientConnection<?> conn) {
-        connections.remove(conn.cookie(), conn);
-        LOG.debug("{}: removed connection {}", persistenceId(), conn);
+        final long stamp = connectionsLock.writeLock();
+        try {
+            if (!connections.remove(conn.cookie(), conn)) {
+                final AbstractClientConnection<T> existing = connections.get(conn.cookie());
+                if (existing != null) {
+                    LOG.warn("{}: failed to remove connection {}, as it was superseded by {}", persistenceId(), conn,
+                        existing);
+                } else {
+                    LOG.warn("{}: failed to remove connection {}, as it was not tracked", persistenceId(), conn);
+                }
+            } else {
+                LOG.info("{}: removed connection {}", persistenceId(), conn);
+            }
+        } finally {
+            connectionsLock.unlockWrite(stamp);
+        }
     }
 
     @SuppressWarnings("unchecked")
@@ -314,11 +333,20 @@ public abstract class ClientActorBehavior<T extends BackendInfo> extends
         final ReconnectingClientConnection<T> conn = (ReconnectingClientConnection<T>)newConn;
         LOG.info("{}: connection {} reconnecting as {}", persistenceId(), oldConn, newConn);
 
-        final boolean replaced = connections.replace(oldConn.cookie(), (AbstractClientConnection<T>)oldConn, conn);
-        if (!replaced) {
-            final AbstractClientConnection<T> existing = connections.get(oldConn.cookie());
-            LOG.warn("{}: old connection {} does not match existing {}, new connection {} in limbo", persistenceId(),
-                oldConn, existing, newConn);
+        final long stamp = connectionsLock.writeLock();
+        try {
+            final boolean replaced = connections.replace(oldConn.cookie(), (AbstractClientConnection<T>)oldConn, conn);
+            if (!replaced) {
+                final AbstractClientConnection<T> existing = connections.get(oldConn.cookie());
+                if (existing != null) {
+                    LOG.warn("{}: failed to replace connection {}, as it was superseded by {}", persistenceId(), conn,
+                        existing);
+                } else {
+                    LOG.warn("{}: failed to replace connection {}, as it was not tracked", persistenceId(), conn);
+                }
+            }
+        } finally {
+            connectionsLock.unlockWrite(stamp);
         }
 
         final Long shard = oldConn.cookie();
-- 
2.36.6