RFR: 8309612: [REDO] JDK-8307153 JVMTI GetThreadState on carrier should return STATE_WAITING [v4]

Chris Plummer cjplummer at openjdk.org
Wed Jun 7 20:15:54 UTC 2023


On Wed, 7 Jun 2023 20:05:45 GMT, Serguei Spitsyn <sspitsyn at 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

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 <jt,thr_obj> 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


More information about the serviceability-dev mailing list