--- old/src/java.base/share/classes/java/util/zip/Deflater.java 2017-12-04 13:50:34.217260334 -0800 +++ new/src/java.base/share/classes/java/util/zip/Deflater.java 2017-12-04 13:50:33.714214957 -0800 @@ -67,12 +67,19 @@ * } * * + * @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. + * * @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 +176,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 +543,37 @@ /** * 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(since="9") - protected void finalize() { - end(); - } + * @implSpec + * If this {@code Deflater} 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 + * 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 phantom-reachable. + */ + @Deprecated(since="9", forRemoval=true) + protected void finalize() {} private void ensureOpen() { assert Thread.holdsLock(zsRef); --- old/src/java.base/share/classes/java/util/zip/Inflater.java 2017-12-04 13:50:35.056336022 -0800 +++ new/src/java.base/share/classes/java/util/zip/Inflater.java 2017-12-04 13:50:34.658300117 -0800 @@ -66,13 +66,20 @@ * } * * + * @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. + * * @see Deflater * @author David Connelly * @since 1.1 * */ -public -class Inflater { + +public class Inflater { private final ZStreamRef zsRef; private byte[] buf = defaultBuf; @@ -101,7 +108,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 +368,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. - */ - @Deprecated(since="9") - protected void finalize() { - end(); - } + * @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 + * 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 phantom-reachable, + */ + @Deprecated(since="9", forRemoval=true) + protected void finalize() {} private void ensureOpen () { assert Thread.holdsLock(zsRef); --- old/src/java.base/share/classes/java/util/zip/ZStreamRef.java 2017-12-04 13:50:35.947416401 -0800 +++ new/src/java.base/share/classes/java/util/zip/ZStreamRef.java 2017-12-04 13:50:35.512377159 -0800 @@ -25,22 +25,89 @@ package java.util.zip; +import java.util.function.LongConsumer; +import java.util.function.LongSupplier; +import java.lang.ref.Cleaner.Cleanable; +import jdk.internal.ref.CleanerFactory; + /** - * A reference to the native zlib's z_stream structure. + * 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 { -class ZStreamRef { - - 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 { + 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(); + } } } --- old/src/java.base/share/classes/java/util/zip/ZipFile.java 2017-12-04 13:50:36.986510132 -0800 +++ new/src/java.base/share/classes/java/util/zip/ZipFile.java 2017-12-04 13:50:36.508467010 -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,14 @@ * 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. + * * @author David Connelly * @since 1.1 */ @@ -81,9 +91,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 +230,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 +303,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 +337,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 +345,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. @@ -348,6 +363,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)) { @@ -363,8 +380,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 +394,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 +408,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) { - super(zfin, inf, 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); - } - if (inf != null) { - releaseInflater(inf); + synchronized (res.istreams) { + res.istreams.remove(this); } + cleanable.clean(); } // Override fill() method to provide an extra "dummy" byte @@ -436,44 +457,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 inflaterCache = new ArrayDeque<>(); - /** * Returns the path name of the ZIP file. * @return the path name of the ZIP file @@ -518,7 +503,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 +521,14 @@ public Enumeration entries() { synchronized (this) { ensureOpen(); - return new ZipEntryIterator(zsrc.total, ZipEntry::new); + return new ZipEntryIterator(res.zsrc.total, ZipEntry::new); } } private Enumeration entries(Function func) { synchronized (this) { ensureOpen(); - return new ZipEntryIterator(zsrc.total, func); + return new ZipEntryIterator(res.zsrc.total, func); } } @@ -568,7 +553,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 +574,13 @@ public Stream 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 +605,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 +623,7 @@ private Stream stream(Function 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 +634,7 @@ /* Checks ensureOpen() before invoke this method */ private ZipEntry getZipEntry(String name, byte[] bname, int pos, Function 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 +683,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 istreams; + + // List of cached Inflater objects for decompression + 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) { + Deque 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 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. + * *

Closing this ZIP file will close all of the input streams * previously returned by invocations of the {@link #getInputStream * getInputStream} method. @@ -717,31 +860,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(); } } } @@ -750,34 +874,31 @@ * 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. + * @implSpec + * If this {@code ZipFile} has been subclassed and the {@code close} method + * has been overridden, the {@code close} method will be called when the + * {@code ZipFile} is unreachable. + * + * @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."); } } @@ -798,7 +919,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 +929,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 +978,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 +1003,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 +1053,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 +1069,7 @@ private String[] getMetaInfEntryNames() { synchronized (this) { ensureOpen(); + Source zsrc = res.zsrc; if (zsrc.metanames == null) { return null; } @@ -972,7 +1090,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 +1198,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; @@ -1105,9 +1223,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(); } --- old/test/jdk/java/util/zip/ZipFile/FinalizeZipFile.java 2017-12-04 13:50:38.083609095 -0800 +++ new/test/jdk/java/util/zip/ZipFile/FinalizeZipFile.java 2017-12-04 13:50:37.648569852 -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 2017-07-29 19:06:40.011000000 -0700 +++ new/test/jdk/java/util/zip/ZipFile/TestCleaner.java 2017-12-04 13:50:38.679662861 -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 instane 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 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()); + } +}