RFR: 8225763: Inflater and Deflater should implement AutoCloseable [v12]
Jaikiran Pai
jpai at openjdk.org
Tue Jan 7 09:33:12 UTC 2025
On Mon, 2 Dec 2024 16:55:23 GMT, Roger Riggs <rriggs at openjdk.org> wrote:
>> 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 22 additional commits since the last revision:
>>
>> - provide guidance to subclasses on which method to override for cleaning up resources
>> - Revert "Roger's suggestion - Make Inflater.close() and Deflater.close() final, also update the new tests to match this change"
>>
>> This reverts commit b60181bbb4be9fac294b16820cd02017de71783e.
>> - merge latest from master branch
>> - update end() to remove mention of other methods throwing IllegalStateException
>> - update the class level documentation of Inflater to match the updates in Deflater
>> - merge latest from master branch
>> - improve Deflater class level doc
>> - Stuart's review - improve end() API doc
>> - merge latest from master branch
>> - missed a few methods for specifying IllegalStateException
>> - ... and 12 more: https://git.openjdk.org/jdk/compare/ec8a7574...42ff9059
>
> src/java.base/share/classes/java/util/zip/Inflater.java line 58:
>
>> 56: * To release resources used by the {@code Inflater}, applications must call the
>> 57: * {@link #end()} method. After {@code end()} has been called, subsequent calls
>> 58: * to several methods of the {@code Inflater} will throw an {@link IllegalStateException}.
>
> Since `close()` is not mentioned here, there may be some come confusion about whether `end()` still needs to be called within T-W-R.
I think that's a good point. I've now updated the PR to reword this part of the text both in `Inflater` and `Deflater`. Does the change look OK?
> test/jdk/java/util/zip/DeflaterClose.java line 43:
>
>> 41:
>> 42: /**
>> 43: * Closes the Deflater multiple times and then expects close() and end() to be called that
>
> Should this be "close() *or* end()"?
Hello Roger, calling `close()` on Inflater/Deflater will just call `end()` on the instance. So I believe "close() and end()" in the test comment here is correct. I now pushed a very minor change just to make it more clear. Let me know if you think we should improve the comment further.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/19675#discussion_r1905158732
PR Review Comment: https://git.openjdk.org/jdk/pull/19675#discussion_r1905161112
More information about the core-libs-dev
mailing list