--- old/src/java.base/share/classes/java/util/zip/ZipFile.java 2017-11-01 12:54:19.983162933 +0100 +++ new/src/java.base/share/classes/java/util/zip/ZipFile.java 2017-11-01 12:54:19.861165011 +0100 @@ -26,27 +26,28 @@ package java.util.zip; import java.io.Closeable; -import java.io.InputStream; -import java.io.IOException; import java.io.EOFException; import java.io.File; +import java.io.IOException; +import java.io.InputStream; 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.nio.file.attribute.BasicFileAttributes; 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.Objects; +import java.util.Set; import java.util.Spliterator; import java.util.Spliterators; import java.util.WeakHashMap; @@ -54,11 +55,10 @@ import java.util.stream.StreamSupport; import jdk.internal.misc.JavaUtilZipFileAccess; import jdk.internal.misc.SharedSecrets; -import jdk.internal.misc.JavaIORandomAccessFileAccess; 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.*; @@ -69,6 +69,19 @@ * or method in this class will cause a {@link NullPointerException} to be * thrown. * + *

+ * @apiNote + * In earlier versions, the {@link Object#finalize} method was overridden and + * specified to close the ZipFile object and release its system resource when + * the instance becomes unreachable. The {@code finalize} method is no longer + * defined. 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. + * + * @implNote + * The resources held by this object will be released when the instance becomes + * phantom-reachable, if the {@code close} is not invoked explicitly. + *

+ * * @author David Connelly * @since 1.1 */ @@ -77,9 +90,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; @@ -210,10 +229,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 = new CleanableResource(this, file, mode); + PerfCounter.getZipFileOpenTime().addElapsedTimeFrom(t0); PerfCounter.getZipFileCount().increment(); } @@ -280,10 +302,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); } } @@ -300,7 +322,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); } @@ -308,10 +330,6 @@ return null; } - // The outstanding inputstreams that need to be closed, - // mapped to the inflater objects they use. - private final Map streams = new WeakHashMap<>(); - /** * Returns an input stream for reading the contents of the specified * zip file entry. @@ -330,6 +348,8 @@ Objects.requireNonNull(entry, "entry"); int pos = -1; ZipFileInputStream in = null; + Source zsrc = res.zsrc; + Set istreams = res.istreams; synchronized (this) { ensureOpen(); if (Objects.equals(lastEntryName, entry.name)) { @@ -345,8 +365,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: @@ -359,10 +379,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: @@ -374,27 +393,30 @@ private class ZipFileInflaterInputStream extends InflaterInputStream { private volatile boolean closeRequested; private boolean eof = false; - private final ZipFileInputStream zfin; + private final Cleanable cleanable; + + ZipFileInflaterInputStream(ZipFileInputStream zfin, + CleanableResource res, int size) { + this(zfin, res.getInflater(), res, size); + } - ZipFileInflaterInputStream(ZipFileInputStream zfin, Inflater inf, - int size) { + private ZipFileInflaterInputStream(ZipFileInputStream zfin, Inflater inf, + CleanableResource res, int size) { super(zfin, inf, size); - this.zfin = zfin; + 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); - } - if (inf != null) { - releaseInflater(inf); + synchronized (res.istreams) { + res.istreams.remove(this); } + cleanable.clean(); + // do this last as it may throw IOException + super.close(); } // Override fill() method to provide an extra "dummy" byte @@ -416,48 +438,12 @@ public int available() throws IOException { if (closeRequested) return 0; - long avail = zfin.size() - inf.getBytesWritten(); + long avail = ((ZipFileInputStream)in).size() - inf.getBytesWritten(); 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 inflaterCache = new ArrayDeque<>(); - /** * Returns the path name of the ZIP file. * @return the path name of the ZIP file @@ -473,7 +459,7 @@ public ZipEntryIterator() { synchronized (ZipFile.this) { ensureOpen(); - this.entryCount = zsrc.total; + this.entryCount = res.zsrc.total; } } @@ -496,7 +482,7 @@ throw new NoSuchElementException(); } // each "entry" has 3 ints in table entries - return getZipEntry(null, null, zsrc.getEntryPos(i++ * 3)); + return getZipEntry(null, null, res.zsrc.getEntryPos(i++ * 3)); } } @@ -535,7 +521,7 @@ /* Checks ensureOpen() before invoke this method */ private ZipEntry getZipEntry(String name, byte[] bname, int pos) { - byte[] cen = zsrc.cen; + byte[] cen = res.zsrc.cen; int nlen = CENNAM(cen, pos); int elen = CENEXT(cen, pos); int clen = CENCOM(cen, pos); @@ -584,88 +570,148 @@ public int size() { synchronized (this) { ensureOpen(); - return zsrc.total; + return res.zsrc.total; } } - /** - * Closes the ZIP file. - *

