8185582: Update Zip implementation to use Cleaner, not finalizers
authorsherman
Mon, 11 Dec 2017 11:45:02 -0800
changeset 48238 9f225d4387e2
parent 48237 ee130cca69e6
child 48239 8067e9cba973
8185582: Update Zip implementation to use Cleaner, not finalizers Reviewed-by: plevart, rriggs, mchung
src/java.base/share/classes/java/util/zip/Deflater.java
src/java.base/share/classes/java/util/zip/Inflater.java
src/java.base/share/classes/java/util/zip/ZStreamRef.java
src/java.base/share/classes/java/util/zip/ZipFile.java
test/jdk/java/util/zip/ZipFile/FinalizeZipFile.java
test/jdk/java/util/zip/ZipFile/TestCleaner.java
--- a/src/java.base/share/classes/java/util/zip/Deflater.java	Mon Dec 11 18:33:53 2017 +0100
+++ b/src/java.base/share/classes/java/util/zip/Deflater.java	Mon Dec 11 11:45:02 2017 -0800
@@ -67,12 +67,26 @@
  * }
  * </pre></blockquote>
  *
+ * @apiNote
+ * To release resources used by this {@code Deflater}, the {@link #end()} method
+ * should be called explicitly. Subclasses are responsible for the cleanup of resources
+ * acquired by the subclass. Subclasses that override {@link #finalize()} in order
+ * to perform cleanup should be modified to use alternative cleanup mechanisms such
+ * as {@link java.lang.ref.Cleaner} and remove the overriding {@code finalize} method.
+ *
+ * @implSpec
+ * If this {@code Deflater} has been subclassed and the {@code end} method has been
+ * overridden, the {@code end} method will be called by the finalization when the
+ * deflater is unreachable. But the subclasses should not depend on this specific
+ * implementation; the finalization is not reliable and the {@code finalize} method
+ * is deprecated to be removed.
+ *
  * @see         Inflater
  * @author      David Connelly
  * @since 1.1
  */
-public
-class Deflater {
+
+public class Deflater {
 
     private final ZStreamRef zsRef;
     private byte[] buf = new byte[0];
@@ -169,7 +183,9 @@
     public Deflater(int level, boolean nowrap) {
         this.level = level;
         this.strategy = DEFAULT_STRATEGY;
-        this.zsRef = new ZStreamRef(init(level, DEFAULT_STRATEGY, nowrap));
+        this.zsRef = ZStreamRef.get(this,
+                () -> init(level, DEFAULT_STRATEGY, nowrap),
+                Deflater::end);
     }
 
     /**
@@ -534,38 +550,32 @@
 
     /**
      * Closes the compressor and discards any unprocessed input.
+     *
      * This method should be called when the compressor is no longer
-     * being used, but will also be called automatically by the
-     * finalize() method. Once this method is called, the behavior
-     * of the Deflater object is undefined.
+     * being used. Once this method is called, the behavior of the
+     * Deflater object is undefined.
      */
     public void end() {
         synchronized (zsRef) {
-            long addr = zsRef.address();
-            zsRef.clear();
-            if (addr != 0) {
-                end(addr);
-                buf = null;
-            }
+            zsRef.clean();
+            buf = null;
         }
     }
 
     /**
      * Closes the compressor when garbage is collected.
      *
-     * @deprecated The {@code finalize} method has been deprecated.
-     *     Subclasses that override {@code finalize} in order to perform cleanup
-     *     should be modified to use alternative cleanup mechanisms and
-     *     to remove the overriding {@code finalize} method.
-     *     When overriding the {@code finalize} method, its implementation must explicitly
-     *     ensure that {@code super.finalize()} is invoked as described in {@link Object#finalize}.
-     *     See the specification for {@link Object#finalize()} for further
-     *     information about migration options.
+     * @deprecated The {@code finalize} method has been deprecated and will be
+     *     removed. It is implemented as a no-op. Subclasses that override
+     *     {@code finalize} in order to perform cleanup should be modified to use
+     *     alternative cleanup mechanisms and to remove the overriding {@code finalize}
+     *     method. The recommended cleanup for compressor is to explicitly call
+     *     {@code end} method when it is no longer in use. If the {@code end} is
+     *     not invoked explicitly the resource of the compressor will be released
+     *     when the instance becomes unreachable.
      */
-    @Deprecated(since="9")
-    protected void finalize() {
-        end();
-    }
+    @Deprecated(since="9", forRemoval=true)
+    protected void finalize() {}
 
     private void ensureOpen() {
         assert Thread.holdsLock(zsRef);
--- a/src/java.base/share/classes/java/util/zip/Inflater.java	Mon Dec 11 18:33:53 2017 +0100
+++ b/src/java.base/share/classes/java/util/zip/Inflater.java	Mon Dec 11 11:45:02 2017 -0800
@@ -66,13 +66,27 @@
  * }
  * </pre></blockquote>
  *
+ * @apiNote
+ * To release resources used by this {@code Inflater}, the {@link #end()} method
+ * should be called explicitly. Subclasses are responsible for the cleanup of resources
+ * acquired by the subclass. Subclasses that override {@link #finalize()} in order
+ * to perform cleanup should be modified to use alternative cleanup mechanisms such
+ * as {@link java.lang.ref.Cleaner} and remove the overriding {@code finalize} method.
+ *
+ * @implSpec
+ * If this {@code Inflater} has been subclassed and the {@code end} method has been
+ * overridden, the {@code end} method will be called by the finalization when the
+ * inflater is unreachable. But the subclasses should not depend on this specific
+ * implementation; the finalization is not reliable and the {@code finalize} method
+ * is deprecated to be removed.
+ *
  * @see         Deflater
  * @author      David Connelly
  * @since 1.1
  *
  */
-public
-class Inflater {
+
+public class Inflater {
 
     private final ZStreamRef zsRef;
     private byte[] buf = defaultBuf;
@@ -101,7 +115,7 @@
      * @param nowrap if true then support GZIP compatible compression
      */
     public Inflater(boolean nowrap) {
-        zsRef = new ZStreamRef(init(nowrap));
+        this.zsRef = ZStreamRef.get(this, () -> init(nowrap), Inflater::end);
     }
 
     /**
@@ -361,38 +375,37 @@
 
     /**
      * Closes the decompressor and discards any unprocessed input.
+     *
      * This method should be called when the decompressor is no longer
-     * being used, but will also be called automatically by the finalize()
-     * method. Once this method is called, the behavior of the Inflater
-     * object is undefined.
+     * being used. Once this method is called, the behavior of the
+     * Inflater object is undefined.
      */
     public void end() {
         synchronized (zsRef) {
-            long addr = zsRef.address();
-            zsRef.clear();
-            if (addr != 0) {
-                end(addr);
-                buf = null;
-            }
+            zsRef.clean();
+            buf = null;
         }
     }
 
     /**
      * Closes the decompressor when garbage is collected.
      *
-     * @deprecated The {@code finalize} method has been deprecated.
-     *     Subclasses that override {@code finalize} in order to perform cleanup
-     *     should be modified to use alternative cleanup mechanisms and
-     *     to remove the overriding {@code finalize} method.
-     *     When overriding the {@code finalize} method, its implementation must explicitly
-     *     ensure that {@code super.finalize()} is invoked as described in {@link Object#finalize}.
-     *     See the specification for {@link Object#finalize()} for further
-     *     information about migration options.
+     * @implSpec
+     * If this {@code Inflater} has been subclassed and the {@code end} method
+     * has been overridden, the {@code end} method will be called when the
+     * inflater is unreachable.
+     *
+     * @deprecated The {@code finalize} method has been deprecated and will be
+     *     removed. It is implemented as a no-op. Subclasses that override
+     *     {@code finalize} in order to perform cleanup should be modified to use
+     *     alternative cleanup mechanisms and remove the overriding {@code finalize}
+     *     method. The recommended cleanup for compressor is to explicitly call
+     *     {@code end} method when it is no longer in use. If the {@code end} is
+     *     not invoked explicitly the resource of the compressor will be released
+     *     when the instance becomes unreachable,
      */
-    @Deprecated(since="9")
-    protected void finalize() {
-        end();
-    }
+    @Deprecated(since="9", forRemoval=true)
+    protected void finalize() {}
 
     private void ensureOpen () {
         assert Thread.holdsLock(zsRef);
--- a/src/java.base/share/classes/java/util/zip/ZStreamRef.java	Mon Dec 11 18:33:53 2017 +0100
+++ b/src/java.base/share/classes/java/util/zip/ZStreamRef.java	Mon Dec 11 11:45:02 2017 -0800
@@ -25,22 +25,89 @@
 
 package java.util.zip;
 
-/**
- * A reference to the native zlib's z_stream structure.
- */
+import java.util.function.LongConsumer;
+import java.util.function.LongSupplier;
+import java.lang.ref.Cleaner.Cleanable;
+import jdk.internal.ref.CleanerFactory;
 
-class ZStreamRef {
+/**
+ * A reference to the native zlib's z_stream structure. It also
+ * serves as the "cleaner" to clean up the native resource when
+ * the deflater or infalter is ended, closed or cleaned.
+ */
+class ZStreamRef implements Runnable {
 
-    private volatile long address;
-    ZStreamRef (long address) {
-        this.address = address;
+    private LongConsumer end;
+    private long address;
+    private final Cleanable cleanable;
+
+    private ZStreamRef (Object owner, LongSupplier addr, LongConsumer end) {
+        this.cleanable = CleanerFactory.cleaner().register(owner, this);
+        this.end = end;
+        this.address = addr.getAsLong();
     }
 
     long address() {
         return address;
     }
 
-    void clear() {
+    void clean() {
+        cleanable.clean();
+    }
+
+    public synchronized void run() {
+        long addr = address;
         address = 0;
+        if (addr != 0) {
+            end.accept(addr);
+        }
+    }
+
+    private ZStreamRef (LongSupplier addr, LongConsumer end) {
+        this.cleanable = null;
+        this.end = end;
+        this.address = addr.getAsLong();
+    }
+
+    /*
+     * If {@code Inflater/Deflater} has been subclassed and the {@code end} method
+     * is overridden, uses {@code finalizer} mechanism for resource cleanup. So
+     * {@code end} method can be called when the {@code Inflater/Deflater} is
+     * unreachable. This mechanism will be removed when the {@code finalize} method
+     * is removed from {@code Inflater/Deflater}.
+     */
+    static ZStreamRef get(Object owner, LongSupplier addr, LongConsumer end) {
+        Class<?> clz = owner.getClass();
+        while (clz != Deflater.class && clz != Inflater.class) {
+            try {
+                clz.getDeclaredMethod("end");
+                return new FinalizableZStreamRef(owner, addr, end);
+            } catch (NoSuchMethodException nsme) {}
+            clz = clz.getSuperclass();
+        }
+        return new ZStreamRef(owner, addr, end);
+    }
+
+    private static class FinalizableZStreamRef extends ZStreamRef {
+        final Object owner;
+
+        FinalizableZStreamRef (Object owner, LongSupplier addr, LongConsumer end) {
+            super(addr, end);
+            this.owner = owner;
+        }
+
+        @Override
+        void clean() {
+            run();
+        }
+
+        @Override
+        @SuppressWarnings("deprecation")
+        protected void finalize() {
+            if (owner instanceof Inflater)
+                ((Inflater)owner).end();
+            else
+                ((Deflater)owner).end();
+        }
     }
 }
--- a/src/java.base/share/classes/java/util/zip/ZipFile.java	Mon Dec 11 18:33:53 2017 +0100
+++ b/src/java.base/share/classes/java/util/zip/ZipFile.java	Mon Dec 11 11:45:02 2017 -0800
@@ -31,22 +31,24 @@
 import java.io.EOFException;
 import java.io.File;
 import java.io.RandomAccessFile;
+import java.io.UncheckedIOException;
+import java.lang.ref.Cleaner.Cleanable;
 import java.nio.charset.Charset;
 import java.nio.charset.StandardCharsets;
 import java.nio.file.attribute.BasicFileAttributes;
-import java.nio.file.Path;
 import java.nio.file.Files;
 
 import java.util.ArrayDeque;
 import java.util.ArrayList;
 import java.util.Arrays;
+import java.util.Collections;
 import java.util.Deque;
 import java.util.Enumeration;
 import java.util.HashMap;
 import java.util.Iterator;
-import java.util.Map;
 import java.util.Objects;
 import java.util.NoSuchElementException;
+import java.util.Set;
 import java.util.Spliterator;
 import java.util.Spliterators;
 import java.util.WeakHashMap;
@@ -61,8 +63,8 @@
 import jdk.internal.misc.SharedSecrets;
 import jdk.internal.misc.VM;
 import jdk.internal.perf.PerfCounter;
+import jdk.internal.ref.CleanerFactory;
 
-import static java.util.zip.ZipConstants.*;
 import static java.util.zip.ZipConstants64.*;
 import static java.util.zip.ZipUtils.*;
 
@@ -73,6 +75,21 @@
  * or method in this class will cause a {@link NullPointerException} to be
  * thrown.
  *
+ * @apiNote
+ * To release resources used by this {@code ZipFile}, the {@link #close()} method
+ * should be called explicitly or by try-with-resources. Subclasses are responsible
+ * for the cleanup of resources acquired by the subclass. Subclasses that override
+ * {@link #finalize()} in order to perform cleanup should be modified to use alternative
+ * cleanup mechanisms such as {@link java.lang.ref.Cleaner} and remove the overriding
+ * {@code finalize} method.
+ *
+ * @implSpec
+ * If this {@code ZipFile} has been subclassed and the {@code close} method has
+ * been overridden, the {@code close} method will be called by the finalization
+ * when {@code ZipFile} is unreachable. But the subclasses should not depend on
+ * this specific implementation; the finalization is not reliable and the
+ * {@code finalize} method is deprecated to be removed.
+ *
  * @author      David Connelly
  * @since 1.1
  */
@@ -81,9 +98,15 @@
 
     private final String name;     // zip file name
     private volatile boolean closeRequested;
-    private Source zsrc;
     private ZipCoder zc;
 
+    // The "resource" used by this zip file that needs to be
+    // cleaned after use.
+    // a) the input streams that need to be closed
+    // b) the list of cached Inflater objects
+    // c) the "native" source of this zip file.
+    private final CleanableResource res;
+
     private static final int STORED = ZipEntry.STORED;
     private static final int DEFLATED = ZipEntry.DEFLATED;
 
@@ -214,10 +237,13 @@
             }
         }
         Objects.requireNonNull(charset, "charset");
+
         this.zc = ZipCoder.get(charset);
         this.name = name;
         long t0 = System.nanoTime();
-        this.zsrc = Source.get(file, (mode & OPEN_DELETE) != 0);
+
+        this.res = CleanableResource.get(this, file, mode);
+
         PerfCounter.getZipFileOpenTime().addElapsedTimeFrom(t0);
         PerfCounter.getZipFileCount().increment();
     }
@@ -284,10 +310,10 @@
     public String getComment() {
         synchronized (this) {
             ensureOpen();
-            if (zsrc.comment == null) {
+            if (res.zsrc.comment == null) {
                 return null;
             }
-            return zc.toString(zsrc.comment);
+            return zc.toString(res.zsrc.comment);
         }
     }
 
@@ -318,7 +344,7 @@
         synchronized (this) {
             ensureOpen();
             byte[] bname = zc.getBytes(name);
-            int pos = zsrc.getEntryPos(bname, true);
+            int pos = res.zsrc.getEntryPos(bname, true);
             if (pos != -1) {
                 return getZipEntry(name, bname, pos, func);
             }
@@ -326,10 +352,6 @@
         return null;
     }
 
-    // The outstanding inputstreams that need to be closed,
-    // mapped to the inflater objects they use.
-    private final Map<InputStream, Inflater> streams = new WeakHashMap<>();
-
     /**
      * Returns an input stream for reading the contents of the specified
      * zip file entry.
@@ -348,6 +370,8 @@
         Objects.requireNonNull(entry, "entry");
         int pos = -1;
         ZipFileInputStream in = null;
+        Source zsrc = res.zsrc;
+        Set<InputStream> istreams = res.istreams;
         synchronized (this) {
             ensureOpen();
             if (Objects.equals(lastEntryName, entry.name)) {
@@ -363,8 +387,8 @@
             in = new ZipFileInputStream(zsrc.cen, pos);
             switch (CENHOW(zsrc.cen, pos)) {
             case STORED:
-                synchronized (streams) {
-                    streams.put(in, null);
+                synchronized (istreams) {
+                    istreams.add(in);
                 }
                 return in;
             case DEFLATED:
@@ -377,10 +401,9 @@
                 if (size <= 0) {
                     size = 4096;
                 }
-                Inflater inf = getInflater();
-                InputStream is = new ZipFileInflaterInputStream(in, inf, (int)size);
-                synchronized (streams) {
-                    streams.put(is, inf);
+                InputStream is = new ZipFileInflaterInputStream(in, res, (int)size);
+                synchronized (istreams) {
+                    istreams.add(is);
                 }
                 return is;
             default:
@@ -392,25 +415,30 @@
     private class ZipFileInflaterInputStream extends InflaterInputStream {
         private volatile boolean closeRequested;
         private boolean eof = false;
+        private final Cleanable cleanable;
 
-        ZipFileInflaterInputStream(ZipFileInputStream zfin, Inflater inf,
-                int size) {
+        ZipFileInflaterInputStream(ZipFileInputStream zfin,
+                                   CleanableResource res, int size) {
+            this(zfin, res, res.getInflater(), size);
+        }
+
+        private ZipFileInflaterInputStream(ZipFileInputStream zfin,
+                                           CleanableResource res,
+                                           Inflater inf, int size) {
             super(zfin, inf, size);
-        }
+            this.cleanable = CleanerFactory.cleaner().register(this,
+                    () -> res.releaseInflater(inf));
+       }
 
         public void close() throws IOException {
             if (closeRequested)
                 return;
             closeRequested = true;
-
             super.close();
-            Inflater inf;
-            synchronized (streams) {
-                inf = streams.remove(this);
+            synchronized (res.istreams) {
+                res.istreams.remove(this);
             }
-            if (inf != null) {
-                releaseInflater(inf);
-            }
+            cleanable.clean();
         }
 
         // Override fill() method to provide an extra "dummy" byte
@@ -436,44 +464,8 @@
             return (avail > (long) Integer.MAX_VALUE ?
                     Integer.MAX_VALUE : (int) avail);
         }
-
-        @SuppressWarnings("deprecation")
-        protected void finalize() throws Throwable {
-            close();
-        }
     }
 
-    /*
-     * Gets an inflater from the list of available inflaters or allocates
-     * a new one.
-     */
-    private Inflater getInflater() {
-        Inflater inf;
-        synchronized (inflaterCache) {
-            while ((inf = inflaterCache.poll()) != null) {
-                if (!inf.ended()) {
-                    return inf;
-                }
-            }
-        }
-        return new Inflater(true);
-    }
-
-    /*
-     * Releases the specified inflater to the list of available inflaters.
-     */
-    private void releaseInflater(Inflater inf) {
-        if (!inf.ended()) {
-            inf.reset();
-            synchronized (inflaterCache) {
-                inflaterCache.add(inf);
-            }
-        }
-    }
-
-    // List of available Inflater objects for decompression
-    private final Deque<Inflater> inflaterCache = new ArrayDeque<>();
-
     /**
      * Returns the path name of the ZIP file.
      * @return the path name of the ZIP file
@@ -518,7 +510,7 @@
                     throw new NoSuchElementException();
                 }
                 // each "entry" has 3 ints in table entries
-                return (T)getZipEntry(null, null, zsrc.getEntryPos(i++ * 3), gen);
+                return (T)getZipEntry(null, null, res.zsrc.getEntryPos(i++ * 3), gen);
             }
         }
 
@@ -536,14 +528,14 @@
     public Enumeration<? extends ZipEntry> entries() {
         synchronized (this) {
             ensureOpen();
-            return new ZipEntryIterator<ZipEntry>(zsrc.total, ZipEntry::new);
+            return new ZipEntryIterator<ZipEntry>(res.zsrc.total, ZipEntry::new);
         }
     }
 
     private Enumeration<JarEntry> entries(Function<String, JarEntry> func) {
         synchronized (this) {
             ensureOpen();
-            return new ZipEntryIterator<JarEntry>(zsrc.total, func);
+            return new ZipEntryIterator<JarEntry>(res.zsrc.total, func);
         }
     }
 
@@ -568,7 +560,7 @@
             if (index >= 0 && index < fence) {
                 synchronized (ZipFile.this) {
                     ensureOpen();
-                    action.accept(gen.apply(zsrc.getEntryPos(index++ * 3)));
+                    action.accept(gen.apply(res.zsrc.getEntryPos(index++ * 3)));
                 }
                 return true;
             }
@@ -589,13 +581,13 @@
     public Stream<? extends ZipEntry> stream() {
         synchronized (this) {
             ensureOpen();
-            return StreamSupport.stream(new EntrySpliterator<>(0, zsrc.total,
+            return StreamSupport.stream(new EntrySpliterator<>(0, res.zsrc.total,
                 pos -> getZipEntry(null, null, pos, ZipEntry::new)), false);
        }
     }
 
     private String getEntryName(int pos) {
-        byte[] cen = zsrc.cen;
+        byte[] cen = res.zsrc.cen;
         int nlen = CENNAM(cen, pos);
         int clen = CENCOM(cen, pos);
         int flag = CENFLG(cen, pos);
@@ -620,7 +612,7 @@
         synchronized (this) {
             ensureOpen();
             return StreamSupport.stream(
-                new EntrySpliterator<>(0, zsrc.total, this::getEntryName), false);
+                new EntrySpliterator<>(0, res.zsrc.total, this::getEntryName), false);
         }
     }
 
@@ -638,7 +630,7 @@
     private Stream<JarEntry> stream(Function<String, JarEntry> func) {
         synchronized (this) {
             ensureOpen();
-            return StreamSupport.stream(new EntrySpliterator<>(0, zsrc.total,
+            return StreamSupport.stream(new EntrySpliterator<>(0, res.zsrc.total,
                 pos -> (JarEntry)getZipEntry(null, null, pos, func)), false);
         }
     }
@@ -649,7 +641,7 @@
     /* Checks ensureOpen() before invoke this method */
     private ZipEntry getZipEntry(String name, byte[] bname, int pos,
                                  Function<String, ? extends ZipEntry> func) {
-        byte[] cen = zsrc.cen;
+        byte[] cen = res.zsrc.cen;
         int nlen = CENNAM(cen, pos);
         int elen = CENEXT(cen, pos);
         int clen = CENCOM(cen, pos);
@@ -698,12 +690,170 @@
     public int size() {
         synchronized (this) {
             ensureOpen();
-            return zsrc.total;
+            return res.zsrc.total;
+        }
+    }
+
+    private static class CleanableResource implements Runnable {
+        // The outstanding inputstreams that need to be closed
+        final Set<InputStream> istreams;
+
+        // List of cached Inflater objects for decompression
+        Deque<Inflater> inflaterCache;
+
+        final Cleanable cleanable;
+
+        Source zsrc;
+
+        CleanableResource(ZipFile zf, File file, int mode) throws IOException {
+            this.cleanable = CleanerFactory.cleaner().register(zf, this);
+            this.istreams = Collections.newSetFromMap(new WeakHashMap<>());
+            this.inflaterCache = new ArrayDeque<>();
+            this.zsrc = Source.get(file, (mode & OPEN_DELETE) != 0);
+        }
+
+        void clean() {
+            cleanable.clean();
+        }
+
+        /*
+         * Gets an inflater from the list of available inflaters or allocates
+         * a new one.
+         */
+        Inflater getInflater() {
+            Inflater inf;
+            synchronized (inflaterCache) {
+                if ((inf = inflaterCache.poll()) != null) {
+                    return inf;
+                }
+            }
+            return new Inflater(true);
+        }
+
+        /*
+         * Releases the specified inflater to the list of available inflaters.
+         */
+        void releaseInflater(Inflater inf) {
+            Deque<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();
+        }
+
+        public void run() {
+            IOException ioe = null;
+
+            // Release cached inflaters and close the cache first
+            Deque<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)...
+                    Inflater inf;
+                    while ((inf = inflaters.poll()) != null) {
+                        inf.end();
+                    }
+                    // close inflaters cache
+                    this.inflaterCache = null;
+                }
+            }
+
+            // Close streams, release their inflaters
+            if (istreams != null) {
+                synchronized (istreams) {
+                    if (!istreams.isEmpty()) {
+                        InputStream[] copy = istreams.toArray(new InputStream[0]);
+                        istreams.clear();
+                        for (InputStream is : copy) {
+                            try {
+                                is.close();
+                            }  catch (IOException e) {
+                                if (ioe == null) ioe = e;
+                                else ioe.addSuppressed(e);
+                            }
+                        }
+                    }
+                }
+            }
+
+            // Release zip src
+            if (zsrc != null) {
+                synchronized (zsrc) {
+                    try {
+                        Source.release(zsrc);
+                        zsrc = null;
+                     }  catch (IOException e) {
+                         if (ioe == null) ioe = e;
+                         else ioe.addSuppressed(e);
+                    }
+                }
+            }
+            if (ioe != null) {
+                throw new UncheckedIOException(ioe);
+            }
+        }
+
+        CleanableResource(File file, int mode)
+            throws IOException {
+            this.cleanable = null;
+            this.istreams = Collections.newSetFromMap(new WeakHashMap<>());
+            this.inflaterCache = new ArrayDeque<>();
+            this.zsrc = Source.get(file, (mode & OPEN_DELETE) != 0);
+        }
+
+        /*
+         * If {@code ZipFile} has been subclassed and the {@code close} method is
+         * overridden, uses the {@code finalizer} mechanism for resource cleanup.
+         * So {@code close} method can be called when the the {@code ZipFile} is
+         * unreachable. This mechanism will be removed when {@code finalize} method
+         * is removed from {@code ZipFile}.
+         */
+        static CleanableResource get(ZipFile zf, File file, int mode)
+            throws IOException {
+            Class<?> clz = zf.getClass();
+            while (clz != ZipFile.class) {
+                try {
+                    clz.getDeclaredMethod("close");
+                    return new FinalizableResource(zf, file, mode);
+                } catch (NoSuchMethodException nsme) {}
+                clz = clz.getSuperclass();
+            }
+            return new CleanableResource(zf, file, mode);
+        }
+
+        static class FinalizableResource extends CleanableResource {
+            ZipFile zf;
+            FinalizableResource(ZipFile zf, File file, int mode)
+                throws IOException {
+                super(file, mode);
+                this.zf = zf;
+            }
+
+            @Override
+            void clean() {
+                run();
+            }
+
+            @Override
+            @SuppressWarnings("deprecation")
+            protected void finalize() throws IOException {
+                zf.close();
+            }
         }
     }
 
     /**
      * Closes the ZIP file.
+     *
      * <p> Closing this ZIP file will close all of the input streams
      * previously returned by invocations of the {@link #getInputStream
      * getInputStream} method.
@@ -717,31 +867,12 @@
         closeRequested = true;
 
         synchronized (this) {
-            // Close streams, release their inflaters
-            synchronized (streams) {
-                if (!streams.isEmpty()) {
-                    Map<InputStream, Inflater> copy = new HashMap<>(streams);
-                    streams.clear();
-                    for (Map.Entry<InputStream, Inflater> e : copy.entrySet()) {
-                        e.getKey().close();
-                        Inflater inf = e.getValue();
-                        if (inf != null) {
-                            inf.end();
-                        }
-                    }
-                }
-            }
-            // Release cached inflaters
-            synchronized (inflaterCache) {
-                Inflater inf;
-                while ((inf = inflaterCache.poll()) != null) {
-                    inf.end();
-                }
-            }
-            // Release zip src
-            if (zsrc != null) {
-                Source.close(zsrc);
-                zsrc = null;
+            // Close streams, release their inflaters, release cached inflaters
+            // and release zip source
+            try {
+                res.clean();
+            } catch (UncheckedIOException ioe) {
+                throw ioe.getCause();
             }
         }
     }
@@ -750,34 +881,26 @@
      * Ensures that the system resources held by this ZipFile object are
      * released when there are no more references to it.
      *
-     * <p>
-     * Since the time when GC would invoke this method is undetermined,
-     * it is strongly recommended that applications invoke the {@code close}
-     * method as soon they have finished accessing this {@code ZipFile}.
-     * This will prevent holding up system resources for an undetermined
-     * length of time.
+     * @deprecated The {@code finalize} method has been deprecated and will be
+     *     removed. It is implemented as a no-op. Subclasses that override
+     *     {@code finalize} in order to perform cleanup should be modified to
+     *     use alternative cleanup mechanisms and to remove the overriding
+     *     {@code finalize} method. The recommended cleanup for ZipFile object
+     *     is to explicitly invoke {@code close} method when it is no longer in
+     *     use, or use try-with-resources. If the {@code close} is not invoked
+     *     explicitly the resources held by this object will be released when
+     *     the instance becomes unreachable.
      *
-     * @deprecated The {@code finalize} method has been deprecated.
-     *     Subclasses that override {@code finalize} in order to perform cleanup
-     *     should be modified to use alternative cleanup mechanisms and
-     *     to remove the overriding {@code finalize} method.
-     *     When overriding the {@code finalize} method, its implementation must explicitly
-     *     ensure that {@code super.finalize()} is invoked as described in {@link Object#finalize}.
-     *     See the specification for {@link Object#finalize()} for further
-     *     information about migration options.
      * @throws IOException if an I/O error has occurred
-     * @see    java.util.zip.ZipFile#close()
      */
-    @Deprecated(since="9")
-    protected void finalize() throws IOException {
-        close();
-    }
+    @Deprecated(since="9", forRemoval=true)
+    protected void finalize() throws IOException {}
 
     private void ensureOpen() {
         if (closeRequested) {
             throw new IllegalStateException("zip file closed");
         }
-        if (zsrc == null) {
+        if (res.zsrc == null) {
             throw new IllegalStateException("The object is not initialized.");
         }
     }
@@ -798,7 +921,7 @@
         protected long rem;     // number of remaining bytes within entry
         protected long size;    // uncompressed size of this entry
 
-        ZipFileInputStream(byte[] cen, int cenpos) throws IOException {
+        ZipFileInputStream(byte[] cen, int cenpos) {
             rem = CENSIZ(cen, cenpos);
             size = CENLEN(cen, cenpos);
             pos = CENOFF(cen, cenpos);
@@ -808,10 +931,10 @@
                 checkZIP64(cen, cenpos);
             }
             // negative for lazy initialization, see getDataOffset();
-            pos = - (pos + ZipFile.this.zsrc.locpos);
+            pos = - (pos + ZipFile.this.res.zsrc.locpos);
         }
 
-        private void checkZIP64(byte[] cen, int cenpos) throws IOException {
+         private void checkZIP64(byte[] cen, int cenpos) {
             int off = cenpos + CENHDR + CENNAM(cen, cenpos);
             int end = off + CENEXT(cen, cenpos);
             while (off + 4 < end) {
@@ -857,7 +980,7 @@
             if (pos <= 0) {
                 byte[] loc = new byte[LOCHDR];
                 pos = -pos;
-                int len = ZipFile.this.zsrc.readFullyAt(loc, 0, loc.length, pos);
+                int len = ZipFile.this.res.zsrc.readFullyAt(loc, 0, loc.length, pos);
                 if (len != LOCHDR) {
                     throw new ZipException("ZipFile error reading zip file");
                 }
@@ -882,7 +1005,7 @@
                 if (len <= 0) {
                     return 0;
                 }
-                len = ZipFile.this.zsrc.readAt(b, off, len, pos);
+                len = ZipFile.this.res.zsrc.readAt(b, off, len, pos);
                 if (len > 0) {
                     pos += len;
                     rem -= len;
@@ -932,15 +1055,11 @@
             }
             closeRequested = true;
             rem = 0;
-            synchronized (streams) {
-                streams.remove(this);
+            synchronized (res.istreams) {
+                res.istreams.remove(this);
             }
         }
 
-        @SuppressWarnings("deprecation")
-        protected void finalize() {
-            close();
-        }
     }
 
     /**
@@ -952,6 +1071,7 @@
     private String[] getMetaInfEntryNames() {
         synchronized (this) {
             ensureOpen();
+            Source zsrc = res.zsrc;
             if (zsrc.metanames == null) {
                 return null;
             }
@@ -972,7 +1092,7 @@
             new JavaUtilZipFileAccess() {
                 @Override
                 public boolean startsWithLocHeader(ZipFile zip) {
-                    return zip.zsrc.startsWithLoc;
+                    return zip.res.zsrc.startsWithLoc;
                 }
                 @Override
                 public String[] getMetaInfEntryNames(ZipFile zip) {
@@ -1080,7 +1200,7 @@
         private static final HashMap<Key, Source> files = new HashMap<>();
 
 
-        public static Source get(File file, boolean toDelete) throws IOException {
+        static Source get(File file, boolean toDelete) throws IOException {
             Key key = new Key(file,
                               Files.readAttributes(file.toPath(), BasicFileAttributes.class));
             Source src = null;
@@ -1105,9 +1225,9 @@
             }
         }
 
-        private static void close(Source src) throws IOException {
+        static void release(Source src) throws IOException {
             synchronized (files) {
-                if (--src.refs == 0) {
+                if (src != null && --src.refs == 0) {
                     files.remove(src.key);
                     src.close();
                 }
--- a/test/jdk/java/util/zip/ZipFile/FinalizeZipFile.java	Mon Dec 11 18:33:53 2017 +0100
+++ b/test/jdk/java/util/zip/ZipFile/FinalizeZipFile.java	Mon Dec 11 11:45:02 2017 -0800
@@ -31,6 +31,7 @@
 import java.util.Random;
 import java.util.zip.*;
 import java.util.concurrent.CountDownLatch;
+import java.util.concurrent.TimeUnit;
 
 public class FinalizeZipFile {
 
@@ -78,10 +79,9 @@
 
     public static void realMain(String[] args) throws Throwable {
         makeGarbage();
-
-        System.gc();
-        finalizersDone.await();
-
+        while (!finalizersDone.await(10, TimeUnit.MILLISECONDS)) {
+            System.gc();
+        }
         // Not all ZipFiles were collected?
         equal(finalizersDone.getCount(), 0L);
     }
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/test/jdk/java/util/zip/ZipFile/TestCleaner.java	Mon Dec 11 11:45:02 2017 -0800
@@ -0,0 +1,246 @@
+/*
+ * Copyright (c) 2017, Oracle and/or its affiliates. All rights reserved.
+ * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
+ *
+ * This code is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ */
+
+/* @test
+ * @bug 8185582
+ * @modules java.base/java.util.zip:open
+ * @summary Check the resources of Inflater, Deflater and ZipFile are always
+ *          cleaned/released when the instance is not unreachable
+ */
+
+import java.io.*;
+import java.lang.reflect.*;
+import java.util.*;
+import java.util.concurrent.CountDownLatch;
+import java.util.concurrent.TimeUnit;
+import java.util.zip.*;
+import static java.nio.charset.StandardCharsets.US_ASCII;
+
+public class TestCleaner {
+
+    public static void main(String[] args) throws Throwable {
+        testDeInflater();
+        testZipFile();
+    }
+
+    private static long addrOf(Object obj) {
+        try {
+            Field addr = obj.getClass().getDeclaredField("address");
+            if (!addr.trySetAccessible()) {
+                return -1;
+            }
+            return addr.getLong(obj);
+        } catch (Exception x) {
+            return -1;
+        }
+    }
+
+    private static class SubclassedInflater extends Inflater {
+        CountDownLatch endCountDown;
+
+        SubclassedInflater(CountDownLatch endCountDown) {
+            this.endCountDown = endCountDown;
+        }
+
+        @Override
+        public void end() {
+            super.end();
+            endCountDown.countDown();
+        }
+    }
+
+    private static class SubclassedDeflater extends Deflater {
+        CountDownLatch endCountDown;
+
+        SubclassedDeflater(CountDownLatch endCountDown) {
+            this.endCountDown = endCountDown;
+        }
+
+        @Override
+        public void end() {
+            super.end();
+            endCountDown.countDown();
+        }
+    }
+
+    // verify the "native resource" of In/Deflater has been cleaned
+    private static void testDeInflater() throws Throwable {
+        Field zsRefDef = Deflater.class.getDeclaredField("zsRef");
+        Field zsRefInf = Inflater.class.getDeclaredField("zsRef");
+        if (!zsRefDef.trySetAccessible() || !zsRefInf.trySetAccessible()) {
+            throw new RuntimeException("'zsRef' is not accesible");
+        }
+        if (addrOf(zsRefDef.get(new Deflater())) == -1 ||
+            addrOf(zsRefInf.get(new Inflater())) == -1) {
+            throw new RuntimeException("'addr' is not accesible");
+        }
+        List<Object> list = new ArrayList<>();
+        byte[] buf1 = new byte[1024];
+        byte[] buf2 = new byte[1024];
+        for (int i = 0; i < 10; i++) {
+            var def = new Deflater();
+            list.add(zsRefDef.get(def));
+            def.setInput("hello".getBytes());
+            def.finish();
+            int n = def.deflate(buf1);
+
+            var inf = new Inflater();
+            list.add(zsRefInf.get(inf));
+            inf.setInput(buf1, 0, n);
+            n = inf.inflate(buf2);
+            if (!"hello".equals(new String(buf2, 0, n))) {
+                throw new RuntimeException("compression/decompression failed");
+            }
+        }
+
+        int n = 10;
+        long cnt = list.size();
+        while (n-- > 0 && cnt != 0) {
+            Thread.sleep(100);
+            System.gc();
+            cnt = list.stream().filter(o -> addrOf(o) != 0).count();
+        }
+        if (cnt != 0)
+            throw new RuntimeException("cleaner failed to clean : " + cnt);
+
+        // test subclassed Deflater/Inflater, for behavioral compatibility.
+        // should be removed if the finalize() method is finally removed.
+        var endCountDown = new CountDownLatch(20);
+        for (int i = 0; i < 10; i++) {
+            var def = new SubclassedDeflater(endCountDown);
+            def.setInput("hello".getBytes());
+            def.finish();
+            n = def.deflate(buf1);
+
+            var inf = new SubclassedInflater(endCountDown);
+            inf.setInput(buf1, 0, n);
+            n = inf.inflate(buf2);
+            if (!"hello".equals(new String(buf2, 0, n))) {
+                throw new RuntimeException("compression/decompression failed");
+            }
+        }
+        while (!endCountDown.await(10, TimeUnit.MILLISECONDS)) {
+            System.gc();
+        }
+        if (endCountDown.getCount() != 0)
+            throw new RuntimeException("finalizer failed to clean : " +
+                endCountDown.getCount());
+    }
+
+    private static class SubclassedZipFile extends ZipFile {
+        CountDownLatch closeCountDown;
+
+        SubclassedZipFile(File f, CountDownLatch closeCountDown)
+            throws IOException {
+            super(f);
+            this.closeCountDown = closeCountDown;
+        }
+
+        @Override
+        public void close() throws IOException {
+            closeCountDown.countDown();
+            super.close();
+        }
+    }
+
+    private static void testZipFile() throws Throwable {
+        File dir = new File(System.getProperty("test.dir", "."));
+        File zip = File.createTempFile("testzf", "zip", dir);
+        Object zsrc = null;
+        try {
+            try (var fos = new FileOutputStream(zip);
+                 var zos = new ZipOutputStream(fos)) {
+                zos.putNextEntry(new ZipEntry("hello"));
+                zos.write("hello".getBytes(US_ASCII));
+                zos.closeEntry();
+            }
+
+            var zf = new ZipFile(zip);
+            var es = zf.entries();
+            while (es.hasMoreElements()) {
+                zf.getInputStream(es.nextElement()).read();
+            }
+
+            Field fieldRes = ZipFile.class.getDeclaredField("res");
+            if (!fieldRes.trySetAccessible()) {
+                throw new RuntimeException("'ZipFile.res' is not accesible");
+            }
+            Object zfRes = fieldRes.get(zf);
+            if (zfRes == null) {
+                throw new RuntimeException("'ZipFile.res' is null");
+            }
+            Field fieldZsrc = zfRes.getClass().getDeclaredField("zsrc");
+            if (!fieldZsrc.trySetAccessible()) {
+                throw new RuntimeException("'ZipFile.zsrc' is not accesible");
+            }
+            zsrc = fieldZsrc.get(zfRes);
+
+        } finally {
+            zip.delete();
+        }
+
+        if (zsrc != null) {
+            Field zfileField = zsrc.getClass().getDeclaredField("zfile");
+            if (!zfileField.trySetAccessible()) {
+                throw new RuntimeException("'ZipFile.Source.zfile' is not accesible");
+            }
+            //System.out.println("zffile: " +  zfileField.get(zsrc));
+            int n = 10;
+            while (n-- > 0 && zfileField.get(zsrc) != null) {
+                System.out.println("waiting gc ... " + n);
+                System.gc();
+                Thread.sleep(100);
+            }
+            if (zfileField.get(zsrc) != null) {
+                throw new RuntimeException("cleaner failed to clean zipfile.");
+            }
+        }
+
+        // test subclassed ZipFile, for behavioral compatibility.
+        // should be removed if the finalize() method is finally removed.
+        var closeCountDown = new CountDownLatch(1);
+        try {
+            try (var fos = new FileOutputStream(zip);
+                 var zos = new ZipOutputStream(fos)) {
+                zos.putNextEntry(new ZipEntry("hello"));
+                zos.write("hello".getBytes(US_ASCII));
+                zos.closeEntry();
+            }
+            var zf = new SubclassedZipFile(zip, closeCountDown);
+            var es = zf.entries();
+            while (es.hasMoreElements()) {
+                zf.getInputStream(es.nextElement()).read();
+            }
+            es = null;
+            zf = null;
+        } finally {
+            zip.delete();
+        }
+        while (!closeCountDown.await(10, TimeUnit.MILLISECONDS)) {
+            System.gc();
+        }
+        if (closeCountDown.getCount() != 0)
+            throw new RuntimeException("finalizer failed to clean : " +
+                closeCountDown.getCount());
+    }
+}