From 003d3a71b765055fd9297b29c9604f0b4e6a8270 Mon Sep 17 00:00:00 2001 From: sherman Date: Mon, 18 Apr 2011 10:51:19 -0700 Subject: [PATCH] 7031076: Retained ZipFile InputStreams increase heap demand Summary: Allow unreferenced ZipFile InputStreams to be finalized, GC'd Reviewed-by: sherman, dholmes Contributed-by: neil.richards@ngmr.net --- src/share/classes/java/util/zip/ZipFile.java | 197 +++++++++++------- .../ClearStaleZipFileInputStreams.java | 148 +++++++++++++ 2 files changed, 270 insertions(+), 75 deletions(-) create mode 100644 test/java/util/zip/ZipFile/ClearStaleZipFileInputStreams.java diff --git a/src/share/classes/java/util/zip/ZipFile.java b/src/share/classes/java/util/zip/ZipFile.java index 914a7ce73..9cfbe8266 100644 --- a/src/share/classes/java/util/zip/ZipFile.java +++ b/src/share/classes/java/util/zip/ZipFile.java @@ -31,11 +31,13 @@ import java.io.IOException; import java.io.EOFException; import java.io.File; import java.nio.charset.Charset; -import java.util.Vector; +import java.util.ArrayDeque; +import java.util.Deque; import java.util.Enumeration; -import java.util.Set; -import java.util.HashSet; +import java.util.HashMap; +import java.util.Map; import java.util.NoSuchElementException; +import java.util.WeakHashMap; import java.security.AccessController; import sun.security.action.GetPropertyAction; import static java.util.zip.ZipConstants64.*; @@ -54,7 +56,7 @@ class ZipFile implements ZipConstants, Closeable { private long jzfile; // address of jzfile data private String name; // zip file name private int total; // total number of entries - private boolean closeRequested; + private volatile boolean closeRequested = false; private static final int STORED = ZipEntry.STORED; private static final int DEFLATED = ZipEntry.DEFLATED; @@ -314,8 +316,9 @@ class ZipFile implements ZipConstants, Closeable { // freeEntry releases the C jzentry struct. private static native void freeEntry(long jzfile, long jzentry); - // the outstanding inputstreams that need to be closed. - private Set streams = new HashSet<>(); + // 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 @@ -351,51 +354,21 @@ class ZipFile implements ZipConstants, Closeable { switch (getEntryMethod(jzentry)) { case STORED: - streams.add(in); + synchronized (streams) { + streams.put(in, null); + } return in; case DEFLATED: - final ZipFileInputStream zfin = in; // MORE: Compute good size for inflater stream: long size = getEntrySize(jzentry) + 2; // Inflater likes a bit of slack if (size > 65536) size = 8192; if (size <= 0) size = 4096; - InputStream is = new InflaterInputStream(zfin, getInflater(), (int)size) { - private boolean isClosed = false; - - public void close() throws IOException { - if (!isClosed) { - super.close(); - releaseInflater(inf); - isClosed = true; - } - } - // Override fill() method to provide an extra "dummy" byte - // at the end of the input stream. This is required when - // using the "nowrap" Inflater option. - protected void fill() throws IOException { - if (eof) { - throw new EOFException( - "Unexpected end of ZLIB input stream"); - } - len = this.in.read(buf, 0, buf.length); - if (len == -1) { - buf[0] = 0; - len = 1; - eof = true; - } - inf.setInput(buf, 0, len); - } - private boolean eof; - - public int available() throws IOException { - if (isClosed) - return 0; - long avail = zfin.size() - inf.getBytesWritten(); - return avail > (long) Integer.MAX_VALUE ? - Integer.MAX_VALUE : (int) avail; - } - }; - streams.add(is); + Inflater inf = getInflater(); + InputStream is = + new ZipFileInflaterInputStream(in, inf, (int)size); + synchronized (streams) { + streams.put(is, inf); + } return is; default: throw new ZipException("invalid compression method"); @@ -403,36 +376,91 @@ class ZipFile implements ZipConstants, Closeable { } } + private class ZipFileInflaterInputStream extends InflaterInputStream { + private volatile boolean closeRequested = false; + private boolean eof = false; + private final ZipFileInputStream zfin; + + ZipFileInflaterInputStream(ZipFileInputStream zfin, Inflater inf, + int size) { + super(zfin, inf, size); + this.zfin = zfin; + } + + 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); + } + } + + // Override fill() method to provide an extra "dummy" byte + // at the end of the input stream. This is required when + // using the "nowrap" Inflater option. + protected void fill() throws IOException { + if (eof) { + throw new EOFException("Unexpected end of ZLIB input stream"); + } + len = in.read(buf, 0, buf.length); + if (len == -1) { + buf[0] = 0; + len = 1; + eof = true; + } + inf.setInput(buf, 0, len); + } + + public int available() throws IOException { + if (closeRequested) + return 0; + long avail = zfin.size() - inf.getBytesWritten(); + return (avail > (long) Integer.MAX_VALUE ? + Integer.MAX_VALUE : (int) avail); + } + + protected void finalize() throws Throwable { + close(); + } + } + /* * Gets an inflater from the list of available inflaters or allocates * a new one. */ private Inflater getInflater() { - synchronized (inflaters) { - int size = inflaters.size(); - if (size > 0) { - Inflater inf = (Inflater)inflaters.remove(size - 1); - return inf; - } else { - return new Inflater(true); + Inflater inf; + synchronized (inflaterCache) { + while (null != (inf = inflaterCache.poll())) { + if (false == inf.ended()) { + return inf; + } } } + return new Inflater(true); } /* * Releases the specified inflater to the list of available inflaters. */ private void releaseInflater(Inflater inf) { - synchronized (inflaters) { - if (inf.ended()) - return; + if (false == inf.ended()) { inf.reset(); - inflaters.add(inf); + synchronized (inflaterCache) { + inflaterCache.add(inf); + } } } // List of available Inflater objects for decompression - private Vector inflaters = new Vector(); + private Deque inflaterCache = new ArrayDeque<>(); /** * Returns the path name of the ZIP file. @@ -540,14 +568,32 @@ class ZipFile implements ZipConstants, Closeable { * @throws IOException if an I/O error has occurred */ public void close() throws IOException { + if (closeRequested) + return; + closeRequested = true; + synchronized (this) { - closeRequested = true; + // Close streams, release their inflaters + synchronized (streams) { + if (false == 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(); + } + } + } + } - if (streams.size() !=0) { - Set copy = streams; - streams = new HashSet<>(); - for (InputStream is: copy) - is.close(); + // Release cached inflaters + Inflater inf; + synchronized (inflaterCache) { + while (null != (inf = inflaterCache.poll())) { + inf.end(); + } } if (jzfile != 0) { @@ -556,23 +602,13 @@ class ZipFile implements ZipConstants, Closeable { jzfile = 0; close(zf); - - // Release inflaters - synchronized (inflaters) { - int size = inflaters.size(); - for (int i = 0; i < size; i++) { - Inflater inf = (Inflater)inflaters.get(i); - inf.end(); - } - } } } } - /** - * Ensures that the close method of this ZIP file is - * called when there are no more references to it. + * 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, @@ -611,6 +647,7 @@ class ZipFile implements ZipConstants, Closeable { * (possibly compressed) zip file entry. */ private class ZipFileInputStream extends InputStream { + private volatile boolean closeRequested = false; protected long jzentry; // address of jzentry data private long pos; // current position within entry data protected long rem; // number of remaining bytes within entry @@ -678,15 +715,25 @@ class ZipFile implements ZipConstants, Closeable { } public void close() { + if (closeRequested) + return; + closeRequested = true; + rem = 0; synchronized (ZipFile.this) { if (jzentry != 0 && ZipFile.this.jzfile != 0) { freeEntry(ZipFile.this.jzfile, jzentry); jzentry = 0; } + } + synchronized (streams) { streams.remove(this); } } + + protected void finalize() { + close(); + } } diff --git a/test/java/util/zip/ZipFile/ClearStaleZipFileInputStreams.java b/test/java/util/zip/ZipFile/ClearStaleZipFileInputStreams.java new file mode 100644 index 000000000..5ab8e835b --- /dev/null +++ b/test/java/util/zip/ZipFile/ClearStaleZipFileInputStreams.java @@ -0,0 +1,148 @@ +/* + * Copyright (c) 2011 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. + */ + +/* + * Portions Copyright (c) 2011 IBM Corporation + */ + +/* + * @test + * @bug 7031076 + * @summary Allow stale InputStreams from ZipFiles to be GC'd + * @author Neil Richards , + */ +import java.lang.ref.ReferenceQueue; +import java.lang.ref.WeakReference; +import java.io.File; +import java.io.FileOutputStream; +import java.io.InputStream; +import java.util.Enumeration; +import java.util.HashSet; +import java.util.Random; +import java.util.Set; +import java.util.zip.ZipEntry; +import java.util.zip.ZipFile; +import java.util.zip.ZipOutputStream; + +public class ClearStaleZipFileInputStreams { + private static final int ZIP_ENTRY_NUM = 5; + + private static final byte[][] data; + + static { + data = new byte[ZIP_ENTRY_NUM][]; + Random r = new Random(); + for (int i = 0; i < ZIP_ENTRY_NUM; i++) { + data[i] = new byte[1000]; + r.nextBytes(data[i]); + } + } + + private static File createTestFile(int compression) throws Exception { + File tempZipFile = + File.createTempFile("test-data" + compression, ".zip"); + tempZipFile.deleteOnExit(); + + ZipOutputStream zos = + new ZipOutputStream(new FileOutputStream(tempZipFile)); + zos.setLevel(compression); + + try { + for (int i = 0; i < ZIP_ENTRY_NUM; i++) { + String text = "Entry" + i; + ZipEntry entry = new ZipEntry(text); + zos.putNextEntry(entry); + try { + zos.write(data[i], 0, data[i].length); + } finally { + zos.closeEntry(); + } + } + } finally { + zos.close(); + } + + return tempZipFile; + } + + private static void startGcInducingThread(final int sleepMillis) { + final Thread gcInducingThread = new Thread() { + public void run() { + while (true) { + System.gc(); + try { + Thread.sleep(sleepMillis); + } catch (InterruptedException e) { } + } + } + }; + + gcInducingThread.setDaemon(true); + gcInducingThread.start(); + } + + public static void main(String[] args) throws Exception { + startGcInducingThread(500); + runTest(ZipOutputStream.DEFLATED); + runTest(ZipOutputStream.STORED); + } + + private static void runTest(int compression) throws Exception { + ReferenceQueue rq = new ReferenceQueue<>(); + + System.out.println("Testing with a zip file with compression level = " + + compression); + File f = createTestFile(compression); + try { + ZipFile zf = new ZipFile(f); + try { + Set refSet = createTransientInputStreams(zf, rq); + + System.out.println("Waiting for 'stale' input streams from ZipFile to be GC'd ..."); + System.out.println("(The test will hang on failure)"); + while (false == refSet.isEmpty()) { + refSet.remove(rq.remove()); + } + System.out.println("Test PASSED."); + System.out.println(); + } finally { + zf.close(); + } + } finally { + f.delete(); + } + } + + private static Set createTransientInputStreams(ZipFile zf, + ReferenceQueue rq) throws Exception { + Enumeration zfe = zf.entries(); + Set refSet = new HashSet<>(); + + while (zfe.hasMoreElements()) { + InputStream is = zf.getInputStream(zfe.nextElement()); + refSet.add(new WeakReference(is, rq)); + } + + return refSet; + } +} -- GitLab