On Thu, 9 Oct 2025 17:45:23 GMT, Patricio Chilano Mateo
<[email protected]> wrote:
> I think JvmtiExport::has_frame_pops should just check if
> thread->jvmti_thread_state() is nullptr and return false, and not try to
> create the state. Same with JvmtiExport::continuation_yield_cleanup(). This
> JvmtiExport::get_jvmti_thread_state() method was added in 8312174 but I don’t
> think we want it for this case in freeze. Not only because no state should
> imply no frame pop requests, but also because it seems the JvmtiThreadState
> created and set to the platform thread will be for the vthread instance, but
> the rebind operation has already been executed in VTMS_unmount_begin so we
> would leave the wrong state in the platform thread until the next transition.
Thank you, Patrico!
I agree with this. Below is the patch for this change.
diff --git a/src/hotspot/share/prims/jvmtiExport.cpp
b/src/hotspot/share/prims/jvmtiExport.cpp
index 077b3fec505..fa6ede86cd9 100644
--- a/src/hotspot/share/prims/jvmtiExport.cpp
+++ b/src/hotspot/share/prims/jvmtiExport.cpp
@@ -1694,7 +1694,7 @@ bool JvmtiExport::has_frame_pops(JavaThread* thread) {
if (!can_post_frame_pop()) {
return false;
}
- JvmtiThreadState *state = get_jvmti_thread_state(thread);
+ JvmtiThreadState *state = thread->jvmti_thread_state();
if (state == nullptr) {
return false;
}
@@ -1713,7 +1713,7 @@ void JvmtiExport::continuation_yield_cleanup(JavaThread*
thread, jint continuati
}
assert(thread == JavaThread::current(), "must be");
- JvmtiThreadState *state = get_jvmti_thread_state(thread);
+ JvmtiThreadState *state = thread->jvmti_thread_state();
if (state == nullptr) {
return;
}
> @sspitsyn IIUC this cleanup of the frame_pop requests should only be needed
> for the plain continuation case, so shouldn’t we have a
> !cont.entry()->is_virtual_thread() check too?
Good idea. I was also thinking about it at some point.
> Also, pre-existing and maybe for a different bug, but seems we are missing a
> call to invalidate_jvmti_stack() for the preempt case.
I can be but could you be more presize about the preempt case? What place do
you mean?
-------------
PR Comment: https://git.openjdk.org/jdk/pull/27716#issuecomment-3388398133
