RFR: 8181085: Race condition in method resolution may produce spurious NullPointerException

[Andrew Haley]

On 26/05/17 10:20, David Holmes wrote:
> Hi Andrew,
> 
> On 26/05/2017 6:26 PM, Andrew Haley wrote:
>> On 26/05/17 03:20, David Holmes wrote:
>>> Any variable passed to an OrderAccess, or Atomic, function should be
>>> volatile to minimise the chances the C compiler will do something
>>> unexpected with it.
>>
>> That's not much more than paranoia, IMO.  If the barriers are strong
>> enough it'll be fine.  The problem was, I suppose, with old compilers
>> which didn't handle memory barriers properly, but we should be moving
>> towards standard ways of doing these things.  Standard atomics have
>> been available since C++11 (I think) and GCC has had support since long
>> before then.
> 
> The issue isn't just the barriers that might be involved inside
> orderAccess methods. If these variables are being used in racy
> lock-free code then they should be marked volatile to ensure other
> compiler optimizations don't interfere. Perhaps that is paranoia,
> but I'd rather a little harmless paranoia than try to debug what
> might otherwise go wrong.

I'm always leery of this kind of reasoning because the hardware I most
care about has a very weakly-ordered memory system and will reorder
everything in the absence of synchronization.  If it is actually
necessary to use volatile on a TSO machine to get multi-thread
ordering then it is almost certainly incorrect code, because volatile
is not sufficient to do what is needed on non-TSO hardware.

So, if you "fix" code on a TSO machine by using volatile, you are
making work for me because I'll have to debug it on a non-TSO machine.
Fix it in a portable way by using the correct primitives and it's
correct everywhere, it's easier to reason about, and you lost nothing.

-- 
Andrew Haley
Java Platform Lead Engineer
Red Hat UK Ltd. <https://www.redhat.com>
EAC8 43EB D3EF DB98 CC77 2FAD A5CD 6035 332F A671