On Wed, 27 May 2026 08:50:23 GMT, David Holmes <[email protected]> wrote:
>> Patricio Chilano Mateo has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> add explicit thread->is_disable_suspend() checks
>
> src/hotspot/share/runtime/javaThread.hpp line 791:
>
>> 789: }
>> 790:
>> 791: bool should_defer_self_suspend() const {
>
> I'm struggling to understand what "self-suspend" means in this context. In
> particular the general inclusion of the jni check doesn't seem applicable to
> all the places this method gets checked.
Yes, checking `jni_deferred_suspension` is not really needed in
`MountUnmountDisabler::start_transition`. We only check the suspend status
there, i.e. whether a `is_suspend_request()` operation has already completed.
The JNI critical case works differently than the other two cases, in that the
`is_suspend_request()` operation is never executed until the target is allowed
to be suspended (out of JNI critical), so a suspended status implies
`jni_deferred_suspension()` is false. It’s just that for
`HandshakeState::get_op_for_self` we do need to check the three conditions, and
since this is a benign check for `MountUnmountDisabler::start_transition` I
grouped them under a common method. But I see that it might be more confusing
(together with the method name) so I removed it and just added the extra
`is_disable_suspend()` check in the mountUnmount disabler logic.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/31281#discussion_r3312972889
