8343426: ConcurrentSkipListMap.spliterator() can no longer split the …

…stream Co-authored-by: Doug Lea <[email protected]> Reviewed-by: vklang
offamitkumar · Nov 14, 2024 · 2b57f40 · 2b57f40
1 parent a8152bd
commit 2b57f40
Show file tree

Hide file tree

Showing 2 changed files with 40 additions and 9 deletions.
diff --git a/src/java.base/share/classes/java/util/concurrent/ConcurrentSkipListMap.java b/src/java.base/share/classes/java/util/concurrent/ConcurrentSkipListMap.java
@@ -3221,14 +3221,14 @@ public final Comparator<? super K> getComparator() {
     }
     // factory method for KeySpliterator
     final KeySpliterator<K,V> keySpliterator() {
-        Index<K,V> h; Node<K,V> n; long est;
+        Index<K,V> h; Node<K,V> hn, n; long est;
         VarHandle.acquireFence();
-        if ((h = head) == null) {
+        if ((h = head) == null || (hn = h.node) == null) {
             n = null;
             est = 0L;
         }
         else {
-            n = h.node;
+            n = hn.next;
             est = getAdderCount();
         }
         return new KeySpliterator<K,V>(comparator, h, n, null, est);
@@ -3307,14 +3307,14 @@ public int characteristics() {
 
     // Almost the same as keySpliterator()
     final ValueSpliterator<K,V> valueSpliterator() {
-        Index<K,V> h; Node<K,V> n; long est;
+        Index<K,V> h; Node<K,V> hn, n; long est;
         VarHandle.acquireFence();
-        if ((h = head) == null) {
+        if ((h = head) == null || (hn = h.node) == null) {
             n = null;
             est = 0L;
         }
         else {
-            n = h.node;
+            n = hn.next;
             est = getAdderCount();
         }
         return new ValueSpliterator<K,V>(comparator, h, n, null, est);
@@ -3411,14 +3411,14 @@ public final Comparator<Map.Entry<K,V>> getComparator() {
 
     // Almost the same as keySpliterator()
     final EntrySpliterator<K,V> entrySpliterator() {
-        Index<K,V> h; Node<K,V> n; long est;
+        Index<K,V> h; Node<K,V> hn, n; long est;
         VarHandle.acquireFence();
-        if ((h = head) == null) {
+        if ((h = head) == null || (hn = h.node) == null) {
             n = null;
             est = 0L;
         }
         else {
-            n = h.node;
+            n = hn.next;
             est = getAdderCount();
         }
         return new EntrySpliterator<K,V>(comparator, h, n, null, est);

diff --git a/test/jdk/java/util/concurrent/tck/ConcurrentSkipListMapTest.java b/test/jdk/java/util/concurrent/tck/ConcurrentSkipListMapTest.java
@@ -1323,6 +1323,37 @@ private int lastAscending() {
         }
     }
 
+    /**
+     * Spliterators.trySplit() returns null with an empty map and non-null when
+     * it is not empty.
+     */
+    public void testSpliterators() {
+        // To test JDK-8343426
+        ConcurrentSkipListMap<Integer, Integer> map = new ConcurrentSkipListMap<>();
+        for (int i = 1; i <= 1000; i++) map.put(i, i);
+        // ensure that the splits do happen
+        assertNotNull(map.keySet().spliterator().trySplit());
+        assertNotNull(map.entrySet().spliterator().trySplit());
+        assertNotNull(map.values().spliterator().trySplit());
+        // ensure that the splits return *all* the items in the set
+        assertEquals(500_500, map.keySet()
+                .parallelStream()
+                .mapToInt(Integer::valueOf)
+                .sum());
+        assertEquals(500_500, map.values()
+                .parallelStream()
+                .mapToInt(Integer::valueOf)
+                .sum());
+        assertEquals(500_500 * 2, map.entrySet()
+                .parallelStream()
+                .mapToInt(entry -> entry.getKey() + entry.getValue())
+                .sum());
+        map.clear();
+        assertNull(map.keySet().spliterator().trySplit());
+        assertNull(map.entrySet().spliterator().trySplit());
+        assertNull(map.values().spliterator().trySplit());
+    }
+
     static void assertEq(Item i, int j) {
         if (i == null)
             mustEqual(j, -1);