[8] Review request for 7186109: Simplify lock machinery for PostEventQueue & EventQueue (original) (raw)

David Holmes 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.

Cheers, David

Thanks, Artem

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, Anthony

More information about the awt-dev mailing list