On Wed, 24 Apr 2024 14:49:55 GMT, Sonia Zaldana Calles <szald...@openjdk.org> 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