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

Mandy Chung mandy.chung at oracle.com
Wed Jun 19 21:40:24 UTC 2013

On 6/19/13 5:05 AM, David Holmes wrote:

Hi Thomas,

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.

It'll be good to get your time to look into it. In fact I suggested this fix to Thomas while I do raise a question to myself whether we should attempt to clean up the synchronization in this fix (this code was implemented before the new Java memory model in 1.5).

Here is my understanding why I think the suggested fix is adequate.

r.queue will be set to ENQUEUED when enqueued and set to NULL when dequeued.

  1. Read access to r.queue

    a) no lock to read r.queue in Reference.enqueue() method and ReferenceHandler thread possibly race is that when calling RQ.enqueue(Reference) method (i) it's already enqueued (ii) it's already dequeued RQ.enqueue(Reference) has to handle that (where the bug is)

    b) Reference.isEnqueued holds the Reference object monitor

    this.next is null initially and is set by the VM to non-null pending for enqueue. I believe checking this.next != null is an optimization.

    Potential race is that the reference may be dequeued which doesn't hold the Reference object lock. Is this a bug?

    This race is acceptable in my opinion as I think the isEnqueued method is useful to determine when a referent has been GC'ed and then followed by dequeuing it from the queue. If there are threads that doing dequeuing, the code calling isEnqueued will prepare for the race when isEnqueued returning true, the reference may have been dequeued shortly. Precisely handling the race that isEnqueued returns true while it may be dequeued doesn't prevent this situation.

  2. Write to r.queue a) acquire Reference object monitor and ReferenceQueue.lock to set r.queue to ENQUEUED Potential race in RQ.enqueue method synchronized (r) { if (r.queue == ENQUEUED) return false; synchronized (lock) { if (r.queue != this) return false; ... Moving if (r.queue == ENQUEUED) line to after acquiring lock

makes it easier to understand and maintain but I don't see a race in the existing code without holding lock.

b) acquire ReferenceQueue.lock to set r.queue to NULL
   ReferenceQueue.poll and remove methods calling reallyPoll method

   I don't know the history.  The nested lock inversion problem
   is probably the reason why it doesn't acquire the Reference object
   monitor to dequeue.

   This one is related to whether Reference.isEnqueued() should
   return (see above).

Mandy

More information about the core-libs-dev mailing list