8217618: JVM TI SuspendThread doesn't suspend the current thread before returning
authordholmes
Sun, 27 Jan 2019 20:48:27 -0500
changeset 53518 2181425e0460
parent 53517 20ba6a7f897a
child 53519 74a5ef4c81cc
8217618: JVM TI SuspendThread doesn't suspend the current thread before returning Reviewed-by: dcubed, sspitsyn, dlong
src/hotspot/share/runtime/thread.cpp
test/hotspot/jtreg/vmTestbase/nsk/jvmti/scenarios/hotswap/HS202/hs202t002/hs202t002.cpp
test/hotspot/jtreg/vmTestbase/nsk/jvmti/scenarios/sampling/SP05/sp05t003/sp05t003.cpp
--- a/src/hotspot/share/runtime/thread.cpp	Mon Jan 28 08:01:06 2019 +0900
+++ b/src/hotspot/share/runtime/thread.cpp	Sun Jan 27 20:48:27 2019 -0500
@@ -2392,8 +2392,19 @@
     }
   }
 
-  VM_ThreadSuspend vm_suspend;
-  VMThread::execute(&vm_suspend);
+  if (Thread::current() == this) {
+    // Safely self-suspend.
+    // If we don't do this explicitly it will implicitly happen
+    // before we transition back to Java, and on some other thread-state
+    // transition paths, but not as we exit a JVM TI SuspendThread call.
+    // As SuspendThread(current) must not return (until resumed) we must
+    // self-suspend here.
+    ThreadBlockInVM tbivm(this);
+    java_suspend_self();
+  } else {
+    VM_ThreadSuspend vm_suspend;
+    VMThread::execute(&vm_suspend);
+  }
 }
 
 // Part II of external suspension.
--- a/test/hotspot/jtreg/vmTestbase/nsk/jvmti/scenarios/hotswap/HS202/hs202t002/hs202t002.cpp	Mon Jan 28 08:01:06 2019 +0900
+++ b/test/hotspot/jtreg/vmTestbase/nsk/jvmti/scenarios/hotswap/HS202/hs202t002/hs202t002.cpp	Sun Jan 27 20:48:27 2019 -0500
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2007, 2018, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2007, 2019, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -43,13 +43,7 @@
 static jint redefineNumber = 0;
 static jvmtiEnv * jvmti = NULL;
 
-typedef enum {
-  suspend_error = -1,
-  not_suspended,
-  suspended
-} thread_suspend_status_t;
-
-static volatile thread_suspend_status_t thread_suspend_status = not_suspended;
+static volatile bool thread_suspend_error = false;
 
 void JNICALL callbackMethodExit(jvmtiEnv *jvmti_env,
                                 JNIEnv* jni_env,
@@ -75,10 +69,10 @@
                 nsk_printf("Agent::SUSPENDING>> \n");
                 err=jvmti_env->SuspendThread(thread);
                 if (err == JVMTI_ERROR_NONE) {
-                    thread_suspend_status = suspended;
-                    nsk_printf("Agent:: Thread successfully suspended..\n");
-                } else if (err == JVMTI_ERROR_THREAD_SUSPENDED) {
-                    thread_suspend_status = suspend_error;
+                  // we don't get here until we are resumed
+                    nsk_printf("Agent:: Thread successfully suspended and was resumed\n");
+                } else {
+                    thread_suspend_error = true;
                     nsk_printf(" ## Error occured %s \n",TranslateError(err));
                 }
             }
@@ -175,7 +169,6 @@
 
     err = jvmti->ResumeThread(thread);
     if (err == JVMTI_ERROR_NONE) {
-        thread_suspend_status = not_suspended;
         retvalue = JNI_TRUE;
         nsk_printf(" Agent:: Thread Resumed.. \n");
     } else {
@@ -189,13 +182,21 @@
 Java_nsk_jvmti_scenarios_hotswap_HS202_hs202t002_hs202t002_isThreadSuspended(JNIEnv* jni,
                                                                              jclass clas,
                                                                              jthread thread) {
-    if (suspend_error == thread_suspend_status) {
+    if (thread_suspend_error) {
         jclass ex_class = jni->FindClass("java/lang/IllegalThreadStateException");
         jni->ThrowNew(ex_class, "Thread has failed to self suspend");
         return JNI_FALSE;
     }
 
-    return suspended == thread_suspend_status;
+    // There is an inherent race here if the suspend fails for some reason but
+    // thread_suspend_error is not yet set. But as long as we report the suspend
+    // state correctly there is no problem as the Java code will simply loop and call
+    // this again until we see thread_suspend_error is true.
+
+    jint state = 0;
+    // No errors possible here: thread is valid, and state is not NULL
+    jvmti->GetThreadState(thread, &state);
+    return (state & JVMTI_THREAD_STATE_SUSPENDED) != 0;
 }
 
-}
+} // extern C
--- a/test/hotspot/jtreg/vmTestbase/nsk/jvmti/scenarios/sampling/SP05/sp05t003/sp05t003.cpp	Mon Jan 28 08:01:06 2019 +0900
+++ b/test/hotspot/jtreg/vmTestbase/nsk/jvmti/scenarios/sampling/SP05/sp05t003/sp05t003.cpp	Sun Jan 27 20:48:27 2019 -0500
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2003, 2018, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2003, 2019, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -65,7 +65,7 @@
 /* references to tested threads */
 static jthread threadsList[THREADS_COUNT];
 
-/* events conunts */
+/* events counts */
 static volatile int eventsStart = 0;
 static volatile int eventsEnd   = 0;
 
@@ -371,17 +371,18 @@
     /* check if event is for tested thread */
     for (i = 0; i < THREADS_COUNT; i++) {
         if (jni->IsSameObject(threadsList[i], thread)) {
-                NSK_DISPLAY0("SUCCESS: expected THREAD_START event\n");
+            NSK_DISPLAY0("SUCCESS: expected THREAD_START event\n");
 
             /* suspend thread */
             NSK_DISPLAY3("  suspend starting thread #%d (%s): %p\n",
                                 i, threadsName[i], (void*)thread);
+            /* must bump the count before we suspend */
+            eventsStart++;
 
             if (!NSK_JVMTI_VERIFY(jvmti->SuspendThread(thread))) {
                 nsk_jvmti_setFailStatus();
                 return;
             }
-            eventsStart++;
 
             break;
         }
@@ -406,6 +407,8 @@
     for (i = 0; i < THREADS_COUNT; i++) {
         if (jni->IsSameObject(threadsList[i], thread)) {
                 NSK_DISPLAY0("SUCCESS: expected THREAD_END event\n");
+            /* must bump the count before we suspend */
+            eventsEnd++;
 
             /* suspend thread */
             NSK_DISPLAY3("  suspend finishing thread #%d (%s): %p\n",
@@ -415,7 +418,6 @@
                 nsk_jvmti_setFailStatus();
                 return;
             }
-            eventsEnd++;
 
             break;
         }