LuciferYang opened a new issue, #12378:
URL: https://github.com/apache/gluten/issues/12378
### Describe the bug
`ConsistentHash` is annotated `@ThreadSafe`, and most of its accessors
return defensive copies — e.g. `getNodes()` returns `new
HashSet<>(nodes.keySet())`. `getPartition()` is the exception: it hands back
the internal set directly.
```java
public Set<Partition<T>> getPartition(T node) {
lock.readLock().lock();
try {
return nodes.get(node); // internal mutable state, not a copy
} finally {
lock.readLock().unlock();
}
}
```
The returned set is the same instance stored in the internal `nodes` map, so
a caller can mutate the ring's state through it (e.g.
`getPartition(node).clear()`), and the reference escapes after the read lock is
released. This is inconsistent with `getNodes()` and weakens the `@ThreadSafe`
contract.
It's latent today — `getPartition()` has no production caller (only tests),
and the internal set isn't mutated after a node is added — but it's a
straightforward encapsulation fix worth making before anything starts to depend
on it.
### Expected behavior
`getPartition()` returns a defensive copy, like `getNodes()`, so callers
can't reach the ring's internal state.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
