On Wed, 7 Jun 2023 21:12:24 GMT, Alex Menkov <amen...@openjdk.org> wrote:
>> Serguei Spitsyn has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> fix trailing space in jvmtiEnvBase.cpp
>
> src/hotspot/share/prims/jvmtiEnvBase.cpp line 765:
>
>> 763: if (is_thread_carrying_vthread(jt, thread_oop)) {
>> 764: state &= ~JVMTI_THREAD_STATE_RUNNABLE;
>> 765: state |= JVMTI_THREAD_STATE_WAITING |
>> JVMTI_THREAD_STATE_WAITING_INDEFINITELY;
>
> This does not look correct.
> GetThreadState spec provides hierarchical set of questions to interpret
> thread state value.
> JVMTI_THREAD_STATE_ALIVE | JVMTI_THREAD_STATE_WAITING |
> JVMTI_THREAD_STATE_WAITING_INDEFINITELY is only one branch and I'd expect all
> other bits are not set for this state.
> Need to decide what do we want to report as carrier thread state for all
> possible values returned by get_thread_state_base().
Good concern.
There are two bits (and the related RUNNABLE bit) that we care in this sub-tree
of state bits: `SUSPENDED` and `INTERRUPTED`. This update clones these two
bits. The RUNNABLE bit must be cleared.
A thread carrying a virtual thread can not be in native, blocked, parked,
sleeping or waiting on some object.
The state returned by the `get_thread_state_base` is based on the call:
` state = (jint)java_lang_Thread::get_thread_status(thread_oop);`
and addition of the derived from JavaThread bits: `SUSPENDED`, `INTERRUPTED`
and `IN_NATIVE`.
The three bit derived from the JavaThread are not relevant.
This call has to be made directly:
` state = (jint)java_lang_Thread::get_thread_status(thread_oop);`
The SUSPEND bit has to be based on the call:
` jt->is_carrier_thread_suspended();`
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/14366#discussion_r1222212064
