RFR: 8365937: post_method_exit might incorrectly set was_popped_by_exception and value in the middle of stack unwinding [v3]

[Patricio Chilano Mateo]

On Thu, 28 Aug 2025 16:41:56 GMT, Leonid Mesnik <lmesnik at openjdk.org> wrote:

>> The void `JvmtiExport::post_method_exit(JavaThread* thread, Method* method, frame current_frame) `calculates
>>   `bool exception_exit = state->is_exception_detected() && !state->is_exception_caught();`
>> to find if method exit normally or by exception.
>> However, JvmtiExport::post_method_exit( method is not called at all in the case of exception. See
>> `void JvmtiExport::notice_unwind_due_to_exception(JavaThread *thread, Method* method, address location, oop exception, bool in_handler_frame)`
>> where post_method_exit_inner is called directly.
>> 
>> The `exception_exit` is true when exception is processed and the current method is called in the middle of stack unwinding. 
>> 
>> 
>> The fix was a part of
>> https://github.com/openjdk/jdk/pull/26713
>> for
>> https://bugs.openjdk.org/browse/JDK-8365192
>
> 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 27 additional commits since the last revision:
> 
>  - Apply suggestions from code review
>    
>    Co-authored-by: David Holmes <62092539+dholmes-ora at users.noreply.github.com>
>  - comment fixed
>  - Merge branch 'master' of https://github.com/openjdk/jdk into 8365937
>  - assertion added.
>  - more comments in the test
>  - fixed ident
>  - bugid fixed:
>  - updated to fix 8365937
>  - fixed comment.
>  - test renamed.
>  - ... and 17 more: https://git.openjdk.org/jdk/compare/01e62e37...4e05639a

LGTM. Thanks for fixing and adding the extra tests.

test/hotspot/jtreg/serviceability/jvmti/events/MethodExit/PendingException/libTestMethodExitWithPendingException.cpp line 48:

> 46:     const char *str = jni->GetStringUTFChars(upcall_result, nullptr);
> 47:     if (str == nullptr) {
> 48:       fatal(jni ,"Failed to convert Java string to C string.");

Suggestion:

      fatal(jni, "Failed to convert Java string to C string.");

test/hotspot/jtreg/serviceability/jvmti/events/MethodExit/PendingException/libTestMethodExitWithPendingException.cpp line 63:

> 61:   jclass main_class = jni->FindClass("TestMethodExitWithPendingException");
> 62:   if (main_class == nullptr) {
> 63:     fatal(jni,"Can't find TestMethodExitWithPendingException class.");

Suggestion:

    fatal(jni, "Can't find TestMethodExitWithPendingException class.");

test/hotspot/jtreg/serviceability/jvmti/events/MethodExit/PendingException/libTestMethodExitWithPendingException.cpp line 69:

> 67:                                                    "upCall", "()Ljava/lang/String;");
> 68:   if (upcall_method == nullptr) {
> 69:     fatal(jni,"Can't find upCall method.");

Suggestion:

    fatal(jni, "Can't find upCall method.");

test/hotspot/jtreg/serviceability/jvmti/events/MethodExit/PendingException/libTestMethodExitWithPendingException.cpp line 76:

> 74:   const char *str = jni->GetStringUTFChars(upcall_result, nullptr);
> 75:   if (str == nullptr) {
> 76:     fatal(jni ,"Failed to convert Java string to C string.");

Suggestion:

    fatal(jni, "Failed to convert Java string to C string.");

test/hotspot/jtreg/serviceability/jvmti/events/MethodExit/PoppedByException/libTestPoppedByException.cpp line 39:

> 37:     }
> 38:     if (return_value.l != nullptr) {
> 39:       fatal(jni ,"return_value should be nullptr.");

Suggestion:

      fatal(jni, "return_value should be nullptr.");

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

Marked as reviewed by pchilanomate (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/26886#pullrequestreview-3169698189
PR Review Comment: https://git.openjdk.org/jdk/pull/26886#discussion_r2310845150
PR Review Comment: https://git.openjdk.org/jdk/pull/26886#discussion_r2310846119
PR Review Comment: https://git.openjdk.org/jdk/pull/26886#discussion_r2310846950
PR Review Comment: https://git.openjdk.org/jdk/pull/26886#discussion_r2310847641
PR Review Comment: https://git.openjdk.org/jdk/pull/26886#discussion_r2310849997