spring/spring-framework
1
0
Fork 0
mirror of https://github.com/spring-projects/spring-framework.git synced 2026-09-25 00:09:02 +00:00
Code Issues Projects Releases Wiki Activity

Fix Spliterator characteristics in ConcurrentReferenceHashMap

Browse Source 
The Spliterators returned by values, entrySet, and keySet incorrectly
reported the SIZED characteristic, instead of CONCURRENT. This could
lead to bugs when the map is concurrently modified during a stream
operation.

For keySet and values, the incorrect characteristics are inherited from
AbstractMap, so to rectify that the respective methods are overridden,
and custom collections are provided that report the correct Spliterator
characteristics.

Closes gh-35817

Signed-off-by: Patrick Strawderman <pstrawderman@netflix.com>
This commit is contained in:
Patrick Strawderman
2025-11-17 11:55:38 +01:00
committed by Sam Brannen
parent 4756a4be23
commit ed75906834
2 changed files with 338 additions and 16 deletions
Download Patch File Download Diff File Expand all files Collapse all files
spring-core/src/test/java/org/springframework/util/ConcurrentReferenceHashMapTests.java
+181 -13
View File
@@ -16,8 +16,8 @@
package org.springframework.util;
import java.util.ArrayList;
import java.util.Comparator;
import java.util.AbstractMap;
import java.util.Collection;
import java.util.HashMap;
import java.util.HashSet;
import java.util.Iterator;
@@ -25,6 +25,8 @@ import java.util.LinkedList;
import java.util.List;
import java.util.Map;
import java.util.Set;
import java.util.Spliterator;
import java.util.stream.Collectors;
import org.jspecify.annotations.Nullable;
import org.junit.jupiter.api.Test;
@@ -32,12 +34,12 @@ import org.junit.jupiter.api.Test;
import org.springframework.util.ConcurrentReferenceHashMap.Entry;
import org.springframework.util.ConcurrentReferenceHashMap.Reference;
import org.springframework.util.ConcurrentReferenceHashMap.Restructure;
import org.springframework.util.comparator.Comparators;
import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException;
import static org.assertj.core.api.Assertions.assertThatIllegalStateException;
import static org.assertj.core.api.Assertions.assertThatNoException;
import static org.assertj.core.api.Assertions.assertThatThrownBy;
/**
* Tests for {@link ConcurrentReferenceHashMap}.
@@ -47,8 +49,6 @@ import static org.assertj.core.api.Assertions.assertThatNoException;
*/
class ConcurrentReferenceHashMapTests {
private static final Comparator<? super String> NULL_SAFE_STRING_SORT = Comparators.nullsLow();
private TestWeakConcurrentCache<Integer, String> map = new TestWeakConcurrentCache<>();
@@ -450,19 +450,176 @@ class ConcurrentReferenceHashMapTests {
assertThat(this.map.keySet()).isEqualTo(expected);
}
@Test
void keySetContains() {
this.map.put(123, "123");
this.map.put(456, null);
this.map.put(null, "789");
assertThat(this.map.keySet()).containsExactlyInAnyOrder(123, 456, null);
}
@Test
void keySetRemove() {
this.map.put(123, "123");
this.map.put(456, null);
this.map.put(null, "789");
assertThat(this.map.keySet().remove(123)).isTrue();
assertThat(this.map).doesNotContainKey(123);
assertThat(this.map.keySet().remove(123)).isFalse();
}
@Test
void keySetIterator() {
this.map.put(123, "123");
this.map.put(456, null);
this.map.put(null, "789");
Iterator<Integer> it = this.map.keySet().iterator();
assertThat(it).toIterable().containsExactlyInAnyOrder(123, 456, null);
assertThat(it).isExhausted();
}
@Test
void keySetIteratorRemove() {
this.map.put(123, "123");
this.map.put(456, null);
this.map.put(null, "789");
Iterator<Integer> keySetIterator = this.map.keySet().iterator();
while (keySetIterator.hasNext()) {
Integer key = keySetIterator.next();
if (key != null && key.equals(456)) {
keySetIterator.remove();
}
}
assertThat(this.map).containsOnlyKeys(123, null);
}
@Test
void keySetClear() {
this.map.put(123, "123");
this.map.put(456, null);
this.map.put(null, "789");
this.map.keySet().clear();
assertThat(this.map).isEmpty();
assertThat(this.map.keySet()).isEmpty();
}
@Test
void keySetAdd() {
assertThatThrownBy(() ->
this.map.keySet().add(12345)
).isInstanceOf(UnsupportedOperationException.class);
}
@Test
void keySetStream() {
this.map.put(123, "123");
this.map.put(456, null);
this.map.put(null, "789");
Set<Integer> keys = this.map.keySet().stream().collect(Collectors.toSet());
assertThat(keys).containsExactlyInAnyOrder(123, 456, null);
}
@Test
void keySetSpliteratorCharacteristics() {
this.map.put(123, "123");
this.map.put(456, null);
this.map.put(null, "789");
Spliterator<Integer> spliterator = this.map.keySet().spliterator();
assertThat(spliterator).hasOnlyCharacteristics(Spliterator.CONCURRENT, Spliterator.DISTINCT);
assertThat(spliterator.estimateSize()).isEqualTo(3L);
assertThat(spliterator.getExactSizeIfKnown()).isEqualTo(-1L);
}
@Test
void valuesCollection() {
this.map.put(123, "123");
this.map.put(456, null);
this.map.put(null, "789");
List<String> actual = new ArrayList<>(this.map.values());
List<String> expected = new ArrayList<>();
expected.add("123");
expected.add(null);
expected.add("789");
actual.sort(NULL_SAFE_STRING_SORT);
expected.sort(NULL_SAFE_STRING_SORT);
assertThat(actual).isEqualTo(expected);
assertThat(this.map.values()).containsExactlyInAnyOrder("123", null, "789");
}
@Test
void valuesCollectionAdd() {
assertThatThrownBy(() ->
this.map.values().add("12345")
).isInstanceOf(UnsupportedOperationException.class);
}
@Test
void valuesCollectionClear() {
Collection<String> values = this.map.values();
this.map.put(123, "123");
this.map.put(456, null);
this.map.put(null, "789");
assertThat(values).isNotEmpty();
values.clear();
assertThat(values).isEmpty();
assertThat(this.map).isEmpty();
}
@Test
void valuesCollectionRemoval() {
Collection<String> values = this.map.values();
assertThat(values).isEmpty();
this.map.put(123, "123");
this.map.put(456, null);
this.map.put(null, "789");
assertThat(values).containsExactlyInAnyOrder("123", null, "789");
values.remove(null);
assertThat(values).containsExactlyInAnyOrder("123", "789");
assertThat(map).containsOnly(new AbstractMap.SimpleEntry<>(123, "123"), new AbstractMap.SimpleEntry<>(null, "789"));
values.remove("123");
values.remove("789");
assertThat(values).isEmpty();
assertThat(map).isEmpty();
}
@Test
void valuesCollectionIterator() {
Iterator<String> iterator = this.map.values().iterator();
assertThat(iterator).isExhausted();
this.map.put(123, "123");
this.map.put(456, null);
this.map.put(null, "789");
iterator = this.map.values().iterator();
assertThat(iterator).toIterable().containsExactlyInAnyOrder("123", null, "789");
}
@Test
void valuesCollectionIteratorRemoval() {
this.map.put(123, "123");
this.map.put(456, null);
this.map.put(null, "789");
Iterator<String> iterator = this.map.values().iterator();
while (iterator.hasNext()) {
String value = iterator.next();
if (value != null && value.equals("789")) {
iterator.remove();
}
}
assertThat(iterator).isExhausted();
assertThat(this.map.values()).containsExactlyInAnyOrder("123", null);
assertThat(this.map).containsOnlyKeys(123, 456);
}
@Test
void valuesCollectionStream() {
this.map.put(123, "123");
this.map.put(456, null);
this.map.put(null, "789");
List<String> values = this.map.values().stream().collect(Collectors.toList());
assertThat(values).containsExactlyInAnyOrder("123", null, "789");
}
@Test
void valuesCollectionSpliteratorCharacteristics() {
this.map.put(123, "123");
this.map.put(456, null);
this.map.put(null, "789");
Spliterator<String> spliterator = this.map.values().spliterator();
assertThat(spliterator).hasOnlyCharacteristics(Spliterator.CONCURRENT);
assertThat(spliterator.estimateSize()).isEqualTo(3L);
assertThat(spliterator.getExactSizeIfKnown()).isEqualTo(-1L);
}
@Test
@@ -541,6 +698,17 @@ class ConcurrentReferenceHashMapTests {
copy.forEach(entry -> assertThat(entrySet).doesNotContain(entry));
}
@Test
void entrySetSpliteratorCharacteristics() {
this.map.put(1, "1");
this.map.put(2, "2");
this.map.put(3, "3");
Spliterator<Map.Entry<Integer, String>> spliterator = this.map.entrySet().spliterator();
assertThat(spliterator).hasOnlyCharacteristics(Spliterator.CONCURRENT, Spliterator.DISTINCT);
assertThat(spliterator.estimateSize()).isEqualTo(3L);
assertThat(spliterator.getExactSizeIfKnown()).isEqualTo(-1L);
}
@Test
void supportNullReference() {
// GC could happen during restructure so we must be able to create a reference for a null entry