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

[Patricio Chilano Mateo]

On Tue, 19 Aug 2025 05:54:13 GMT, Leonid Mesnik <lmesnik at openjdk.org> wrote:

>> The method
>> get_jvmti_thread_state()
>> should be called only while thread is in vm state.
>> 
>> The post_method_exit is doing some preparation before switching to vm state. This cause issues if thread is needed to initialize jvmti thread state.
>> 
>> The fix was found using jvmti stress agent and thus no additional regression test is required.
>
> 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 nine additional commits since the last revision:
> 
>  - The oop preservation and exception handling has been fixed.
>  - Test added.
>  - Merge branch 'master' of https://github.com/openjdk/jdk into 8365192
>  - Merge branch 'master' of https://github.com/openjdk/jdk into 8365192
>  - added _
>  - wong phase
>  - fixed name
>  - simplified after feedback
>  - 8365192: post_meth_exit should be in vm state when calling get_jvmti_thread_state

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

> 1842:   // Additionally, the result oop should be preserved while the thread is in java.
> 1843:   BasicType type = current_frame.interpreter_frame_result(&oop_result, &value);
> 1844: 

We could add the following assert here (or in `frame::interpreter_frame_result`):

`assert(type == T_VOID || current_frame.interpreter_frame_expression_stack_size() > 0, “”);`

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

> 1848:   // Deferred transition to VM, so we can stash away the return oop before GC
> 1849:   // Note that this transition is not needed when throwing an exception, because
> 1850:   // there is no oop to retain.

This sentence about not needing the transition should be removed.

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

> 1851:   JvmtiThreadState *state;
> 1852:   {
> 1853:     ThreadInVMfromJava tiv(thread);

The `JRT_BLOCK` below can be moved up. We always transition to vm anyways, so we don’t need the extra `ThreadInVMfromJava`.

test/hotspot/jtreg/serviceability/jvmti/events/MethodExit/ExceptionOccurred.java line 1:

> 1: /*

Shouldn't this file be placed in `test/hotspot/jtreg/serviceability/jvmti/events/MethodExit/ExceptionOccurred` along with the cpp file?

test/hotspot/jtreg/serviceability/jvmti/events/MethodExit/ExceptionOccurred.java line 52:

> 50:             enable();
> 51:             exceptionExit();
> 52:             disableAndCheck();

`disableAndCheck` should be moved after the try-catch block otherwise we will never call it.

test/hotspot/jtreg/serviceability/jvmti/events/MethodExit/ExceptionOccurred/libExceptionOccurred.cpp line 62:

> 60:   if (upcall_method == nullptr) {
> 61:     fatal(jni,"Can't find upCall method.");
> 62:   }

Missing the upcall (not noticed now because of never calling `disableAndCheck`).

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

PR Review Comment: https://git.openjdk.org/jdk/pull/26713#discussion_r2286328220
PR Review Comment: https://git.openjdk.org/jdk/pull/26713#discussion_r2286358481
PR Review Comment: https://git.openjdk.org/jdk/pull/26713#discussion_r2286331388
PR Review Comment: https://git.openjdk.org/jdk/pull/26713#discussion_r2286355238
PR Review Comment: https://git.openjdk.org/jdk/pull/26713#discussion_r2286335516
PR Review Comment: https://git.openjdk.org/jdk/pull/26713#discussion_r2286337948