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

[Anton Artemov]

On Sun, 20 Jul 2025 21:39:24 GMT, David Holmes <dholmes at openjdk.org> wrote:

>> Anton Artemov has updated the pull request incrementally with two additional commits since the last revision:
>> 
>>  - 8359820: Fixed spaces
>>  - 8359820: Addressed reviewer's comments
>
> src/hotspot/share/runtime/handshake.cpp line 214:
> 
>> 212:   }
>> 213:   if (target != nullptr) {
>> 214:     fatal("Thread " PTR_FORMAT " has not cleared handshake op: " PTR_FORMAT ", then failed to terminate JVM", p2i(target), p2i(op));
> 
> Suggestion:
> 
>     fatal("Thread " PTR_FORMAT " has not cleared handshake op %s, and failed to terminate the JVM", p2i(target), op->name());
> 
> The earlier logging statement that uses `p2i(op)` relies on an even earlier logging statement (line 189/190) that reports the name and the `p2i` value. But the fatal error message can't rely on using logging information to map the `p2i` value back to a name, so we need the name directly.

Thanks, addressed in the latest commit.

> src/hotspot/share/utilities/vmError.cpp line 108:
> 
>> 106: const intptr_t    VMError::segfault_address = pd_segfault_address;
>> 107: volatile intptr_t VMError::handshakeTimedOutThread = p2i(nullptr);
>> 108: volatile intptr_t VMError::safepointTimedOutThread = p2i(nullptr);
> 
> Suggestion:
> 
> volatile intptr_t VMError::_handshake_timeout_thread = p2i(nullptr);
> volatile intptr_t VMError::_safepoint_timeout_thread = p2i(nullptr);

Addressed in the latest commit.

> src/hotspot/share/utilities/vmError.cpp line 1340:
> 
>> 1338: }
>> 1339: 
>> 1340: void VMError::set_handshake_timed_out_thread(intptr_t x) {
> 
> Suggestion:
> 
> void VMError::set_handshake_timed_out_thread(intptr_t thread_addr) {

Addressed in the latest commit.

> src/hotspot/share/utilities/vmError.cpp line 1344:
> 
>> 1342: }
>> 1343: 
>> 1344: void VMError::set_safepoint_timed_out_thread(intptr_t x) {
> 
> Suggestion:
> 
> void VMError::set_safepoint_timed_out_thread(intptr_t thread_addr) {

Addressed in the latest commit.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/26309#discussion_r2235680354
PR Review Comment: https://git.openjdk.org/jdk/pull/26309#discussion_r2235682175
PR Review Comment: https://git.openjdk.org/jdk/pull/26309#discussion_r2235681293
PR Review Comment: https://git.openjdk.org/jdk/pull/26309#discussion_r2235681597