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

[Patricio Chilano Mateo]

On Thu, 23 Oct 2025 03:56:07 GMT, Serguei Spitsyn <sspitsyn at openjdk.org> wrote:

>> Strangely, the test `serviceability/jvmti/vthread/ContStackDepthTest` is failing with the `assert(_cur_stack_depth == num_frames)`. Obviously, some of the code paths is missed to call `invalidate_jvmti_stack()`.
>
> As I see, the calls to `invalidate_jvmti_stack()` are needed for plain continuations only (under condition `if (!cont.entry()->is_virtual_thread()`).
> 
> The following patch does not cause regressions in mach5 tier 6 (mach5 tiers 1-5 are in progress):
> 
> diff --git a/src/hotspot/share/prims/jvmtiExport.cpp b/src/hotspot/share/prims/jvmtiExport.cpp
> index 0884fce2ff7..dbbf5526602 100644
> --- a/src/hotspot/share/prims/jvmtiExport.cpp
> +++ b/src/hotspot/share/prims/jvmtiExport.cpp
> @@ -1711,7 +1711,6 @@ void JvmtiExport::continuation_yield_cleanup(JavaThread* thread, jint continuati
>    if (state == nullptr) {
>      return;
>    }
> -  state->invalidate_cur_stack_depth();
>  
>    // Clear frame_pop requests in frames popped by yield
>    if (can_post_frame_pop()) {
> diff --git a/src/hotspot/share/runtime/continuationFreezeThaw.cpp b/src/hotspot/share/runtime/continuationFreezeThaw.cpp
> index 0750f611876..f8406fb33c1 100644
> --- a/src/hotspot/share/runtime/continuationFreezeThaw.cpp
> +++ b/src/hotspot/share/runtime/continuationFreezeThaw.cpp
> @@ -1626,11 +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)) {
> +  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);
>    }
>  }
>  
> @@ -1685,7 +1689,7 @@ 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()));
> +  JVMTI_ONLY(if (!cont.entry()->is_virtual_thread()) 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);
>  ```

Ah, we need to call `invalidate_jvmti_stack` at the end in `jvmti_yield_cleanup`! The problem is that in `JvmtiExport::continuation_yield_cleanup` we are setting again the stack depth when calling `state->cur_stack_depth()`. We count the enterSpecial frame at the top but we don’t decrement the depth when removing it (done in assembly code).

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

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