On Wed, 24 Apr 2024 14:49:55 GMT, Sonia Zaldana Calles
wrote:
>> Hi folks,
>>
>> This PR aims to fix
>> [JDK-8329581](https://bugs.openjdk.org/browse/JDK-8329581).
>>
>> I think the regression got introduced in
>> [JDK-8315458](https://bugs.openjdk.org/browse/JDK-8315458).
>>
>> In the issue linked above,
>> [LauncherHelper#getMainType](https://github.com/openjdk/jdk/pull/16461/files#diff-108a3a3e3c2d108c8c7f19ea498f641413b7c9239ecd2975a6c27d904c2ba226)
>> got removed to simplify launcher code.
>>
>> Previously, we used ```getMainType``` to do the appropriate main method
>> invocation in ```JavaMain```. However, we currently attempt to do all types
>> of main method invocations at the same time
>> [here](https://github.com/openjdk/jdk/blob/master/src/java.base/share/native/libjli/java.c#L623).
>>
>>
>> Note how all of these invocations clear the exception reported with
>> [CHECK_EXCEPTION_FAIL](https://github.com/openjdk/jdk/blob/140f56718bbbfc31bb0c39255c68568fad285a1f/src/java.base/share/native/libjli/java.c#L390).
>>
>>
>> Therefore, if a legitimate exception comes up during one of these
>> invocations, it does not get reported.
>>
>> I propose reintroducing ```LauncherHelper#getMainType``` but I'm looking
>> forward to your suggestions.
>>
>> Cheers,
>> Sonia
>
> Sonia Zaldana Calles has updated the pull request incrementally with one
> additional commit since the last revision:
>
> Using new macro to avoid reporting JNI error
Looks good. Can you add a new test for this? You can reference
MainClassCantBeLoadedTest.java which does something similar to what you need.
src/java.base/share/native/libjli/java.c line 621:
> 619: helperClass = GetLauncherHelperClass(env);
> 620: isStaticMainField =
> 621: (*env)->GetStaticFieldID(env, helperClass, "isStaticMain", "Z");
Nit: this line isn't long. Can do in 1 line.
Same for line 623-624, 626-627.
-
PR Review: https://git.openjdk.org/jdk/pull/18786#pullrequestreview-2020321074
PR Review Comment: https://git.openjdk.org/jdk/pull/18786#discussion_r1578158252