Closing this ZIP file will close all of the input streams - * previously returned by invocations of the {@link #getInputStream - * getInputStream} method. - * - * @throws IOException if an I/O error has occurred - */ - public void close() throws IOException { - if (closeRequested) { - return; + private static class CleanableResource implements Runnable { + private final Cleanable cleanable; + + // The outstanding inputstreams that need to be closed + final Set istreams; + + // List of cached Inflater objects for decompression, + // set to null when closed + private Deque inflaters; + + // zip source + final 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.inflaters = new ArrayDeque<>(); + this.zsrc = Source.get(file, (mode & OPEN_DELETE) != 0); } - closeRequested = true; - synchronized (this) { - // Close streams, release their inflaters - synchronized (streams) { - if (!streams.isEmpty()) { - Map copy = new HashMap<>(streams); - streams.clear(); - for (Map.Entry e : copy.entrySet()) { - e.getKey().close(); - Inflater inf = e.getValue(); + void clean() throws IOException { + try { + cleanable.clean(); + } catch (UncheckedIOException e) { + // unwrap UncheckedIOException and re-throw it + throw e.getCause(); + } + } + + Inflater getInflater() { + Deque inflaters = this.inflaters; + if (inflaters != null) { + synchronized (inflaters) { + // double checked! + if (this.inflaters == inflaters) { + Inflater inf = inflaters.poll(); if (inf != null) { - inf.end(); + return inf; } } } } - // Release cached inflaters - synchronized (inflaterCache) { - Inflater inf; - while ((inf = inflaterCache.poll()) != null) { - inf.end(); + // inflaters cache already closed or empty - allocate new + return new Inflater(true); + } + + void releaseInflater(Inflater inf) { + Deque inflaters = this.inflaters; + if (inflaters != null) { + synchronized (inflaters) { + // double checked! + if (this.inflaters == inflaters) { + inf.reset(); + inflaters.add(inf); + return; + } + } + } + // inflaters cache already closed - just end late comers + inf.end(); + } + + public void run() { + // Release cached inflaters and close inflaters cache 1st + Deque inflaters = this.inflaters; + 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.inflaters = null; + } + } + // collect IOException(s)... + IOException ioe = null; + // close streams + 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) { - Source.close(zsrc); - zsrc = null; + synchronized (zsrc) { + try { + Source.release(zsrc); + } catch (IOException e) { + if (ioe == null) ioe = e; else ioe.addSuppressed(e); + } + } + } + // throw possible IOException wrapped into unchecked + if (ioe != null) { + throw new UncheckedIOException(ioe); } } } /** - * Ensures that the system resources held by this ZipFile object are - * released when there are no more references to it. + * Closes the ZIP file. + * + *

Closing this ZIP file will close all of the input streams + * previously returned by invocations of the {@link #getInputStream + * getInputStream} method. * - *

- * 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. - * 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(); + public void close() throws IOException { + if (closeRequested) { + return; + } + closeRequested = true; + + synchronized (this) { + // Close streams, release their inflaters, release cached inflaters + // and release zip source + res.clean(); + } } private void ensureOpen() { if (closeRequested) { throw new IllegalStateException("zip file closed"); } - if (zsrc == null) { - throw new IllegalStateException("The object is not initialized."); - } } private void ensureOpenOrZipException() throws IOException { @@ -694,7 +740,7 @@ 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 { @@ -743,7 +789,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"); } @@ -768,7 +814,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; @@ -819,15 +865,11 @@ } closeRequested = true; rem = 0; - synchronized (streams) { - streams.remove(this); + synchronized (res.istreams) { + res.istreams.remove(this); } } - @SuppressWarnings("deprecation") - protected void finalize() { - close(); - } } /** @@ -839,6 +881,7 @@ private String[] getMetaInfEntryNames() { synchronized (this) { ensureOpen(); + Source zsrc = res.zsrc; if (zsrc.metanames == null) { return null; } @@ -858,7 +901,7 @@ SharedSecrets.setJavaUtilZipFileAccess( new JavaUtilZipFileAccess() { public boolean startsWithLocHeader(ZipFile zip) { - return zip.zsrc.startsWithLoc; + return zip.res.zsrc.startsWithLoc; } public String[] getMetaInfEntryNames(ZipFile zip) { return zip.getMetaInfEntryNames(); @@ -946,7 +989,7 @@ private static final HashMap 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; @@ -971,9 +1014,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(); }