Re: RFR: 8329581: Java launcher no longer prints a stack trace
On Mon, 15 Apr 2024 18:25:02 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 I tested now pull/18753 and pull/18786: both solve both issues JDK-8329420 and JDK-8329581 - PR Comment: https://git.openjdk.org/jdk/pull/18786#issuecomment-2061302794
Re: RFR: 8329420: Java 22 (and 23) launcher calls default constructor although main() is static [v4]
On Wed, 17 Apr 2024 09:20:24 GMT, Jan Lahoda wrote:
>> Consider code like:
>>
>> public class MainClass {
>> public MainClass() {
>> System.out.println("Constructor called!");
>> }
>> public static void main() {
>> System.out.println("main called!");
>> }
>> }
>>
>> and compile and run it, with preview enabled, like:
>>
>> $ javac /tmp/MainClass.java
>> $ java --enable-preview -classpath /tmp MainClass
>> Constructor called!
>> main called!
>>
>>
>> That is wrong, as the `main` method is static, and there is no need to
>> create a new instance of the class.
>>
>> The reason is that as launcher attempts to invoke the main method, it goes
>> in the following order: 1) static-main-with-args; 2)
>> instance-main-with-args; 3) static-main-without-args; 4)
>> instance-main-without-args. But, for the instance variants, the code first
>> creates a new instance of the given class, and only then attempts to lookup
>> the `main` method, and will pass to the next option when the `main` method
>> lookup fails. So, when invoking static-main-without-args, the current class
>> instance may be created for instance-main-with-args, which will then fail
>> due to the missing `main(String[])` method.
>>
>> The proposed solution to this problem is to simply first do a lookup for the
>> `main` method (skipping to the next variant when the given main method does
>> not exist, without instantiating the current class).
>>
>> There is also a relatively closely related problem: what happens when the
>> constructor throws an exception?
>>
>> public class MainClass {
>> public MainClass() {
>> if (true) throw new RuntimeException();
>> }
>> public void main() {
>> System.out.println("main called!");
>> }
>> }
>>
>>
>> when compiled an run, this produces no output whatsoever:
>>
>> $ javac /tmp/MainClass.java
>> $ java --enable-preview -classpath /tmp MainClass
>> $
>>
>>
>> This is because any exceptions thrown from the constructor are effectively
>> ignored, and the launcher will continue with the next variant. This seems
>> wrong - the exception should be printed for the user, like:
>>
>> $ java --enable-preview -classpath /tmp/ MainClass
>> Exception in thread "main" java.lang.RuntimeException
>> at MainClass.(MainClass.java:3)
>>
>>
>> This patch proposes to do that by not consuming the exceptions thrown from
>> the constructor, and stop the propagation to the next variant.
>
> Jan Lahoda has updated the pull request incrementally with one additional
> commit since the last revision:
>
> Fixing typo (should not continue after an exception from a constructor).
I tested now pull/18753 and pull/18786: both solve both issues JDK-8329420 and
JDK-8329581
-
PR Comment: https://git.openjdk.org/jdk/pull/18753#issuecomment-2061301645
Re: RFR: 8329581: Java launcher no longer prints a stack trace
On Mon, 15 Apr 2024 18:25:02 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 Just tried to avoid unnecessary duplication of work. - PR Comment: https://git.openjdk.org/jdk/pull/18786#issuecomment-2061191285
Re: RFR: 8329581: Java launcher no longer prints a stack trace
On Mon, 15 Apr 2024 18:25:02 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 Isn't this already fixed by https://github.com/openjdk/jdk/pull/18753? - PR Comment: https://git.openjdk.org/jdk/pull/18786#issuecomment-2058185032
Re: RFR: 8329420: Java 22 (and 23) launcher calls default constructor although main() is static [v2]
On Mon, 15 Apr 2024 07:36:05 GMT, Jan Lahoda wrote:
>> Consider code like:
>>
>> public class MainClass {
>> public MainClass() {
>> System.out.println("Constructor called!");
>> }
>> public static void main() {
>> System.out.println("main called!");
>> }
>> }
>>
>> and compile and run it, with preview enabled, like:
>>
>> $ javac /tmp/MainClass.java
>> $ java --enable-preview -classpath /tmp MainClass
>> Constructor called!
>> main called!
>>
>>
>> That is wrong, as the `main` method is static, and there is no need to
>> create a new instance of the class.
>>
>> The reason is that as launcher attempts to invoke the main method, it goes
>> in the following order: 1) static-main-with-args; 2)
>> instance-main-with-args; 3) static-main-without-args; 4)
>> instance-main-without-args. But, for the instance variants, the code first
>> creates a new instance of the given class, and only then attempts to lookup
>> the `main` method, and will pass to the next option when the `main` method
>> lookup fails. So, when invoking static-main-without-args, the current class
>> instance may be created for instance-main-with-args, which will then fail
>> due to the missing `main(String[])` method.
>>
>> The proposed solution to this problem is to simply first do a lookup for the
>> `main` method (skipping to the next variant when the given main method does
>> not exist, without instantiating the current class).
>>
>> There is also a relatively closely related problem: what happens when the
>> constructor throws an exception?
>>
>> public class MainClass {
>> public MainClass() {
>> if (true) throw new RuntimeException();
>> }
>> public void main() {
>> System.out.println("main called!");
>> }
>> }
>>
>>
>> when compiled an run, this produces no output whatsoever:
>>
>> $ javac /tmp/MainClass.java
>> $ java --enable-preview -classpath /tmp MainClass
>> $
>>
>>
>> This is because any exceptions thrown from the constructor are effectively
>> ignored, and the launcher will continue with the next variant. This seems
>> wrong - the exception should be printed for the user, like:
>>
>> $ java --enable-preview -classpath /tmp/ MainClass
>> Exception in thread "main" java.lang.RuntimeException
>> at MainClass.(MainClass.java:3)
>>
>>
>> This patch proposes to do that by not consuming the exceptions thrown from
>> the constructor, and stop the propagation to the next variant.
>
> Jan Lahoda has updated the pull request incrementally with one additional
> commit since the last revision:
>
> Reflecting review feedback.
Does this also fix [JDK-8329581](https://bugs.openjdk.org/browse/JDK-8329581)?
-
PR Comment: https://git.openjdk.org/jdk/pull/18753#issuecomment-2057206293
