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

[Anton Artemov]

On Thu, 7 Aug 2025 07:34:50 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: Addressed reviewer's comments

I think we have 2 orthogonal problems here:

1) We want to make a value stored in that global variables to be visible to all threads.

2) We may have multiple updates of that value cause by (theoretically) more than one thread timed out. 

I came to conclusion that the 1st issue is not an issue, as signal firing and receiving is very heavy compared by a simple store to a variable. One can safely assume that by the time the signal is received on the target thread the memory operation has been already finished and the value will be visible. Hence we do not need a fence. We may have it, but it is not a must. 

The 2nd issue has more a hypothetical one. If we ever see more than one thread timing out, the variable keeping the thread address of the timed out thread will be updated more than once. More than one signal will be sent. But, as the comment on top of `VMError::report()` says, "Only one thread can call this function, so we don't need to worry about MT-safety." And we do not know which thread will execute this method anyway. Therefore, I suggest to use `Atomic::replace_if_null()` in the setter with the default (conservative) memory order. This will discard all updates except the 1st one and give us fences addressing the 1st not-really-an-issue. In case of a mismatch between the reporting thread and the thread stored in the variable we fall back to default reporting, but I think it will be an extremely rare case. 

 I also brought back the `volatile` keyword for indicative purposes.

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

PR Comment: https://git.openjdk.org/jdk/pull/26656#issuecomment-3178476744