RFR: 8359820: Improve handshake/safepoint timeout diagnostic messages [v4]

[Thomas Stuefe]

On Fri, 18 Jul 2025 09:14:39 GMT, Anton Artemov <duke at openjdk.org> wrote:

>> Hi, please consider the following changes:
>> 
>> The problem in the issue description is not a problem by itself, the behavior is not unexpected, but it is somewhat difficult to find out what caused SIGILL to be fired.
>> 
>> We propagate this information from `handshake::handle_timeout()` to `VMError::report()` with a help of a global variable.  The same mechanism is used to address a similar issue in the safepoint timeout handler. 
>> 
>> Tested in tiers 1-3.
>
> Anton Artemov has updated the pull request incrementally with one additional commit since the last revision:
> 
>   8359820: Addressed reviewer's comments

src/hotspot/share/runtime/safepoint.cpp line 70:

> 68: #include "utilities/systemMemoryBarrier.hpp"
> 69: 
> 70: intptr_t safepointTimedOutThread = p2i(nullptr);

Maybe not a global variable. 

I would either make this a static member of SafePoint and the other one a static member of Handshake, or (maybe simpler) both static members of VMError, plus accessors.

src/hotspot/share/utilities/vmError.cpp line 822:

> 820:       if (_siginfo != nullptr && os::signal_sent_by_kill(_siginfo)) {
> 821:         if (handshakeTimedOutThread == p2i(_thread)) {
> 822:           st->print(" (sent by handshake timeout handler, timed out thread " PTR_FORMAT ")", handshakeTimedOutThread);

since this thread is the timeouted thread, I think printing the thread's pointer is redundant and possibly confusing (it confused me). I would just write "sent by handshake timeout handler".

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

PR Review Comment: https://git.openjdk.org/jdk/pull/26309#discussion_r2216056575
PR Review Comment: https://git.openjdk.org/jdk/pull/26309#discussion_r2216039993