RFR: 8132306: java/lang/ref/ReferenceEnqueue.java fails with "RuntimeException: Error: poll() returned null; expected ref object"

[David Holmes]

Hi Kim,

On 29/07/2015 5:57 PM, Kim Barrett wrote:
> Please review this fix of a race condition in
> j.l.r.Reference/ReferenceQueue.  See comments in the bug report for a
> description of the race.  The race is being fixed by reordering a pair
> of volatile assignments.

While this seems logical for the failure at hand it isn't immediately 
obvious to me that setting next before setting the ENQUEUED state won't 
cause a problem for some other piece of code. Really these things need 
to be tightly synchronized - I think the real bug is the unsynchronized 
fast-path for the empty-queue case in poll(). While that change was 
deliberate in 6666739 this side-effect was not realized and would have 
affected the change. I hope Martin and/or Tom see this and chime in.

That aside the commentary is rather verbose, a simple:

// Only set ENQUEUED state after the reference is enqueued

would suffice (and add "volatiles ensure ordering" if you want that to 
be clearer).

Thanks,
David

> CR:
> https://bugs.openjdk.java.net/browse/JDK-8132306
>
> Webrev:
> http://cr.openjdk.java.net/~kbarrett/8132306/webrev.00/
>
> Testing:
> jprt with default and hotspot testsets
>
> Locally tested race in original code by insertion of sleep in enqueue and
> verifying ReferenceEnqueue regression test fails consistently as
> described in the bug report.
>
> Locally tested fixed code by insertion of sleeps at various points and
> verifying ReferenceEnqueue regression test still passes.
>