On Tue, 2 Jan 2024 10:37:55 GMT, Jaikiran Pai <j...@openjdk.org> wrote:
>> 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.
Updated the code to align with `FilterOutputStream.close()`:
- Avoid the suppression try/catch for the case where finishException is null
- Move `closed = true;` to the top of the method, this makes sure the wrapped
stream is only closed once even in the case where it throws during close. Close
should only be attempted once.
I added more test cases including one where both finish and close operations
fail, one where finish and close operations throw identical IOException
instances and one testing that the wrapped stream is closed only once, even if
the close operation throws.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/17209#discussion_r1439403755
