RFR: 8293613: need to properly handle and hide tmp VTMS transitions [v3]

[Chris Plummer]

On Tue, 20 Sep 2022 22:15:49 GMT, Serguei Spitsyn <sspitsyn at openjdk.org> wrote:

>> src/hotspot/share/prims/jvmtiEnvBase.cpp line 713:
>> 
>>> 711:   if (!jt->is_in_tmp_VTMS_transition()) {
>>> 712:     jvf = check_and_skip_hidden_frames(jt, jvf);
>>> 713:   }
>> 
>> I think this comment needs some help. It's hard to match it up with what the code is doing. I think you are saying you want to avoid skipping hidden frames when in transition, although if that's the case, it's not clear to me why not skipping is ok.
>> 
>> Also is skipping (or not skipping) ok regardless of the JvmtiEnvBase::is_cthread_with_continuation() result?
>
> Thank you for reviewing and the comment, Chris.
> I agree, this part and related comment is kind of obscure.
> I'll think how to make the comment better.
> In fact, all temporary VTMS transitions do temporary switch the `JavaThread` identity from virtual thread to carrier. There is no need to skip frames because there are no real carrier thread frames at the top. Moreover, any attempt to skip frames which are in transition works incorrectly and gives an empty stack. The check `JvmtiEnvBase::is_cthread_with_continuation()` is needed to make sure we have a deal with a continuation. There is no need to skip frames otherwise.

It's still not clear to me if the result of `JvmtiEnvBase::is_cthread_with_continuation()` impacts if you have to possibly skip hidden frames. In other words, do you always have to do the `if` block that follows, no matter what `JvmtiEnvBase::is_cthread_with_continuation()` returns? If you don't, then maybe instead of a "? :"  expression, an if/else would be better. I'm not sure if the following is correct, but something like:

    javaVFrame *jvf;
    if (JvmtiEnvBase::is_cthread_with_continuation(jt)) {
      jvf = jt->carrier_last_java_vframe(reg_map_p);
    } else {
      jvf - jt->last_java_vframe(reg_map_p);
      // Skipping hidden frames when jt->is_in_tmp_VTMS_transition()==true results
      // in returning jvf==NULL, and so, empty stack traces for carrier threads.
      if (!jt->is_in_tmp_VTMS_transition()) {
        jvf = check_and_skip_hidden_frames(jt, jvf);
      }
    }

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

PR: https://git.openjdk.org/jdk/pull/10321