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

[Serguei Spitsyn]

On Thu, 23 Oct 2025 17:59:51 GMT, Patricio Chilano Mateo <pchilanomate at openjdk.org> wrote:

>> 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.

Okay, thanks! I've pushed my latest changes which are aligned with your patch above and invalidate the stack for plain/pure continuations only. So, the `VTMS_unmount_end()` still does not include it.

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

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