--- old/src/java.base/share/classes/java/lang/ref/Cleaner.java 2017-10-31 23:27:34.856594904 +0100 +++ new/src/java.base/share/classes/java/lang/ref/Cleaner.java 2017-10-31 23:27:34.711597374 +0100 @@ -29,7 +29,11 @@ import java.util.Objects; import java.util.concurrent.ThreadFactory; +import java.util.function.Consumer; import java.util.function.Function; +import java.util.function.LongConsumer; +import java.util.function.LongSupplier; +import java.util.function.Supplier; /** * {@code Cleaner} manages a set of object references and corresponding cleaning actions. @@ -234,4 +238,114 @@ void clean(); } + /** + * 1st registers an object then allocates a reference-valued resource by + * invoking resource allocator function and associates it with de-allocator + * function which will be called with the resource as cleaning action when + * the registered object becomes phantom reachable.

+ * Using this method (when applicable) is preferable to using + * {@link #register(Object, Runnable)}, because it ensures correct + * order of actions - 1st register the object, then allocate the resource - + * which prevents resource leaks in rare circumstances when registration fails + * because of insufficient heap memory. The resource allocator function still + * bares all the responsibility for either returning normally with the + * allocated resource or throwing an unchecked exception in which case the + * resource should not be allocated or should already be de-allocated, because + * in such case, the de-allocator function is not called. + * Refer to the API Note above for + * cautions about the behavior of cleaning actions. + * + * @param obj the object to monitor + * @param resourceAllocator the resource allocator function + * @param resourceDeallocator the resource de-allocator function, invoked as + * cleaning action with the allocated resource + * @param the type of resource + * @return a {@link CleanableResource} instance holding the allocated resource + * with associated de-allocator function as cleaning action + * @since 10 + */ + public CleanableResource createResource( + Object obj, + Supplier resourceAllocator, + Consumer resourceDeallocator) + { + Objects.requireNonNull(obj, "obj"); + Objects.requireNonNull(resourceAllocator, "resourceConstructor"); + Objects.requireNonNull(resourceDeallocator, "resourceDestructor"); + return new CleanerImpl.PhantomCleanableResource<>(obj, this, resourceAllocator, resourceDeallocator); + } + + /** + * {@code CleanableResource} represents an object holding some reference + * valued resource and a cleaning action for the resource registered in a + * {@code Cleaner}. + * + * @since 10 + * + * @param the type of allocated resource + */ + public interface CleanableResource extends Cleanable { + /** + * Obtains the allocated resource. + * + * @return the allocated resource + * @throws IllegalStateException if the resource has already been + * {@link #clean() cleaned} + */ + T value(); + } + + /** + * 1st registers an object then allocates a {@code long}-valued resource by + * invoking resource allocator function and associates it with de-allocator + * function which will be called with the resource as cleaning action when + * the registered object becomes phantom reachable.

+ * Using this method (when applicable) is preferable to using + * {@link #register(Object, Runnable)}, because it ensures correct + * order of actions - 1st register the object, then allocate the resource - + * which prevents resource leaks in rare circumstances when registration fails + * because of insufficient heap memory. The resource allocator function still + * bares all the responsibility for either returning normally with the + * allocated resource or throwing an unchecked exception in which case the + * resource should not be allocated or should already be de-allocated, because + * in such case, the de-allocator function is not called. + * Refer to the API Note above for + * cautions about the behavior of cleaning actions. + * + * @param obj the object to monitor + * @param resourceAllocator the resource allocator function + * @param resourceDeallocator the resource de-allocator function, invoked as + * cleaning action with the allocated resource + * @return a {@link LongCleanableResource} instance holding the allocated resource + * with associated de-allocator function as cleaning action + * @since 10 + */ + public LongCleanableResource createLongResource( + Object obj, + LongSupplier resourceAllocator, + LongConsumer resourceDeallocator) + { + Objects.requireNonNull(obj, "obj"); + Objects.requireNonNull(resourceAllocator, "resourceConstructor"); + Objects.requireNonNull(resourceDeallocator, "resourceDestructor"); + return new CleanerImpl.PhantomLongCleanableResource(obj, this, resourceAllocator, resourceDeallocator); + } + + /** + * {@code LongCleanableResource} represents an object holding some {@code long} + * valued resource and a cleaning action for the resource registered in a + * {@code Cleaner}. + * + * @since 10 + */ + public interface LongCleanableResource extends Cleanable { + /** + * Obtains the allocated resource. + * + * @return the allocated resource + * @throws IllegalStateException if the resource has already been + * {@link #clean() cleaned} + */ + long value(); + } } --- old/src/java.base/share/classes/java/util/zip/Deflater.java 2017-10-31 23:27:35.188589249 +0100 +++ new/src/java.base/share/classes/java/util/zip/Deflater.java 2017-10-31 23:27:35.068591293 +0100 @@ -25,6 +25,11 @@ package java.util.zip; +import jdk.internal.ref.CleanerFactory; + +import java.lang.ref.Cleaner; +import java.lang.ref.Reference; + /** * This class provides support for general purpose compression using the * popular ZLIB compression library. The ZLIB compression library was @@ -67,6 +72,20 @@ * } * * + *

+ * @apiNote + * In earlier versions the {@link Object#finalize} method was overridden and + * specified to call the {@code end} method to close the {@code deflater} and + * release the resource when the instance becomes unreachable. + * The {@code finalize} method is no longer defined. The recommended cleanup + * for compressor is to explicitly call {@code end} method when it is no + * longer in use. + * + * @implNote + * The resource of the compressor will be released when the instance becomes + * phantom-reachable, if the {@code end} is not invoked explicitly. + *

+ * * @see Inflater * @author David Connelly * @since 1.1 @@ -74,7 +93,7 @@ public class Deflater { - private final ZStreamRef zsRef; + private final Cleaner.LongCleanableResource zsRef; private byte[] buf = new byte[0]; private int off, len; private int level, strategy; @@ -169,7 +188,11 @@ public Deflater(int level, boolean nowrap) { this.level = level; this.strategy = DEFAULT_STRATEGY; - this.zsRef = new ZStreamRef(init(level, DEFAULT_STRATEGY, nowrap)); + this.zsRef = CleanerFactory + .cleaner() + .createLongResource(this, + () -> init(level, DEFAULT_STRATEGY, nowrap), + Deflater::end); } /** @@ -242,7 +265,8 @@ } synchronized (zsRef) { ensureOpen(); - setDictionary(zsRef.address(), b, off, len); + setDictionary(zsRef.value(), b, off, len); + Reference.reachabilityFence(this); } } @@ -445,7 +469,8 @@ if (flush == NO_FLUSH || flush == SYNC_FLUSH || flush == FULL_FLUSH) { int thisLen = this.len; - int n = deflateBytes(zsRef.address(), b, off, len, flush); + int n = deflateBytes(zsRef.value(), b, off, len, flush); + Reference.reachabilityFence(this); bytesWritten += n; bytesRead += (thisLen - this.len); return n; @@ -461,7 +486,9 @@ public int getAdler() { synchronized (zsRef) { ensureOpen(); - return getAdler(zsRef.address()); + int adler = getAdler(zsRef.value()); + Reference.reachabilityFence(this); + return adler; } } @@ -524,7 +551,8 @@ public void reset() { synchronized (zsRef) { ensureOpen(); - reset(zsRef.address()); + reset(zsRef.value()); + Reference.reachabilityFence(this); finish = false; finished = false; off = len = 0; @@ -534,43 +562,25 @@ /** * 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(); - } - private void ensureOpen() { assert Thread.holdsLock(zsRef); - if (zsRef.address() == 0) + try { + zsRef.value(); + } catch (IllegalStateException e) { throw new NullPointerException("Deflater has been closed"); + } } private static native void initIDs(); --- old/src/java.base/share/classes/java/util/zip/Inflater.java 2017-10-31 23:27:35.524583525 +0100 +++ new/src/java.base/share/classes/java/util/zip/Inflater.java 2017-10-31 23:27:35.402585603 +0100 @@ -25,6 +25,11 @@ package java.util.zip; +import jdk.internal.ref.CleanerFactory; + +import java.lang.ref.Cleaner; +import java.lang.ref.Reference; + /** * This class provides support for general purpose decompression using the * popular ZLIB compression library. The ZLIB compression library was @@ -66,6 +71,20 @@ * } * * + *

+ * @apiNote + * In earlier versions the {@link Object#finalize} method was overridden and + * specified to call the {@code end} method to close the {@code inflater} and + * release the resource when the instance becomes unreachable. + * The {@code finalize} method is no longer defined. The recommended cleanup + * for decompressor is to explicitly call {@code end} method when it is no + * longer in use. + * + * @implNote + * The resource of the decompressor will be released when the instance becomes + * phantom-reachable, if the {@code end} is not invoked explicitly. + *

+ * * @see Deflater * @author David Connelly * @since 1.1 @@ -74,7 +93,7 @@ public class Inflater { - private final ZStreamRef zsRef; + private final Cleaner.LongCleanableResource zsRef; private byte[] buf = defaultBuf; private int off, len; private boolean finished; @@ -101,7 +120,9 @@ * @param nowrap if true then support GZIP compatible compression */ public Inflater(boolean nowrap) { - zsRef = new ZStreamRef(init(nowrap)); + this.zsRef = CleanerFactory + .cleaner() + .createLongResource(this, () -> init(nowrap), Inflater::end); } /** @@ -165,7 +186,8 @@ } synchronized (zsRef) { ensureOpen(); - setDictionary(zsRef.address(), b, off, len); + setDictionary(zsRef.value(), b, off, len); + Reference.reachabilityFence(this); needDict = false; } } @@ -257,7 +279,8 @@ synchronized (zsRef) { ensureOpen(); int thisLen = this.len; - int n = inflateBytes(zsRef.address(), b, off, len); + int n = inflateBytes(zsRef.value(), b, off, len); + Reference.reachabilityFence(this); bytesWritten += n; bytesRead += (thisLen - this.len); return n; @@ -288,7 +311,9 @@ public int getAdler() { synchronized (zsRef) { ensureOpen(); - return getAdler(zsRef.address()); + int adler = getAdler(zsRef.value()); + Reference.reachabilityFence(this); + return adler; } } @@ -350,7 +375,8 @@ public void reset() { synchronized (zsRef) { ensureOpen(); - reset(zsRef.address()); + reset(zsRef.value()); + Reference.reachabilityFence(this); buf = defaultBuf; finished = false; needDict = false; @@ -361,48 +387,24 @@ /** * 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(); - } - private void ensureOpen () { assert Thread.holdsLock(zsRef); - if (zsRef.address() == 0) + try { + zsRef.value(); + } catch (IllegalStateException e) { throw new NullPointerException("Inflater has been closed"); - } - - boolean ended() { - synchronized (zsRef) { - return zsRef.address() == 0; } } --- old/src/java.base/share/classes/java/util/zip/InflaterInputStream.java 2017-10-31 23:27:35.854577904 +0100 +++ new/src/java.base/share/classes/java/util/zip/InflaterInputStream.java 2017-10-31 23:27:35.734579948 +0100 @@ -25,10 +25,15 @@ package java.util.zip; +import jdk.internal.ref.CleanerFactory; + +import java.io.EOFException; import java.io.FilterInputStream; -import java.io.InputStream; import java.io.IOException; -import java.io.EOFException; +import java.io.InputStream; +import java.lang.ref.Cleaner; +import java.util.function.Consumer; +import java.util.function.Supplier; /** * This class implements a stream filter for uncompressing data in the @@ -60,6 +65,9 @@ // this flag is set to true after EOF has reached private boolean reachEOF = false; + // in case this stream manages own Inflater, else null + private final Cleaner.CleanableResource infRes; + /** * Check to make sure that this stream has not been closed */ @@ -85,8 +93,9 @@ } else if (size <= 0) { throw new IllegalArgumentException("buffer size <= 0"); } + this.buf = new byte[size]; + this.infRes = null; // uses specified/default decompressor this.inf = inf; - buf = new byte[size]; } /** @@ -110,6 +119,31 @@ usesDefaultInflater = true; } + /** + * Creates a new input stream with a decompressor allocated by inflaterAllocator + * and deallocated by inflaterDeallocator and buffer size. + * @param in the input stream + * @param inflaterAllocator the inflater allocator function + * @param inflaterDeallocator the inflater de-allocator function + * @param size the input buffer size + */ + InflaterInputStream(InputStream in, + Supplier inflaterAllocator, + Consumer inflaterDeallocator, + int size) { + super(in); + if (in == null) { + throw new NullPointerException(); + } else if (size <= 0) { + throw new IllegalArgumentException("buffer size <= 0"); + } + this.buf = new byte[size]; + this.infRes = CleanerFactory + .cleaner() + .createResource(this, inflaterAllocator, inflaterDeallocator); + this.inf = infRes.value(); + } + private byte[] singleByteBuf = new byte[1]; /** @@ -227,7 +261,9 @@ */ public void close() throws IOException { if (!closed) { - if (usesDefaultInflater) + if (infRes != null) + infRes.clean(); + else if (usesDefaultInflater) inf.end(); in.close(); closed = true; --- old/src/java.base/share/classes/java/util/zip/ZipFile.java 2017-10-31 23:27:36.179572368 +0100 +++ new/src/java.base/share/classes/java/util/zip/ZipFile.java 2017-10-31 23:27:36.061574378 +0100 @@ -31,34 +31,37 @@ import java.io.EOFException; import java.io.File; import java.io.RandomAccessFile; +import java.io.UncheckedIOException; +import java.lang.ref.Cleaner; 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; +import java.util.function.Consumer; +import java.util.function.Supplier; import java.util.stream.Stream; 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 +72,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 */ @@ -80,6 +96,15 @@ private Source zsrc; private ZipCoder zc; + // The outstanding inputstreams that need to be closed + private final Set streams; + + // List of cached Inflater objects for decompression + private final Deque inflaterCache; + + // Cleanable reference to Source + private final Cleaner.CleanableResource zsrcRes; + private static final int STORED = ZipEntry.STORED; private static final int DEFLATED = ZipEntry.DEFLATED; @@ -213,7 +238,20 @@ this.zc = ZipCoder.get(charset); this.name = name; long t0 = System.nanoTime(); - this.zsrc = Source.get(file, (mode & OPEN_DELETE) != 0); + this.streams = Collections.newSetFromMap(new WeakHashMap<>()); + this.inflaterCache = new ArrayDeque<>(); + try { + this.zsrcRes = CleanerFactory + .cleaner() + .createResource( + this, + () -> Source.get(file, (mode & OPEN_DELETE) != 0), + Source::release + ); + } catch (UncheckedIOException e) { + throw e.getCause(); + } + this.zsrc = zsrcRes.value(); PerfCounter.getZipFileOpenTime().addElapsedTimeFrom(t0); PerfCounter.getZipFileCount().increment(); } @@ -308,10 +346,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. @@ -346,7 +380,7 @@ switch (CENHOW(zsrc.cen, pos)) { case STORED: synchronized (streams) { - streams.put(in, null); + streams.add(in); } return in; case DEFLATED: @@ -359,10 +393,10 @@ if (size <= 0) { size = 4096; } - Inflater inf = getInflater(); - InputStream is = new ZipFileInflaterInputStream(in, inf, (int)size); + InputStream is = new ZipFileInflaterInputStream( + in, this::getInflater, this::releaseInflater, (int)size); synchronized (streams) { - streams.put(is, inf); + streams.add(is); } return is; default: @@ -374,12 +408,12 @@ private class ZipFileInflaterInputStream extends InflaterInputStream { private volatile boolean closeRequested; private boolean eof = false; - private final ZipFileInputStream zfin; - ZipFileInflaterInputStream(ZipFileInputStream zfin, Inflater inf, - int size) { - super(zfin, inf, size); - this.zfin = zfin; + ZipFileInflaterInputStream(ZipFileInputStream zfin, + Supplier inflaterAllocator, + Consumer inflaterDeallocator, + int size) { + super(zfin, inflaterAllocator, inflaterDeallocator, size); } public void close() throws IOException { @@ -388,12 +422,8 @@ closeRequested = true; super.close(); - Inflater inf; synchronized (streams) { - inf = streams.remove(this); - } - if (inf != null) { - releaseInflater(inf); + streams.remove(this); } } @@ -416,15 +446,10 @@ 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(); - } } /* @@ -435,9 +460,7 @@ Inflater inf; synchronized (inflaterCache) { while ((inf = inflaterCache.poll()) != null) { - if (!inf.ended()) { - return inf; - } + return inf; } } return new Inflater(true); @@ -447,17 +470,12 @@ * 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); - } + 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 @@ -596,67 +614,50 @@ * * @throws IOException if an I/O error has occurred */ - public void close() throws IOException { - if (closeRequested) { - return; - } + public synchronized void close() throws IOException { + if (closeRequested) return; 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(); + IOException ioe = null; + + // Close streams + synchronized (streams) { + if (!streams.isEmpty()) { + InputStream[] copy = streams.toArray(new InputStream[0]); + streams.clear(); + for (InputStream is : copy) { + try { + is.close(); + } catch (IOException e) { + if (ioe == null) { + ioe = e; + } else { + ioe.addSuppressed(e); } } } } - // Release cached inflaters - synchronized (inflaterCache) { - Inflater inf; - while ((inf = inflaterCache.poll()) != 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; + } + // Release zip src + try { + zsrcRes.clean(); + } catch (UncheckedIOException e) { + if (ioe == null) { + ioe = e.getCause(); + } else { + ioe.addSuppressed(e.getCause()); } } - } - - /** - * 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. - * @throws IOException if an I/O error has occurred - * @see java.util.zip.ZipFile#close() - */ - @Deprecated(since="9") - protected void finalize() throws IOException { - close(); + if (ioe != null) { + throw ioe; + } } private void ensureOpen() { @@ -824,10 +825,6 @@ } } - @SuppressWarnings("deprecation") - protected void finalize() { - close(); - } } /** @@ -946,36 +943,44 @@ private static final HashMap files = new HashMap<>(); - public static Source get(File file, boolean toDelete) throws IOException { - Key key = new Key(file, - Files.readAttributes(file.toPath(), BasicFileAttributes.class)); - Source src = null; - synchronized (files) { - src = files.get(key); - if (src != null) { - src.refs++; - return src; + static Source get(File file, boolean toDelete) { + try { + Key key = new Key(file, + Files.readAttributes(file.toPath(), BasicFileAttributes.class)); + Source src = null; + synchronized (files) { + src = files.get(key); + if (src != null) { + src.refs++; + return src; + } } - } - src = new Source(key, toDelete); + src = new Source(key, toDelete); - synchronized (files) { - if (files.containsKey(key)) { // someone else put in first - src.close(); // close the newly created one - src = files.get(key); - src.refs++; + synchronized (files) { + if (files.containsKey(key)) { // someone else put in first + src.close(); // close the newly created one + src = files.get(key); + src.refs++; + return src; + } + files.put(key, src); return src; } - files.put(key, src); - return src; + } catch (IOException e) { + throw new UncheckedIOException(e); } } - private static void close(Source src) throws IOException { + static void release(Source src) { synchronized (files) { if (--src.refs == 0) { files.remove(src.key); - src.close(); + try { + src.close(); + } catch (IOException e) { + throw new UncheckedIOException(e); + } } } } --- old/src/java.base/share/classes/jdk/internal/ref/CleanerImpl.java 2017-10-31 23:27:36.526566457 +0100 +++ new/src/java.base/share/classes/jdk/internal/ref/CleanerImpl.java 2017-10-31 23:27:36.404568535 +0100 @@ -25,6 +25,8 @@ package jdk.internal.ref; +import jdk.internal.misc.InnocuousThread; + import java.lang.ref.Cleaner; import java.lang.ref.Cleaner.Cleanable; import java.lang.ref.ReferenceQueue; @@ -32,9 +34,11 @@ import java.security.PrivilegedAction; import java.util.concurrent.ThreadFactory; import java.util.concurrent.atomic.AtomicInteger; +import java.util.function.Consumer; import java.util.function.Function; - -import jdk.internal.misc.InnocuousThread; +import java.util.function.LongConsumer; +import java.util.function.LongSupplier; +import java.util.function.Supplier; /** * CleanerImpl manages a set of object references and corresponding cleaning actions. @@ -187,6 +191,171 @@ } /** + * Prevent access to referent even when it is still alive. + * + * @throws UnsupportedOperationException always + */ + @Override + public Object get() { + throw new UnsupportedOperationException("get"); + } + + /** + * Direct clearing of the referent is not supported. + * + * @throws UnsupportedOperationException always + */ + @Override + public void clear() { + throw new UnsupportedOperationException("clear"); + } + } + + /** + * Reference-valued resource, automatically cleaned when + * the tracked referent becomes phantom-reachable. + * + * @param the type of resource + * @since 10 + */ + public static final class PhantomCleanableResource + extends PhantomCleanable + implements Cleaner.CleanableResource { + + private final T resource; + private Consumer deallocator; + + /** + * Constructor for a phantom cleanable resource. + * @param obj the object to monitor + * @param cleaner the cleaner + * @param allocator the resource allocator function + * @param deallocator the resource de-allocator function + */ + public PhantomCleanableResource(Object obj, Cleaner cleaner, + Supplier allocator, + Consumer deallocator) { + super(obj, cleaner); + try { + this.resource = allocator.get(); + } catch (Throwable t) { + // effectively de-register this cleanable in case + // resource allocation fails + super.clear(); + throw t; + } + this.deallocator = deallocator; + } + + + @Override + protected void performCleanup() { + Consumer deallocator = this.deallocator; + // only when deallocator was assigned, we can be sure that + // allocator.get() returned successfully + if (deallocator != null) { + // mark that cleanup has been performed + this.deallocator = null; + deallocator.accept(resource); + } + } + + /** + * Access to the allocated resource. + * + * @return the allocated resource + */ + @Override + public T value() { + if (deallocator == null) { + throw new IllegalStateException("Already cleaned"); + } + return resource; + } + + /** + * Prevent access to referent even when it is still alive. + * + * @throws UnsupportedOperationException always + */ + @Override + public Object get() { + throw new UnsupportedOperationException("get"); + } + + /** + * Direct clearing of the referent is not supported. + * + * @throws UnsupportedOperationException always + */ + @Override + public void clear() { + throw new UnsupportedOperationException("clear"); + } + } + + /** + * Long-valued resource, automatically cleaned when + * the tracked referent becomes phantom-reachable. + * + * @since 10 + */ + public static final class PhantomLongCleanableResource + extends PhantomCleanable + implements Cleaner.LongCleanableResource { + + private final long resource; + private LongConsumer deallocator; + + /** + * Constructor for a phantom cleanable resource. + * @param obj the object to monitor + * @param cleaner the cleaner + * @param allocator the resource allocator function + * @param deallocator the resource de-allocator function + */ + public PhantomLongCleanableResource(Object obj, Cleaner cleaner, + LongSupplier allocator, + LongConsumer deallocator) { + super(obj, cleaner); + try { + this.resource = allocator.getAsLong(); + } catch (Throwable t) { + // effectively de-register this cleanable in case + // resource allocation fails + super.clear(); + throw t; + } + this.deallocator = deallocator; + } + + + @Override + protected void performCleanup() { + LongConsumer deallocator = this.deallocator; + // only when deallocator was assigned, we can be sure that + // allocator.get() returned successfully + if (deallocator != null) { + // mark that cleanup has been performed + this.deallocator = null; + deallocator.accept(resource); + } + } + + /** + * Access to the allocated resource. + * + * @return the allocated resource + */ + @Override + public long value() { + if (deallocator == null) { + throw new IllegalStateException("Already cleaned"); + } + return resource; + } + + /** * Prevent access to referent even when it is still alive. * * @throws UnsupportedOperationException always --- old/test/jdk/java/util/zip/ZipFile/FinalizeZipFile.java 2017-10-31 23:27:36.857560819 +0100 +++ new/test/jdk/java/util/zip/ZipFile/FinalizeZipFile.java 2017-10-31 23:27:36.734562914 +0100 @@ -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 { @@ -43,7 +44,7 @@ System.out.printf("Using %s%n", f.getPath()); } @Override - protected void finalize() throws IOException { + protected void finalize() throws Throwable { System.out.printf("Killing %s%n", getName()); super.finalize(); finalizersDone.countDown(); @@ -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-10-31 17:22:21.921871366 +0100 +++ new/test/jdk/java/util/zip/ZipFile/TestCleaner.java 2017-10-31 23:27:37.062557327 +0100 @@ -0,0 +1,138 @@ +/* + * 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.ref.Cleaner; +import java.lang.reflect.*; +import java.util.*; +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 { + return ((Cleaner.LongCleanableResource) obj).value(); + } catch (IllegalStateException x) { // already cleaned + return 0; + } + } + + // 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"); + } + List list = new ArrayList<>(); + byte[] buf1 = new byte[1024]; + byte[] buf2 = new byte[1024]; + for (int i = 0; i < 10; i++) { + Deflater def = new Deflater(); + list.add(zsRefDef.get(def)); + def.setInput("hello".getBytes()); + def.finish(); + int n = def.deflate(buf1); + + Inflater 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); + } + + 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 (FileOutputStream fos = new FileOutputStream(zip); + ZipOutputStream zos = new ZipOutputStream(fos)) { + zos.putNextEntry(new ZipEntry("hello")); + zos.write("hello".getBytes(US_ASCII)); + zos.closeEntry(); + } + + ZipFile zf = new ZipFile(zip); + Enumeration es = zf.entries(); + while (es.hasMoreElements()) { + zf.getInputStream(es.nextElement()).read(); + } + + Field fieldZsrc = ZipFile.class.getDeclaredField("zsrc"); + if (!fieldZsrc.trySetAccessible()) { + throw new RuntimeException("'ZipFile.zsrc' is not accesible"); + } + zsrc = fieldZsrc.get(zf); + + } 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."); + } + } + } + +} --- old/src/java.base/share/classes/java/util/zip/ZStreamRef.java 2017-10-31 23:27:37.499549883 +0100 +++ /dev/null 2017-10-31 17:22:21.921871366 +0100 @@ -1,46 +0,0 @@ -/* - * Copyright (c) 2009, 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. Oracle designates this - * particular file as subject to the "Classpath" exception as provided - * by Oracle in the LICENSE file that accompanied this code. - * - * 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. - */ - -package java.util.zip; - -/** - * A reference to the native zlib's z_stream structure. - */ - -class ZStreamRef { - - private volatile long address; - ZStreamRef (long address) { - this.address = address; - } - - long address() { - return address; - } - - void clear() { - address = 0; - } -}