RFR: 8369181: InflaterOutputStream: writing after finish() results in IllegalStateException instead of an IOException

[Daniel Fuchs]

On Thu, 26 Feb 2026 11:28:17 GMT, Jaikiran Pai <jpai at openjdk.org> wrote:

> Can I please get a review of this change which addresses the issue noted in https://bugs.openjdk.org/browse/JDK-8369181?
> 
> As noted in that issue, if `finish()` is called on a `InflaterOutputStream` that was constructed without passing a `Inflater`, then any subsequent `write()`s on that `InflaterOutputStream` result in an `IllegalStateException`, because we close the `Inflater` in `finish()`. The commit in this PR, fixes the issue by throwing the specified `IOException` in place of the `IllegalStateException`. 
> 
> At the same time, the documentation of `finish()` has been enhanced to clarify the current behaviour, through a `@implSpec`.
> 
> Alternative approaches of deprecating finish() and/or not closing the default Inflater were considered, but given the current long standing implementation of finish(), it was decided to merely specify the current behaviour of closing the  default Inflater in finish().
> 
> A new jtreg test has been added to reproduce the issue and verify the fix. tier1, tier2, tier3 testing of this change completed without any related issues.
> 
> I'll file a CSR shortly for this change.

I like this approach. I believe it's the best for keeping compatibility risks low. I was a bit uncertain about specifying calling `Inflater::close()`  when the implementation actually call `Inflater::end()` but this probably don't matter since it's only called on the default inflater which is not user-supplied.

src/java.base/share/classes/java/util/zip/InflaterOutputStream.java line 243:

> 241:         flush();
> 242:         if (usesDefaultInflater) {
> 243:             inf.end();

close() and end() are the same thing for the default inflater so I guess this is fine and not violating the spec.

src/java.base/share/classes/java/util/zip/InflaterOutputStream.java line 288:

> 286:         if (usesDefaultInflater && defaultInflaterClosed) {
> 287:             throw new IOException("Inflater closed");
> 288:         }

Since we're already checking for `closed` before checking for `null` I guess it makes sense to make this check here in second position. I wonder if it could/should be included in `ensureOpen()`? Maybe, or maybe  not - I see `ensureOpen()` is called from `finish()` too. So if we moved that check to `ensureOpen()` then calling `finish()` twice in succession would throw. What you have now will probably keep compatibility risk at minima.

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

PR Review: https://git.openjdk.org/jdk/pull/29935#pullrequestreview-3860485396
PR Review Comment: https://git.openjdk.org/jdk/pull/29935#discussion_r2858632959
PR Review Comment: https://git.openjdk.org/jdk/pull/29935#discussion_r2858648728