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

Aleksey Shipilev shade at openjdk.org
Wed Aug 6 09:12:36 UTC 2025


On Mon, 4 Aug 2025 08:37:23 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 with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains 17 additional commits since the last revision:
> 
>  - 8359820: Addressed reviewer's comments
>  - Merge remote-tracking branch 'origin/master' into JDK-8359820-SIGILL-with-low-handshake-timeout-on-intel-sde
>  - 8359820: Addressed reviewer's comments
>  - 8359820: Addressed reviewer's comments
>  - Merge remote-tracking branch 'origin/master' into JDK-8359820-SIGILL-with-low-handshake-timeout-on-intel-sde
>  - 8359820: Fixed spaces
>  - 8359820: Addressed reviewer's comments
>  - 8359820: Addressed reviewer's comments
>  - 8359820: Addressed reviewer's comments
>  - Merge remote-tracking branch 'origin/master' into JDK-8359820-SIGILL-with-low-handshake-timeout-on-intel-sde
>  - ... and 7 more: https://git.openjdk.org/jdk/compare/1bb5a3bd...d85769af

A few post-integration notes, maybe do a little follow-up cleanup?

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

> 105: const size_t      VMError::_reattempt_required_stack_headroom = 64 * K;
> 106: const intptr_t    VMError::segfault_address = pd_segfault_address;
> 107: volatile intptr_t VMError::_handshake_timed_out_thread = p2i(nullptr);

Not sure why we do `intptr_t` if we only ever expect real `Thread*` here? Would avoid doing awkward `p2i` conversions in comparisons.

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

> 823:       if (_siginfo != nullptr && os::signal_sent_by_kill(_siginfo)) {
> 824:         if (_handshake_timed_out_thread == p2i(_thread)) {
> 825:           st->print(" (sent by handshake timeout handler");

Looks like this message misses the closing parenthesis? `(sent by kill)`, but `(sent by handshake timeout handler`.

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

PR Review: https://git.openjdk.org/jdk/pull/26309#pullrequestreview-3091525451
PR Review Comment: https://git.openjdk.org/jdk/pull/26309#discussion_r2256490100
PR Review Comment: https://git.openjdk.org/jdk/pull/26309#discussion_r2256491930


More information about the hotspot-runtime-dev mailing list