On Fri, 5 Sep 2025 16:08:13 GMT, Leonid Mesnik <lmes...@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
