On Tue, 1 Apr 2025 07:22:45 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 14 additional > commits since the last revision: > > - 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 > - change "@implSpec" to "@apiNote" and update their text > - add class level documentation > - ... and 4 more: https://git.openjdk.org/jdk/compare/acbfb6c5...f2d08bad src/java.base/share/classes/java/util/zip/DeflaterInputStream.java line 39: > 37: * <h2 id="compressor-usage">Compressor Usage</h2> > 38: * An {@linkplain DeflaterInputStream input stream} that is created > without specifying > 39: * a {@linkplain Deflater compressor} will use a default compressor. The > default Reading this again makes it wonder if "will use a default compressor" should be adjusted to say "will create a compressor", only to avoid anyone thinking there is a system wide default compressor. src/java.base/share/classes/java/util/zip/DeflaterInputStream.java line 88: > 86: * <p> > 87: * The default compressor will be closed when this input stream > 88: * is {@linkplain #close() closed}. There is another case where "the default compressor" may be mis-read to mean a system-wide compressor. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/23655#discussion_r2023231878 PR Review Comment: https://git.openjdk.org/jdk/pull/23655#discussion_r2023234064
