On Mon, 5 May 2025 23:53:02 GMT, Serguei Spitsyn <[email protected]> wrote:
>> src/hotspot/share/prims/jvmtiEnvBase.cpp line 1798:
>>
>>> 1796: return JVMTI_ERROR_THREAD_SUSPENDED;
>>> 1797: }
>>> 1798: if (!java_thread->java_suspend(single_suspend)) {
>>
>> We could use `is_virtual && single_suspend` (same in resume) and change
>> `_handshakee->is_vthread_mounted()` to be an assert in
>> `HandshakeState::set_suspended()`.
>
> Thank you for suggestion. Let me check if I understand you right.
> We can rename the parameter `update_vthread_list` to `register_vthread_SR`
> and pass `is_virtual && single_suspend` instead of `single_suspend` to
> `java_suspend()` and `java_resume()`.
> We also want to change the `HandshakeState::set_suspended()` as below:
>
> void HandshakeState::set_suspended(bool is_suspend, bool register_vthread_SR)
> {
> #if INCLUDE_JVMTI
> if (register_vthread_SR) {
> assert(_handshakee->is_vthread_mounted(), "sanity check");
> if (is_suspend) {
> JvmtiVTSuspender::register_vthread_suspend(_handshakee->vthread());
> } else {
> JvmtiVTSuspender::register_vthread_resume(_handshakee->vthread());
> }
> }
> #endif
> Atomic::store(&_suspended, is_suspend);
> }
>
>
> Is this correct? If so, then I think it is a good suggestion.
It feels like all the `HandshakeState` SR code can be moved from
`handshake.?pp` to` jvmtiEnvBase.?pp` as it seems to be a little bit unnatural
for `HandshakeState`. The `JvmtiThreadState_lock` or some other lock can be
used for waiting in the suspended state. Then some attempts to simplify this
code could be made. But it does not look as very important at this point in
time.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/24269#discussion_r2074721380
