RFR: 8338379: Accesses to class init state should be properly synchronized [v2]

[David Holmes]

On Mon, 23 Sep 2024 07:14:52 GMT, Aleksey Shipilev <shade at openjdk.org> wrote:

>> src/hotspot/share/oops/instanceKlass.cpp line 4099:
>> 
>>> 4097: #endif
>>> 4098:   assert(_init_thread == nullptr, "should be cleared before state change");
>>> 4099:   Atomic::release_store_fence(&_init_state, state);
>> 
>> Why not just a release_store ?? Why do we need the trailing fence?
>
> Says in PR: "Current patch makes a seqcst write, which is stronger than strictly necessary. I think it is okay to be extra paranoid on rarely-executed class initialization path." But I can turn it into just a weaker release, sure.

I thought a seqcst write would be `fence(); store; fence()`?

Anyway I don't like "paranoid when it comes to memory barriers because that says to me "hey we don't understand what is going on here so we're just going to do the heaviest barrier we can 'just in case'."

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

PR Review Comment: https://git.openjdk.org/jdk/pull/21110#discussion_r1771041123