RFR: 8066583: DeflaterInput/OutputStream and InflaterInput/OutputStream should explain responsibility for freeing resources [v8]
Alan Bateman
alanb at openjdk.org
Thu Apr 3 12:09:57 UTC 2025
On Thu, 3 Apr 2025 11:40:23 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 16 additional commits since the last revision:
>
> - additional inputs
> - merge latest from master branch
> - Alan's input - replace "@apiNote" in favour of formal spec
> - merge latest from master branch
> - 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
> - ... and 6 more: https://git.openjdk.org/jdk/compare/9d89f79e...cfeef86d
The wording is good, thanks for persisting through all the iterations to get this right.
src/java.base/share/classes/java/util/zip/DeflaterInputStream.java line 38:
> 36: *
> 37: * <h2 id="compressor-usage">Compressor Usage</h2>
> 38: * A {@linkplain DeflaterInputStream} created without
I don't think you meant to use @linkplain here, should be @code or @link.
-------------
Marked as reviewed by alanb (Reviewer).
PR Review: https://git.openjdk.org/jdk/pull/23655#pullrequestreview-2739616795
PR Review Comment: https://git.openjdk.org/jdk/pull/23655#discussion_r2026860948
More information about the core-libs-dev
mailing list