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

[Mandy Chung]

On 6/19/13 2:05 AM, Peter Levart wrote:
>
> It doesn't make a difference between Reference.isEnqueued() and 
> ReferenceQueue.poll(), since there isn't any synchronization between 
> the two. So isEnqueued() can still be returning true while another 
> thread has already de-queued the instance. I guess the real use of 
> isEnqueued() method is reliable detection of false -> true transition 
> only.
>

I think the isEnqueued method is useful to determine when a
referent has been GC'ed. If there are threads that remove them
from the queue, the code calling isEnqueued should prepare for
the race when isEnqueued returning true, the reference may
have been dequeued shortly.  That seems to be acceptable to
return true when another thread has already dequeued it while
not costing much performance tradeoff.

On the other hand, I agree that it'd be nice if we can clean up
the synchronization logic.

> I can't see why the isEnqueued() method checks for both (next != null 
> && queue == ENQUEUED). Wouldn't the check for (queue == ENQUEUED) be 
> enough? In that case the Reference.queue field could be made volatile 
> and synchronization on Reference instance removed.
>

I also thought about that.  The uncertainty I have is any performance 
implication that we need to concern about.   Thomas - can you also find 
out from the GC team?

It looks to me that next != null is an optimization since next == null 
if not pending to be enqueued or not enqueued.

> While you're at it, the reallyPoll() could optimize the double 
> volatile read from head and only perform it once:
>
>     private Reference<? extends T> reallyPoll() {       /* Must hold 
> lock */
>         Reference<? extends T> r = head;
>         if (r != null) {
>             head = (r.next == r) ? null : r.next;
>             r.queue = NULL;
>             r.next = r;
>             queueLength--;
>             if (r instanceof FinalReference) {
>                 sun.misc.VM.addFinalRefCount(-1);
>             }
>             return r;
>         }
>         return null;
>     }
>
>

Good catch.

Mandy