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