On Thu, 27 Mar 2025 07:23:07 GMT, Jaikiran Pai <j...@openjdk.org> wrote:

>> Can I please get a review of this doc-only change which proposes to improve 
>> the API documentation of `DeflaterInputStream`, `DeflaterOutputStream`, 
>> `InflaterInputStream` and `InflaterOutputStream` classes?
>> 
>> As noted in https://bugs.openjdk.org/browse/JDK-8066583 some of the 
>> constructors of these classes allow callers to pass a `Deflater`/`Inflater` 
>> instance. The implementation of these classes do not close the given 
>> `Deflater`/`Inflater` when the corresponding instance of the class itself is 
>> closed. This isn't documented and can lead to situations where callers 
>> aren't aware that they are responsible for closing the given 
>> `Deflater`/`Inflater` instance. That can then lead to resource leaks of 
>> resources held by the `Deflater`/`Inflater`.
>> 
>> The commit in this PR updates the relevant constructors of these classes to 
>> add an `@implSpec` explaining the responsibility of closing the given 
>> `Inflater`/`Deflater`. I chose the `@implSpec` since each of these classes 
>> whose documentation is being updated are `public` and can be sub-classed and 
>> the `close()` method overridden. The text being added merely specifies the 
>> implementation of these classes and not the sub-classes.
>> 
>> I'll draft a CSR once we agree on the proposed text.
>
> Jaikiran Pai has updated the pull request with a new target base due to a 
> merge or a rebase. The incremental webrev excludes the unrelated changes 
> brought in by the merge/rebase. The pull request contains 12 additional 
> commits since the last revision:
> 
>  - Lance's inputs
>  - merge latest from master branch
>  - merge latest from master branch
>  - merge latest from master branch
>  - additional updates to the doc
>  - merge latest from master branch
>  - change "@implSpec" to "@apiNote" and update their text
>  - add class level documentation
>  - merge latest from master branch
>  - add tests
>  - ... and 2 more: https://git.openjdk.org/jdk/compare/73e4df95...ed7cf59e

src/java.base/share/classes/java/util/zip/InflaterOutputStream.java line 89:

> 87:      * @apiNote
> 88:      * The default decompressor will be closed when this output stream
> 89:      * is {@linkplain #close() closed}.

This can be normative, meaning you can drop the apiNote.

src/java.base/share/classes/java/util/zip/InflaterOutputStream.java line 106:

> 104:      * {@linkplain #close() Closing} this output stream
> 105:      * {@linkplain ##decompressor-usage will not close} the given
> 106:      * {@linkplain Inflater decompressor}.

This can be normative too.

src/java.base/share/classes/java/util/zip/InflaterOutputStream.java line 123:

> 121:      * {@linkplain #close() Closing} this output stream
> 122:      * {@linkplain ##decompressor-usage will not close} the given
> 123:      * {@linkplain Inflater decompressor}.

This can be normative too.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/23655#discussion_r2021201415
PR Review Comment: https://git.openjdk.org/jdk/pull/23655#discussion_r2021201761
PR Review Comment: https://git.openjdk.org/jdk/pull/23655#discussion_r2021203278

Reply via email to