OpenJDK / portola / portola
changeset 7983:fb83b663cadc
6829919: URLClassLoader.close() doesn't close resource file if getResourceAsStream(...) was called before
Reviewed-by: chegar
author | michaelm |
---|---|
date | Wed, 12 Jan 2011 15:05:10 +0000 |
parents | dffe8439eb20 |
children | bfcd8150ae1c |
files | jdk/src/share/classes/java/net/URLClassLoader.java jdk/test/java/net/URLClassLoader/closetest/CloseTest.java jdk/test/java/net/URLClassLoader/closetest/Common.java jdk/test/java/net/URLClassLoader/closetest/GetResourceAsStream.java jdk/test/java/net/URLClassLoader/closetest/build2.sh |
diffstat | 5 files changed, 374 insertions(+), 113 deletions(-) [+] |
line wrap: on
line diff
--- 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 ..