diff --git a/dubbo-cluster/src/main/java/org/apache/dubbo/rpc/cluster/router/state/BitList.java b/dubbo-cluster/src/main/java/org/apache/dubbo/rpc/cluster/router/state/BitList.java index 013a4010e00..6fe37712213 100644 --- a/dubbo-cluster/src/main/java/org/apache/dubbo/rpc/cluster/router/state/BitList.java +++ b/dubbo-cluster/src/main/java/org/apache/dubbo/rpc/cluster/router/state/BitList.java @@ -55,7 +55,6 @@ public class BitList extends AbstractList implements Cloneable { private final BitSet rootSet; private volatile List originList; - private static final BitList emptyList = new BitList(Collections.emptyList()); private volatile List tailList = null; public BitList(List originList) { @@ -174,9 +173,12 @@ public synchronized E randomSelectOne() { return get(ThreadLocalRandom.current().nextInt(cardinality + tailSize)); } - @SuppressWarnings("unchecked") + /** + * Returns a new empty BitList. A new instance is created on each call because BitList is mutable; + * a shared singleton would cause cross-caller state contamination (see issue #16131). + */ public static BitList emptyList() { - return emptyList; + return new BitList<>(Collections.emptyList()); } // Provided by JDK List interface diff --git a/dubbo-cluster/src/test/java/org/apache/dubbo/rpc/cluster/router/state/BitListTest.java b/dubbo-cluster/src/test/java/org/apache/dubbo/rpc/cluster/router/state/BitListTest.java index 8885f445334..cb9c7b55a3e 100644 --- a/dubbo-cluster/src/test/java/org/apache/dubbo/rpc/cluster/router/state/BitListTest.java +++ b/dubbo-cluster/src/test/java/org/apache/dubbo/rpc/cluster/router/state/BitListTest.java @@ -579,6 +579,69 @@ void testClone2() { Assertions.assertEquals(2, set.size()); } + @Test + void testEmptyListReturnsIndependentInstances() { + // Verify that emptyList() returns separate instances to prevent shared mutable state + BitList empty1 = BitList.emptyList(); + BitList empty2 = BitList.emptyList(); + + Assertions.assertNotSame(empty1, empty2, "emptyList() must return distinct instances"); + } + + @Test + void testEmptyListMutationDoesNotAffectOtherEmptyList() { + // This test reproduces the bug from issue #16131: + // When emptyList() returned a shared singleton, adding elements to one + // empty list would contaminate all other empty lists. + BitList empty1 = BitList.emptyList(); + BitList empty2 = BitList.emptyList(); + + // Simulate what AbstractDirectory does: start with emptyList, then add invokers + empty1.add("InvokerA"); + empty1.add("InvokerB"); + + // empty2 must remain empty - this was the core bug + Assertions.assertTrue(empty2.isEmpty(), "Mutating one emptyList must not affect another"); + Assertions.assertEquals(0, empty2.size()); + } + + @Test + void testEmptyListTailListMutationDoesNotAffectOtherEmptyList() { + // Test that addToTailList on one emptyList doesn't leak to another + BitList empty1 = BitList.emptyList(); + BitList empty2 = BitList.emptyList(); + + empty1.addToTailList("X"); + empty1.addToTailList("Y"); + + Assertions.assertEquals(2, empty1.size()); + Assertions.assertTrue(empty2.isEmpty()); + Assertions.assertFalse(empty2.hasMoreElementInTailList()); + } + + @Test + void testEmptyListAndOperationDoesNotAffectOtherEmptyList() { + // The and() method is used in AbstractStateRouter.route() to mutate invokers in-place. + // This test verifies that and() on one emptyList doesn't contaminate another. + BitList source = new BitList<>(Arrays.asList("A", "B", "C")); + BitList empty1 = BitList.emptyList(); + BitList empty2 = BitList.emptyList(); + + // and() mutates rootSet and may add tailList elements in-place + empty1.and(source); + + Assertions.assertTrue(empty2.isEmpty(), "and() on one emptyList must not affect another"); + Assertions.assertEquals(0, empty2.size()); + } + + @Test + void testEmptyListIsInitiallyEmpty() { + BitList empty = BitList.emptyList(); + Assertions.assertTrue(empty.isEmpty()); + Assertions.assertEquals(0, empty.size()); + Assertions.assertFalse(empty.iterator().hasNext()); + } + @Test void testConcurrent() throws InterruptedException { for (int i = 0; i < 100000; i++) {