RFR: 8066583: DeflaterInput/OutputStream and InflaterInput/OutputStream should explain responsibility for freeing resources [v3]

[Volkan Yazici]

On Sat, 1 Mar 2025 07:50:44 GMT, Jaikiran Pai <jpai at 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 six additional commits since the last revision:
> 
>  - change "@implSpec" to "@apiNote" and update their text
>  - add class level documentation
>  - merge latest from master branch
>  - add tests
>  - update DeflaterInputStream and InflaterInputStream
>  - 8066583: DeflaterOutputStream and InflaterOutputStream should explain responsibility for freeing resources

src/java.base/share/classes/java/util/zip/DeflaterInputStream.java line 40:

> 38:  * <p>This class uses a {@link Deflater} for compressing the data. Two forms of constructors
> 39:  * are available for constructing a {@code DeflaterInputStream} - one which accepts
> 40:  * a {@code Deflater} and one which doesn't. The constructors that don't accept a {@code Deflater}

> `The constructors that don't accept a {@code Deflater}`

Regarding the internal `Deflater` that will be created and its life cycle will be managed by the `DIS`... This sounds like an _internal implementation detail_ and `DIS` takes care of this internal resource nicely. IMHO, providing this information brings no benefit to users and sort of hand-cuffs ourselves to this behavior. Shall we remove that part from the docs? (Note that this remark applies to all 4 classes whose JavaDoc is updated.)

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

PR Review Comment: https://git.openjdk.org/jdk/pull/23655#discussion_r1977130720