Hi Kim, I agree with your proposed fix.
I see that you have added a comment for future maintainers. Thanks for that - as the implication of the ordering between the two volatile writes is not immediately perceptible to the casual reader. You have good eyes - I hadn't spotted the race condition, but it is definitely there. I have tried to see if the proposed reordering could have other side effects but couldn't find any: The reference might now temporarily appear unenqueued though it's already at the head of the queue - but since 'head' is private in ReferenceQueue and access to the head reference are adequately protected by 'lock' then it looks like this is not going to be an issue. +1 Thanks for finding the cause of the failure and the devising the fix! best regards, -- daniel On 29/07/15 09:57, 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. 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.
