From 60ea17e8482936a6acbc442bb1be199e01008072 Mon Sep 17 00:00:00 2001 From: Serguei Spitsyn Date: Wed, 5 Jun 2024 21:46:41 +0000 Subject: [PATCH] 8311177: Switching to interpreter only mode in carrier thread can lead to crashes Reviewed-by: pchilanomate, amenkov --- .../share/prims/jvmtiEventController.cpp | 16 ++- .../share/prims/jvmtiEventController.hpp | 2 +- src/hotspot/share/prims/jvmtiThreadState.cpp | 8 -- src/hotspot/share/prims/jvmtiThreadState.hpp | 3 +- .../share/prims/jvmtiThreadState.inline.hpp | 14 +- src/hotspot/share/runtime/javaThread.cpp | 3 + .../CarrierThreadEventNotification.java | 76 ++++++++++ .../libCarrierThreadEventNotification.cpp | 131 ++++++++++++++++++ .../MethodExitTest/libMethodExitTest.cpp | 8 +- 9 files changed, 239 insertions(+), 22 deletions(-) create mode 100644 test/hotspot/jtreg/serviceability/jvmti/vthread/CarrierThreadEventNotification/CarrierThreadEventNotification.java create mode 100644 test/hotspot/jtreg/serviceability/jvmti/vthread/CarrierThreadEventNotification/libCarrierThreadEventNotification.cpp diff --git a/src/hotspot/share/prims/jvmtiEventController.cpp b/src/hotspot/share/prims/jvmtiEventController.cpp index ac508fb841f..d5c5d53ba81 100644 --- a/src/hotspot/share/prims/jvmtiEventController.cpp +++ b/src/hotspot/share/prims/jvmtiEventController.cpp @@ -205,11 +205,17 @@ JvmtiEnvEventEnable::~JvmtiEnvEventEnable() { class EnterInterpOnlyModeClosure : public HandshakeClosure { private: bool _completed; + JvmtiThreadState* _state; + public: - EnterInterpOnlyModeClosure() : HandshakeClosure("EnterInterpOnlyMode"), _completed(false) { } + EnterInterpOnlyModeClosure(JvmtiThreadState* state) + : HandshakeClosure("EnterInterpOnlyMode"), + _completed(false), + _state(state) { } + void do_thread(Thread* th) { JavaThread* jt = JavaThread::cast(th); - JvmtiThreadState* state = jt->jvmti_thread_state(); + JvmtiThreadState* state = _state; assert(state != nullptr, "sanity check"); assert(state->get_thread() == jt, "handshake unsafe conditions"); @@ -369,7 +375,7 @@ void JvmtiEventControllerPrivate::enter_interp_only_mode(JvmtiThreadState *state if (target == nullptr) { // an unmounted virtual thread return; // EnterInterpOnlyModeClosure will be executed right after mount. } - EnterInterpOnlyModeClosure hs; + EnterInterpOnlyModeClosure hs(state); if (target->is_handshake_safe_for(current)) { hs.do_thread(target); } else { @@ -1101,9 +1107,9 @@ JvmtiEventController::set_extension_event_callback(JvmtiEnvBase *env, // Called by just mounted virtual thread if pending_interp_only_mode() is set. void -JvmtiEventController::enter_interp_only_mode() { +JvmtiEventController::enter_interp_only_mode(JvmtiThreadState* state) { Thread *current = Thread::current(); - EnterInterpOnlyModeClosure hs; + EnterInterpOnlyModeClosure hs(state); hs.do_thread(current); } diff --git a/src/hotspot/share/prims/jvmtiEventController.hpp b/src/hotspot/share/prims/jvmtiEventController.hpp index 84070a3098c..afe1d700d96 100644 --- a/src/hotspot/share/prims/jvmtiEventController.hpp +++ b/src/hotspot/share/prims/jvmtiEventController.hpp @@ -228,7 +228,7 @@ public: jint extension_event_index, jvmtiExtensionEvent callback); - static void enter_interp_only_mode(); + static void enter_interp_only_mode(JvmtiThreadState* state); static void set_frame_pop(JvmtiEnvThreadState *env_thread, JvmtiFramePop fpop); static void clear_frame_pop(JvmtiEnvThreadState *env_thread, JvmtiFramePop fpop); diff --git a/src/hotspot/share/prims/jvmtiThreadState.cpp b/src/hotspot/share/prims/jvmtiThreadState.cpp index c95f5646193..931071c7f00 100644 --- a/src/hotspot/share/prims/jvmtiThreadState.cpp +++ b/src/hotspot/share/prims/jvmtiThreadState.cpp @@ -639,14 +639,6 @@ JvmtiVTMSTransitionDisabler::VTMS_mount_end(jobject vthread) { thread->rebind_to_jvmti_thread_state_of(vt); - JvmtiThreadState* state = thread->jvmti_thread_state(); - if (state != nullptr && state->is_pending_interp_only_mode()) { - MutexLocker mu(JvmtiThreadState_lock); - state = thread->jvmti_thread_state(); - if (state != nullptr && state->is_pending_interp_only_mode()) { - JvmtiEventController::enter_interp_only_mode(); - } - } assert(thread->is_in_VTMS_transition(), "sanity check"); assert(!thread->is_in_tmp_VTMS_transition(), "sanity check"); finish_VTMS_transition(vthread, /* is_mount */ true); diff --git a/src/hotspot/share/prims/jvmtiThreadState.hpp b/src/hotspot/share/prims/jvmtiThreadState.hpp index ff7b6a1ebde..41d52dd2486 100644 --- a/src/hotspot/share/prims/jvmtiThreadState.hpp +++ b/src/hotspot/share/prims/jvmtiThreadState.hpp @@ -1,5 +1,5 @@ /* - * Copyright (c) 2003, 2023, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2003, 2024, 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 @@ -279,6 +279,7 @@ class JvmtiThreadState : public CHeapObj { static void unbind_from(JvmtiThreadState* state, JavaThread* thread); static void bind_to(JvmtiThreadState* state, JavaThread* thread); + static void process_pending_interp_only(JavaThread* current); // access to the linked list of all JVMTI thread states static JvmtiThreadState *first() { diff --git a/src/hotspot/share/prims/jvmtiThreadState.inline.hpp b/src/hotspot/share/prims/jvmtiThreadState.inline.hpp index a8f00fdce32..220001bb1b6 100644 --- a/src/hotspot/share/prims/jvmtiThreadState.inline.hpp +++ b/src/hotspot/share/prims/jvmtiThreadState.inline.hpp @@ -1,5 +1,5 @@ /* - * Copyright (c) 2006, 2023, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2006, 2024, 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 @@ -163,4 +163,16 @@ inline void JvmtiThreadState::bind_to(JvmtiThreadState* state, JavaThread* threa state->set_thread(thread); } } + +inline void JvmtiThreadState::process_pending_interp_only(JavaThread* current) { + JvmtiThreadState* state = current->jvmti_thread_state(); + + if (state != nullptr && state->is_pending_interp_only_mode()) { + MutexLocker mu(JvmtiThreadState_lock); + state = current->jvmti_thread_state(); + if (state != nullptr && state->is_pending_interp_only_mode()) { + JvmtiEventController::enter_interp_only_mode(state); + } + } +} #endif // SHARE_PRIMS_JVMTITHREADSTATE_INLINE_HPP diff --git a/src/hotspot/share/runtime/javaThread.cpp b/src/hotspot/share/runtime/javaThread.cpp index e4992f36842..70bb47c213c 100644 --- a/src/hotspot/share/runtime/javaThread.cpp +++ b/src/hotspot/share/runtime/javaThread.cpp @@ -1860,6 +1860,9 @@ JvmtiThreadState* JavaThread::rebind_to_jvmti_thread_state_of(oop thread_oop) { // bind new JvmtiThreadState to JavaThread JvmtiThreadState::bind_to(java_lang_Thread::jvmti_thread_state(thread_oop), this); + // enable interp_only_mode for virtual or carrier thread if it has pending bit + JvmtiThreadState::process_pending_interp_only(this); + return jvmti_thread_state(); } #endif diff --git a/test/hotspot/jtreg/serviceability/jvmti/vthread/CarrierThreadEventNotification/CarrierThreadEventNotification.java b/test/hotspot/jtreg/serviceability/jvmti/vthread/CarrierThreadEventNotification/CarrierThreadEventNotification.java new file mode 100644 index 00000000000..683b73fc068 --- /dev/null +++ b/test/hotspot/jtreg/serviceability/jvmti/vthread/CarrierThreadEventNotification/CarrierThreadEventNotification.java @@ -0,0 +1,76 @@ +/* + * Copyright (c) 2024, 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 + * under the terms of the GNU General Public License version 2 only, as + * published by the Free Software Foundation. + * + * This code is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License + * version 2 for more details (a copy is included in the LICENSE file that + * accompanied this code). + * + * You should have received a copy of the GNU General Public License version + * 2 along with this work; if not, write to the Free Software Foundation, + * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA. + * + * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA + * or visit www.oracle.com if you need additional information or have any + * questions. + */ + +/** + * @test + * @bug 8311177 + * @requires vm.continuations + * @library /test/lib + * @run main/othervm/native -agentlib:CarrierThreadEventNotification CarrierThreadEventNotification + */ + +import java.util.concurrent.*; +import java.util.ArrayList; +import java.util.List; + +public class CarrierThreadEventNotification { + static final int VTHREAD_COUNT = 64; + static volatile boolean stopRunning = false; + + private static native void setSingleSteppingMode(boolean enable); + + final Runnable FOO = () -> { + while(!stopRunning) { + recurse(10); + } + }; + + private void recurse(int depth) { + if (depth > 0) { + recurse(depth -1); + } else { + Thread.yield(); + } + } + + private void runTest() throws Exception { + List virtualThreads = new ArrayList<>(); + for (int i = 0; i < VTHREAD_COUNT; i++) { + virtualThreads.add(Thread.ofVirtual().start(FOO)); + } + for (int cnt = 0; cnt < 500; cnt++) { + setSingleSteppingMode(true); + Thread.sleep(10); + setSingleSteppingMode(false); + } + stopRunning = true; + for (Thread t : virtualThreads) { + t.join(); + } + } + + public static void main(String[] args) throws Exception { + CarrierThreadEventNotification obj = new CarrierThreadEventNotification(); + obj.runTest(); + } +} diff --git a/test/hotspot/jtreg/serviceability/jvmti/vthread/CarrierThreadEventNotification/libCarrierThreadEventNotification.cpp b/test/hotspot/jtreg/serviceability/jvmti/vthread/CarrierThreadEventNotification/libCarrierThreadEventNotification.cpp new file mode 100644 index 00000000000..f04b9def94c --- /dev/null +++ b/test/hotspot/jtreg/serviceability/jvmti/vthread/CarrierThreadEventNotification/libCarrierThreadEventNotification.cpp @@ -0,0 +1,131 @@ +/* + * Copyright (c) 2024, 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 + * under the terms of the GNU General Public License version 2 only, as + * published by the Free Software Foundation. + * + * This code is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License + * version 2 for more details (a copy is included in the LICENSE file that + * accompanied this code). + * + * You should have received a copy of the GNU General Public License version + * 2 along with this work; if not, write to the Free Software Foundation, + * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA. + * + * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA + * or visit www.oracle.com if you need additional information or have any + * questions. + */ + +#include +#include +#include +#include +#include "jvmti_common.hpp" + + +extern "C" { + +// set by Agent_OnLoad +static jvmtiEnv* jvmti = nullptr; + +static jthread* carrier_threads = nullptr; +static jint cthread_cnt = 0; + +static const char* CTHREAD_NAME_START = "ForkJoinPool"; +static const size_t CTHREAD_NAME_START_LEN = strlen("ForkJoinPool"); + +static jint +get_cthreads(JNIEnv* jni, jthread** cthreads_p) { + jthread* cthreads = nullptr; + jint all_cnt = 0; + jint ct_cnt = 0; + + jvmtiError err = jvmti->GetAllThreads(&all_cnt, &cthreads); + check_jvmti_status(jni, err, "get_cthreads: error in JVMTI GetAllThreads"); + + for (int idx = 0; idx < all_cnt; idx++) { + jthread thread = cthreads[idx]; + char* tname = get_thread_name(jvmti, jni, thread); + + if (strncmp(tname, CTHREAD_NAME_START, CTHREAD_NAME_START_LEN) == 0) { + cthreads[ct_cnt++] = jni->NewGlobalRef(thread); + } + deallocate(jvmti, jni, tname); + } + *cthreads_p = cthreads; + return ct_cnt; +} + +static void JNICALL +SingleStep(jvmtiEnv *jvmti, JNIEnv* jni, jthread thread, + jmethodID method, jlocation location) { + jboolean is_virtual = jni->IsVirtualThread(thread); + if (is_virtual) { + jni->FatalError("Virtual thread should not have posted single stepping event"); + } +} + +JNIEXPORT void JNICALL +Java_CarrierThreadEventNotification_setSingleSteppingMode(JNIEnv* jni, jclass klass, jboolean enable) { + if (enable) { + if (cthread_cnt != 0 || carrier_threads != nullptr) { + jni->FatalError("Should not be set"); + } + cthread_cnt = get_cthreads(jni, &carrier_threads); + for (int i = 0; i < cthread_cnt; i++) { + jthread thread = carrier_threads[i]; + jvmtiError err = jvmti->SetEventNotificationMode(JVMTI_ENABLE, JVMTI_EVENT_SINGLE_STEP, thread); + check_jvmti_status(jni, err, "event handler: error in JVMTI SetEventNotificationMode for event JVMTI_EVENT_SINGLE_STEP"); + } + } else { + if (carrier_threads == nullptr) { + jni->FatalError("Should be set"); + } + for (int i = 0; i < cthread_cnt; i++) { + jthread thread = carrier_threads[i]; + jvmtiError err = jvmti->SetEventNotificationMode(JVMTI_DISABLE, JVMTI_EVENT_SINGLE_STEP, thread); + check_jvmti_status(jni, err, "event handler: error in JVMTI SetEventNotificationMode for event JVMTI_EVENT_SINGLE_STEP"); + jni->DeleteGlobalRef(thread); + } + deallocate(jvmti, jni, carrier_threads); + cthread_cnt = 0; + carrier_threads = nullptr; + } +} + +JNIEXPORT jint JNICALL Agent_OnLoad(JavaVM* jvm, char* options, void* reserved) { + jvmtiEventCallbacks callbacks; + jvmtiCapabilities caps; + jvmtiError err; + + printf("Agent_OnLoad: started\n"); + if (jvm->GetEnv((void **) (&jvmti), JVMTI_VERSION) != JNI_OK) { + LOG("error in GetEnv"); + return JNI_ERR; + } + + memset(&caps, 0, sizeof(caps)); + caps.can_generate_single_step_events = 1; + caps.can_support_virtual_threads = 1; + + err = jvmti->AddCapabilities(&caps); + if (err != JVMTI_ERROR_NONE) { + LOG("error in JVMTI AddCapabilities: %d\n", err); + } + + memset(&callbacks, 0, sizeof(callbacks)); + callbacks.SingleStep = &SingleStep; + err = jvmti->SetEventCallbacks(&callbacks, sizeof(jvmtiEventCallbacks)); + if (err != JVMTI_ERROR_NONE) { + LOG("Agent_OnLoad: Error in JVMTI SetEventCallbacks: %d\n", err); + } + + return 0; +} + +} // extern "C" diff --git a/test/hotspot/jtreg/serviceability/jvmti/vthread/MethodExitTest/libMethodExitTest.cpp b/test/hotspot/jtreg/serviceability/jvmti/vthread/MethodExitTest/libMethodExitTest.cpp index 78e14223377..b7077b10c81 100644 --- a/test/hotspot/jtreg/serviceability/jvmti/vthread/MethodExitTest/libMethodExitTest.cpp +++ b/test/hotspot/jtreg/serviceability/jvmti/vthread/MethodExitTest/libMethodExitTest.cpp @@ -197,12 +197,8 @@ breakpoint_hit2(jvmtiEnv *jvmti, JNIEnv* jni, jboolean is_virtual, char* mname) { jvmtiError err; - // Verify that we did not get a METHOD_EXIT events when enabled on the cthread. - if (received_method_exit_event) { - passed = JNI_FALSE; - received_method_exit_event = JNI_FALSE; - LOG("FAILED: got METHOD_EXIT event on the cthread: %p\n", cthread); - } + // need to reset this value after the breakpoint_hit1 + received_method_exit_event = JNI_FALSE; // Disable METHOD_EXIT events on the cthread. LOG("Hit #2: Breakpoint: %s: disabling MethodExit events on carrier thread: %p\n",