RFR: 8318812: LauncherHelper.checkAndLoadMain closes jar file that's about to be re-opened

[Christian Stein]

On Wed, 14 Feb 2024 11:41:43 GMT, Alan Bateman <alanb at openjdk.org> wrote:

>> Please review this PR that makes the launcher helper keep a reference to the executable JAR file active after extracting the name of the main class and returning it as Class instance. Now, when loading classes from the JAR file, it hasn't to be re-opened.
>
> src/java.base/share/classes/sun/launcher/LauncherHelper.java line 596:
> 
>> 594:         String mainValue;
>> 595:         try {
>> 596:             Manifest manifest = jarFile.getManifest();
> 
> I think the try-catch around the block can be dropped and you can put a more targeted try-catch around getManifest, at least I think that is the only case remaining in getMainClassFromJar that needs error handling now.

Dropping that outer try-catch will lead to many whitespace-only changes due to un-indenting lines of the former block. Proceed or keep the indentation stable by making that internal method throw IOE and insert a comment-only block like:


/* keep indentation stable */ {
    Manifest manifest = jarFile.getManifest();
// ...
}

> src/java.base/share/classes/sun/launcher/LauncherHelper.java line 876:
> 
>> 874:                     jarFile.close();
>> 875:                 } catch (IOException ioe) {
>> 876:                     abort(ioe, "java.launcher.jar.error1", what);
> 
> java.launcher.jar.error1 is "Error: An unexpected error occurred while trying to open file". I can't think of any cases where it might fail but the error message would be confusing if it did.

Good catch! Introducing and using:

java.launcher.jar.error5=\
    Error: An unexpected error occurred while trying to close file {0}

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

PR Review Comment: https://git.openjdk.org/jdk/pull/17843#discussion_r1489697776
PR Review Comment: https://git.openjdk.org/jdk/pull/17843#discussion_r1489699548