FIX: Concurrency problem in locator allNodes.

naver · Jul 3, 2024 · dfcd905 · dfcd905
1 parent 715f73a
commit dfcd905
Show file tree

Hide file tree

Showing 2 changed files with 12 additions and 7 deletions.
diff --git a/src/main/java/net/spy/memcached/ArcusKetamaNodeLocator.java b/src/main/java/net/spy/memcached/ArcusKetamaNodeLocator.java
@@ -41,7 +41,7 @@
 public class ArcusKetamaNodeLocator extends SpyObject implements NodeLocator {
 
   private final TreeMap<Long, SortedSet<MemcachedNode>> ketamaNodes;
-  private final Collection<MemcachedNode> allNodes;
+  private Collection<MemcachedNode> allNodes;
 
   /* ENABLE_MIGRATION if */
   private TreeMap<Long, SortedSet<MemcachedNode>> ketamaAlterNodes;
@@ -206,15 +206,16 @@ public void update(Collection<MemcachedNode> toAttach,
                      Collection<MemcachedNode> toDelete) {
     lock.lock();
     try {
+      ArrayList<MemcachedNode> newAllNodes = new ArrayList<>(allNodes);
       // Add memcached nodes.
       for (MemcachedNode node : toAttach) {
-        allNodes.add(node);
+        newAllNodes.add(node);
         insertHash(node);
       }
 
       // Remove memcached nodes.
       for (MemcachedNode node : toDelete) {
-        allNodes.remove(node);
+        newAllNodes.remove(node);
         removeHash(node);
         try {
           node.closeChannel();
@@ -223,6 +224,7 @@ public void update(Collection<MemcachedNode> toAttach,
                   "Failed to closeChannel the node : " + node);
         }
       }
+      allNodes = newAllNodes;
     } finally {
       /* ENABLE_MIGRATION if */
       if (migrationInProgress && alterNodes.isEmpty()) {

diff --git a/src/main/java/net/spy/memcached/ArcusReplKetamaNodeLocator.java b/src/main/java/net/spy/memcached/ArcusReplKetamaNodeLocator.java
@@ -32,6 +32,7 @@
 import java.util.SortedSet;
 import java.util.TreeMap;
 import java.util.TreeSet;
+import java.util.concurrent.CopyOnWriteArrayList;
 import java.util.concurrent.locks.Lock;
 import java.util.concurrent.locks.ReentrantLock;
 
@@ -42,7 +43,7 @@ public class ArcusReplKetamaNodeLocator extends SpyObject implements NodeLocator
 
   private final TreeMap<Long, SortedSet<MemcachedReplicaGroup>> ketamaGroups;
   private final HashMap<String, MemcachedReplicaGroup> allGroups;
-  private final Collection<MemcachedNode> allNodes;
+  private Collection<MemcachedNode> allNodes;
 
   /* ENABLE_MIGRATION if */
   private TreeMap<Long, SortedSet<MemcachedReplicaGroup>> ketamaAlterGroups;
@@ -65,7 +66,7 @@ public class ArcusReplKetamaNodeLocator extends SpyObject implements NodeLocator
 
   public ArcusReplKetamaNodeLocator(List<MemcachedNode> nodes) {
     super();
-    allNodes = nodes;
+    allNodes = new CopyOnWriteArrayList<>(nodes);
     ketamaGroups = new TreeMap<>();
     allGroups = new HashMap<>();
 
@@ -251,9 +252,10 @@ public void update(Collection<MemcachedNode> toAttach,
      */
     lock.lock();
     try {
+      ArrayList<MemcachedNode> newAllNodes = new ArrayList<>(allNodes);
       // Remove memcached nodes.
       for (MemcachedNode node : toDelete) {
-        allNodes.remove(node);
+        newAllNodes.remove(node);
         removeNodeFromGroup(node);
         try {
           node.closeChannel();
@@ -269,7 +271,7 @@ public void update(Collection<MemcachedNode> toAttach,
 
       // Add memcached nodes.
       for (MemcachedNode node : toAttach) {
-        allNodes.add(node);
+        newAllNodes.add(node);
         insertNodeIntoGroup(node);
       }
 
@@ -280,6 +282,7 @@ public void update(Collection<MemcachedNode> toAttach,
         removeHash(group);
       }
       toDeleteGroups.clear();
+      allNodes = newAllNodes;
     } finally {
       /* ENABLE_MIGRATION if */
       if (migrationInProgress && alterNodes.isEmpty()) {