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

[Jaikiran Pai]

On Tue, 11 Mar 2025 15:28:59 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 87:
> 
>> 85:     // the ZipCoder instance is derived from the Charset passed to the ZipFile constructor
>> 86:     // and will be used for decoding the non-UTF-8 entry names and the ZIP file comment.
>> 87:     private final ZipCoder entryNameCommentCoder;
> 
> The Source `ZipCoder` field had a `@Stable` annotation. Any reason why this did not survive the move?

Hello Eirik, the `ZipCoder` field here is `final` so it's doesn't require a `@Stable` annotation. I realize this field in `Source` too was `final` and yet it was annotated `@Stable`, but my understanding is that, that was redundant and unnecessary. The documentation of `@Stable` states that a `@Stable` for `final` fields is only relevant when the field is a array type, which it isn't in the case here.

> src/java.base/share/classes/java/util/zip/ZipFile.java line 87:
> 
>> 85:     // the ZipCoder instance is derived from the Charset passed to the ZipFile constructor
>> 86:     // and will be used for decoding the non-UTF-8 entry names and the ZIP file comment.
>> 87:     private final ZipCoder entryNameCommentCoder;
> 
> Since we do not have separate ZipCoders for decoding the different ZIP values, I'm not sure it's useful to put the field names ("name"/"comment") into the instance field name here. Especially if the comment already has this information.
> 
> Could we call this just `zc` or `zipCoder`?

I have now renamed the field to `zipCoder`.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/23986#discussion_r1990700514
PR Review Comment: https://git.openjdk.org/jdk/pull/23986#discussion_r1990700773