Skip to content

Commit c862f97

Browse files
committed
fix: return new BitList instance from emptyList() to prevent shared mutable state
BitList.emptyList() previously returned a shared static singleton instance. Since BitList is mutable, modifications to one caller's "empty" list would contaminate all other callers. This caused cross-interface invoker leakage in AbstractDirectory, leading to NoSuchMethodError when traffic was routed to instances of a previous interface. The fix returns a new BitList instance on each call, consistent with how mutable collections should behave. Fixes #16131
1 parent 63fe8c3 commit c862f97

File tree

2 files changed

+68
-3
lines changed
  • dubbo-cluster/src

2 files changed

+68
-3
lines changed

dubbo-cluster/src/main/java/org/apache/dubbo/rpc/cluster/router/state/BitList.java

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,6 @@
5555
public class BitList<E> extends AbstractList<E> implements Cloneable {
5656
private final BitSet rootSet;
5757
private volatile List<E> originList;
58-
private static final BitList emptyList = new BitList(Collections.emptyList());
5958
private volatile List<E> tailList = null;
6059

6160
public BitList(List<E> originList) {
@@ -174,9 +173,12 @@ public synchronized E randomSelectOne() {
174173
return get(ThreadLocalRandom.current().nextInt(cardinality + tailSize));
175174
}
176175

177-
@SuppressWarnings("unchecked")
176+
/**
177+
* Returns a new empty BitList. A new instance is created on each call because BitList is mutable;
178+
* a shared singleton would cause cross-caller state contamination (see issue #16131).
179+
*/
178180
public static <T> BitList<T> emptyList() {
179-
return emptyList;
181+
return new BitList<>(Collections.emptyList());
180182
}
181183

182184
// Provided by JDK List interface

dubbo-cluster/src/test/java/org/apache/dubbo/rpc/cluster/router/state/BitListTest.java

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -579,6 +579,69 @@ void testClone2() {
579579
Assertions.assertEquals(2, set.size());
580580
}
581581

582+
@Test
583+
void testEmptyListReturnsIndependentInstances() {
584+
// Verify that emptyList() returns separate instances to prevent shared mutable state
585+
BitList<String> empty1 = BitList.emptyList();
586+
BitList<String> empty2 = BitList.emptyList();
587+
588+
Assertions.assertNotSame(empty1, empty2, "emptyList() must return distinct instances");
589+
}
590+
591+
@Test
592+
void testEmptyListMutationDoesNotAffectOtherEmptyList() {
593+
// This test reproduces the bug from issue #16131:
594+
// When emptyList() returned a shared singleton, adding elements to one
595+
// empty list would contaminate all other empty lists.
596+
BitList<String> empty1 = BitList.emptyList();
597+
BitList<String> empty2 = BitList.emptyList();
598+
599+
// Simulate what AbstractDirectory does: start with emptyList, then add invokers
600+
empty1.add("InvokerA");
601+
empty1.add("InvokerB");
602+
603+
// empty2 must remain empty - this was the core bug
604+
Assertions.assertTrue(empty2.isEmpty(), "Mutating one emptyList must not affect another");
605+
Assertions.assertEquals(0, empty2.size());
606+
}
607+
608+
@Test
609+
void testEmptyListTailListMutationDoesNotAffectOtherEmptyList() {
610+
// Test that addToTailList on one emptyList doesn't leak to another
611+
BitList<String> empty1 = BitList.emptyList();
612+
BitList<String> empty2 = BitList.emptyList();
613+
614+
empty1.addToTailList("X");
615+
empty1.addToTailList("Y");
616+
617+
Assertions.assertEquals(2, empty1.size());
618+
Assertions.assertTrue(empty2.isEmpty());
619+
Assertions.assertFalse(empty2.hasMoreElementInTailList());
620+
}
621+
622+
@Test
623+
void testEmptyListAndOperationDoesNotAffectOtherEmptyList() {
624+
// The and() method is used in AbstractStateRouter.route() to mutate invokers in-place.
625+
// This test verifies that and() on one emptyList doesn't contaminate another.
626+
BitList<String> source = new BitList<>(Arrays.asList("A", "B", "C"));
627+
BitList<String> empty1 = BitList.emptyList();
628+
BitList<String> empty2 = BitList.emptyList();
629+
630+
// and() mutates rootSet and may add tailList elements in-place
631+
empty1.and(source);
632+
633+
Assertions.assertTrue(empty2.isEmpty(), "and() on one emptyList must not affect another");
634+
Assertions.assertEquals(0, empty2.size());
635+
}
636+
637+
@Test
638+
void testEmptyListIsInitiallyEmpty() {
639+
BitList<String> empty = BitList.emptyList();
640+
Assertions.assertTrue(empty.isEmpty());
641+
Assertions.assertEquals(0, empty.size());
642+
Assertions.assertFalse(empty.iterator().hasNext());
643+
}
644+
582645
@Test
583646
void testConcurrent() throws InterruptedException {
584647
for (int i = 0; i < 100000; i++) {

0 commit comments

Comments
 (0)