On Wed, 7 Jun 2023 20:05:45 GMT, Serguei Spitsyn <sspit...@openjdk.org> wrote:

>> This is REDO the fix of 
>> [JDK-8307153](https://bugs.openjdk.org/browse/JDK-8307153).
>> The last update of the fix in the review cycle was incorrect and incorrectly 
>> tested, so the issue has not been noticed. It is why the fix was backed out.
>> The issue is that the SUSPEND bit was missed in the JVMTI thread state of 
>> platform/carrier threads carrying virtual threads 
>> (see`JvmtiEnvBase::get_thread_state` function).
>> 
>> The first push/patch is the original fix of JDK-8307153.
>> The fix of the SUSPEND bit issue will be in the incremental update.
>> It is to simplify the review.
>> 
>> Testing:
>>  - TBD: mach5 tiers 1-5
>
> 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().

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/14366#discussion_r1222182733

Reply via email to