On Thu, 26 Feb 2026 13:39:51 GMT, Eirik Bjørsnøs <[email protected]> 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` > > Eirik Bjørsnøs has updated the pull request incrementally with one additional > commit since the last revision: > > Reduce memory retained by a closed ZipFile I don't think this patch brings much value; the old model was working fine, and people already identified a few regressions with this supposedly optimization. It's not worth reviewers' time to go through "hey there's a specific undocumented caller constraint to this method that I hope will continue to work by chance" for a supposedly stylistic improvement. ------------- PR Comment: https://git.openjdk.org/jdk/pull/29937#issuecomment-3967639980
