RFR: 8317600: VtableStubs::stub_containing() table load not ordered wrt to stores

[Thomas Schatzl]

On Sat, 7 Oct 2023 21:48:35 GMT, Dean Long <dlong at openjdk.org> wrote:

> Atomic acquire loads protected by VtableStubs_lock seem unnecessary if all stores are also protected by VtableStubs_lock.

Can you clarify what particular line/interaction you are referring to: `Vtablestubs::stub_containing` is not locking by the `VtableStubs_lock` so afaict that's why the acquire is required (from the release in the `enter`). I can't find obvious explicit locking of `VtableStubs_lock` for the few `contains` and `stub_containing` calls.

If you are referring to the `Atomic:load` in `VtableStubs::entry_point` does not have any particular meaning apart from telling "this is an access to a variable that is potentially accessed in parallel" (apart from telling the compiler to not optimize that load). This has been (at least) in the GC team the requirement for *any* access to such a variable (which we mark volatile). This makes it easier to discern such variables.

The reason why the release is needed is that `Vtablestubs::stub_containing` can run in parallel to `enter`, and without that there is a reordering that allows the problem to occur:

I.e.
thread A in `enter`:

set_next(_table[h]);
_table[h]= s

switch to thread B in `stub_containing`:

    for (VtableStub* s = _table[i]; s != nullptr; s = s->next()) { ... }

These reads of `_table[i]` can receive `next` and `_table[i]` in the wrong order without a release/acquire pair.

switch to thread A:
  unlock `Vtablestubs_lock` (and do the implicit release).

The release is too late (and the corresponding acquire is missing anyway).

For the call to `vtable_stubs_do` I can only find a `CodeCache_lock` to be locked in the vicinity, not the `Vtablestubs_lock`.

-------------

PR Comment: https://git.openjdk.org/jdk/pull/16056#issuecomment-1752585677