From 82e99fae178b930cca56adc01645f8d76382f83a Mon Sep 17 00:00:00 2001 From: Serguei Spitsyn Date: Wed, 9 Apr 2025 00:47:52 -0700 Subject: [PATCH 1/4] 8352773: JVMTI should disable events during java upcalls --- src/hotspot/share/prims/jvmtiEnv.cpp | 3 ++- src/hotspot/share/prims/jvmtiEnvBase.cpp | 1 + src/hotspot/share/prims/jvmtiEnvBase.hpp | 18 ++++++++++++++++++ src/hotspot/share/prims/jvmtiExport.cpp | 4 ++++ src/hotspot/share/runtime/javaThread.cpp | 1 + src/hotspot/share/runtime/javaThread.hpp | 7 ++++++- 6 files changed, 32 insertions(+), 2 deletions(-) diff --git a/src/hotspot/share/prims/jvmtiEnv.cpp b/src/hotspot/share/prims/jvmtiEnv.cpp index fe1578aedf278..7a0c9a428c5a3 100644 --- a/src/hotspot/share/prims/jvmtiEnv.cpp +++ b/src/hotspot/share/prims/jvmtiEnv.cpp @@ -1212,7 +1212,7 @@ JvmtiEnv::StopThread(jthread thread, jobject exception) { // thread - NOT protected by ThreadsListHandle and NOT pre-checked jvmtiError JvmtiEnv::InterruptThread(jthread thread) { - JavaThread* current_thread = JavaThread::current(); + JavaThread* current_thread = JavaThread::current(); HandleMark hm(current_thread); JvmtiVTMSTransitionDisabler disabler(thread); @@ -1228,6 +1228,7 @@ JvmtiEnv::InterruptThread(jthread thread) { if (java_lang_VirtualThread::is_instance(thread_obj)) { // For virtual threads we have to call into Java to interrupt: Handle obj(current_thread, thread_obj); + JvmtiJavaUpcallMark jjum(current_thread); // hide JVMTI events for Java upcall JavaValue result(T_VOID); JavaCalls::call_virtual(&result, obj, diff --git a/src/hotspot/share/prims/jvmtiEnvBase.cpp b/src/hotspot/share/prims/jvmtiEnvBase.cpp index 71b2c9d216ea4..017bd800ffeab 100644 --- a/src/hotspot/share/prims/jvmtiEnvBase.cpp +++ b/src/hotspot/share/prims/jvmtiEnvBase.cpp @@ -864,6 +864,7 @@ JvmtiEnvBase::get_subgroups(JavaThread* current_thread, Handle group_hdl, jint * // This call collects the strong and weak groups JavaThread* THREAD = current_thread; + JvmtiJavaUpcallMark jjum(current_thread); JavaValue result(T_OBJECT); JavaCalls::call_virtual(&result, group_hdl, diff --git a/src/hotspot/share/prims/jvmtiEnvBase.hpp b/src/hotspot/share/prims/jvmtiEnvBase.hpp index 5d6b5a2209e29..17b7dcb7ca0ea 100644 --- a/src/hotspot/share/prims/jvmtiEnvBase.hpp +++ b/src/hotspot/share/prims/jvmtiEnvBase.hpp @@ -454,6 +454,24 @@ class JvmtiEnvIterator : public StackObj { JvmtiEnv* next(JvmtiEnvBase* env) { return env->next_environment(); } }; +// This helper class marks current thread as making a Java upcall. +// It is needed to hide JVMTI events during JVMTI operation. +class JvmtiJavaUpcallMark : public StackObj { + private: + JavaThread* _current; + public: + JvmtiJavaUpcallMark(JavaThread* current) { + _current = current; + assert(!_current->is_in_java_upcall(), "sanity check"); + _current->toggle_is_in_java_upcall(); + } + + ~JvmtiJavaUpcallMark() { + assert(_current->is_in_java_upcall(), "sanity check"); + _current->toggle_is_in_java_upcall(); + } +}; + // Used in combination with the JvmtiHandshake class. // It is intended to support both platform and virtual threads. class JvmtiUnitedHandshakeClosure : public HandshakeClosure { diff --git a/src/hotspot/share/prims/jvmtiExport.cpp b/src/hotspot/share/prims/jvmtiExport.cpp index 9bfe1f2ab8585..07cf51cdc4932 100644 --- a/src/hotspot/share/prims/jvmtiExport.cpp +++ b/src/hotspot/share/prims/jvmtiExport.cpp @@ -1413,6 +1413,10 @@ void JvmtiExport::post_class_prepare(JavaThread *thread, Klass* klass) { return; } if (thread->should_hide_jvmti_events()) { + // All events can be disabled if current thread is doing a Java upcall originated by JVMTI. + // ClassPrepare events are important for JDWP agent but not expected during such upcalls. + // Catch if this invariant is not broken. + assert(!thread->is_in_java_upcall(), "unexpected ClassPrepare event during JVMTI upcall"); return; } diff --git a/src/hotspot/share/runtime/javaThread.cpp b/src/hotspot/share/runtime/javaThread.cpp index f33f51518557d..b5c7709ac88f1 100644 --- a/src/hotspot/share/runtime/javaThread.cpp +++ b/src/hotspot/share/runtime/javaThread.cpp @@ -450,6 +450,7 @@ JavaThread::JavaThread(MemTag mem_tag) : _carrier_thread_suspended(false), _is_in_VTMS_transition(false), _is_disable_suspend(false), + _is_in_java_upcall(false), _VTMS_transition_mark(false), _on_monitor_waited_event(false), _contended_entered_monitor(nullptr), diff --git a/src/hotspot/share/runtime/javaThread.hpp b/src/hotspot/share/runtime/javaThread.hpp index 5448903340b9b..25d3b7906e69b 100644 --- a/src/hotspot/share/runtime/javaThread.hpp +++ b/src/hotspot/share/runtime/javaThread.hpp @@ -331,6 +331,7 @@ class JavaThread: public Thread { volatile bool _carrier_thread_suspended; // Carrier thread is externally suspended bool _is_in_VTMS_transition; // thread is in virtual thread mount state transition bool _is_disable_suspend; // JVMTI suspend is temporarily disabled; used on current thread only + bool _is_in_java_upcall; // JVMTI is doing a Java upcall, so JVMTI events must be hidden bool _VTMS_transition_mark; // used for sync between VTMS transitions and disablers bool _on_monitor_waited_event; // Avoid callee arg processing for enterSpecial when posting waited event ObjectMonitor* _contended_entered_monitor; // Monitor for pending monitor_contended_entered callback @@ -722,13 +723,17 @@ class JavaThread: public Thread { bool is_disable_suspend() const { return _is_disable_suspend; } void toggle_is_disable_suspend() { _is_disable_suspend = !_is_disable_suspend; }; + bool is_in_java_upcall() const { return _is_in_java_upcall; } + void toggle_is_in_java_upcall() { _is_in_java_upcall = !_is_in_java_upcall; }; + bool VTMS_transition_mark() const { return Atomic::load(&_VTMS_transition_mark); } void set_VTMS_transition_mark(bool val) { Atomic::store(&_VTMS_transition_mark, val); } // Temporarily skip posting JVMTI events for safety reasons when executions is in a critical section: // - is in a VTMS transition (_is_in_VTMS_transition) // - is in an interruptLock or similar critical section (_is_disable_suspend) - bool should_hide_jvmti_events() const { return _is_in_VTMS_transition || _is_disable_suspend; } + // - JVMTI is making a Java upcall (_is_in_java_upcall) + bool should_hide_jvmti_events() const { return _is_in_VTMS_transition || _is_disable_suspend || _is_in_java_upcall; } bool on_monitor_waited_event() { return _on_monitor_waited_event; } void set_on_monitor_waited_event(bool val) { _on_monitor_waited_event = val; } From 342e8a78d4b7016e28e766481791aaa60407d20d Mon Sep 17 00:00:00 2001 From: Serguei Spitsyn Date: Wed, 9 Apr 2025 15:37:06 -0700 Subject: [PATCH 2/4] review: 1) added an assert for ClassLoad events same as for ClassPrepare; 2) added one minor comment --- src/hotspot/share/prims/jvmtiEnvBase.cpp | 2 +- src/hotspot/share/prims/jvmtiExport.cpp | 4 ++++ 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/src/hotspot/share/prims/jvmtiEnvBase.cpp b/src/hotspot/share/prims/jvmtiEnvBase.cpp index 017bd800ffeab..9e0929e69cffd 100644 --- a/src/hotspot/share/prims/jvmtiEnvBase.cpp +++ b/src/hotspot/share/prims/jvmtiEnvBase.cpp @@ -864,7 +864,7 @@ JvmtiEnvBase::get_subgroups(JavaThread* current_thread, Handle group_hdl, jint * // This call collects the strong and weak groups JavaThread* THREAD = current_thread; - JvmtiJavaUpcallMark jjum(current_thread); + JvmtiJavaUpcallMark jjum(current_thread); // hide JVMTI events for Java upcall JavaValue result(T_OBJECT); JavaCalls::call_virtual(&result, group_hdl, diff --git a/src/hotspot/share/prims/jvmtiExport.cpp b/src/hotspot/share/prims/jvmtiExport.cpp index 07cf51cdc4932..b421705f4e974 100644 --- a/src/hotspot/share/prims/jvmtiExport.cpp +++ b/src/hotspot/share/prims/jvmtiExport.cpp @@ -1376,6 +1376,10 @@ void JvmtiExport::post_class_load(JavaThread *thread, Klass* klass) { return; } if (thread->should_hide_jvmti_events()) { + // All events can be disabled if current thread is doing a Java upcall originated by JVMTI. + // ClassLoad events are important for JDWP agent but not expected during such upcalls. + // Catch if this invariant is not broken. + assert(!thread->is_in_java_upcall(), "unexpected ClassLoad event during JVMTI upcall"); return; } From f0b70372df84f917ee25f4cc0e8cc23ae1190f3e Mon Sep 17 00:00:00 2001 From: Serguei Spitsyn Date: Wed, 9 Apr 2025 22:45:38 -0700 Subject: [PATCH 3/4] review: minor tweak in two similar comments --- src/hotspot/share/prims/jvmtiExport.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/hotspot/share/prims/jvmtiExport.cpp b/src/hotspot/share/prims/jvmtiExport.cpp index b421705f4e974..d288baaab54e1 100644 --- a/src/hotspot/share/prims/jvmtiExport.cpp +++ b/src/hotspot/share/prims/jvmtiExport.cpp @@ -1378,7 +1378,7 @@ void JvmtiExport::post_class_load(JavaThread *thread, Klass* klass) { if (thread->should_hide_jvmti_events()) { // All events can be disabled if current thread is doing a Java upcall originated by JVMTI. // ClassLoad events are important for JDWP agent but not expected during such upcalls. - // Catch if this invariant is not broken. + // Catch if this invariant is broken. assert(!thread->is_in_java_upcall(), "unexpected ClassLoad event during JVMTI upcall"); return; } @@ -1419,7 +1419,7 @@ void JvmtiExport::post_class_prepare(JavaThread *thread, Klass* klass) { if (thread->should_hide_jvmti_events()) { // All events can be disabled if current thread is doing a Java upcall originated by JVMTI. // ClassPrepare events are important for JDWP agent but not expected during such upcalls. - // Catch if this invariant is not broken. + // Catch if this invariant is broken. assert(!thread->is_in_java_upcall(), "unexpected ClassPrepare event during JVMTI upcall"); return; } From 908de6c0e22bdf3426114a4b0190bad52fd25718 Mon Sep 17 00:00:00 2001 From: Serguei Spitsyn Date: Thu, 10 Apr 2025 10:55:12 -0700 Subject: [PATCH 4/4] fix a build time error for minimal VM config --- src/hotspot/share/prims/jvmtiEnvBase.hpp | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/hotspot/share/prims/jvmtiEnvBase.hpp b/src/hotspot/share/prims/jvmtiEnvBase.hpp index 17b7dcb7ca0ea..66f10c85bc9af 100644 --- a/src/hotspot/share/prims/jvmtiEnvBase.hpp +++ b/src/hotspot/share/prims/jvmtiEnvBase.hpp @@ -454,6 +454,8 @@ class JvmtiEnvIterator : public StackObj { JvmtiEnv* next(JvmtiEnvBase* env) { return env->next_environment(); } }; +#if INCLUDE_JVMTI + // This helper class marks current thread as making a Java upcall. // It is needed to hide JVMTI events during JVMTI operation. class JvmtiJavaUpcallMark : public StackObj { @@ -471,6 +473,7 @@ class JvmtiJavaUpcallMark : public StackObj { _current->toggle_is_in_java_upcall(); } }; +#endif // INCLUDE_JVMTI // Used in combination with the JvmtiHandshake class. // It is intended to support both platform and virtual threads.