RFR: 8313739: ZipOutputStream.close() should always close the wrapped stream [v2]

[Jaikiran Pai]

On Tue, 2 Jan 2024 09:14:14 GMT, Eirik Bjørsnøs <eirbjo at openjdk.org> wrote:

>> Please consider this PR which makes `DeflaterOutputStream.close()` always close its wrapped output stream.
>> 
>> 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.
>> 
>> 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:
> 
>   Prevent IOException thrown during finish() from being lost if an IOException is thrown while closing the wrapped stream

src/java.base/share/classes/java/util/zip/DeflaterOutputStream.java line 257:

> 255:                 } catch (IOException ioe) {
> 256:                     if (finishException != ioe) {
> 257:                         ioe.addSuppressed(finishException);

A null check for `finishException` will be needed here to prevent a `NullPointerException` being thrown from within `addSuppressed`. I think it's better to just copy over (rest of) the finally block from `FilterOutputStream`'s close() method.

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/17209#discussion_r1439328181