RFR (XS): 8014890 : Reference queues may return more entries than expected

[Aleksey Shipilev]

On 06/19/2013 04:05 PM, David Holmes wrote:
> I think I'm going to need a lot more time to understand this issue 
> completely. The synchronization being used seems complex and
> somewhat ill-defined, and also inadequate. I'm not convinced that
> adding code inside a synchronized block really fixes a race condition
> caused by unsynchronized access - but it may well narrow the window
> significantly.

+1. Having spent 30 minutes trying to figure out the synchronization
protocol for R and RQ, I can say that is a haunted part of JDK.

That said, I think the patch fixes the concrete issue. We are
piggybacking on the mutual exclusion on RQ.lock to protect ourselves
from the concurrent mutation of r.queue. The only two places where we
mutate it is RQ.enqueue() and RQ.poll(), both secured with RQ.lock.
Given the r.queue is not volatile, we should also acquire the same lock
while reading r.queue, otherwise races obliterate the reasoning about
the correctness.

With that notion in mind, I start to wonder if we just need to move the
check in enqueue() down into the synchronized(lock), and extend it to
capture NULL:

--- a/src/share/classes/java/lang/ref/ReferenceQueue.java	Mon Jun 17
16:28:22 2013 +0400
+++ b/src/share/classes/java/lang/ref/ReferenceQueue.java	Wed Jun 19
17:53:24 2013 +0400
@@ -56,8 +56,9 @@

     boolean enqueue(Reference<? extends T> r) { /* Called only by
Reference class */
         synchronized (r) {
-            if (r.queue == ENQUEUED) return false;
             synchronized (lock) {
+                if (r.queue == ENQUEUED || r.queue == NULL)
+                    return false;
                 r.queue = ENQUEUED;
                 r.next = (head == null) ? r : head;
                 head = r;

Will it be equivalent to what Thomas is trying to accomplish?

-Aleksey.