From f91e9ba757f04983655c23542e06973805465249 Mon Sep 17 00:00:00 2001 From: Serguei Spitsyn Date: Fri, 9 Jun 2023 06:12:48 +0000 Subject: [PATCH] 8309612: [REDO] JDK-8307153 JVMTI GetThreadState on carrier should return STATE_WAITING Reviewed-by: cjplummer, amenkov --- src/hotspot/share/prims/jvmtiEnvBase.cpp | 42 +++++++++++--- src/hotspot/share/prims/jvmtiEnvBase.hpp | 9 +-- .../ThreadStateTest/ThreadStateTest.java | 8 ++- .../ThreadStateTest/libThreadStateTest.cpp | 57 +++++++++++++++++-- 4 files changed, 96 insertions(+), 20 deletions(-) diff --git a/src/hotspot/share/prims/jvmtiEnvBase.cpp b/src/hotspot/share/prims/jvmtiEnvBase.cpp index 12fd4755e17..ce9d7fab5ed 100644 --- a/src/hotspot/share/prims/jvmtiEnvBase.cpp +++ b/src/hotspot/share/prims/jvmtiEnvBase.cpp @@ -730,7 +730,7 @@ JvmtiEnvBase::get_cthread_last_java_vframe(JavaThread* jt, RegisterMap* reg_map_ } jint -JvmtiEnvBase::get_thread_state(oop thread_oop, JavaThread* jt) { +JvmtiEnvBase::get_thread_state_base(oop thread_oop, JavaThread* jt) { jint state = 0; if (thread_oop != nullptr) { @@ -756,6 +756,30 @@ JvmtiEnvBase::get_thread_state(oop thread_oop, JavaThread* jt) { return state; } +jint +JvmtiEnvBase::get_thread_state(oop thread_oop, JavaThread* jt) { + jint state = 0; + + if (is_thread_carrying_vthread(jt, thread_oop)) { + state = (jint)java_lang_Thread::get_thread_status(thread_oop); + + // This is for extra safety. Other bits are not expected nor needed. + state &= (JVMTI_THREAD_STATE_ALIVE | JVMTI_THREAD_STATE_INTERRUPTED); + + 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 not needed and the WAITING bits are added. + state |= JVMTI_THREAD_STATE_WAITING | JVMTI_THREAD_STATE_WAITING_INDEFINITELY; + } else { + state = get_thread_state_base(thread_oop, jt); + } + return state; +} + jint JvmtiEnvBase::get_vthread_state(oop thread_oop, JavaThread* java_thread) { jint state = 0; @@ -770,7 +794,7 @@ JvmtiEnvBase::get_vthread_state(oop thread_oop, JavaThread* java_thread) { jint filtered_bits = JVMTI_THREAD_STATE_SUSPENDED | JVMTI_THREAD_STATE_INTERRUPTED; // This call can trigger a safepoint, so thread_oop must not be used after it. - state = get_thread_state(ct_oop, java_thread) & ~filtered_bits; + state = get_thread_state_base(ct_oop, java_thread) & ~filtered_bits; } else { jshort vt_state = java_lang_VirtualThread::state(thread_oop); state = (jint)java_lang_VirtualThread::map_state_to_thread_status(vt_state); @@ -1708,13 +1732,13 @@ JvmtiEnvBase::suspend_thread(oop thread_oop, JavaThread* java_thread, bool singl if (java_thread->is_hidden_from_external_view()) { return JVMTI_ERROR_NONE; } - bool is_passive_cthread = is_passive_carrier_thread(java_thread, thread_h()); + bool is_thread_carrying = is_thread_carrying_vthread(java_thread, thread_h()); // A case of non-virtual thread. if (!is_virtual) { // Thread.suspend() is used in some tests. It sets jt->is_suspended() only. if (java_thread->is_carrier_thread_suspended() || - (!is_passive_cthread && java_thread->is_suspended())) { + (!is_thread_carrying && java_thread->is_suspended())) { return JVMTI_ERROR_THREAD_SUSPENDED; } java_thread->set_carrier_thread_suspended(); @@ -1725,10 +1749,10 @@ JvmtiEnvBase::suspend_thread(oop thread_oop, JavaThread* java_thread, bool singl (is_virtual && JvmtiVTSuspender::is_vthread_suspended(thread_h())), "sanity check"); - // An attempt to handshake-suspend a passive carrier thread will result in + // An attempt to handshake-suspend a thread carrying a virtual thread will result in // suspension of mounted virtual thread. So, we just mark it as suspended // and it will be actually suspended at virtual thread unmount transition. - if (!is_passive_cthread) { + if (!is_thread_carrying) { assert(thread_h() != nullptr, "sanity check"); assert(single_suspend || thread_h()->is_a(vmClasses::BaseVirtualThread_klass()), "SuspendAllVirtualThreads should never suspend non-virtual threads"); @@ -1778,19 +1802,19 @@ JvmtiEnvBase::resume_thread(oop thread_oop, JavaThread* java_thread, bool single if (java_thread->is_hidden_from_external_view()) { return JVMTI_ERROR_NONE; } - bool is_passive_cthread = is_passive_carrier_thread(java_thread, thread_h()); + bool is_thread_carrying = is_thread_carrying_vthread(java_thread, thread_h()); // A case of a non-virtual thread. if (!is_virtual) { if (!java_thread->is_carrier_thread_suspended() && - (is_passive_cthread || !java_thread->is_suspended())) { + (is_thread_carrying || !java_thread->is_suspended())) { return JVMTI_ERROR_THREAD_NOT_SUSPENDED; } java_thread->clear_carrier_thread_suspended(); } assert(!java_thread->is_in_VTMS_transition(), "sanity check"); - if (!is_passive_cthread) { + if (!is_thread_carrying) { assert(thread_h() != nullptr, "sanity check"); assert(single_resume || thread_h()->is_a(vmClasses::BaseVirtualThread_klass()), "ResumeAllVirtualThreads should never resume non-virtual threads"); diff --git a/src/hotspot/share/prims/jvmtiEnvBase.hpp b/src/hotspot/share/prims/jvmtiEnvBase.hpp index b5d1b1520cd..6fdeb11f406 100644 --- a/src/hotspot/share/prims/jvmtiEnvBase.hpp +++ b/src/hotspot/share/prims/jvmtiEnvBase.hpp @@ -96,8 +96,8 @@ class JvmtiEnvBase : public CHeapObj { static jvmtiError check_thread_list(jint count, const jthread* list); static bool is_in_thread_list(jint count, const jthread* list, oop jt_oop); - // check if thread_oop represents a passive carrier thread - static bool is_passive_carrier_thread(JavaThread* java_thread, oop thread_oop) { + // check if thread_oop represents a thread carrying a virtual thread + static bool is_thread_carrying_vthread(JavaThread* java_thread, oop thread_oop) { return java_thread != nullptr && java_thread->jvmti_vthread() != nullptr && java_thread->jvmti_vthread() != thread_oop && java_thread->threadObj() == thread_oop; @@ -180,7 +180,7 @@ class JvmtiEnvBase : public CHeapObj { static oop current_thread_obj_or_resolve_external_guard(jthread thread); // Return true if the thread identified with a pair is current. - // A passive carrier thread is not treated as current. + // A thread carrying a virtual thread is not treated as current. static bool is_JavaThread_current(JavaThread* jt, oop thr_obj) { JavaThread* current = JavaThread::current(); // jt can be null in case of a virtual thread @@ -384,7 +384,8 @@ class JvmtiEnvBase : public CHeapObj { // get carrier thread last java vframe static javaVFrame* get_cthread_last_java_vframe(JavaThread* jt, RegisterMap* reg_map); - // get ordinary thread thread state + // get platform thread state + static jint get_thread_state_base(oop thread_oop, JavaThread* jt); static jint get_thread_state(oop thread_oop, JavaThread* jt); // get virtual thread thread state diff --git a/test/hotspot/jtreg/serviceability/jvmti/vthread/ThreadStateTest/ThreadStateTest.java b/test/hotspot/jtreg/serviceability/jvmti/vthread/ThreadStateTest/ThreadStateTest.java index cd4cda510ef..4c8b9bb030f 100644 --- a/test/hotspot/jtreg/serviceability/jvmti/vthread/ThreadStateTest/ThreadStateTest.java +++ b/test/hotspot/jtreg/serviceability/jvmti/vthread/ThreadStateTest/ThreadStateTest.java @@ -42,8 +42,12 @@ public class ThreadStateTest { private static native void setSingleSteppingMode(boolean enable); private static native void setMonitorContendedMode(boolean enable); + private static native void testGetThreadState(Thread thread); + private static native void testGetThreadListStackTraces(Thread thread); final Runnable FOO = () -> { + testGetThreadState(Thread.currentThread()); + testGetThreadListStackTraces(Thread.currentThread()); Thread.yield(); }; @@ -59,7 +63,9 @@ public class ThreadStateTest { List virtualThreads = new ArrayList<>(); for (int i = 0; i < VTHREAD_COUNT; i++) { - virtualThreads.add(factory.newThread(FOO)); + Thread vt = factory.newThread(FOO); + vt.setName("VT-" + i); + virtualThreads.add(vt); } for (Thread t : virtualThreads) { diff --git a/test/hotspot/jtreg/serviceability/jvmti/vthread/ThreadStateTest/libThreadStateTest.cpp b/test/hotspot/jtreg/serviceability/jvmti/vthread/ThreadStateTest/libThreadStateTest.cpp index b5f826ff8d5..895a2ba198e 100644 --- a/test/hotspot/jtreg/serviceability/jvmti/vthread/ThreadStateTest/libThreadStateTest.cpp +++ b/test/hotspot/jtreg/serviceability/jvmti/vthread/ThreadStateTest/libThreadStateTest.cpp @@ -29,6 +29,10 @@ // set by Agent_OnLoad static jvmtiEnv* jvmti = nullptr; +static const jint EXP_VT_STATE = JVMTI_THREAD_STATE_ALIVE | JVMTI_THREAD_STATE_RUNNABLE; +static const jint EXP_CT_STATE = JVMTI_THREAD_STATE_ALIVE | JVMTI_THREAD_STATE_WAITING | + JVMTI_THREAD_STATE_WAITING_INDEFINITELY; +static const jint MAX_FRAME_COUNT = 32; extern "C" { @@ -38,20 +42,60 @@ SingleStep(jvmtiEnv *jvmti, JNIEnv* jni, jthread thread, } static void JNICALL -MonitorContended(jvmtiEnv* jvmti, JNIEnv* jni_env, jthread thread, +MonitorContended(jvmtiEnv* jvmti, JNIEnv* jni, jthread thread, jobject object) { } +static void JNICALL +check_thread_state(jvmtiEnv* jvmti, JNIEnv* jni, jthread thread, jint state, jint exp_state, const char* msg) { + if (state != exp_state) { + const char* tname = get_thread_name(jvmti, jni, thread); + + LOG("FAILED: %p: %s: thread state: %x expected state: %x\n", + (void*)thread, tname, state, exp_state); + + deallocate(jvmti, jni, (void*)tname); + jni->FatalError(msg); + } +} + JNIEXPORT void JNICALL Java_ThreadStateTest_setSingleSteppingMode(JNIEnv* jni, jclass klass, jboolean enable) { jvmtiError err = jvmti->SetEventNotificationMode(enable ? JVMTI_ENABLE : JVMTI_DISABLE, JVMTI_EVENT_SINGLE_STEP, nullptr); - check_jvmti_status(jni, err, "event handler: error in JVMTI SetEventNotificationMode for event JVMTI_EVENT_SINGLE_STEP"); + check_jvmti_status(jni, err, "setSingleSteppingMode: error in JVMTI SetEventNotificationMode for JVMTI_EVENT_SINGLE_STEP"); } JNIEXPORT void JNICALL Java_ThreadStateTest_setMonitorContendedMode(JNIEnv* jni, jclass klass, jboolean enable) { jvmtiError err = jvmti->SetEventNotificationMode(enable ? JVMTI_ENABLE : JVMTI_DISABLE, JVMTI_EVENT_MONITOR_CONTENDED_ENTER, nullptr); - check_jvmti_status(jni, err, "event handler: error in JVMTI SetEventNotificationMode for event JVMTI_EVENT_MONITOR_CONTENDED_ENTER"); + check_jvmti_status(jni, err, "setMonitorContendedMode: error in JVMTI SetEventNotificationMode for JVMTI_EVENT_MONITOR_CONTENDED_ENTER"); +} + +JNIEXPORT void JNICALL +Java_ThreadStateTest_testGetThreadState(JNIEnv* jni, jclass klass, jthread vthread) { + jthread cthread = get_carrier_thread(jvmti, jni, vthread); + jint ct_state = get_thread_state(jvmti, jni, cthread); + jint vt_state = get_thread_state(jvmti, jni, vthread); + + check_thread_state(jvmti, jni, cthread, ct_state, EXP_CT_STATE, + "Failed: unexpected carrier thread state from JVMTI GetThreadState"); + check_thread_state(jvmti, jni, vthread, vt_state, EXP_VT_STATE, + "Failed: unexpected virtual thread state from JVMTI GetThreadState"); +} + +JNIEXPORT void JNICALL +Java_ThreadStateTest_testGetThreadListStackTraces(JNIEnv* jni, jclass klass, jthread vthread) { + jthread cthread = get_carrier_thread(jvmti, jni, vthread); + jthread threads[2] = { cthread, vthread }; + jvmtiStackInfo* stackInfo = NULL; + + jvmtiError err = jvmti->GetThreadListStackTraces(2, threads, MAX_FRAME_COUNT, &stackInfo); + check_jvmti_status(jni, err, "testGetThreadState: error in JVMTI GetThreadListStackTraces"); + + check_thread_state(jvmti, jni, cthread, stackInfo[0].state, EXP_CT_STATE, + "Failed: unexpected carrier thread state from JVMTI GetThreadListStackTraces"); + check_thread_state(jvmti, jni, vthread, stackInfo[1].state, EXP_VT_STATE, + "Failed: unexpected virtual thread state from JVMTI GetThreadListStackTraces"); } JNIEXPORT jint JNICALL Agent_OnLoad(JavaVM* jvm, char* options, void* reserved) { @@ -59,9 +103,9 @@ JNIEXPORT jint JNICALL Agent_OnLoad(JavaVM* jvm, char* options, void* reserved) jvmtiCapabilities caps; jvmtiError err; - printf("Agent_OnLoad started\n"); + printf("Agent_OnLoad: started\n"); if (jvm->GetEnv((void **) (&jvmti), JVMTI_VERSION) != JNI_OK) { - LOG("error in GetEnv"); + LOG("Agent_OnLoad: error in GetEnv"); return JNI_ERR; } @@ -72,7 +116,7 @@ JNIEXPORT jint JNICALL Agent_OnLoad(JavaVM* jvm, char* options, void* reserved) err = jvmti->AddCapabilities(&caps); if (err != JVMTI_ERROR_NONE) { - LOG("error in JVMTI AddCapabilities: %d\n", err); + LOG("Agent_OnLoad: error in JVMTI AddCapabilities: %d\n", err); } memset(&callbacks, 0, sizeof(callbacks)); @@ -82,6 +126,7 @@ JNIEXPORT jint JNICALL Agent_OnLoad(JavaVM* jvm, char* options, void* reserved) if (err != JVMTI_ERROR_NONE) { LOG("Agent_OnLoad: Error in JVMTI SetEventCallbacks: %d\n", err); } + printf("Agent_OnLoad: finished\n"); return 0; }