8189387: ConcurrentLinkedDeque linearizability continued ..

Reviewed-by: martin, psandoz, dholmes

src/java.base/share/classes/java/util/concurrent/ConcurrentLinkedDeque.java

@@ -695,8 +695,9 @@ public class ConcurrentLinkedDeque<E>
      * stale pointer that is now off the list.
      */
     final Node<E> pred(Node<E> p) {
-        Node<E> q = p.prev;
+        if (p == (p = p.prev))
-        return (p == q) ? last() : q;
+            p = last();
+        return p;
     }
 
     /**
@@ -867,32 +868,32 @@ public class ConcurrentLinkedDeque<E>
 
     public E peekFirst() {
         restart: for (;;) {
-            for (Node<E> first = first(), p = first;;) {
+            E item;
-                final E item;
+            Node<E> first = first(), p = first;
-                if ((item = p.item) != null) {
+            while ((item = p.item) == null) {
+                if (p == (p = p.next)) continue restart;
+                if (p == null)
+                    break;
+            }
             // recheck for linearizability
             if (first.prev != null) continue restart;
             return item;
         }
-                if ((p = succ(p)) == null)
-                    return null;
-            }
-        }
     }
 
     public E peekLast() {
         restart: for (;;) {
-            for (Node<E> last = last(), p = last;;) {
+            E item;
-                final E item;
+            Node<E> last = last(), p = last;
-                if ((item = p.item) != null) {
+            while ((item = p.item) == null) {
+                if (p == (p = p.prev)) continue restart;
+                if (p == null)
+                    break;
+            }
             // recheck for linearizability
             if (last.next != null) continue restart;
             return item;
         }
-                if ((p = pred(p)) == null)
-                    return null;
-            }
-        }
     }
 
     /**
@@ -921,11 +922,14 @@ public class ConcurrentLinkedDeque<E>
                         return item;
                     }
                 }
-                if ((p = succ(p)) == null)
+                if (p == (p = p.next)) continue restart;
+                if (p == null) {
+                    if (first.prev != null) continue restart;
                     return null;
                 }
             }
         }
+    }
 
     public E pollLast() {
         restart: for (;;) {
@@ -939,11 +943,14 @@ public class ConcurrentLinkedDeque<E>
                         return item;
                     }
                 }
-                if ((p = pred(p)) == null)
+                if (p == (p = p.prev)) continue restart;
+                if (p == null) {
+                    if (last.next != null) continue restart;
                     return null;
                 }
             }
         }
+    }
 
     /**
      * @throws NoSuchElementException {@inheritDoc}

test/jdk/java/util/concurrent/tck/ConcurrentLinkedDequeTest.java

@@ -936,6 +936,14 @@ public class ConcurrentLinkedDequeTest extends JSR166TestCase {
         }
     }
 
+    void runAsync(Runnable r1, Runnable r2) {
+        boolean b = ThreadLocalRandom.current().nextBoolean();
+        CompletableFuture<Void> f1 = CompletableFuture.runAsync(b ? r1 : r2);
+        CompletableFuture<Void> f2 = CompletableFuture.runAsync(b ? r2 : r1);
+        f1.join();
+        f2.join();
+    }
+
     /**
      * Non-traversing Deque operations are linearizable.
      * https://bugs.openjdk.java.net/browse/JDK-8188900
@@ -959,18 +967,9 @@ public class ConcurrentLinkedDequeTest extends JSR166TestCase {
                             x, nulls.sum(), zeros.sum()));
             };
 
-            Runnable adder = () -> {
+            Runnable adder = () -> { d.addFirst(0); d.addLast(42); };
-                d.addFirst(0);
-                d.addLast(42);
-            };
 
-            boolean b = rnd.nextBoolean();
+            runAsync(getter, adder);
-            Runnable r1 = b ? getter : adder;
-            Runnable r2 = b ? adder : getter;
-            CompletableFuture<Void> f1 = CompletableFuture.runAsync(r1);
-            CompletableFuture<Void> f2 = CompletableFuture.runAsync(r2);
-            f1.join();
-            f2.join();
         }
     }
 
@@ -995,18 +994,48 @@ public class ConcurrentLinkedDequeTest extends JSR166TestCase {
                             x, nulls.sum(), zeros.sum()));
             };
 
-            Runnable adder = () -> {
+            Runnable adder = () -> { d.addLast(0); d.addFirst(42); };
-                d.addLast(0);
-                d.addFirst(42);
-            };
 
-            boolean b = rnd.nextBoolean();
+            runAsync(getter, adder);
-            Runnable r1 = b ? getter : adder;
+        }
-            Runnable r2 = b ? adder : getter;
+    }
-            CompletableFuture<Void> f1 = CompletableFuture.runAsync(r1);
+
-            CompletableFuture<Void> f2 = CompletableFuture.runAsync(r2);
+    <T> T chooseRandomly(T... choices) {
-            f1.join();
+        return choices[ThreadLocalRandom.current().nextInt(choices.length)];
-            f2.join();
+    }
+
+    /**
+     * Non-traversing Deque operations (that return null) are linearizable.
+     * Don't return null when the deque is observably never empty.
+     * https://bugs.openjdk.java.net/browse/JDK-8189387
+     * ant -Djsr166.expensiveTests=true -Djsr166.tckTestClass=ConcurrentLinkedDequeTest -Djsr166.methodFilter=testBug8189387 tck
+     */
+    public void testBug8189387() {
+        final ThreadLocalRandom rnd = ThreadLocalRandom.current();
+        Object x = new Object();
+        for (int n = expensiveTests ? 100_000 : 10; n--> 0; ) {
+            ConcurrentLinkedDeque<Object> d = new ConcurrentLinkedDeque<>();
+            Runnable add = chooseRandomly(
+                () -> d.addFirst(x),
+                () -> d.offerFirst(x),
+                () -> d.addLast(x),
+                () -> d.offerLast(x));
+
+            Runnable get = chooseRandomly(
+                () -> assertFalse(d.isEmpty()),
+                () -> assertSame(x, d.peekFirst()),
+                () -> assertSame(x, d.peekLast()),
+                () -> assertSame(x, d.pollFirst()),
+                () -> assertSame(x, d.pollLast()));
+
+            Runnable addRemove = chooseRandomly(
+                () -> { d.addFirst(x); d.pollLast(); },
+                () -> { d.offerFirst(x); d.removeFirst(); },
+                () -> { d.offerLast(x); d.removeLast(); },
+                () -> { d.addLast(x); d.pollFirst(); });
+
+            add.run();
+            runAsync(get, addRemove);
         }
     }
 }