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

David Holmes david.holmes at oracle.com
Mon Jul 1 11:34:54 UTC 2013

On 1/07/2013 8:05 PM, Thomas Schatzl wrote:
> 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/

Yes I think this handles it better. Get rid of synchronization of the 
Reference, make the fields volatile and let things race until the 
ReferenceQueue lock is taken, then sort it out.


> 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):
> 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

More information about the core-libs-dev mailing list