RFR: 8339332: Clean up the java launcher code

[Jaikiran Pai]

On Thu, 12 Sep 2024 06:33:18 GMT, David Holmes <dholmes at openjdk.org> wrote:

>> Can I please get a review of this change which cleans up the code in the `java` launcher? The original motivation for the change was to prevent the `java` launcher's C code from parsing a jar's manifest when launching an application through `java -jar foo.jar`. The jar parsing code in C currently doesn't have the same level of robustness and features as what's available in the Java side implementation of `Zip/JarFile` API in `java.util.zip`. Among them, one is the lack of ZIP64 support in the launcher's jar parsing code. That issue was noticed in https://bugs.openjdk.org/browse/JDK-8328995 and a PR (https://github.com/openjdk/jdk/pull/18479) was proposed to enhance this C code in the launcher to support ZIP64 jar files. Even before the proposed changes in that PR, there already were a few concerns about long term maintainability of the jar parsing code in the launcher given that it duplicates what we already do in the Java side implementation, so adding new C code here isn't prefe
 rable.
>> 
>> The change in this current PR removes the part where the launcher's C code was parsing the jar's manifest file to identify "Main-Class" and "SplashScreen-Image" attributes from the manifest of the jar that was being launched. This was being done in the C code to support a long outdated "JRE selection" feature (mJRE https://bugs.openjdk.org/browse/JDK-8058407) which required it to parse the jar's manifest. After that feature was removed, the sole reason the jar's manifest was continued to be parsed in this launcher code was for convenience.
>> 
>> With the changes proposed in this PR, the launcher will use the jar parsing C code only if the jar has a "SplashScreen-Image" in its manifest. The number of such jars, based on an experiment against a large corpus of jar files, is very minimal (we found just 26 jars in around 900K odd jar files with a "SplashScreen-Image"). The updated code in this PR also delegates the identification of the "SplashScreen-Image" attribute to the java code (in `LauncherHelper`) thus removing the need to parse the manifest in C code. The only time the C code will now do the jar parsing is to display a splash screen image (if any is present) from the jar file. I think even that can be avoided, but I haven't experimented with it and I would like to pursue that separately in future, instead of this PR.
>> 
>> Finally, a few of the utility functions that were introduced in the launcher C code in a recent JEP to support "Implicitly Declared Classes and...
>
> src/java.base/share/native/libjli/java.c line 1138:
> 
>> 1136:         has_arg = IsOptionWithArgument(argc, argv);
>> 1137:         if (JLI_StrCCmp(arg, "-version:") == 0) {
>> 1138:             JLI_ReportErrorMessage(SPC_ERROR1);
> 
> Are these error message definitions, `SPC_ERROR1` etc, unused now?

The `SPC_ERROR1` and similar error messages were being thrown when the launcher arguments `-version:`, `-jre-restrict-search` and `-jre-no-restrict-search` were being parsed. That parsing used to happen in this `SelectVersion()` function which has been removed in the PR. The parsing of these arguments has been moved to the existing `ParseArguments` function where it continues to throw this `SPC_ERROR1` and other related error messages. So these error message definitions are still in use.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/20929#discussion_r1756252290