RFR: 8349907: jdk.tools.jlink.internal.plugins.ZipPlugin does not close the Deflater in exceptional cases [v2]
Chen Liang
liach at openjdk.org
Wed Feb 12 15:45:26 UTC 2025
On Wed, 12 Feb 2025 15:42:36 GMT, Jaikiran Pai <jpai at openjdk.org> wrote:
>> Can I please get a review of this change in `jdk.tools.jlink.internal.plugins.ZipPlugin` which proposes to close the `Deflater` instance cleanly? This addresses https://bugs.openjdk.org/browse/JDK-8349907.
>>
>> As noted in that issue, the `Deflater` instance wasn't being closed in the exception code path of this method. The commit in this PR merely changes the code to use a try-with-resources block on the `Deflater` instance.
>>
>> No new tests have been introduced given the nature of the change. I've added a `noreg-hard` to the issue. Existing tier1, tier2 and tier3 tests continue to pass with this change without any related failures.
>
> Jaikiran Pai has updated the pull request incrementally with one additional commit since the last revision:
>
> Lance's suggestion
src/jdk.jlink/share/classes/jdk/tools/jlink/internal/plugins/ZipPlugin.java line 99:
> 97: stream.close();
> 98: } catch (IOException ex) {
> 99: return bytesIn; // return the original uncompressed input
This really won't happen. The close for byte array in/out streams are no-op and can be omitted. I believe putting an assertion here is better too.
And by practice, calling methods on an in/out stream after close doesn't make much sense too. Here we are calling `toByteArray` after close, which looks weird.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/23588#discussion_r1952903341
More information about the core-libs-dev
mailing list