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

David Holmes dholmes at openjdk.org
Sun Aug 17 21:39: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

Our convention for lock-free code is that we should use `Atomic::load` and `Atomic::store` for all accesses to the shared variable. - this serves as clear documentation of the lock-free nature of the code. The use, or not,  of `volatile` on the declaration of the shared variable is not a convention we have firmly established or documented. I am in the camp that the `volatile` should not be present on the declaration as we have internalised/encapsulated the `volatile` within the `Atomic` functions.

@albertnetymk suggestion to use explicit `release`  between the store and the `pthread_kill` does also address the visibility concern unlike the earlier `release_store` suggestion which puts the `release` in the wrong place. On further reflection my suggestion about the `fence` should also make the `fence` explicit between the store and the `pthread_kill` rather than being inside the setter method (which again by our own conventions should be renamed to have `fence` in the name if we did that).

Arguably, as stated much much earlier, we don't need any explicit memory ordering here in practice as the signal implementation must provide its own ordering guarantees to actually function - and there is no way a compiler would ever re-order the store with the `pthread_kill` call!

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

PR Comment: https://git.openjdk.org/jdk/pull/26656#issuecomment-3194666749


More information about the hotspot-dev mailing list