RFR: 8347712: IllegalStateException on multithreaded ZipFile access with non-UTF8 charset [v2]

[Jaikiran Pai]

On Tue, 11 Mar 2025 15:32:53 GMT, Eirik Bjørsnøs <eirbjo at openjdk.org> wrote:

>> Jaikiran Pai has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   tiny typo fix in newly introduced documentation
>
> src/java.base/share/classes/java/util/zip/ZipFile.java line 1222:
> 
>> 1220:                 table[hsh] = index;
>> 1221:                 // Record the CEN offset and the name hash in our hash cell.
>> 1222:                 entries[index] = hash;
> 
> Seems unrelated to the issue at hand. Perhaps better left for a separate PR, backed by a benchmark.

This isn't a performance related change. I merely marked the method parameters as `final` and the `index` method parameter was being updated here.

> src/java.base/share/classes/java/util/zip/ZipFile.java line 1426:
> 
>> 1424:             private final Charset entryNameCommentCharset;
>> 1425: 
>> 1426:             public Key(File file, BasicFileAttributes attrs, Charset entryNameCommentCharset) {
> 
> I feel this parameter and the field it is assigned to could also just be called `charset`. The comment has the information about what it's used for.

I've updated the PR to rename this to `charset`.

> src/java.base/share/classes/java/util/zip/ZipFile.java line 1434:
> 
>> 1432:             @Override
>> 1433:             public int hashCode() {
>> 1434:                 long t = entryNameCommentCharset.hashCode();
> 
> This represents a behavioral change, right? Is a CSR warranted?
> 
> EDIT: Scratch that, I guess the caching mechanism here is unspecified and and more of implementation detail.

This is an internal implementation detail, so doesn't require a CSR.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/23986#discussion_r1990706483
PR Review Comment: https://git.openjdk.org/jdk/pull/23986#discussion_r1990704378
PR Review Comment: https://git.openjdk.org/jdk/pull/23986#discussion_r1990704708