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

[Volker Simonis]

On Fri, May 26, 2017 at 6:09 PM, Andrew Haley <aph at redhat.com> wrote:
> On 26/05/17 17:03, Volker Simonis wrote:
>
>> Volatile not only prevents reordering by the compiler. It also
>> prevents other, otherwise legal transformations/optimizations (like
>> for example reloading a variable [1]) which have to be prevented in
>> order to write correct, lock free programs.
>
> Yes, but so do compiler barriers.
>

Please correct me if I'm wrong, but I thought "compiler barriers" are
to prevent reordering by the compiler. However, this is a question of
optimization. If you have two subsequent loads from the same address,
the compiler is free to do only the first load and keep the value in a
register if the address is not pointing to a volatile value. This is
one of the well known semantics of volatile.

But there's another, less known 'optimization' which is possible, if
an address is not pointing to a volatile value. If there's just a
single load, the compiler is free to reload that value a second time
later on (instead of spilling it to the stack or to another register).
And that was exactly the problem with JDK-8129440 [2]:

static inline oop       load_heap_oop(oop* p)       { return *p; }
...
template <class T>
inline void G1RootRegionScanClosure::do_oop_nv(T* p) {
  // 1. load 'heap_oop' from 'p'
  T heap_oop = oopDesc::load_heap_oop(p);
  if (!oopDesc::is_null(heap_oop)) {
    // 2. Compiler reloads 'heap_oop' from 'p' which may now be null!
    oop obj = oopDesc::decode_heap_oop_not_null(heap_oop);
    HeapRegion* hr = _g1h->heap_region_containing((HeapWord*) obj);
    _cm->grayRoot(obj, hr);
  }
}

How would a compiler barrier help here? How would it look like and
where would it have to be placed to?

I think this problem can currently only be solved reliably by
declaring the loaded value 'volatile'.

Regards,
Volker

[2] https://bugs.openjdk.java.net/browse/JDK-8129440

>> So I think declaring the variables involved in such algorithms
>> volatile is currently still necessary.
>
> IMO, only if compiler barriers don't work; and that implies broken
> compilers.  But from the responses I've seen, the assumption is that
> the compilers used to build HotSpot are broken.
>
> --
> Andrew Haley
> Java Platform Lead Engineer
> Red Hat UK Ltd. <https://www.redhat.com>
> EAC8 43EB D3EF DB98 CC77 2FAD A5CD 6035 332F A671