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

[David Holmes]

On Wed, 6 Aug 2025 10:48:43 GMT, Anton Artemov <duke at openjdk.org> wrote:

>> Hi, please consider the following changes:
>> 
>> This is a cleanup after https://github.com/openjdk/jdk/pull/26309
>> 
>> 1) `_handshake_timed_out_thread `and `_safepoint_timed_out_thread` are now `Thread*` and not `intptr_t`, no conversions `p2i <-> i2p` needed.
>> 
>> 2) Added a missed brace in the error message. 
>> 
>> Trivial change.
>
> Anton Artemov has updated the pull request incrementally with one additional commit since the last revision:
> 
>   8364819: Fixed brace in the test

Thanks for catching these @shipilev . I have to confess I thought there was a reason we stored the thread as just an id rather than a true pointer, but I was mistaken.

@toxaart thanks for the speedy follow up here. I have one other nit I just noticed - @shipilev may want to weigh in on it too.

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.)

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

PR Review: https://git.openjdk.org/jdk/pull/26656#pullrequestreview-3092261002
PR Review Comment: https://git.openjdk.org/jdk/pull/26656#discussion_r2256961025