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

[David Holmes]

On 26/05/2017 10:35 PM, Andrew Haley wrote:
> 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.

No we do not "fix" the code by adding volatile. We as rule mark all 
variables involved as "volatile" because it is the only thing we can do 
to tell the compiler that there are things going on it is not aware of. 
In addition we use barriers and atomic instructions to be correct on 
every platform strong or weakly ordered - at least that is the intent. 
Now it may be that if your compiler is truly multi-thread aware and 
fence aware and atomic aware, and you use all those things directly that 
you don't need to also use "volatile". But the JVM does not at this time 
exist in that world.

David

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