RFR: 8369609: Continuations preempt_epilog is missing a call to invalidate_jvmti_stack [v3]

[Patricio Chilano Mateo]

On Thu, 23 Oct 2025 17:32:51 GMT, Serguei Spitsyn <sspitsyn at openjdk.org> wrote:

>> Okay, thanks! Did you verify it with run of test `serviceability/jvmti/vthread/ContStackDepthTest`? I see it is still failing with the call to `invalidate_jvmti_stack()` at the end of `jvmti_yield_cleanup()`.
>
> This test does not fail with the following patch:
> 
> diff --git a/src/hotspot/share/runtime/continuationFreezeThaw.cpp b/src/hotspot/share/runtime/continuationFreezeThaw.cpp
> index 0750f611876..a12e99903af 100644
> --- a/src/hotspot/share/runtime/continuationFreezeThaw.cpp
> +++ b/src/hotspot/share/runtime/continuationFreezeThaw.cpp
> @@ -1626,12 +1626,17 @@ static void invalidate_jvmti_stack(JavaThread* thread) {
>  }
>  
>  static void jvmti_yield_cleanup(JavaThread* thread, ContinuationWrapper& cont) {
> -  if (!cont.entry()->is_virtual_thread() && JvmtiExport::has_frame_pops(thread)) {
> +  if (cont.entry()->is_virtual_thread()) {
> +    return;
> +  }
> +  invalidate_jvmti_stack(thread);
> +  if (JvmtiExport::has_frame_pops(thread)) {
>      int num_frames = num_java_frames(cont);
>  
>      ContinuationWrapper::SafepointOp so(Thread::current(), cont);
> -    JvmtiExport::continuation_yield_cleanup(JavaThread::current(), num_frames);
> +    JvmtiExport::continuation_yield_cleanup(thread, num_frames);
>    }
> +  invalidate_jvmti_stack(thread);
>  }
>  
>  static void jvmti_mount_end(JavaThread* current, ContinuationWrapper& cont, frame top) {
> @@ -1685,7 +1690,6 @@ static inline freeze_result freeze_epilog(ContinuationWrapper& cont) {
>  
>    log_develop_debug(continuations)("=== End of freeze cont ### #" INTPTR_FORMAT, cont.hash());
>  
> -  JVMTI_ONLY(invalidate_jvmti_stack(JavaThread::current()));
>    return freeze_ok;
>  }
>  
> @@ -2311,7 +2315,7 @@ NOINLINE intptr_t* Thaw<ConfigT>::thaw_slow(stackChunkOop chunk, Continuation::t
>  
>    assert(_cont.chunk_invariant(), "");
>  
> -  JVMTI_ONLY(invalidate_jvmti_stack(_thread));
> +  JVMTI_ONLY(if (!_cont.entry()->is_virtual_thread()) invalidate_jvmti_stack(_thread));
>  
>    _thread->set_cont_fastpath(_fastpath);
> 
> 
> The call to `invalidate_jvmti_stack()` is still needed in the `thaw_slow()`.
> I can go ahead with this update if you are okay with it.

This is the patch I tested out with `serviceability/jvmti/vthread/ContStackDepthTest` (on top of mainline):

diff --git a/src/hotspot/share/prims/jvmtiThreadState.cpp b/src/hotspot/share/prims/jvmtiThreadState.cpp
index 00a48dec111..181d77f4d10 100644
--- a/src/hotspot/share/prims/jvmtiThreadState.cpp
+++ b/src/hotspot/share/prims/jvmtiThreadState.cpp
@@ -678,6 +678,10 @@ void
 JvmtiVTMSTransitionDisabler::VTMS_unmount_end(jobject vthread) {
   JavaThread* thread = JavaThread::current();
   assert(thread->is_in_VTMS_transition(), "sanity check");
+  JvmtiThreadState *state = thread->jvmti_thread_state();
+  if (state != nullptr) {
+    state->invalidate_cur_stack_depth();
+  }
   finish_VTMS_transition(vthread, /* is_mount */ false);
 }

diff --git a/src/hotspot/share/runtime/continuationFreezeThaw.cpp b/src/hotspot/share/runtime/continuationFreezeThaw.cpp
index 3e509e71551..a493fca076e 100644
--- a/src/hotspot/share/runtime/continuationFreezeThaw.cpp
+++ b/src/hotspot/share/runtime/continuationFreezeThaw.cpp
@@ -1626,13 +1626,15 @@ static void invalidate_jvmti_stack(JavaThread* thread) {
 }

 static void jvmti_yield_cleanup(JavaThread* thread, ContinuationWrapper& cont) {
-  if (!cont.entry()->is_virtual_thread() && JvmtiExport::has_frame_pops(thread)) {
-    int num_frames = num_java_frames(cont);
+  if (!cont.entry()->is_virtual_thread()) {
+    if (JvmtiExport::has_frame_pops(thread)) {
+      int num_frames = num_java_frames(cont);

-    ContinuationWrapper::SafepointOp so(Thread::current(), cont);
-    JvmtiExport::continuation_yield_cleanup(JavaThread::current(), num_frames);
+      ContinuationWrapper::SafepointOp so(Thread::current(), cont);
+      JvmtiExport::continuation_yield_cleanup(JavaThread::current(), num_frames);
+    }
+    invalidate_jvmti_stack(thread);
   }
-  invalidate_jvmti_stack(thread);
 }

 static void jvmti_mount_end(JavaThread* current, ContinuationWrapper& cont, frame top) {

Originally I had `invalidate_jvmti_stack()` before calling `JvmtiExport::continuation_yield_cleanup` so I could reproduce the issue.

I think you are right that for virtual threads we shouldn't need to invalidate the stack, so your patch is an improvement.

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/27878#discussion_r2456495915