On Tue, 19 Aug 2025 05:54:13 GMT, Leonid Mesnik <[email protected]> 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
