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

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");

src/hotspot/share/prims/jvmtiEnvBase.hpp

@@ -96,8 +96,8 @@ class JvmtiEnvBase : public CHeapObj<mtInternal> {
   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<mtInternal> {
   static oop current_thread_obj_or_resolve_external_guard(jthread thread);
 
   // Return true if the thread identified with a pair <jt,thr_obj> 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<mtInternal> {
   // 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

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 @@ private void runTest() throws Exception {
 
             List<Thread> 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) {

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,30 +42,70 @@ 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) {
   jvmtiEventCallbacks callbacks;
   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;
 }