(gluten) branch main updated: [GLUTEN-12378][CORE] Return a defensive copy from ConsistentHash.getPartition() (#12379)

[ja...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

jackylee-ch pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/gluten.git


The following commit(s) were added to refs/heads/main by this push:
     new 9dd8e4a6b8 [GLUTEN-12378][CORE] Return a defensive copy from ConsistentHash.getPartition() (#12379)
9dd8e4a6b8 is described below

commit 9dd8e4a6b8713e0da06552da5f5f11a978d6d332
Author: YangJie <ya...@baidu.com>
AuthorDate: Mon Jun 29 16:05:56 2026 +0800

    [GLUTEN-12378][CORE] Return a defensive copy from ConsistentHash.getPartition() (#12379)
---
 .../main/java/org/apache/gluten/hash/ConsistentHash.java   | 11 +++++++++--
 .../java/org/apache/gluten/hash/ConsistentHashTest.java    | 14 ++++++++++++++
 2 files changed, 23 insertions(+), 2 deletions(-)

diff --git a/gluten-core/src/main/java/org/apache/gluten/hash/ConsistentHash.java b/gluten-core/src/main/java/org/apache/gluten/hash/ConsistentHash.java
index b432049645..31fdebe6c6 100644
--- a/gluten-core/src/main/java/org/apache/gluten/hash/ConsistentHash.java
+++ b/gluten-core/src/main/java/org/apache/gluten/hash/ConsistentHash.java
@@ -162,7 +162,10 @@ public class ConsistentHash<T extends ConsistentHash.Node> {
   public Set<Partition<T>> getPartition(T node) {
     lock.readLock().lock();
     try {
-      return nodes.get(node);
+      // Return a defensive copy: the map value is internal mutable state, and getNodes() copies
+      // for the same reason. Callers get a snapshot they can't use to mutate the ring.
+      Set<Partition<T>> partitions = nodes.get(node);
+      return partitions == null ? null : new HashSet<>(partitions);
     } finally {
       lock.readLock().unlock();
     }
@@ -255,7 +258,11 @@ public class ConsistentHash<T extends ConsistentHash.Node> {
       return node;
     }
 
-    public void setSlot(long slot) {
+    // Non-public on purpose: only ConsistentHash.add() assigns the slot, during ring construction
+    // (accessible as a nestmate). Keeping it out of the public API stops callers of getPartition()
+    // from mutating ring state through a returned Partition (e.g. changing a slot so removeNode()
+    // drops the wrong entry).
+    private void setSlot(long slot) {
       this.slot = slot;
     }
 
diff --git a/gluten-core/src/test/java/org/apache/gluten/hash/ConsistentHashTest.java b/gluten-core/src/test/java/org/apache/gluten/hash/ConsistentHashTest.java
index 875fc46082..aacaade84d 100644
--- a/gluten-core/src/test/java/org/apache/gluten/hash/ConsistentHashTest.java
+++ b/gluten-core/src/test/java/org/apache/gluten/hash/ConsistentHashTest.java
@@ -163,6 +163,20 @@ public class ConsistentHashTest {
         });
   }
 
+  @Test
+  public void testGetPartitionReturnsDefensiveCopy() {
+    HostNode node = new HostNode("executor-1");
+    consistentHash.addNode(node);
+
+    Set<ConsistentHash.Partition<ConsistentHash.Node>> partitions =
+        consistentHash.getPartition(node);
+    Assert.assertEquals(REPLICAS, partitions.size());
+
+    // Mutating the returned set must not affect the ring's internal state.
+    partitions.clear();
+    Assert.assertEquals(REPLICAS, consistentHash.getPartition(node).size());
+  }
+
   private static class HostNode implements ConsistentHash.Node {
     private final String host;
 


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org