RFR: 8365192: post_meth_exit should be in vm state when calling get_jvmti_thread_state [v3]

[David Holmes]

On Fri, 5 Sep 2025 16:08:13 GMT, Leonid Mesnik <lmesnik at openjdk.org> wrote:

>> This is the second attempt to fix method `post_meth_exit` to correctly set state and preserve result.
>> The related fix here:
>> https://github.com/openjdk/jdk/commit/b7b64bb6c800b45e32ff37b1b92b5927a3b3fb56
>> Hope fix became clarere now.
>> 
>> There 2 problems in this post_meth_exit:
>> 1) The result is preserved only if `state->is_enabled(JVMTI_EVENT_METHOD_EXIT)` however transition in the JRT_BLOCK_END happens always. So there is a risk of loosing method results in the interp_only mode.
>> 2) The method `get_jvmti_thread_state` should be called when thread is in vm state only.
>> 
>> The fix adds `post_method_exit_transition` to have single exit point with oop restoration.
>
> Leonid Mesnik has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains five additional commits since the last revision:
> 
>  - small improvements
>  - Merge branch 'master' of https://github.com/openjdk/jdk into 8365192-2
>  - comment
>  - comment is fixed.
>  - fix

I've taken a first pass through and the code reorg seems okay. It would have been a little clearer without factoring out the transition method.

Thanks

src/hotspot/share/prims/jvmtiExport.cpp line 1833:

> 1831:   // we just call into the interpreter to convert this into a jvalue.
> 1832:   // The post_method_exit_transition always makes transition to vm and back
> 1833:   // where GC can happen. So it is needed to preserve result  and then restore it

Suggestion:

  // where GC can happen. So it is needed to preserve result and then restore it

src/hotspot/share/prims/jvmtiExport.cpp line 1857:

> 1855:                                     BasicType type, Handle result, jvalue value) {
> 1856:   JvmtiThreadState* state; // should be initialized in vm state only
> 1857:   JavaThread* current = thread; // for JRT_BLOCK

If `thread` is always the current thread then I suggest just calling it `current` in the first place.

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

PR Review: https://git.openjdk.org/jdk/pull/27112#pullrequestreview-3195023988
PR Review Comment: https://git.openjdk.org/jdk/pull/27112#discussion_r2329182820
PR Review Comment: https://git.openjdk.org/jdk/pull/27112#discussion_r2329187504