RFR: 8378759: Make ZipFile.CleanableResource.inflaterCache field final
Sanne Grinovero
sgrinovero at openjdk.org
Thu Feb 26 13:30:49 UTC 2026
On Thu, 26 Feb 2026 12:23:44 GMT, Eirik Bjørsnøs <eirbjo at openjdk.org> wrote:
> Please review this cleanup PR where we simplify synchronization on ZipFile's inflater cache.
>
> Currently, the `ZipFile.CleanableResource.inflaterCache` field is non-final, is used in synchronization and is set to `null` to indicate a closed inflater cache. This complicates the state considerations for synchronization, requiring double-checking that the cache does not close under us. Generally, correctness of synchronizing on a non-final field which can also be null is hard to reason about.
>
> This PR marks the `inflaterCache` field as `final` and introduces a boolean flag field to model the closed state explicitly. This allows synchronization to be simplified, double-checking to be removed and the closed state to be more obvious. If we want the future ZipFile to be less mutable, this is one step in that direction.
>
> Cleanup refactoring, `noreg-cleanup`
src/java.base/share/classes/java/util/zip/ZipFile.java line 753:
> 751: }
> 752: // close inflaters cache
> 753: inflaterCache.clear();
I agree that the pattern you propose looks easier to maintain, but I'm concerned about this possibly holding on to memory unnecessarily.
_Personally_ I'd prefer the JVM to handle a minor little complication for the sake of lower memory consumption, but if you all think this should be done, could you at least invoke `trimToSize()` on the list after clearing it?
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/29937#discussion_r2859038105
More information about the core-libs-dev
mailing list