From bc03e21eca162341732c4fe42a09e96fd94cd450 Mon Sep 17 00:00:00 2001
From: Robert Varga <robert.varga@pantheon.tech>
Date: Mon, 19 Nov 2018 20:20:20 +0100
Subject: [PATCH] Do not leak implementation CompletableFutures

Rather than LockListener performing a lookup and modification of
CompletableFuture, expose a package-private method to remove the
lock -- thus properly encapsulating the operation.

Change-Id: Ib968b42c2e032aed49125ceac523435d7a1b41c6
Signed-off-by: Robert Varga <robert.varga@pantheon.tech>
---
 .../genius/lockmanager/impl/LockListener.java       | 12 +-----------
 .../lockmanager/impl/LockManagerServiceImpl.java    | 13 +++++++++++--
 2 files changed, 12 insertions(+), 13 deletions(-)

diff --git a/lockmanager/lockmanager-impl/src/main/java/org/opendaylight/genius/lockmanager/impl/LockListener.java b/lockmanager/lockmanager-impl/src/main/java/org/opendaylight/genius/lockmanager/impl/LockListener.java
index 5503aa828..6e7aee14a 100644
--- a/lockmanager/lockmanager-impl/src/main/java/org/opendaylight/genius/lockmanager/impl/LockListener.java
+++ b/lockmanager/lockmanager-impl/src/main/java/org/opendaylight/genius/lockmanager/impl/LockListener.java
@@ -7,7 +7,6 @@
  */
 package org.opendaylight.genius.lockmanager.impl;
 
-import java.util.concurrent.CompletableFuture;
 import javax.annotation.Nonnull;
 import javax.annotation.PreDestroy;
 import javax.inject.Inject;
@@ -39,16 +38,7 @@ public class LockListener extends AbstractClusteredAsyncDataTreeChangeListener<L
 
     @Override
     public void remove(@Nonnull InstanceIdentifier<Lock> instanceIdentifier, @Nonnull Lock removedLock) {
-        String lockName = removedLock.getLockName();
-        LOG.debug("Received remove for lock {} : {}", lockName, removedLock);
-        CompletableFuture<Void> lock = lockManager.getSynchronizerForLock(lockName);
-        if (lock != null) {
-            // FindBugs flags a false violation here - "passes a null value as the parameter of a method which must be
-            // non-null. Either this parameter has been explicitly marked as @Nonnull, or analysis has determined that
-            // this parameter is always dereferenced.". However neither is true. The type param is Void so you have to
-            // pas null.
-            lock.complete(null);
-        }
+        lockManager.removeLock(removedLock);
     }
 
     @Override
diff --git a/lockmanager/lockmanager-impl/src/main/java/org/opendaylight/genius/lockmanager/impl/LockManagerServiceImpl.java b/lockmanager/lockmanager-impl/src/main/java/org/opendaylight/genius/lockmanager/impl/LockManagerServiceImpl.java
index 1b5feb41f..deb84bb04 100644
--- a/lockmanager/lockmanager-impl/src/main/java/org/opendaylight/genius/lockmanager/impl/LockManagerServiceImpl.java
+++ b/lockmanager/lockmanager-impl/src/main/java/org/opendaylight/genius/lockmanager/impl/LockManagerServiceImpl.java
@@ -120,8 +120,17 @@ public class LockManagerServiceImpl implements LockManagerService {
             }), unused -> new UnlockOutputBuilder().build(), MoreExecutors.directExecutor())).build();
     }
 
-    public CompletableFuture<Void> getSynchronizerForLock(String lockName) {
-        return lockSynchronizerMap.get(lockName);
+    void removeLock(final Lock removedLock) {
+        final String lockName = removedLock.getLockName();
+        LOG.debug("Received remove for lock {} : {}", lockName, removedLock);
+        CompletableFuture<Void> lock = lockSynchronizerMap.get(lockName);
+        if (lock != null) {
+            // FindBugs flags a false violation here - "passes a null value as the parameter of a method which must be
+            // non-null. Either this parameter has been explicitly marked as @Nonnull, or analysis has determined that
+            // this parameter is always dereferenced.". However neither is true. The type param is Void so you have to
+            // pas null.
+            lock.complete(null);
+        }
     }
 
     /**
-- 
2.36.6