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 <[email protected]>
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: [email protected]
For additional commands, e-mail: [email protected]
