On Mon, 24 Mar 2025 15:01:09 GMT, Eirik Bjørsnøs <eir...@openjdk.org> wrote:
>> src/java.base/share/classes/java/util/zip/ZipCoder.java line 44:
>>
>>> 42: * <p>
>>> 43: * The {@code ZipCoder} for UTF-8 charset is thread safe, {@code
>>> ZipCoder}
>>> 44: * for other charsets require external synchronization.
>>
>> I think the "thread safe" feature is already implied by the comment on UTF8:
>> "Encoding/decoding is stateless". So I recommend just mentioning that a
>> ZipCoder may carry states, and a ZipCoder obtained from `ZipCoder.get`
>> should only be used locally.
>
> I agree with @liach here. The fact that one ZipCoder is stateless and
> thread-safe is a class-internal implementation detail and not information
> which any call site of this API makes use of.
> So I recommend just mentioning that a ZipCoder may carry states, and a
> ZipCoder obtained from ZipCoder.get should only be used locally.
The `UTF8ZipCoder` may not just be accessed through the `ZipCoder.UTF8` field.
It could even be obtained through `ZipCoder.get(UTF_8)`. So the ZipCode
returned by `ZipCoder.get()` can still be used concurrently (if it is for
UTF-8).
> The fact that one ZipCoder is stateless and thread-safe is a class-internal
> implementation detail and not information which any call site of this API
> makes use of.
The `ZipCoder` itself is an internal implementation detail (it's a package
protected class), so the javadoc that we are discussing here won't be published
by the `java.base` module. I believe that the thread safety aspect of the
ZipCoder instances, even if it for some specific charsets, is an important
detail even for internal call sites to be aware of. In fact, the current issue
that we are fixing shows that this detail might have helped prevent this issue.
I think the brief comment that we have added here, in its current form, is good
enough. So unless either of you or others feel strongly about it, I would like
to keep it in its current form.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/23986#discussion_r2068550089
