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

[Thomas Schatzl]

Hi David,

On Mon, 2013-07-01 at 17:51 +1000, David Holmes wrote:
> Well you can ignore what I wrote below - sorry. Somehow I got it in my 
> head that multiple enqueue's were intended/supported when of course they 
> are not. :(
> 
> So the proposed fix is okay - though I'd simplify the comment to just:
> 
> // Check that since getting the lock this reference hasn't already been
> // enqueued (and even then removed)

Done, see the new webrev at
http://cr.openjdk.java.net/~tschatzl/8014890/webrev.01/

I will send this version to Mandy for pushing if nobody objects in the next few days.

> The synchronization is problematic as I mention below but there is no 
> easy fix due to the lock-ordering problem, and any attempt at such a fix 
> would be much riskier. So this fix is fine - thanks.

Fyi, while waiting for your approval, I tried to clean up this a little
taking into account the comments from Peter and Aleksey (sorry if I
forgot somebody) into account.

A webrev for this is at
http://cr.openjdk.java.net/~tschatzl/8014890/webrev.refactor/ 

The changes are:
 - avoid the double read from the volatile head member in
ReferenceQueue.reallyPoll()
 - the fix for 8014890 you just reviewed
 - make Reference.queue volatile, and remove the synchronization on it
from Reference.isEnqueued() and ReferenceQueue.enqueue(). I do not see a
performance problem: a volatile read (in Reference.isEnqueued()) should
be at least as fast as the synchronization on that reference.

I think the change is okay as in functionally correct; I ran it through
jprt (running all the available test cases, including the one for the
original 8014890 patch, in the process), passing it.

Maybe it is useful to you for future reference.

Thanks everyone for your suggestions,
  Thomas

Standard information about the patch (the original one, not the
suggested cleanup):

JIRA:
https://jbs.oracle.com/bugs/browse/JDK-8014890

bugs.sun.com
http://bugs.sun.com/view_bug.do?bug_id=8014890

Testing:
jck, jprt, manual testing on most platforms