<AWT Dev>  Review request for 7186109: Simplify lock machinery for PostEventQueue & EventQueue
david.holmes at oracle.com
Mon Sep 10 17:50:30 PDT 2012
On 11/09/2012 1:17 AM, Artem Ananiev wrote:
> On 9/10/2012 6:50 PM, Anthony Petrov wrote:
>> On 09/10/12 15:27, Artem Ananiev wrote:
>>>> ** This is safe because a thread only ever writes its own value to
>>>> flushThread so even if it reads a stale value that value will either be
>>>> null or some other thread - either way it is not the current thread so
>>>> it proceeds with the main logic.
>>> The "flushThread" field is not volatile, so we can't check its value
>>> outside of synchronized blocks.
>> In this particular case you can do that, and in the above quote David
>> explains why.
> Got it. I didn't even think about writing the code that survive stale
> values, and therefore missed David's comment. Anyway, I don't think such
> an anti-pattern as reading non-volatile field value outside of
> synchronized block is acceptable. And I'm pretty sure static analyzers
> like FindBugs will find this violation.
Yes they might, which is shame because this is one of a few patterns for
data-races that are perfectly safe and valid. But if you are that
concerned then make it volatile - I don't think it will affect
performance in this context and I think the overall code simplification
is worth it.
>> In other words: you only want to check whether the flushThread refers to
>> the current thread or not. If it's been actually set by the current
>> thread, then this thread must see the correct value w/o any
>> synchronization needed. Otherwise, (if it's null or set by another
>> thread,) your code will see a value that doesn't refer to the current
>> thread, and this is exactly what you wanted to check.
>> So I agree with David, this test needs no synchronization.
>> best regards,
More information about the awt-dev