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

David Holmes dholmes at openjdk.org
Sun Aug 17 21:47:13 UTC 2025


On Tue, 12 Aug 2025 12:07:53 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. 
>> 
>> 3) Updates are done with `Atomic::replace_if_null()` to address possible multiple updates and visibility among all threads. 
>> 
>> Trivial change.
>
> Anton Artemov has updated the pull request incrementally with one additional commit since the last revision:
> 
>   8364819: Added atomic loads to getters

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

> 1343:   // updates except the 1st one. Those can hypothetically happen
> 1344:   // if more than one thread times out.
> 1345:   // The default memory ordering guarantees visibility to other threads.

The use of `replace_if_null` is a good alternative here. The comment is a bit wordy. I would suggest a simpler:

// Only preserve the first thread to time-out this way. The atomic operation ensures
// visibility to the target thread.

and of course the same comment needs to be in `set_safepoint_timed_out_thread` (or else use a comment block before the two functions to describe both their operations at once).

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

PR Review Comment: https://git.openjdk.org/jdk/pull/26656#discussion_r2281031302


More information about the hotspot-dev mailing list