RFR: 8316682: serviceability/jvmti/vthread/SelfSuspendDisablerTest timed out [v6]

[Serguei Spitsyn]

On Mon, 5 May 2025 23:53:02 GMT, Serguei Spitsyn <sspitsyn at openjdk.org> 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