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

David Holmes dholmes at openjdk.org
Wed Aug 6 21:06:16 UTC 2025


On Wed, 6 Aug 2025 15:20:44 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: Fixed volatility issue on getters.

Changes requested by dholmes (Reviewer).

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

> 106: const intptr_t    VMError::segfault_address = pd_segfault_address;
> 107: Thread* volatile VMError::_handshake_timed_out_thread = nullptr;
> 108: Thread* volatile VMError::_safepoint_timed_out_thread = nullptr;

`volatile` is still not useful or needed here.

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

> 1344: 
> 1345: void VMError::set_safepoint_timed_out_thread(Thread* thread) {
> 1346:   Atomic::release_store(&_safepoint_timed_out_thread, thread);

Acquire/release is not what is needed here. You are not trying to synchronize with previous memory accesses. If you want to force this to be visible to the other thread a `fence()` is required.

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

PR Review: https://git.openjdk.org/jdk/pull/26656#pullrequestreview-3094270934
PR Review Comment: https://git.openjdk.org/jdk/pull/26656#discussion_r2258292221
PR Review Comment: https://git.openjdk.org/jdk/pull/26656#discussion_r2258295173


More information about the hotspot-dev mailing list