6829919: URLClassLoader.close() doesn't close resource file if getResourceAsStream(...) was called before
Reviewed-by: chegar
--- a/jdk/src/share/classes/java/net/URLClassLoader.java Mon Jan 10 17:06:10 2011 -0800
+++ b/jdk/src/share/classes/java/net/URLClassLoader.java Wed Jan 12 15:05:10 2011 +0000
@@ -27,19 +27,15 @@
import java.lang.reflect.Method;
import java.lang.reflect.Modifier;
-import java.io.File;
-import java.io.FilePermission;
-import java.io.InputStream;
-import java.io.IOException;
-import java.io.Closeable;
+import java.lang.ref.*;
+import java.io.*;
import java.net.URL;
import java.net.URLConnection;
import java.net.URLStreamHandlerFactory;
import java.util.Enumeration;
-import java.util.List;
-import java.util.NoSuchElementException;
-import java.util.StringTokenizer;
+import java.util.*;
import java.util.jar.Manifest;
+import java.util.jar.JarFile;
import java.util.jar.Attributes;
import java.util.jar.Attributes.Name;
import java.security.CodeSigner;
@@ -194,6 +190,65 @@
acc = AccessController.getContext();
}
+ /* A map (used as a set) to keep track of closeable local resources
+ * (either JarFiles or FileInputStreams). We don't care about
+ * Http resources since they don't need to be closed.
+ *
+ * If the resource is coming from a jar file
+ * we keep a (weak) reference to the JarFile object which can
+ * be closed if URLClassLoader.close() called. Due to jar file
+ * caching there will typically be only one JarFile object
+ * per underlying jar file.
+ *
+ * For file resources, which is probably a less common situation
+ * we have to keep a weak reference to each stream.
+ */
+
+ private WeakHashMap<Closeable,Void>
+ closeables = new WeakHashMap<>();
+
+ /**
+ * Returns an input stream for reading the specified resource.
+ * If this loader is closed, then any resources opened by this method
+ * will be closed.
+ *
+ * <p> The search order is described in the documentation for {@link
+ * #getResource(String)}. </p>
+ *
+ * @param name
+ * The resource name
+ *
+ * @return An input stream for reading the resource, or <tt>null</tt>
+ * if the resource could not be found
+ *
+ * @since 1.7
+ */
+ public InputStream getResourceAsStream(String name) {
+ URL url = getResource(name);
+ try {
+ if (url == null) {
+ return null;
+ }
+ URLConnection urlc = url.openConnection();
+ InputStream is = urlc.getInputStream();
+ if (urlc instanceof JarURLConnection) {
+ JarURLConnection juc = (JarURLConnection)urlc;
+ JarFile jar = juc.getJarFile();
+ synchronized (closeables) {
+ if (!closeables.containsKey(jar)) {
+ closeables.put(jar, null);
+ }
+ }
+ } else if (urlc instanceof sun.net.www.protocol.file.FileURLConnection) {
+ synchronized (closeables) {
+ closeables.put(is, null);
+ }
+ }
+ return is;
+ } catch (IOException e) {
+ return null;
+ }
+ }
/**
* Closes this URLClassLoader, so that it can no longer be used to load
@@ -202,8 +257,8 @@
* delegation hierarchy are still accessible. Also, any classes or resources
* that are already loaded, are still accessible.
* <p>
- * In the case of jar: and file: URLs, it also closes any class files,
- * or JAR files that were opened by it. If another thread is loading a
+ * In the case of jar: and file: URLs, it also closes any files
+ * that were opened by it. If another thread is loading a
* class when the {@code close} method is invoked, then the result of
* that load is undefined.
* <p>
@@ -213,10 +268,10 @@
* loader has no effect.
* <p>
* @throws IOException if closing any file opened by this class loader
- * resulted in an IOException. Any such exceptions are caught, and a
- * single IOException is thrown after the last file has been closed.
- * If only one exception was thrown, it will be set as the <i>cause</i>
- * of this IOException.
+ * resulted in an IOException. Any such exceptions are caught internally.
+ * If only one is caught, then it is re-thrown. If more than one exception
+ * is caught, then the second and following exceptions are added
+ * as suppressed exceptions of the first one caught, which is then re-thrown.
*
* @throws SecurityException if a security manager is set, and it denies
* {@link RuntimePermission}<tt>("closeClassLoader")</tt>
@@ -229,21 +284,33 @@
security.checkPermission(new RuntimePermission("closeClassLoader"));
}
List<IOException> errors = ucp.closeLoaders();
+
+ // now close any remaining streams.
+
+ synchronized (closeables) {
+ Set<Closeable> keys = closeables.keySet();
+ for (Closeable c : keys) {
+ try {
+ c.close();
+ } catch (IOException ioex) {
+ errors.add(ioex);
+ }
+ }
+ closeables.clear();
+ }
+
if (errors.isEmpty()) {
return;
}
- if (errors.size() == 1) {
- throw new IOException (
- "Error closing URLClassLoader resource",
- errors.get(0)
- );
+
+ IOException firstex = errors.remove(0);
+
+ // Suppress any remaining exceptions
+
+ for (IOException error: errors) {
+ firstex.addSuppressed(error);
}
- // Several exceptions. So, just combine the error messages
- String errormsg = "Error closing resources: ";
- for (IOException error: errors) {
- errormsg = errormsg + "[" + error.toString() + "] ";
- }
- throw new IOException (errormsg);
+ throw firstex;
}
/**
--- a/jdk/test/java/net/URLClassLoader/closetest/CloseTest.java Mon Jan 10 17:06:10 2011 -0800
+++ b/jdk/test/java/net/URLClassLoader/closetest/CloseTest.java Wed Jan 12 15:05:10 2011 +0000
@@ -36,94 +36,7 @@
import java.lang.reflect.*;
import com.sun.net.httpserver.*;
-public class CloseTest {
-
- static void copyFile (String src, String dst) {
- copyFile (new File(src), new File(dst));
- }
-
- static void copyDir (String src, String dst) {
- copyDir (new File(src), new File(dst));
- }
-
- static void copyFile (File src, File dst) {
- try {
- if (!src.isFile()) {
- throw new RuntimeException ("File not found: " + src.toString());
- }
- dst.delete();
- dst.createNewFile();
- FileInputStream i = new FileInputStream (src);
- FileOutputStream o = new FileOutputStream (dst);
- byte[] buf = new byte [1024];
- int count;
- while ((count=i.read(buf)) >= 0) {
- o.write (buf, 0, count);
- }
- i.close();
- o.close();
- } catch (IOException e) {
- throw new RuntimeException (e);
- }
- }
-
- static void rm_minus_rf (File path) {
- if (!path.exists()) {
- return;
- }
- if (path.isFile()) {
- if (!path.delete()) {
- throw new RuntimeException ("Could not delete " + path);
- }
- } else if (path.isDirectory ()) {
- String[] names = path.list();
- File[] files = path.listFiles();
- for (int i=0; i<files.length; i++) {
- rm_minus_rf (new File(path, names[i]));
- }
- if (!path.delete()) {
- throw new RuntimeException ("Could not delete " + path);
- }
- } else {
- throw new RuntimeException ("Trying to delete something that isn't a file or a directory");
- }
- }
-
- static void copyDir (File src, File dst) {
- if (!src.isDirectory()) {
- throw new RuntimeException ("Dir not found: " + src.toString());
- }
- if (dst.exists()) {
- throw new RuntimeException ("Dir exists: " + dst.toString());
- }
- dst.mkdir();
- String[] names = src.list();
- File[] files = src.listFiles();
- for (int i=0; i<files.length; i++) {
- String f = names[i];
- if (files[i].isDirectory()) {
- copyDir (files[i], new File (dst, f));
- } else {
- copyFile (new File (src, f), new File (dst, f));
- }
- }
- }
-
- /* expect is true if you expect to find it, false if you expect not to */
- static Class loadClass (String name, URLClassLoader loader, boolean expect){
- try {
- Class clazz = Class.forName (name, true, loader);
- if (!expect) {
- throw new RuntimeException ("loadClass: "+name+" unexpected");
- }
- return clazz;
- } catch (ClassNotFoundException e) {
- if (expect) {
- throw new RuntimeException ("loadClass: " +name + " not found");
- }
- }
- return null;
- }
+public class CloseTest extends Common {
//
// needs two jar files test1.jar and test2.jar with following structure
--- /dev/null Thu Jan 01 00:00:00 1970 +0000
+++ b/jdk/test/java/net/URLClassLoader/closetest/Common.java Wed Jan 12 15:05:10 2011 +0000
@@ -0,0 +1,115 @@
+/*
+ * 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.
+ */
+
+import java.io.*;
+import java.net.*;
+
+public class Common {
+
+ static void copyFile (String src, String dst) {
+ copyFile (new File(src), new File(dst));
+ }
+
+ static void copyDir (String src, String dst) {
+ copyDir (new File(src), new File(dst));
+ }
+
+ static void copyFile (File src, File dst) {
+ try {
+ if (!src.isFile()) {
+ throw new RuntimeException ("File not found: " + src.toString());
+ }
+ dst.delete();
+ dst.createNewFile();
+ FileInputStream i = new FileInputStream (src);
+ FileOutputStream o = new FileOutputStream (dst);
+ byte[] buf = new byte [1024];
+ int count;
+ while ((count=i.read(buf)) >= 0) {
+ o.write (buf, 0, count);
+ }
+ i.close();
+ o.close();
+ } catch (IOException e) {
+ throw new RuntimeException (e);
+ }
+ }
+
+ static void rm_minus_rf (File path) {
+ if (!path.exists()) {
+ return;
+ }
+ if (path.isFile()) {
+ if (!path.delete()) {
+ throw new RuntimeException ("Could not delete " + path);
+ }
+ } else if (path.isDirectory ()) {
+ String[] names = path.list();
+ File[] files = path.listFiles();
+ for (int i=0; i<files.length; i++) {
+ rm_minus_rf (new File(path, names[i]));
+ }
+ if (!path.delete()) {
+ throw new RuntimeException ("Could not delete " + path);
+ }
+ } else {
+ throw new RuntimeException ("Trying to delete something that isn't a file or a directory");
+ }
+ }
+
+ static void copyDir (File src, File dst) {
+ if (!src.isDirectory()) {
+ throw new RuntimeException ("Dir not found: " + src.toString());
+ }
+ if (dst.exists()) {
+ throw new RuntimeException ("Dir exists: " + dst.toString());
+ }
+ dst.mkdir();
+ String[] names = src.list();
+ File[] files = src.listFiles();
+ for (int i=0; i<files.length; i++) {
+ String f = names[i];
+ if (files[i].isDirectory()) {
+ copyDir (files[i], new File (dst, f));
+ } else {
+ copyFile (new File (src, f), new File (dst, f));
+ }
+ }
+ }
+
+ /* expect is true if you expect to find it, false if you expect not to */
+ static Class loadClass (String name, URLClassLoader loader, boolean expect){
+ try {
+ Class clazz = Class.forName (name, true, loader);
+ if (!expect) {
+ throw new RuntimeException ("loadClass: "+name+" unexpected");
+ }
+ return clazz;
+ } catch (ClassNotFoundException e) {
+ if (expect) {
+ throw new RuntimeException ("loadClass: " +name + " not found");
+ }
+ }
+ return null;
+ }
+}
--- /dev/null Thu Jan 01 00:00:00 1970 +0000
+++ b/jdk/test/java/net/URLClassLoader/closetest/GetResourceAsStream.java Wed Jan 12 15:05:10 2011 +0000
@@ -0,0 +1,108 @@
+/*
+ * 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.
+ */
+
+/**
+ * @test
+ * @bug 6899919
+ * @run shell build2.sh
+ * @run main/othervm GetResourceAsStream
+ */
+
+import java.io.*;
+import java.net.*;
+
+public class GetResourceAsStream extends Common {
+
+/*
+ * We simply test various scenarios with class/resource files
+ * and make sure the files can be deleted after closing
+ * the loader. Therefore, the test will only really be verified
+ * on Windows. It will still run correctly on other platforms
+ */
+ public static void main (String args[]) throws Exception {
+
+ String workdir = System.getProperty("test.classes");
+ if (workdir == null) {
+ workdir = args[0];
+ }
+
+ /* the jar we copy for each test */
+ File srcfile = new File (workdir, "foo.jar");
+
+ /* the jar we use for the test */
+ File testfile = new File (workdir, "test.jar");
+
+ copyFile (srcfile, testfile);
+ test (testfile, false, false);
+
+ copyFile (srcfile, testfile);
+ test (testfile, true, false);
+
+ copyFile (srcfile, testfile);
+ test (testfile, true, true);
+
+ // repeat test using a directory of files
+
+ File testdir= new File (workdir, "testdir");
+ File srcdir= new File (workdir, "test3");
+
+ copyDir (srcdir, testdir);
+ test (testdir, true, false);
+
+ }
+
+ // create a loader on jarfile (or directory)
+ // load a class , then look for a resource
+ // then close the loader
+ // check further new classes/resources cannot be loaded
+ // check jar (or dir) can be deleted
+
+ static void test (File file, boolean loadclass, boolean readall)
+ throws Exception
+ {
+ URL[] urls = new URL[] {file.toURL()};
+ System.out.println ("Doing tests with URL: " + urls[0]);
+ URLClassLoader loader = new URLClassLoader (urls);
+ if (loadclass) {
+ Class testclass = loadClass ("com.foo.TestClass", loader, true);
+ }
+ InputStream s = loader.getResourceAsStream ("hello.txt");
+ s.read();
+ if (readall) {
+ while (s.read() != -1) ;
+ s.close();
+ }
+
+ loader.close ();
+
+ // shouuld not find bye.txt now
+ InputStream s1 = loader.getResourceAsStream("bye.txt");
+ if (s1 != null) {
+ throw new RuntimeException ("closed loader returned resource");
+ }
+
+ // now check we can delete the path
+ rm_minus_rf (file);
+ System.out.println (" ... OK");
+ }
+}
--- /dev/null Thu Jan 01 00:00:00 1970 +0000
+++ b/jdk/test/java/net/URLClassLoader/closetest/build2.sh Wed Jan 12 15:05:10 2011 +0000
@@ -0,0 +1,58 @@
+#!/bin/sh
+#
+# 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.
+#
+if [ "${TESTSRC}" = "" ]
+then
+ echo "TESTSRC not set. Test cannot execute. Failed."
+ exit 1
+fi
+echo "TESTSRC=${TESTSRC}"
+
+if [ "${TESTJAVA}" = "" ]
+then
+ echo "TESTJAVA not set. Test cannot execute. Failed."
+ exit 1
+fi
+echo "TESTJAVA=${TESTJAVA}"
+
+if [ "${TESTCLASSES}" = "" ]
+then
+ echo "TESTCLASSES not set. Test cannot execute. Failed."
+ exit 1
+fi
+
+JAVAC="${TESTJAVA}/bin/javac"
+JAR="${TESTJAVA}/bin/jar"
+
+rm -rf ${TESTCLASSES}/test3
+mkdir -p ${TESTCLASSES}/test3
+
+echo "Hello world" > ${TESTCLASSES}/test3/hello.txt
+echo "Bye world" > ${TESTCLASSES}/test3/bye.txt
+cp ${TESTSRC}/test1/com/foo/TestClass.java ${TESTCLASSES}/test3
+cd ${TESTCLASSES}/test3
+${JAVAC} -d . TestClass.java
+
+${JAR} cvf foo.jar hello.txt bye.txt com/foo/TestClass.class
+rm -f ../foo.jar
+mv foo.jar ..