From a031b408c549bdb24ae353e452fdadcf07d9f654 Mon Sep 17 00:00:00 2001 From: Leonid Mesnik Date: Thu, 4 Sep 2025 23:07:43 -0700 Subject: [PATCH 1/9] fix --- src/hotspot/share/prims/jvmtiExport.cpp | 71 +++++++++++++------------ src/hotspot/share/prims/jvmtiExport.hpp | 7 ++- 2 files changed, 43 insertions(+), 35 deletions(-) diff --git a/src/hotspot/share/prims/jvmtiExport.cpp b/src/hotspot/share/prims/jvmtiExport.cpp index 8c10a371e5afd..2d1f70c123573 100644 --- a/src/hotspot/share/prims/jvmtiExport.cpp +++ b/src/hotspot/share/prims/jvmtiExport.cpp @@ -418,6 +418,7 @@ JvmtiExport::get_jvmti_interface(JavaVM *jvm, void **penv, jint version) { JvmtiThreadState* JvmtiExport::get_jvmti_thread_state(JavaThread *thread, bool allow_suspend) { assert(thread == JavaThread::current(), "must be current thread"); + assert(thread->thread_state() == _thread_in_vm, "thread should be in vm"); if (thread->is_vthread_mounted() && thread->jvmti_thread_state() == nullptr) { JvmtiEventController::thread_started(thread); if (allow_suspend && thread->is_suspended()) { @@ -1826,58 +1827,60 @@ void JvmtiExport::post_method_entry(JavaThread *thread, Method* method, frame cu } void JvmtiExport::post_method_exit(JavaThread* thread, Method* method, frame current_frame) { + // At this point we only have the address of a "raw result" and + // we just call into the interpreter to convert this into a jvalue. + // The post_method_exit_transition always makes transition to vm and back + // where GC can happen. So it is needed to preserve result and then restore it + // even if events are not actually posted. + // Saving oop_result into valu.j is deferred until jvmti state is ready. HandleMark hm(thread); methodHandle mh(thread, method); - - JvmtiThreadState *state = get_jvmti_thread_state(thread); - - if (state == nullptr || !state->is_interp_only_mode()) { - // for any thread that actually wants method exit, interp_only_mode is set - return; - } - - Handle result; + oop oop_result; jvalue value; value.j = 0L; - - if (state->is_enabled(JVMTI_EVENT_METHOD_EXIT)) { - // At this point we only have the address of a "raw result" and - // we just call into the interpreter to convert this into a jvalue. - oop oop_result; - BasicType type = current_frame.interpreter_frame_result(&oop_result, &value); - assert(type == T_VOID || current_frame.interpreter_frame_expression_stack_size() > 0, - "Stack shouldn't be empty"); - if (is_reference_type(type)) { - result = Handle(thread, oop_result); - value.l = JNIHandles::make_local(thread, result()); - } + BasicType type = current_frame.interpreter_frame_result(&oop_result, &value); + assert(type == T_VOID || current_frame.interpreter_frame_expression_stack_size() > 0, + "Stack shouldn't be empty"); + Handle result = Handle(thread, oop_result); + post_method_exit_transition(thread, mh, type, result, value); + if (result.not_null() && !mh->is_native()) { + *(oop*)current_frame.interpreter_frame_tos_address() = result(); } +} - // Do not allow NotifyFramePop to add new FramePop event request at - // depth 0 as it is already late in the method exiting dance. - state->set_top_frame_is_exiting(); - - // Deferred transition to VM, so we can stash away the return oop before GC. +void JvmtiExport::post_method_exit_transition(JavaThread* thread, methodHandle mh, + BasicType type, Handle result, jvalue value) { + JvmtiThreadState * state; // should be initialized in vm state only JavaThread* current = thread; // for JRT_BLOCK JRT_BLOCK - post_method_exit_inner(thread, mh, state, false /* not exception exit */, current_frame, value); + state = get_jvmti_thread_state(thread); + if (state == nullptr || !state->is_interp_only_mode()) { + // The transition from vm to java + return; + } + + if (state->is_enabled(JVMTI_EVENT_METHOD_EXIT)) { + if (is_reference_type(type)) { + value.l = JNIHandles::make_local(thread, result()); + } + } + + // Do not allow NotifyFramePop to add new FramePop event request at + // depth 0 as it is already late in the method exiting dance. + state->set_top_frame_is_exiting(); + + post_method_exit_inner(thread, mh, state, false /* not exception exit */, value); JRT_BLOCK_END // The JRT_BLOCK_END can safepoint in ThreadInVMfromJava desctructor. Now it is safe to allow // adding FramePop event requests as no safepoint can happen before removing activation. state->clr_top_frame_is_exiting(); - - if (result.not_null() && !mh->is_native()) { - // We have to restore the oop on the stack for interpreter frames - *(oop*)current_frame.interpreter_frame_tos_address() = result(); - } } void JvmtiExport::post_method_exit_inner(JavaThread* thread, methodHandle& mh, JvmtiThreadState *state, bool exception_exit, - frame current_frame, jvalue& value) { if (mh->jvmti_mount_transition() || thread->should_hide_jvmti_events()) { return; @@ -2107,7 +2110,7 @@ void JvmtiExport::notice_unwind_due_to_exception(JavaThread *thread, Method* met // When these events are enabled code should be in running in interp mode. jvalue no_value; no_value.j = 0L; - JvmtiExport::post_method_exit_inner(thread, mh, state, true, thread->last_frame(), no_value); + JvmtiExport::post_method_exit_inner(thread, mh, state, true, no_value); // The cached cur_stack_depth might have changed from the // operations of frame pop or method exit. We are not 100% sure // the cached cur_stack_depth is still valid depth so invalidate diff --git a/src/hotspot/share/prims/jvmtiExport.hpp b/src/hotspot/share/prims/jvmtiExport.hpp index 4f8c3016908d2..04f5541ae293d 100644 --- a/src/hotspot/share/prims/jvmtiExport.hpp +++ b/src/hotspot/share/prims/jvmtiExport.hpp @@ -210,11 +210,16 @@ class JvmtiExport : public AllStatic { // dependency information is complete or not. static bool _all_dependencies_are_recorded; + static void post_method_exit_transition(JavaThread* thread, + methodHandle mh, + BasicType type, + Handle result, + jvalue value); + static void post_method_exit_inner(JavaThread* thread, methodHandle& mh, JvmtiThreadState *state, bool exception_exit, - frame current_frame, jvalue& value); public: From c1de44f6d230688d7c8d4a05c7849d40af340559 Mon Sep 17 00:00:00 2001 From: Leonid Mesnik Date: Thu, 4 Sep 2025 23:26:26 -0700 Subject: [PATCH 2/9] comment is fixed. --- src/hotspot/share/prims/jvmtiExport.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/hotspot/share/prims/jvmtiExport.cpp b/src/hotspot/share/prims/jvmtiExport.cpp index 2d1f70c123573..d1711dd36d9b3 100644 --- a/src/hotspot/share/prims/jvmtiExport.cpp +++ b/src/hotspot/share/prims/jvmtiExport.cpp @@ -1832,7 +1832,7 @@ void JvmtiExport::post_method_exit(JavaThread* thread, Method* method, frame cur // The post_method_exit_transition always makes transition to vm and back // where GC can happen. So it is needed to preserve result and then restore it // even if events are not actually posted. - // Saving oop_result into valu.j is deferred until jvmti state is ready. + // Saving oop_result into value.j is deferred until jvmti state is ready. HandleMark hm(thread); methodHandle mh(thread, method); oop oop_result; From 77c9c7a33a3a14b0239f998b217827177ae6320a Mon Sep 17 00:00:00 2001 From: Leonid Mesnik Date: Thu, 4 Sep 2025 23:35:17 -0700 Subject: [PATCH 3/9] comment --- src/hotspot/share/prims/jvmtiExport.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/src/hotspot/share/prims/jvmtiExport.cpp b/src/hotspot/share/prims/jvmtiExport.cpp index d1711dd36d9b3..47e503adea91d 100644 --- a/src/hotspot/share/prims/jvmtiExport.cpp +++ b/src/hotspot/share/prims/jvmtiExport.cpp @@ -1860,6 +1860,7 @@ void JvmtiExport::post_method_exit_transition(JavaThread* thread, methodHandle m } if (state->is_enabled(JVMTI_EVENT_METHOD_EXIT)) { + // Deferred saving Object result into value. if (is_reference_type(type)) { value.l = JNIHandles::make_local(thread, result()); } From 0bd9c98d08108d6a94d13d6a71981a8e076fe49f Mon Sep 17 00:00:00 2001 From: Leonid Mesnik Date: Fri, 5 Sep 2025 09:04:15 -0700 Subject: [PATCH 4/9] small improvements --- src/hotspot/share/prims/jvmtiExport.cpp | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/hotspot/share/prims/jvmtiExport.cpp b/src/hotspot/share/prims/jvmtiExport.cpp index 47e503adea91d..2077097fa1851 100644 --- a/src/hotspot/share/prims/jvmtiExport.cpp +++ b/src/hotspot/share/prims/jvmtiExport.cpp @@ -1835,13 +1835,16 @@ void JvmtiExport::post_method_exit(JavaThread* thread, Method* method, frame cur // Saving oop_result into value.j is deferred until jvmti state is ready. HandleMark hm(thread); methodHandle mh(thread, method); + Handle result; oop oop_result; jvalue value; value.j = 0L; BasicType type = current_frame.interpreter_frame_result(&oop_result, &value); assert(type == T_VOID || current_frame.interpreter_frame_expression_stack_size() > 0, "Stack shouldn't be empty"); - Handle result = Handle(thread, oop_result); + if (is_reference_type(type)) { + result = Handle(thread, oop_result); + } post_method_exit_transition(thread, mh, type, result, value); if (result.not_null() && !mh->is_native()) { *(oop*)current_frame.interpreter_frame_tos_address() = result(); @@ -1850,7 +1853,7 @@ void JvmtiExport::post_method_exit(JavaThread* thread, Method* method, frame cur void JvmtiExport::post_method_exit_transition(JavaThread* thread, methodHandle mh, BasicType type, Handle result, jvalue value) { - JvmtiThreadState * state; // should be initialized in vm state only + JvmtiThreadState* state; // should be initialized in vm state only JavaThread* current = thread; // for JRT_BLOCK JRT_BLOCK state = get_jvmti_thread_state(thread); From 118d39e163d2da1c8a133ce70fb04c937620b567 Mon Sep 17 00:00:00 2001 From: Leonid Mesnik Date: Mon, 8 Sep 2025 08:28:49 -0700 Subject: [PATCH 5/9] reverted to single method. --- src/hotspot/share/prims/jvmtiExport.cpp | 49 +++++++++++-------------- src/hotspot/share/prims/jvmtiExport.hpp | 6 --- 2 files changed, 21 insertions(+), 34 deletions(-) diff --git a/src/hotspot/share/prims/jvmtiExport.cpp b/src/hotspot/share/prims/jvmtiExport.cpp index 2077097fa1851..0b341f367156c 100644 --- a/src/hotspot/share/prims/jvmtiExport.cpp +++ b/src/hotspot/share/prims/jvmtiExport.cpp @@ -1829,8 +1829,8 @@ void JvmtiExport::post_method_entry(JavaThread *thread, Method* method, frame cu void JvmtiExport::post_method_exit(JavaThread* thread, Method* method, frame current_frame) { // At this point we only have the address of a "raw result" and // we just call into the interpreter to convert this into a jvalue. - // The post_method_exit_transition always makes transition to vm and back - // where GC can happen. So it is needed to preserve result and then restore it + // This method always makes transition to vm and back where GC can happen. + // So it is needed to preserve result and then restore it // even if events are not actually posted. // Saving oop_result into value.j is deferred until jvmti state is ready. HandleMark hm(thread); @@ -1845,40 +1845,33 @@ void JvmtiExport::post_method_exit(JavaThread* thread, Method* method, frame cur if (is_reference_type(type)) { result = Handle(thread, oop_result); } - post_method_exit_transition(thread, mh, type, result, value); - if (result.not_null() && !mh->is_native()) { - *(oop*)current_frame.interpreter_frame_tos_address() = result(); - } -} - -void JvmtiExport::post_method_exit_transition(JavaThread* thread, methodHandle mh, - BasicType type, Handle result, jvalue value) { JvmtiThreadState* state; // should be initialized in vm state only JavaThread* current = thread; // for JRT_BLOCK JRT_BLOCK state = get_jvmti_thread_state(thread); - if (state == nullptr || !state->is_interp_only_mode()) { - // The transition from vm to java - return; - } - - if (state->is_enabled(JVMTI_EVENT_METHOD_EXIT)) { - // Deferred saving Object result into value. - if (is_reference_type(type)) { - value.l = JNIHandles::make_local(thread, result()); + if (state != nullptr && state->is_interp_only_mode()) { + if (state->is_enabled(JVMTI_EVENT_METHOD_EXIT)) { + // Deferred saving Object result into value. + if (is_reference_type(type)) { + value.l = JNIHandles::make_local(thread, result()); + } } - } - // Do not allow NotifyFramePop to add new FramePop event request at - // depth 0 as it is already late in the method exiting dance. - state->set_top_frame_is_exiting(); + // Do not allow NotifyFramePop to add new FramePop event request at + // depth 0 as it is already late in the method exiting dance. + state->set_top_frame_is_exiting(); - post_method_exit_inner(thread, mh, state, false /* not exception exit */, value); + post_method_exit_inner(thread, mh, state, false /* not exception exit */, value); + } JRT_BLOCK_END - - // The JRT_BLOCK_END can safepoint in ThreadInVMfromJava desctructor. Now it is safe to allow - // adding FramePop event requests as no safepoint can happen before removing activation. - state->clr_top_frame_is_exiting(); + if (state != nullptr && state->is_interp_only_mode()) { + // The JRT_BLOCK_END can safepoint in ThreadInVMfromJava desctructor. Now it is safe to allow + // adding FramePop event requests as no safepoint can happen before removing activation. + state->clr_top_frame_is_exiting(); + } + if (result.not_null() && !mh->is_native()) { + *(oop*)current_frame.interpreter_frame_tos_address() = result(); + } } void JvmtiExport::post_method_exit_inner(JavaThread* thread, diff --git a/src/hotspot/share/prims/jvmtiExport.hpp b/src/hotspot/share/prims/jvmtiExport.hpp index 04f5541ae293d..904a8a56e646e 100644 --- a/src/hotspot/share/prims/jvmtiExport.hpp +++ b/src/hotspot/share/prims/jvmtiExport.hpp @@ -210,12 +210,6 @@ class JvmtiExport : public AllStatic { // dependency information is complete or not. static bool _all_dependencies_are_recorded; - static void post_method_exit_transition(JavaThread* thread, - methodHandle mh, - BasicType type, - Handle result, - jvalue value); - static void post_method_exit_inner(JavaThread* thread, methodHandle& mh, JvmtiThreadState *state, From ea941c2d21ea765058ff1bd14ced80370809caaf Mon Sep 17 00:00:00 2001 From: Leonid Mesnik Date: Mon, 8 Sep 2025 08:35:53 -0700 Subject: [PATCH 6/9] more changes reverted --- src/hotspot/share/prims/jvmtiExport.cpp | 6 ++++-- src/hotspot/share/prims/jvmtiExport.hpp | 1 + 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/src/hotspot/share/prims/jvmtiExport.cpp b/src/hotspot/share/prims/jvmtiExport.cpp index 0b341f367156c..8f601bbfce3ba 100644 --- a/src/hotspot/share/prims/jvmtiExport.cpp +++ b/src/hotspot/share/prims/jvmtiExport.cpp @@ -1861,7 +1861,7 @@ void JvmtiExport::post_method_exit(JavaThread* thread, Method* method, frame cur // depth 0 as it is already late in the method exiting dance. state->set_top_frame_is_exiting(); - post_method_exit_inner(thread, mh, state, false /* not exception exit */, value); + post_method_exit_inner(thread, mh, state, false /* not exception exit */, current_frame, value); } JRT_BLOCK_END if (state != nullptr && state->is_interp_only_mode()) { @@ -1870,6 +1870,7 @@ void JvmtiExport::post_method_exit(JavaThread* thread, Method* method, frame cur state->clr_top_frame_is_exiting(); } if (result.not_null() && !mh->is_native()) { + // We have to restore the oop on the stack for interpreter frames *(oop*)current_frame.interpreter_frame_tos_address() = result(); } } @@ -1878,6 +1879,7 @@ void JvmtiExport::post_method_exit_inner(JavaThread* thread, methodHandle& mh, JvmtiThreadState *state, bool exception_exit, + frame current_frame, jvalue& value) { if (mh->jvmti_mount_transition() || thread->should_hide_jvmti_events()) { return; @@ -2107,7 +2109,7 @@ void JvmtiExport::notice_unwind_due_to_exception(JavaThread *thread, Method* met // When these events are enabled code should be in running in interp mode. jvalue no_value; no_value.j = 0L; - JvmtiExport::post_method_exit_inner(thread, mh, state, true, no_value); + JvmtiExport::post_method_exit_inner(thread, mh, state, true, thread->last_frame(), no_value); // The cached cur_stack_depth might have changed from the // operations of frame pop or method exit. We are not 100% sure // the cached cur_stack_depth is still valid depth so invalidate diff --git a/src/hotspot/share/prims/jvmtiExport.hpp b/src/hotspot/share/prims/jvmtiExport.hpp index 904a8a56e646e..4f8c3016908d2 100644 --- a/src/hotspot/share/prims/jvmtiExport.hpp +++ b/src/hotspot/share/prims/jvmtiExport.hpp @@ -214,6 +214,7 @@ class JvmtiExport : public AllStatic { methodHandle& mh, JvmtiThreadState *state, bool exception_exit, + frame current_frame, jvalue& value); public: From 03c172a94dadf27b3ce9e60861329280baa7dc6f Mon Sep 17 00:00:00 2001 From: Leonid Mesnik Date: Mon, 8 Sep 2025 08:56:53 -0700 Subject: [PATCH 7/9] comments fixed. --- src/hotspot/share/prims/jvmtiExport.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/hotspot/share/prims/jvmtiExport.cpp b/src/hotspot/share/prims/jvmtiExport.cpp index 8f601bbfce3ba..a248c86da863d 100644 --- a/src/hotspot/share/prims/jvmtiExport.cpp +++ b/src/hotspot/share/prims/jvmtiExport.cpp @@ -1830,7 +1830,7 @@ void JvmtiExport::post_method_exit(JavaThread* thread, Method* method, frame cur // At this point we only have the address of a "raw result" and // we just call into the interpreter to convert this into a jvalue. // This method always makes transition to vm and back where GC can happen. - // So it is needed to preserve result and then restore it + // So it is needed to preserve result and then restore it // even if events are not actually posted. // Saving oop_result into value.j is deferred until jvmti state is ready. HandleMark hm(thread); @@ -1840,8 +1840,8 @@ void JvmtiExport::post_method_exit(JavaThread* thread, Method* method, frame cur jvalue value; value.j = 0L; BasicType type = current_frame.interpreter_frame_result(&oop_result, &value); - assert(type == T_VOID || current_frame.interpreter_frame_expression_stack_size() > 0, - "Stack shouldn't be empty"); + assert(mh->is_native() || type == T_VOID || current_frame.interpreter_frame_expression_stack_size() > 0, + "Stack shouldn't be empty"); if (is_reference_type(type)) { result = Handle(thread, oop_result); } From 3a169797614a710e4ebfe31cfbbf1e46e06bd746 Mon Sep 17 00:00:00 2001 From: Leonid Mesnik Date: Mon, 8 Sep 2025 11:27:56 -0700 Subject: [PATCH 8/9] interopnly_state should be saved --- src/hotspot/share/prims/jvmtiExport.cpp | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/hotspot/share/prims/jvmtiExport.cpp b/src/hotspot/share/prims/jvmtiExport.cpp index a248c86da863d..e8f75fd9c9925 100644 --- a/src/hotspot/share/prims/jvmtiExport.cpp +++ b/src/hotspot/share/prims/jvmtiExport.cpp @@ -1847,9 +1847,11 @@ void JvmtiExport::post_method_exit(JavaThread* thread, Method* method, frame cur } JvmtiThreadState* state; // should be initialized in vm state only JavaThread* current = thread; // for JRT_BLOCK + bool interp_only; // might be changed in JRT_BLOCK_END JRT_BLOCK state = get_jvmti_thread_state(thread); - if (state != nullptr && state->is_interp_only_mode()) { + interp_only = state != nullptr && state->is_interp_only_mode(); + if (interp_only) { if (state->is_enabled(JVMTI_EVENT_METHOD_EXIT)) { // Deferred saving Object result into value. if (is_reference_type(type)) { @@ -1864,7 +1866,7 @@ void JvmtiExport::post_method_exit(JavaThread* thread, Method* method, frame cur post_method_exit_inner(thread, mh, state, false /* not exception exit */, current_frame, value); } JRT_BLOCK_END - if (state != nullptr && state->is_interp_only_mode()) { + if (interp_only) { // The JRT_BLOCK_END can safepoint in ThreadInVMfromJava desctructor. Now it is safe to allow // adding FramePop event requests as no safepoint can happen before removing activation. state->clr_top_frame_is_exiting(); From 85482ae4ee77ea57088aa14c7158710d76388faf Mon Sep 17 00:00:00 2001 From: Leonid Mesnik Date: Tue, 9 Sep 2025 07:24:51 -0700 Subject: [PATCH 9/9] Update src/hotspot/share/prims/jvmtiExport.cpp Co-authored-by: David Holmes <62092539+dholmes-ora@users.noreply.github.com> --- src/hotspot/share/prims/jvmtiExport.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/hotspot/share/prims/jvmtiExport.cpp b/src/hotspot/share/prims/jvmtiExport.cpp index e8f75fd9c9925..2da9a074fb633 100644 --- a/src/hotspot/share/prims/jvmtiExport.cpp +++ b/src/hotspot/share/prims/jvmtiExport.cpp @@ -1867,7 +1867,7 @@ void JvmtiExport::post_method_exit(JavaThread* thread, Method* method, frame cur } JRT_BLOCK_END if (interp_only) { - // The JRT_BLOCK_END can safepoint in ThreadInVMfromJava desctructor. Now it is safe to allow + // The JRT_BLOCK_END can safepoint in ThreadInVMfromJava destructor. Now it is safe to allow // adding FramePop event requests as no safepoint can happen before removing activation. state->clr_top_frame_is_exiting(); }