8217618: JVM TI SuspendThread doesn't suspend the current thread before returning
Reviewed-by: dcubed, sspitsyn, dlong
--- 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;
}