RFR: 8313739: ZipOutputStream.close() should always close the wrapped stream [v4]
Jaikiran Pai
jpai at openjdk.org
Tue Jan 16 15:07:20 UTC 2024
On Mon, 15 Jan 2024 10:26:53 GMT, Eirik Bjørsnøs <eirbjo at openjdk.org> wrote:
>> Please consider this PR which makes `DeflaterOutputStream.close()` always close its wrapped output stream exactly once.
>>
>> Currently, closing of the wrapped output stream happens outside the finally block where `finish()` is called. If `finish()` throws, this means the wrapped stream will not be closed. This can potentially lead to leaking resources such as file descriptors or sockets.
>>
>> This fix is to move the closing of the wrapped stream inside the finally block.
>>
>> Additionally, the `closed = true;` statement is moved to the start of the close method. This makes sure we only ever close the wrapped stream once (this aligns with the overridden method `FilterOutputStream.close´)
>>
>> Specification: This change brings the implementation of `DeflaterOutputStream.close()` in line with its specification: *Writes remaining compressed data to the output stream and closes the underlying stream.*
>>
>> Risk: This is a behavioural change. There is a small risk that existing code depends on the close method not following its specification.
>>
>> Testing: The PR adds a new JUnit 5 test `CloseWrappedStream.java` which simulates the failure condition and verifies that the wrapped stream was closed under failing and non-failing conditions.
>
> Eirik Bjørsnøs has updated the pull request incrementally with one additional commit since the last revision:
>
> Update test/jdk/java/util/zip/ZipOutputStream/CloseWrappedStream.java
>
> Remove extra whitespace
>
> Co-authored-by: Andrey Turbanov <turbanoff at gmail.com>
The source changes as well as the test looks good to me.
-------------
Marked as reviewed by jpai (Reviewer).
PR Review: https://git.openjdk.org/jdk/pull/17209#pullrequestreview-1823702967
More information about the core-libs-dev
mailing list