# HG changeset patch # User dl # Date 1516155872 28800 # Node ID 221cf830760659cde23f7f4d366477f9c8aea8e4 # Parent 20ed1cebe5f8d28ba360312030b5a79fd8b3fd94 8191483: AbstractQueuedSynchronizer cancel/cancel race Reviewed-by: martin diff -r 20ed1cebe5f8 -r 221cf8307606 src/java.base/share/classes/java/util/concurrent/locks/AbstractQueuedLongSynchronizer.java --- a/src/java.base/share/classes/java/util/concurrent/locks/AbstractQueuedLongSynchronizer.java Wed Jan 17 07:55:20 2018 +0800 +++ b/src/java.base/share/classes/java/util/concurrent/locks/AbstractQueuedLongSynchronizer.java Tue Jan 16 18:24:32 2018 -0800 @@ -320,7 +320,9 @@ // predNext is the apparent node to unsplice. CASes below will // fail if not, in which case, we lost race vs another cancel - // or signal, so no further action is necessary. + // or signal, so no further action is necessary, although with + // a possibility that a cancelled node may transiently remain + // reachable. Node predNext = pred.next; // Can use unconditional write instead of CAS here. @@ -912,13 +914,13 @@ * at any time, a {@code true} return does not guarantee that any * other thread will ever acquire. * - *

In this implementation, this operation returns in - * constant time. - * * @return {@code true} if there may be other threads waiting to acquire */ public final boolean hasQueuedThreads() { - return head != tail; + for (Node p = tail, h = head; p != h && p != null; p = p.prev) + if (p.waitStatus <= 0) + return true; + return false; } /** @@ -1067,17 +1069,21 @@ * @since 1.7 */ public final boolean hasQueuedPredecessors() { - // The correctness of this depends on head being initialized - // before tail and on head.next being accurate if the current - // thread is first in queue. - Node t = tail; // Read fields in reverse initialization order - Node h = head; - Node s; - return h != t && - ((s = h.next) == null || s.thread != Thread.currentThread()); + Node h, s; + if ((h = head) != null) { + if ((s = h.next) == null || s.waitStatus > 0) { + s = null; // traverse in case of concurrent cancellation + for (Node p = tail; p != h && p != null; p = p.prev) { + if (p.waitStatus <= 0) + s = p; + } + } + if (s != null && s.thread != Thread.currentThread()) + return true; + } + return false; } - // Instrumentation and monitoring methods /** diff -r 20ed1cebe5f8 -r 221cf8307606 src/java.base/share/classes/java/util/concurrent/locks/AbstractQueuedSynchronizer.java --- a/src/java.base/share/classes/java/util/concurrent/locks/AbstractQueuedSynchronizer.java Wed Jan 17 07:55:20 2018 +0800 +++ b/src/java.base/share/classes/java/util/concurrent/locks/AbstractQueuedSynchronizer.java Tue Jan 16 18:24:32 2018 -0800 @@ -800,7 +800,9 @@ // predNext is the apparent node to unsplice. CASes below will // fail if not, in which case, we lost race vs another cancel - // or signal, so no further action is necessary. + // or signal, so no further action is necessary, although with + // a possibility that a cancelled node may transiently remain + // reachable. Node predNext = pred.next; // Can use unconditional write instead of CAS here. @@ -1392,13 +1394,13 @@ * at any time, a {@code true} return does not guarantee that any * other thread will ever acquire. * - *

In this implementation, this operation returns in - * constant time. - * * @return {@code true} if there may be other threads waiting to acquire */ public final boolean hasQueuedThreads() { - return head != tail; + for (Node p = tail, h = head; p != h && p != null; p = p.prev) + if (p.waitStatus <= 0) + return true; + return false; } /** @@ -1547,17 +1549,21 @@ * @since 1.7 */ public final boolean hasQueuedPredecessors() { - // The correctness of this depends on head being initialized - // before tail and on head.next being accurate if the current - // thread is first in queue. - Node t = tail; // Read fields in reverse initialization order - Node h = head; - Node s; - return h != t && - ((s = h.next) == null || s.thread != Thread.currentThread()); + Node h, s; + if ((h = head) != null) { + if ((s = h.next) == null || s.waitStatus > 0) { + s = null; // traverse in case of concurrent cancellation + for (Node p = tail; p != h && p != null; p = p.prev) { + if (p.waitStatus <= 0) + s = p; + } + } + if (s != null && s.thread != Thread.currentThread()) + return true; + } + return false; } - // Instrumentation and monitoring methods /** diff -r 20ed1cebe5f8 -r 221cf8307606 test/jdk/java/util/concurrent/tck/AbstractQueuedSynchronizerTest.java --- a/test/jdk/java/util/concurrent/tck/AbstractQueuedSynchronizerTest.java Wed Jan 17 07:55:20 2018 +0800 +++ b/test/jdk/java/util/concurrent/tck/AbstractQueuedSynchronizerTest.java Tue Jan 16 18:24:32 2018 -0800 @@ -1289,11 +1289,10 @@ } /** - * Disabled demo test for (unfixed as of 2017-11) * JDK-8191483: AbstractQueuedSynchronizer cancel/cancel race * ant -Djsr166.tckTestClass=AbstractQueuedSynchronizerTest -Djsr166.methodFilter=testCancelCancelRace -Djsr166.runsPerTest=100 tck */ - public void DISABLED_testCancelCancelRace() throws InterruptedException { + public void testCancelCancelRace() throws InterruptedException { class Sync extends AbstractQueuedSynchronizer { protected boolean tryAcquire(int acquires) { return !hasQueuedPredecessors() && compareAndSetState(0, 1);