RFR: 8364819: Post-integration cleanups for JDK-8359820 [v2]

[David Holmes]

On Wed, 6 Aug 2025 12:59:01 GMT, Aleksey Shipilev <shade at openjdk.org> wrote:

>> src/hotspot/share/utilities/vmError.cpp line 108:
>> 
>>> 106: const intptr_t    VMError::segfault_address = pd_segfault_address;
>>> 107: volatile Thread* VMError::_handshake_timed_out_thread = nullptr;
>>> 108: volatile Thread* VMError::_safepoint_timed_out_thread = nullptr;
>> 
>> I should have picked this up previously but `volatile` generally serves no purpose for inter-thread consistency. If we need to guarantee visibility between the signaller and signalee, then we need a fence to ensure that. Or we can skip the fence (and volatile) and note that this will usually, but perhaps not always, work fine. (I note that `pthread_kill` is not specified as a memory synchronizing operation, but I strongly suspect it has to have its own internal memory barriers that we would be piggy-backing on.)
>
> I am pretty sure the memory consistency here is irrelevant, given that: a) the `Thread` in question is likely already initialized for a long time, so it is unlikely we will lose anything release-acquire-wise; b) we only use these for pointer comparisons.
> 
> But since we are here, and in the interest of clarity and avoiding future surprises, we can just summarily wrap these with `Atomic::release_store` and `Atomic::load_acquire` to be extra paranoidly safe. This is a failing path, so we don't care about performance, and would like to avoid a secondary crash in error handler some time in the future, if anyone reads anything from these threads.

@shipilev The memory consistency is about seeing the writes to these fields in the thread that is signalled. acquire/release has no meaning in this context. I would not add acquire/release just in case someone in the future actually tried to follow those pointers - they are only for identifying purposes. If you are worried about that then lets go back to changing them to a value (intptr_t) so it will never be an  issue.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/26656#discussion_r2259542119