RFR: 8376403: Avoid loading ArrayDeque in java.util.zip.ZipFile [v4]

Eirik Bjørsnøs eirbjo at openjdk.org
Sat Feb 21 17:01:10 UTC 2026


On Sat, 21 Feb 2026 16:45:44 GMT, Eirik Bjørsnøs <eirbjo at openjdk.org> wrote:

> Setting a synchronized field to null is a bit of a code smell, so ideally I think that `inflaterCache` should be made final, the list should be cleared instead of nulled and we should introduce a separate `boolean` field to track the closed state of the cache. But that's perhaps for another PR.

Another benefit of making the closed state explicit is that it would remove the awkward `null` and double `==` checking.

Diff on top of this PR:


Index: src/java.base/share/classes/java/util/zip/ZipFile.java
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/src/java.base/share/classes/java/util/zip/ZipFile.java b/src/java.base/share/classes/java/util/zip/ZipFile.java
--- a/src/java.base/share/classes/java/util/zip/ZipFile.java	(revision 02de2507929a1431a1070e0d6660c1585152cf9f)
+++ b/src/java.base/share/classes/java/util/zip/ZipFile.java	(date 1771693058851)
@@ -691,7 +691,9 @@
         final Set<InputStream> istreams;
 
         // List of cached Inflater objects for decompression
-        List<Inflater> inflaterCache;
+        final List<Inflater> inflaterCache;
+        // Closed state, access guarded by inflaterCache
+        boolean inflaterCacheClosed;
 
         final Cleanable cleanable;
 
@@ -727,36 +729,28 @@
          * Releases the specified inflater to the list of available inflaters.
          */
         void releaseInflater(Inflater inf) {
-            List<Inflater> inflaters = this.inflaterCache;
-            if (inflaters != null) {
-                synchronized (inflaters) {
-                    // double checked!
-                    if (inflaters == this.inflaterCache) {
-                        inf.reset();
-                        inflaters.add(inf);
-                        return;
-                    }
-                }
-            }
-            // inflaters cache already closed - just end it.
-            inf.end();
+            synchronized (inflaterCache) {
+                if (inflaterCacheClosed) {
+                    // inflaters cache already closed - just end it.
+                    inf.end();
+                } else {
+                    inf.reset();
+                    inflaterCache.add(inf);
+                }
+            }
         }
 
         public void run() {
             IOException ioe = null;
 
             // Release cached inflaters and close the cache first
-            List<Inflater> inflaters = this.inflaterCache;
-            if (inflaters != null) {
-                synchronized (inflaters) {
-                    // no need to double-check as only one thread gets a
-                    // chance to execute run() (Cleaner guarantee)...
-                    for (Inflater inf : inflaters) {
-                        inf.end();
-                    }
-                    // close inflaters cache
-                    this.inflaterCache = null;
-                }
+            synchronized (inflaterCache) {
+                for (Inflater inf : inflaterCache) {
+                    inf.end();
+                }
+                inflaterCache.clear();
+                // close inflaters cache
+                this.inflaterCacheClosed = true;
             }
 
             // Close streams, release their inflaters

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

PR Review Comment: https://git.openjdk.org/jdk/pull/29430#discussion_r2836359284


More information about the core-libs-dev mailing list