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
