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

2024-01-01 Thread Eirik Bjørsnøs
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 c

Re: RFR: 8313739: ZipOutputStream.close() should always close the wrapped stream

2024-01-02 Thread Jaikiran Pai
On Mon, 1 Jan 2024 16:12:13 GMT, Eirik Bjørsnøs wrote: > 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 behavioura

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

2024-02-02 Thread Lance Andersen
On Mon, 22 Jan 2024 13:58:00 GMT, Eirik Bjørsnøs 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 c

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

2024-01-02 Thread Eirik Bjørsnøs
> 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 cl

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

2024-01-02 Thread Eirik Bjørsnøs
On Tue, 2 Jan 2024 08:35:47 GMT, Jaikiran Pai wrote: > This call has a potential to throw an `IOException` in which case any > original `IOException` thrown from the `finish()` call will be lost. Good catch. Adopted your suggestion with a few changes: - The catch for IOException during finish(

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

2024-01-02 Thread Jaikiran Pai
On Tue, 2 Jan 2024 09:14:14 GMT, Eirik Bjørsnøs 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 `fin

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

2024-01-02 Thread Eirik Bjørsnøs
> 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 cl

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

2024-01-02 Thread Eirik Bjørsnøs
On Tue, 2 Jan 2024 10:37:55 GMT, Jaikiran Pai 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 s

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

2024-01-02 Thread Eirik Bjørsnøs
On Tue, 2 Jan 2024 12:21:21 GMT, Eirik Bjørsnøs 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 ca

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

2024-01-15 Thread Andrey Turbanov
On Tue, 2 Jan 2024 12:21:21 GMT, Eirik Bjørsnøs 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 ca

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

2024-01-15 Thread Eirik Bjørsnøs
> 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 w

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

2024-01-15 Thread Alan Bateman
On Tue, 2 Jan 2024 22:06:36 GMT, Eirik Bjørsnøs wrote: > A CSR for this change has been proposed and is ready for review: > [JDK-8322871](https://bugs.openjdk.org/browse/JDK-8322871) I've reviewed the CSR so you can finalize. The implementation change looks fine. - PR Comment: htt

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

2024-01-16 Thread Jaikiran Pai
On Mon, 15 Jan 2024 10:26:53 GMT, Eirik Bjørsnøs 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 c

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

2024-01-16 Thread Lance Andersen
On Mon, 15 Jan 2024 10:26:53 GMT, Eirik Bjørsnøs 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 c

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

2024-01-22 Thread Eirik Bjørsnøs
> 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 w

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

2024-01-22 Thread Eirik Bjørsnøs
On Tue, 16 Jan 2024 18:18:38 GMT, Lance Andersen wrote: > This comment could use a bit of wordsmithing to indicate what "correct" means It's hard to write good prose for these tricky error scenarios. But just saying "correct" without defining it is a bit too lazy, yes :-) Please take a look at