8189387: ConcurrentLinkedDeque linearizability continued ...
authordl
Thu, 09 Nov 2017 16:10:46 -0800
changeset 47729 1563167c9520
parent 47728 0a65c8231efa
child 47730 c7b5b1ce8145
8189387: ConcurrentLinkedDeque linearizability continued ... Reviewed-by: martin, psandoz, dholmes
src/java.base/share/classes/java/util/concurrent/ConcurrentLinkedDeque.java
test/jdk/java/util/concurrent/tck/ConcurrentLinkedDequeTest.java
--- a/src/java.base/share/classes/java/util/concurrent/ConcurrentLinkedDeque.java	Thu Nov 09 16:07:21 2017 -0800
+++ b/src/java.base/share/classes/java/util/concurrent/ConcurrentLinkedDeque.java	Thu Nov 09 16:10:46 2017 -0800
@@ -695,8 +695,9 @@
      * 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 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 @@
                         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 @@
                         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;
+                }
             }
         }
     }
--- a/test/jdk/java/util/concurrent/tck/ConcurrentLinkedDequeTest.java	Thu Nov 09 16:07:21 2017 -0800
+++ b/test/jdk/java/util/concurrent/tck/ConcurrentLinkedDequeTest.java	Thu Nov 09 16:10:46 2017 -0800
@@ -936,6 +936,14 @@
         }
     }
 
+    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 @@
                             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 @@
                             x, nulls.sum(), zeros.sum()));
             };
 
-            Runnable adder = () -> {
-                d.addLast(0);
-                d.addFirst(42);
-            };
+            Runnable adder = () -> { d.addLast(0); d.addFirst(42); };
+
+            runAsync(getter, adder);
+        }
+    }
+
+    <T> T chooseRandomly(T... choices) {
+        return choices[ThreadLocalRandom.current().nextInt(choices.length)];
+    }
 
-            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();
+    /**
+     * 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);
         }
     }
 }