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

[Serguei Spitsyn]

On Thu, 23 Oct 2025 14:24:05 GMT, Patricio Chilano Mateo <pchilanomate at openjdk.org> wrote:

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

Okay, thanks! Did you verify it the test `serviceability/jvmti/vthread/ContStackDepthTest`? I see it is still failing.

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

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