On Mon, 15 Apr 2024 18:25:02 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 My personal comments here: - I am fine with a solution like this. In 18753, I wanted to avoid a change of dynamics between the Java helper and the native part. But if we can change that, it looks better. I would suggest to take the test from 18753 though - doing a change like this without a test may lead to hard-to-find regressions in the future. (Note the current test should guard against both JDK-8329420 and JDK-8329581.) Or write a different test. - as Mandy points out, `LaucherHelper` already reads/has variables for "is-static" and "no-arguments" in `validateMainMethod`, so it should be possible to just use that values; also as Mandy points out, we can probably get rid of `CHECK_EXCEPTION_FAIL` and `CHECK_EXCEPTION_NULL_FAIL`, and use the `..._LEAVE` variants, no? (The `..._FAIL` variants where needed so that the launcher could continue with the next variant, but since we now only call the correct variant, we can just stop if something goes wrong?) ------------- PR Comment: https://git.openjdk.org/jdk/pull/18786#issuecomment-2063217552
