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

[Christian Stein]

On Mon, 19 Feb 2024 10:41:03 GMT, Alan Bateman <alanb at openjdk.org> wrote:

>> src/java.base/share/classes/sun/launcher/LauncherHelper.java line 821:
>> 
>>> 819:         // In LM_JAR mode, put the underlying file in the JarFile/ZipFile cache.
>>> 820:         // This will avoid needing to re-parse the manifest when the JAR file
>>> 821:         // is opened on the class path, triggered by Class.forName below.
>> 
>> Nice improvement to have. would it be ok to pad out this comment a bit more ? I found it difficult to understand the specifics. Perhaps :
>> 
>> 
>> 		// In LM_JAR mode, keep the underlying file open to retain it in 
>> 		// the ZipFile HashMap cache. This will avoid needing to re-parse
>>                 // the central directory when the file is opened on the class path,
>> 		// triggered by Class.forName below.
>
>> Nice improvement to have. would it be ok to pad out this comment a bit more ? I found it difficult to understand the specifics. Perhaps :
>> 
>> ```
>> 		// In LM_JAR mode, keep the underlying file open to retain it in 
>> 		// the ZipFile HashMap cache. This will avoid needing to re-parse
>>                 // the central directory when the file is opened on the class path,
>> 		// triggered by Class.forName below.
>> ```
> 
> No issue with expanding the comment but I think leave it as "ZipFile cache" as a more specific comment will quickly bit rot and confuse reader, and of course the Launcher code doesn't care which collection is used internally in the ZipFile code.

I'll expand the comment as suggest, keeping the "JarFile/ZipFile cache" phrase.

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

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