--- old/src/java.base/share/classes/java/util/zip/ZipFile.java 2017-11-07 11:19:31.154669633 -0800 +++ new/src/java.base/share/classes/java/util/zip/ZipFile.java 2017-11-07 11:19:30.726631040 -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; @@ -54,11 +56,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.*; @@ -77,9 +78,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 +217,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 +290,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 +310,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 +318,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 +336,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 +353,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,42 +367,45 @@ 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: throw new ZipException("invalid compression method"); } } + } private class ZipFileInflaterInputStream extends InflaterInputStream { private volatile boolean closeRequested; private boolean eof = false; - private final ZipFileInputStream zfin; + 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.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(); } // Override fill() method to provide an extra "dummy" byte @@ -416,48 +427,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 +448,7 @@ public ZipEntryIterator() { synchronized (ZipFile.this) { ensureOpen(); - this.entryCount = zsrc.total; + this.entryCount = res.zsrc.total; } } @@ -496,7 +471,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 +510,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,12 +559,105 @@ 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 istreams; + + // List of cached Inflater objects for decompression + final Deque 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) { + inf.reset(); + synchronized (inflaterCache) { + inflaterCache.add(inf); + } + } + + public void run() { + IOException ioe = 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 cached inflaters + if (inflaterCache != null) { + synchronized (inflaterCache) { + Inflater inf; + while ((inf = inflaterCache.poll()) != null) { + inf.end(); + } + } + } + // 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); + } } } /** * 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. @@ -603,31 +671,12 @@ 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(); - 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(); } } } @@ -636,34 +685,26 @@ * Ensures that the system resources held by this ZipFile object are * released when there are no more references to it. * - *

- * 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. + * @deprecated The {@code finalize} method has been deprecated and + * 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 phantom-reachable. + * * @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."); } } @@ -684,7 +725,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); @@ -694,10 +735,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) { @@ -743,7 +784,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 +809,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 +860,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 +876,7 @@ private String[] getMetaInfEntryNames() { synchronized (this) { ensureOpen(); + Source zsrc = res.zsrc; if (zsrc.metanames == null) { return null; } @@ -858,7 +896,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 +984,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 +1009,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(); }