RFR: 8066583: DeflaterInput/OutputStream and InflaterInput/OutputStream should explain responsibility for freeing resources [v6]
Jaikiran Pai
jpai at openjdk.org
Thu Mar 27 09:01:20 UTC 2025
On Thu, 27 Mar 2025 07:23:07 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 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/90dc8a3b...ed7cf59e
Hello Eirik,
> The term 'default compressor' may raise more questions than it answers. What is the default compressor, where is it specified
The "default compressor" and "default decompressor" are not new terms. They are already part of these classes javadocs. For example, the constructor of DeflaterInputStream already has:
> Creates a new input stream with a default compressor and buffer size.
-------------
PR Comment: https://git.openjdk.org/jdk/pull/23655#issuecomment-2757242629
More information about the core-libs-dev
mailing list