On Fri, 16 May 2025 15:35:18 GMT, Lance Andersen <lan...@openjdk.org> wrote:
>> src/jdk.zipfs/share/classes/jdk/nio/zipfs/ZipFileSystem.java line 259:
>>
>>> 257:
>>> 258: // Pass "this" as a parameter after everything else is set up.
>>> 259: this.rootdir = new ZipPath(this, new byte[]{'/'});
>>
>> This could probably be set above release version etc. but it's a choice of
>> either:
>> 1. waiting until everything is set up before passing "this"
>> 2. letting an incomplete "this" instance get passed to another class
>> (possible escape risk?)
>>
>> Though in this case the "incompleteness" is limited to it not being set up
>> for multi-jar reading yet, which absolutely shouldn't affect the root path.
>> It feels safer to me to leave it to the end, or perhaps we should dig in to
>> ZipPath, figure out what it really wants here, and just call a different
>> constructor?
>
> I think it was fine where it was originally given how rootdir is used
I'd rather at least have it over the last part where the non-final fields are
modified (in that state it's an instance functionally identical to one opened
on a non-MR JAR).
Looking through the ZipPath code, it seems to (currently) only need to read
back the ZipCoder field (but I'm still nervous about restoring the "this"
escape to its original location since the ZipPath code might change one day and
rely on something not initialized).
I moved it so it's the last final field initialized, but I'll revert it
completely if you want.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/25178#discussion_r2095549535
