From e2cbace23d9ec6b46417511b1f671a13912b701a Mon Sep 17 00:00:00 2001
From: Doug Lea <dl@openjdk.org>
Date: Thu, 9 Nov 2017 16:10:46 -0800
Subject: [PATCH] 8189387: ConcurrentLinkedDeque linearizability continued ..

Reviewed-by: martin, psandoz, dholmes
---
 .../concurrent/ConcurrentLinkedDeque.java     | 51 +++++++------
 .../tck/ConcurrentLinkedDequeTest.java        | 73 +++++++++++++------
 2 files changed, 80 insertions(+), 44 deletions(-)

diff --git a/src/java.base/share/classes/java/util/concurrent/ConcurrentLinkedDeque.java b/src/java.base/share/classes/java/util/concurrent/ConcurrentLinkedDeque.java
index 3885370dbc9..64b6bf1fa80 100644
--- a/src/java.base/share/classes/java/util/concurrent/ConcurrentLinkedDeque.java
+++ b/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;
-        return (p == q) ? last() : q;
+        if (p == (p = p.prev))
+            p = last();
+        return p;
     }
 
     /**
@@ -867,31 +868,31 @@ public class ConcurrentLinkedDeque<E>
 
     public E peekFirst() {
         restart: for (;;) {
-            for (Node<E> first = first(), p = first;;) {
-                final E item;
-                if ((item = p.item) != null) {
-                    // recheck for linearizability
-                    if (first.prev != null) continue restart;
-                    return item;
-                }
-                if ((p = succ(p)) == null)
-                    return null;
+            E item;
+            Node<E> first = first(), p = first;
+            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;
         }
     }
 
     public E peekLast() {
         restart: for (;;) {
-            for (Node<E> last = last(), p = last;;) {
-                final E item;
-                if ((item = p.item) != null) {
-                    // recheck for linearizability
-                    if (last.next != null) continue restart;
-                    return item;
-                }
-                if ((p = pred(p)) == null)
-                    return null;
+            E item;
+            Node<E> last = last(), p = last;
+            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;
         }
     }
 
@@ -921,8 +922,11 @@ 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;
+                }
             }
         }
     }
@@ -939,8 +943,11 @@ 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;
+                }
             }
         }
     }
diff --git a/test/jdk/java/util/concurrent/tck/ConcurrentLinkedDequeTest.java b/test/jdk/java/util/concurrent/tck/ConcurrentLinkedDequeTest.java
index ad17d9b01aa..5431db36039 100644
--- a/test/jdk/java/util/concurrent/tck/ConcurrentLinkedDequeTest.java
+++ b/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 = () -> {
-                d.addFirst(0);
-                d.addLast(42);
-            };
+            Runnable adder = () -> { d.addFirst(0); d.addLast(42); };
 
-            boolean b = rnd.nextBoolean();
-            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();
+            runAsync(getter, adder);
         }
     }
 
@@ -995,18 +994,48 @@ public class ConcurrentLinkedDequeTest extends JSR166TestCase {
                             x, nulls.sum(), zeros.sum()));
             };
 
-            Runnable adder = () -> {
-                d.addLast(0);
-                d.addFirst(42);
-            };
+            Runnable adder = () -> { d.addLast(0); d.addFirst(42); };
 
-            boolean b = rnd.nextBoolean();
-            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();
+            runAsync(getter, adder);
+        }
+    }
+
+    <T> T chooseRandomly(T... choices) {
+        return choices[ThreadLocalRandom.current().nextInt(choices.length)];
+    }
+
+    /**
+     * 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);
         }
     }
 }