RFR (XS): JDK-8025728 Missing volatile specifier for field G1AllocRegion::_alloc_region

[Thomas Schatzl]

Hi all,

  can I have review for the following small change? It changes G1AllocRegion::_alloc_region to be a volatile pointer, to avoid compiler optimizations that defeat the intent of the code.

I.e. the code loads _alloc_region, and if it's not NULL uses it further; the compiler may reload the variable after the NULL check, leading to races with other code that changes it to NULL in the meantime.
(The race is not a problem, the code just needs a consistent view on the value of the variable).

E.g. in G1CollectedHeap::max_unsafe_tlab_alloc()

  HeapRegion* hr = _mutator_alloc_region.get();
  if (hr == NULL) {
    return 0;
  }
  return hr->free();

At the same time G1AllocRegion::retire(bool fill_up) may change _alloc_region.

The fix is to make sure that the compiler does not reload the value of _alloc_region by making it volatile.

This analysis and the suggested fix has been proposed from the original bug reporters.

There is one catch with doing only this: G1AllocRegion::get() also contains a similar race where it is possible (and with that change guaranteed) that the code reloads the value.
This is even more serious, as this would make the code return an internal HeapRegion instance. This screws up some internal logic.

Bug:
https://bugs.openjdk.java.net/browse/JDK-8025728

Webrev:
http://cr.openjdk.java.net/~tschatzl/8025728/webrev

Testing:
jprt

@Volker: can you try the change? We do not have the compiler/platform to check.

Thanks,
Thomas