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

[Andrew Haley]

On 29/05/17 13:20, Erik Österlund wrote:

> As a conclusion, by using volatile in addition to OrderAccess you
> rely on standardized compiler semantics (at least for
> volatile-to-volatile re-orderings and re-loading, but not for
> volatile-to-nonvolatile, but that's another can of worms), and
> regrettably if you rely on OrderAccess memory model doing what it
> says it will do, then it should indeed work without volatile, but to
> make that work, OrderAccess relies on non-standardized
> compiler-specific barriers. In practice it should work well on all
> our supported compilers without volatile. And if it didn't, it would
> indeed be a bug in OrderAccess that needs to be solved in
> OrderAccess.

Right.  And, target by target, we can rework OrderAccess to use real
C++11 atomics, and everything will be better.  Eventually.

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

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

> Now one might argue that by using C++11 atomics that are
> standardized, all these problems would go away as we would rely in
> standardized primitives and then just trust the compiler.

And I absolutely do argue that.  In fact, it is the only correct way
to go with C++11 compilers.  IMO.

> But then there could arise problems when the C++ compiler decides to
> be less conservative than we want, e.g. by not doing fence in
> sequentially consistent loads to optimize for non-multiple copy
> atomic CPUs arguing that IRIW issues that violate sequential
> consistency are non-issues in practice.

A C++ compiler will not decide to do that.  C++ compiler authors know
well enough what sequential consistency means.  Besides, if there is
any idiom in the JVM that actually requires IRIW we should remove it
as soon as possible.

> That makes those loads "almost" sequentially consistent, which might
> be good enough. But it feels good to have a choice here to be more
> conservative.  To have the synchronization helmet on.

I have no real problem with that.  Using volatile has the problem,
from my point of view, that it might conceal bugs that would be
revealed on a weakly-ordered machine that you or I then have to fix,
but I can live with it.

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