[jdk8u-dev] RFR: 8339810: Clean up the code in sun.tools.jar.Main to properly close resources and use ZipFile during extract
Andrew John Hughes
andrew at openjdk.org
Thu Mar 27 22:12:15 UTC 2025
On Sat, 22 Mar 2025 05:21:24 GMT, Martin Balao <mbalao at openjdk.org> wrote:
> Hi,
>
> I'd like to have a review for the 8u backport of [8339810](https://bugs.openjdk.org/browse/JDK-8339810).
>
> Changes to the 11u patch:
>
> * Paths.
>
> * jdk/src/share/classes/sun/tools/jar/Main.java
> * Copyright date. Manually fixed.
> * 8u does not have 8199871, 8058520, 8114827, 8156497, 8142968, 8172432, 8217375, 8169069 so the context is different and changes do not apply cleanly.
> * I decided to include the part of 8217375 affecting Main.java because it's related to resources leakage, relevant to 8339810. Given that this part is an insignificant part of 8217375, I propose not add a reference and give the (misleading) impression that it's a full backport of it.
>
> Testing:
>
> * No regressions found in jdk/sun/security/tools/jarsigner.
>
> Thanks,
> Martin.-
All the later changes do make this quite tricky to review (and to backport, I imagine). After comparing the 11u & 8u patches (both as a diff and line by line) and the resulting patched files, I think this looks good.
The inclusion of the hunk from 8217375 seems right. Given that hunk is changed by this patch, it would be odd to not include that and instead alter the incoming hunk to be knowingly inferior to what is in 11u. To that end, I think it may be worth following up this change with an 8u specific change to bring over the similar changes to `run`, `create` and `copy` which are hiding in [8158295](def15478ebc213eeff8eb4da9178f8bac4c72604) and [8172432](https://github.com/openjdk/jdk11u/commit/cb9f76175c61478fcd24e340a943a8c0c6103202)
-------------
Marked as reviewed by andrew (Reviewer).
PR Review: https://git.openjdk.org/jdk8u-dev/pull/639#pullrequestreview-2723477077
More information about the jdk8u-dev
mailing list