Re: RFR: 8309612: [REDO] JDK-8307153 JVMTI GetThreadState on carrier should return STATE_WAITING [v4]
On Thu, 8 Jun 2023 06:25:20 GMT, Alan Bateman wrote: >> It was decided with Alan that it is okay to be in a waiting state. The >> `JVMTI_THREAD_STATE_BLOCKED_ON_MONITOR_ENTER` state requires a monitor to be >> blocked on, so it can be confusing. Alan's comment in the original PR >> [https://github.com/openjdk/jdk/pull/14298](https://github.com/openjdk/jdk/pull/14298) >> was: >>> if the jt is carrying thread_oop and it's okay for the JVMTI state to >>> reported as WAITING when waiting for something other than Object.wait. > > The mental model is that the carrier is blocked so this is what an observer > using the APIs should see. My recollection is that JVMTI_THREAD_STATE_WAITING > was okay because there is a wriggle room in the JVM TI spec, it only uses > Object.wait as an example. There may be a few rough edges to smooth down in > this area. It's okay to take time with this PR and expand the tests to cover > more cases and get more confident that there aren't more issues. We agreed with Alex to file a test RFE to improve test coverage in this area. - PR Review Comment: https://git.openjdk.org/jdk/pull/14366#discussion_r1223883294
Re: RFR: 8309612: [REDO] JDK-8307153 JVMTI GetThreadState on carrier should return STATE_WAITING [v4]
On Thu, 8 Jun 2023 04:41:10 GMT, Serguei Spitsyn wrote: >> Thanks for clarifying - it gets very confusing as to which "thread" is being >> talked about. But if a virtual thread is mounted on this JavaThread then I >> thought the carrier thread's thread-oop is supposed to be in a blocked state? > > It was decided with Alan that it is okay to be in a waiting state. The > `JVMTI_THREAD_STATE_BLOCKED_ON_MONITOR_ENTER` state requires a monitor to be > blocked on, so it can be confusing. Alan's comment in the original PR > [https://github.com/openjdk/jdk/pull/14298](https://github.com/openjdk/jdk/pull/14298) > was: >> if the jt is carrying thread_oop and it's okay for the JVMTI state to >> reported as WAITING when waiting for something other than Object.wait. The mental model is that the carrier is blocked so this is what an observer using the APIs should see. My recollection is that JVMTI_THREAD_STATE_WAITING was okay because there is a wriggle room in the JVM TI spec, it only uses Object.wait as an example. There may be a few rough edges to smooth down in this area. It's okay to take time with this PR and expand the tests to cover more cases and get more confident that there aren't more issues. - PR Review Comment: https://git.openjdk.org/jdk/pull/14366#discussion_r1222511997
Re: RFR: 8309612: [REDO] JDK-8307153 JVMTI GetThreadState on carrier should return STATE_WAITING [v4]
On Thu, 8 Jun 2023 04:29:47 GMT, David Holmes wrote: >>> > A thread carrying a virtual thread can not be in native, blocked, parked, >>> > sleeping or waiting on some object. >> >>>A virtual thread can call native code, be blocked on an object monitor, or >>>waiting on an object monitor. Only parking and sleeping are specialized for >>>virtual threads in the list you gave. >> >> This statement was about a carrier thread (not a `JavaThread` and not a ` >> java.lang.VirtualThread`) when there is a virtual thread executed at the >> top. We are getting state bits with the >> `java_lang_Thread::get_thread_status(thread_oop)` where the `thread_oop` >> belongs to the carrier thread. But you are talking about a virtual thread >> which, of course, can be in almost any state. > > Thanks for clarifying - it gets very confusing as to which "thread" is being > talked about. But if a virtual thread is mounted on this JavaThread then I > thought the carrier thread's thread-oop is supposed to be in a blocked state? It was decided with Alan that it is okay to be in a waiting state. The `JVMTI_THREAD_STATE_BLOCKED_ON_MONITOR_ENTER` state requires a monitor to be blocked on, so it can be confusing. Alan's comment in the original PR [https://github.com/openjdk/jdk/pull/14298](https://github.com/openjdk/jdk/pull/14298) was: > if the jt is carrying thread_oop and it's okay for the JVMTI state to > reported as WAITING when waiting for something other than Object.wait. - PR Review Comment: https://git.openjdk.org/jdk/pull/14366#discussion_r1222442892
Re: RFR: 8309612: [REDO] JDK-8307153 JVMTI GetThreadState on carrier should return STATE_WAITING [v4]
On Thu, 8 Jun 2023 03:36:52 GMT, Serguei Spitsyn wrote: >>> A thread carrying a virtual thread can not be in native, blocked, parked, >>> sleeping or waiting on some object. >> >> A virtual thread can call native code, be blocked on an object monitor, or >> waiting on an object monitor. Only parking and sleeping are specialized for >> virtual threads in the list you gave. > >> > A thread carrying a virtual thread can not be in native, blocked, parked, >> > sleeping or waiting on some object. > >>A virtual thread can call native code, be blocked on an object monitor, or >>waiting on an object monitor. Only parking and sleeping are specialized for >>virtual threads in the list you gave. > > This statement was about a carrier thread (not a `JavaThread` and not a ` > java.lang.VirtualThread`) when there is a virtual thread executed at the top. > We are getting state bits with the > `java_lang_Thread::get_thread_status(thread_oop)` where the `thread_oop` > belongs to the carrier thread. But you are talking about a virtual thread > which, of course, can be in almost any state. Thanks for clarifying - it gets very confusing as to which "thread" is being talked about. But if a virtual thread is mounted on this JavaThread then I thought the carrier thread's thread-oop is supposed to be in a blocked state? - PR Review Comment: https://git.openjdk.org/jdk/pull/14366#discussion_r1222437953
Re: RFR: 8309612: [REDO] JDK-8307153 JVMTI GetThreadState on carrier should return STATE_WAITING [v4]
On Thu, 8 Jun 2023 02:07:00 GMT, David Holmes wrote: > > A thread carrying a virtual thread can not be in native, blocked, parked, > > sleeping or waiting on some object. >A virtual thread can call native code, be blocked on an object monitor, or >waiting on an object monitor. Only parking and sleeping are specialized for >virtual threads in the list you gave. This statement was about carrier thread when there is a virtual thread executed at the top. We are getting state bits with the `java_lang_Thread::get_thread_status(thread_oop)` where the `thread_oop` belongs to the carrier thread. But you are talking about a virtual thread which, of course, can be in almost any state. - PR Review Comment: https://git.openjdk.org/jdk/pull/14366#discussion_r1222413499
Re: RFR: 8309612: [REDO] JDK-8307153 JVMTI GetThreadState on carrier should return STATE_WAITING [v4]
On Thu, 8 Jun 2023 01:40:06 GMT, Serguei Spitsyn wrote: >> The INTERRUPTED bit we need has to be returned by the >> `java_lang_Thread::get_thread_status`. >> Not completely sure but the bit jt->is_interrupted(false) can be set for the >> mounted virtual thread. >> The JVMTI InterruptThread calls this function to set interrupt bit for >> non-virtual threads: >> ` java_lang_Thread::set_interrupted(thread_obj, true);` > > Corrected the function `get_thread_state()` to make it more safe. > Only `ALIVE` and `INTERRUPTED` bits are taken from result of > `java_lang_Thread::get_thread_status(thread_oop)`. > A thread carrying a virtual thread can not be in native, blocked, parked, > sleeping or waiting on some object. A virtual thread can call native code, be blocked on an object monitor, or waiting on an object monitor. Only parking and sleeping are specialized for virtual threads in the list you gave. - PR Review Comment: https://git.openjdk.org/jdk/pull/14366#discussion_r1222364495
Re: RFR: 8309612: [REDO] JDK-8307153 JVMTI GetThreadState on carrier should return STATE_WAITING [v4]
On Wed, 7 Jun 2023 23:08:52 GMT, Serguei Spitsyn wrote: >>> A thread carrying a virtual thread can not be in native, blocked, parked, >>> sleeping or waiting on some object. >> >> Actually it can be in native. >> And if I remember correctly synchronized block pins virtual thread, so >> inside synchronized we can get other states > > The INTERRUPTED bit we need has to be returned by the > `java_lang_Thread::get_thread_status`. > Not completely sure but the bit jt->is_interrupted(false) can be set for the > mounted virtual thread. > The JVMTI InterruptThread calls this function to set interrupt bit for > non-virtual threads: > ` java_lang_Thread::set_interrupted(thread_obj, true);` Corrected the function `get_thread_state()` to make it more safe. Only `ALIVE` and `INTERRUPTED` bits are taken from result of `java_lang_Thread::get_thread_status(thread_oop)`. - PR Review Comment: https://git.openjdk.org/jdk/pull/14366#discussion_r1222352148
Re: RFR: 8309612: [REDO] JDK-8307153 JVMTI GetThreadState on carrier should return STATE_WAITING [v4]
On Wed, 7 Jun 2023 22:48:47 GMT, Alex Menkov wrote:
>> Do you need to check jt->is_interrupted(false) and set INTERRUPTED bit?
>> It looks like java_lang_Thread::get_thread_status(thread_oop) can only
>> return RUNNABLE in the case and we clear it, so the call is not needed:
>>
>> if (is_thread_carrying_vthread(jt, thread_oop)) {
>> jint state = JVMTI_THREAD_STATE_WAITING |
>> JVMTI_THREAD_STATE_WAITING_INDEFINITELY;
>> if (jt->is_carrier_thread_suspended()) {
>> state |= JVMTI_THREAD_STATE_SUSPENDED;
>> }
>> if (jt->is_interrupted(false)) {
>> state |= JVMTI_THREAD_STATE_INTERRUPTED;
>> }
>> return state;
>> } else ...
>
>> A thread carrying a virtual thread can not be in native, blocked, parked,
>> sleeping or waiting on some object.
>
> Actually it can be in native.
> And if I remember correctly synchronized block pins virtual thread, so inside
> synchronized we can get other states
The INTERRUPTED bit we need has to be returned by the
`java_lang_Thread::get_thread_status`.
Not completely sure but the bit jt->is_interrupted(false) can be set for the
mounted virtual thread.
The JVMTI InterruptThread calls this function to set interrupt bit for
non-virtual threads:
` java_lang_Thread::set_interrupted(thread_obj, true);`
-
PR Review Comment: https://git.openjdk.org/jdk/pull/14366#discussion_r168031
Re: RFR: 8309612: [REDO] JDK-8307153 JVMTI GetThreadState on carrier should return STATE_WAITING [v4]
On Wed, 7 Jun 2023 22:42:49 GMT, Alex Menkov wrote:
>> 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 bits 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();`
>>
>> The function `get_thread_state` will look as below:
>>
>> if (is_thread_carrying_vthread(jt, thread_oop)) {
>> jint state = (jint)java_lang_Thread::get_thread_status(thread_oop);
>> if (jt->is_carrier_thread_suspended()) {
>> state |= JVMTI_THREAD_STATE_SUSPENDED;
>> }
>> // It's okay for the JVMTI state to be reported as WAITING when waiting
>> // for something other than an Object.wait. So, we treat a thread
>> carrying
>> // a virtual thread as waiting indefinitely which is not runnable.
>> // It is why the RUNNABLE bit is cleared and the WAITING bits are added.
>> state &= ~JVMTI_THREAD_STATE_RUNNABLE;
>> state |= JVMTI_THREAD_STATE_WAITING |
>> JVMTI_THREAD_STATE_WAITING_INDEFINITELY;
>> return state;
>> } else {
>> return get_thread_state_base(thread_oop, jt);
>> }
>
> Do you need to check jt->is_interrupted(false) and set INTERRUPTED bit?
> It looks like java_lang_Thread::get_thread_status(thread_oop) can only return
> RUNNABLE in the case and we clear it, so the call is not needed:
>
> if (is_thread_carrying_vthread(jt, thread_oop)) {
> jint state = JVMTI_THREAD_STATE_WAITING |
> JVMTI_THREAD_STATE_WAITING_INDEFINITELY;
> if (jt->is_carrier_thread_suspended()) {
> state |= JVMTI_THREAD_STATE_SUSPENDED;
> }
> if (jt->is_interrupted(false)) {
> state |= JVMTI_THREAD_STATE_INTERRUPTED;
> }
> return state;
> } else ...
> A thread carrying a virtual thread can not be in native, blocked, parked,
> sleeping or waiting on some object.
Actually it can be in native.
And if I remember correctly synchronized block pins virtual thread, so inside
synchronized we can get other states
-
PR Review Comment: https://git.openjdk.org/jdk/pull/14366#discussion_r155498
Re: RFR: 8309612: [REDO] JDK-8307153 JVMTI GetThreadState on carrier should return STATE_WAITING [v4]
On Wed, 7 Jun 2023 21:52:33 GMT, Serguei Spitsyn wrote:
>> 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 bits 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();`
>
> The function `get_thread_state` will look as below:
>
> if (is_thread_carrying_vthread(jt, thread_oop)) {
> jint state = (jint)java_lang_Thread::get_thread_status(thread_oop);
> if (jt->is_carrier_thread_suspended()) {
> state |= JVMTI_THREAD_STATE_SUSPENDED;
> }
> // It's okay for the JVMTI state to be reported as WAITING when waiting
> // for something other than an Object.wait. So, we treat a thread carrying
> // a virtual thread as waiting indefinitely which is not runnable.
> // It is why the RUNNABLE bit is cleared and the WAITING bits are added.
> state &= ~JVMTI_THREAD_STATE_RUNNABLE;
> state |= JVMTI_THREAD_STATE_WAITING |
> JVMTI_THREAD_STATE_WAITING_INDEFINITELY;
> return state;
> } else {
> return get_thread_state_base(thread_oop, jt);
> }
Do you need to check jt->is_interrupted(false) and set INTERRUPTED bit?
It looks like java_lang_Thread::get_thread_status(thread_oop) can only return
RUNNABLE in the case and we clear it, so the call is not needed:
if (is_thread_carrying_vthread(jt, thread_oop)) {
jint state = JVMTI_THREAD_STATE_WAITING |
JVMTI_THREAD_STATE_WAITING_INDEFINITELY;
if (jt->is_carrier_thread_suspended()) {
state |= JVMTI_THREAD_STATE_SUSPENDED;
}
if (jt->is_interrupted(false)) {
state |= JVMTI_THREAD_STATE_INTERRUPTED;
}
return state;
} else ...
-
PR Review Comment: https://git.openjdk.org/jdk/pull/14366#discussion_r152628
Re: RFR: 8309612: [REDO] JDK-8307153 JVMTI GetThreadState on carrier should return STATE_WAITING [v4]
On Wed, 7 Jun 2023 21:12:24 GMT, Alex Menkov 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_r112064
Re: RFR: 8309612: [REDO] JDK-8307153 JVMTI GetThreadState on carrier should return STATE_WAITING [v4]
On Wed, 7 Jun 2023 20:10:24 GMT, Chris Plummer 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;
>
> How about a comment here:
>
> "Clear RUNNABLE state and add WAITING state because..."
Thanks. Added comment.
> src/hotspot/share/prims/jvmtiEnvBase.cpp line 1739:
>
>> 1737: "sanity check");
>> 1738:
>> 1739: // An attempt to handshake-suspend a thread carrying virtual thread
>> will result in
>
> Suggestion:
>
> // An attempt to handshake-suspend a thread carrying a virtual thread will
> result in
Thanks. Updated now.
> src/hotspot/share/prims/jvmtiEnvBase.hpp line 99:
>
>> 97: static bool is_in_thread_list(jint count, const jthread* list, oop
>> jt_oop);
>> 98:
>> 99: // check if thread_oop represents a thread carrying virtual thread
>
> Suggestion:
>
> // check if thread_oop represents a thread carrying a virtual thread
Thanks. Updated now.
> src/hotspot/share/prims/jvmtiEnvBase.hpp line 183:
>
>> 181:
>> 182: // Return true if the thread identified with a pair is
>> current.
>> 183: // A thread carrying virtual thread is not treated as current.
>
> Suggestion:
>
> // A thread carrying a virtual thread is not treated as current.
Thanks. Updated now.
-
PR Review Comment: https://git.openjdk.org/jdk/pull/14366#discussion_r1222185513
PR Review Comment: https://git.openjdk.org/jdk/pull/14366#discussion_r1222185817
PR Review Comment: https://git.openjdk.org/jdk/pull/14366#discussion_r1222185985
PR Review Comment: https://git.openjdk.org/jdk/pull/14366#discussion_r1222186135
Re: RFR: 8309612: [REDO] JDK-8307153 JVMTI GetThreadState on carrier should return STATE_WAITING [v4]
On Wed, 7 Jun 2023 20:05:45 GMT, Serguei Spitsyn 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
Re: RFR: 8309612: [REDO] JDK-8307153 JVMTI GetThreadState on carrier should return STATE_WAITING [v4]
On Wed, 7 Jun 2023 20:05:45 GMT, Serguei Spitsyn 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
Changes requested by cjplummer (Reviewer).
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;
How about a comment here:
"Clear RUNNABLE state and add WAITING state because..."
src/hotspot/share/prims/jvmtiEnvBase.cpp line 1739:
> 1737: "sanity check");
> 1738:
> 1739: // An attempt to handshake-suspend a thread carrying virtual thread
> will result in
Suggestion:
// An attempt to handshake-suspend a thread carrying a virtual thread will
result in
src/hotspot/share/prims/jvmtiEnvBase.hpp line 99:
> 97: static bool is_in_thread_list(jint count, const jthread* list, oop
> jt_oop);
> 98:
> 99: // check if thread_oop represents a thread carrying virtual thread
Suggestion:
// check if thread_oop represents a thread carrying a virtual thread
src/hotspot/share/prims/jvmtiEnvBase.hpp line 183:
> 181:
> 182: // Return true if the thread identified with a pair is
> current.
> 183: // A thread carrying virtual thread is not treated as current.
Suggestion:
// A thread carrying a virtual thread is not treated as current.
-
PR Review: https://git.openjdk.org/jdk/pull/14366#pullrequestreview-1468479443
PR Review Comment: https://git.openjdk.org/jdk/pull/14366#discussion_r1222104282
PR Review Comment: https://git.openjdk.org/jdk/pull/14366#discussion_r1222104787
PR Review Comment: https://git.openjdk.org/jdk/pull/14366#discussion_r1222105165
PR Review Comment: https://git.openjdk.org/jdk/pull/14366#discussion_r1222105551
Re: RFR: 8309612: [REDO] JDK-8307153 JVMTI GetThreadState on carrier should return STATE_WAITING [v4]
> 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 - Changes: - all: https://git.openjdk.org/jdk/pull/14366/files - new: https://git.openjdk.org/jdk/pull/14366/files/094b5f28..4defcf2e Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=14366=03 - incr: https://webrevs.openjdk.org/?repo=jdk=14366=02-03 Stats: 2 lines in 1 file changed: 0 ins; 0 del; 2 mod Patch: https://git.openjdk.org/jdk/pull/14366.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/14366/head:pull/14366 PR: https://git.openjdk.org/jdk/pull/14366
