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 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
