-
Bug
-
Resolution: Fixed
-
P2
-
7
-
b51
-
generic
-
generic
Issue | Fix Version | Assignee | Priority | Status | Resolution | Resolved In Build |
---|---|---|---|---|---|---|
JDK-2175757 | OpenJDK6 | Martin Buchholz | P3 | Resolved | Fixed | b16 |
Martin Buchholz reports:
While writing a test for this, I unearthed yet another race condition in AQS.
Fortunately, it's in new jdk7 code.
In the expression
(h = head) != tail &&
head may be read as null,
then head and tail are both initialized before tail on RHS is read,
yielding NPE
Caused by: java.lang.NullPointerException
at java.util.concurrent.locks.AbstractQueuedSynchronizer.hasQueuedPredecessors(AbstractQueuedSynchronizer.java:1510)
at java.util.concurrent.Semaphore$FairSync.tryAcquireShared(Semaphore.java:245)
at java.util.concurrent.locks.AbstractQueuedSynchronizer.acquireSharedInterruptibly(AbstractQueuedSynchronizer.java:1263)
at java.util.concurrent.Semaphore.acquire(Semaphore.java:312)
We need to read fields in the reverse order.
@@ -1445,8 +1502,10 @@
// 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 h, s;
- return (h = head) != tail &&
+ 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());
}
While writing a test for this, I unearthed yet another race condition in AQS.
Fortunately, it's in new jdk7 code.
In the expression
(h = head) != tail &&
head may be read as null,
then head and tail are both initialized before tail on RHS is read,
yielding NPE
Caused by: java.lang.NullPointerException
at java.util.concurrent.locks.AbstractQueuedSynchronizer.hasQueuedPredecessors(AbstractQueuedSynchronizer.java:1510)
at java.util.concurrent.Semaphore$FairSync.tryAcquireShared(Semaphore.java:245)
at java.util.concurrent.locks.AbstractQueuedSynchronizer.acquireSharedInterruptibly(AbstractQueuedSynchronizer.java:1263)
at java.util.concurrent.Semaphore.acquire(Semaphore.java:312)
We need to read fields in the reverse order.
@@ -1445,8 +1502,10 @@
// 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 h, s;
- return (h = head) != tail &&
+ 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());
}
- backported by
-
JDK-2175757 Race condition in AbstractQueuedSynchronizer
- Resolved
- relates to
-
JDK-6801020 Concurrent Semaphore release may cause some require thread not signaled
- Closed