On Wed, 8 Nov 2023 16:11:44 GMT, Serguei Spitsyn <sspit...@openjdk.org> wrote:
>> src/hotspot/share/prims/jvmtiEnvBase.cpp line 1974:
>>
>>> 1972:
>>> 1973: if (java_lang_VirtualThread::is_instance(target_h())) { // virtual
>>> thread
>>> 1974: if (!JvmtiEnvBase::is_vthread_alive(target_h())) {
>>
>> There is only one issue I see in how this check is implemented and the
>> removal of the VM_op for unmounted vthreads. The change of state to
>> TERMINATED happens after notifyJvmtiUnmount(), i.e we can see that this
>> vthread is alive here but a check later can return is not. This might hit
>> the assert in JvmtiEnvBase::get_vthread_jvf() (maybe this the issue you saw
>> on your first prototype). We can either change that order at the Java level,
>> or maybe better change this function to read the state and add a case where
>> if the state is RUNNING check whether the continuation is done or not
>> (jdk_internal_vm_Continuation::done(cont)).
>
> Thank you for the suggestion. Will check it.
I've added the check for `!jdk_internal_vm_Continuation::done(cont)` into `
JvmtiEnvBase::is_vthread_alive(oop vt)` but then decided to remove it again.
This is racy for `JvmtiHandshake` execution. As you correctly stated, the
change of state to `TERMINATED` happens after `notifyJvmtiUnmount()`. The
target virtual thread will be blocked in the `notifyJvmtiUnmount()` because the
`JvmtiVTMSTransitionDisabler` is set. This gives us a guaranty that the target
virtual thread won't change its state to `TERMINATED` while a handshake is
executed. But it becomes not true if we add the
`!jdk_internal_vm_Continuation::done(cont)` check.
Form the other hand, absence of this check allows for target virtual thread
stack to become empty (with no frames). This is a known problem but I'd prefer
to attack it separately.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/16460#discussion_r1395238429
