On Tue, 19 Aug 2025 21:02:26 GMT, Patricio Chilano Mateo <pchilanom...@openjdk.org> wrote:
>> 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, “”);` Thanks, added. > 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`. I tried to do this in the first implementation, probably because of other problems. Seems, it can be done now. I run some testing to verify this fix. > 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`). Thanks, I removed it right before final clean up. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/26713#discussion_r2286603703 PR Review Comment: https://git.openjdk.org/jdk/pull/26713#discussion_r2286603775 PR Review Comment: https://git.openjdk.org/jdk/pull/26713#discussion_r2286604585
