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

[Andrew Haley]

On 30/05/17 11:57, Erik Österlund wrote:
> 
> Having said that, I am not convinced C++11 is not a good idea either. I 
> would just like to balance out the view that C++11 is a synchronization 
> silver bullet for the JVM that is simply a superior solution without any 
> pitfalls and that doing anything else is wrong. There are things to be 
> considered there as well, like the extent of possible ABI incompatibilities.

Sure, and I appreciate your comment, but you never got the idea that
using C++11 atomics is a synchronization silver bullet from me: IMO
it's necessary but (probably) not sufficient.

>> It's important that we do so because racy accesses are undefined
>> behaviour in C++11.  (And, arguably, before that, but I'm not going to
>> go there.)
> 
> What paragraph are we referring to here that would break OrderAccess
> in C++11?

Nowhere: it's racy accesses *without* synchronization that are UB.

>>> Personally though, I am a helmet-on-synchronization kind of person,
>>> so I would take precaution anyway and use volatile whenever
>>> possible, because 1) it makes the code more readable, and 2) it
>>> provides one extra layer of safety that is more standardized. It
>>> seems that over the years it has happened multiple times that we
>>> assumed OrderAccess is bullet proof, and then realized that it
>>> wasn't and observed a crash that would never have happened if the
>>> code was written in a helmet-on-synchronization way. At least that's
>>> how I feel about it.
>>
>> I have no problem with that.  What I *do* have a problem with is the
>> use of volatile to fix bugs that really need to be corrected with
>> proper barriers.
> 
> I think you misunderstood me here. I did not propose to use volatile so 
> we don't have to fix bugs in OrderAccess. Conversely, I said if we find 
> such issues, we should definitely fix them in OrderAccess. But despite 
> that, I personally like the pragmatic safety approach, and would use 
> volatile in my lock-free code anyway to make it a) more readable, and b) 
> provide an extra level of safety against our increasingly aggressive 
> compilers. It's like wearing a helmet when biking. You don't expect to 
> fall and should not fall, but why take risks if you don't have to and 
> there is an easy way of preventing disaster if that happens. At least 
> that's how I think about it myself.

As I said, I have no problem with that.  I'm happy with that
justification for volatile, even when not strictly necessary, as long
as it's not done in places that would significantly impede
performance.  I'm sure you would agree with that anyway.

You have to remember where this discussion started, which was a
proposed use of volatile to fix a bug where a barrier was needed.

> The issue is not whether an algorithm depends on IRIW or not. The issue 
> is that we have to explicitly reason about IRIW to prove that it works. 
> The lack of IRIW violates seq_cst and by extension linearizaiton points 
> that rely in seq_cst, and by extension algorithms that rely on 
> linearization points. By breaking the very building blocks that were 
> used to reason about algorithms and their correctness, we rely on chance 
> for it to work. The algorithm may or may not work. It probably does work 
> without IRIW constraints in the vast majority of cases. But we have to 
> explicitly reason about that expanded state machine of possible races 
> caused by IRIW issues to actually know that it works rather than leaving 
> it to chance. Reasoning about this extended state machine can take a lot 
> of work and puts the bar unreasonably high for writing synchronized code 
> in my opinion. And I think the alternative of leaving it to chance 
> (albeit with good odds) seems like an unfortunate choice.

Sure, and I know all of that, and it sounds like you are arguing with
a point that someone else made.  Where sequential consistency really
is required in the VM, seq_cst in the C++ compiler must really be
seq_cst.  But whoever thought otherwise?  No-one, as far as I know.

> Either way, relying on C++11 atomics might also conceal bugs that would 
> be revealed on a weakly-ordered machine due to conflicting ABIs between 
> the statically generated C++ code and the dynamically generated code, as 
> previously mentioned.

We have to make sure that the generated code is ABI-compatible, of
course.

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