# HG changeset patch # User michaelm # Date 1294844710 0 # Node ID fb83b663cadc3b250f4dcbe56f9bdc2d7f7428f7 # Parent dffe8439eb204a9fd89757d3d0fdf1fec26d841d 6829919: URLClassLoader.close() doesn't close resource file if getResourceAsStream(...) was called before Reviewed-by: chegar diff -r dffe8439eb20 -r fb83b663cadc jdk/src/share/classes/java/net/URLClassLoader.java --- 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 + 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. + * + *

The search order is described in the documentation for {@link + * #getResource(String)}.

+ * + * @param name + * The resource name + * + * @return An input stream for reading the resource, or null + * 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. *

- * 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. *

@@ -213,10 +268,10 @@ * loader has no effect. *

* @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 cause - * 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}("closeClassLoader") @@ -229,21 +284,33 @@ security.checkPermission(new RuntimePermission("closeClassLoader")); } List errors = ucp.closeLoaders(); + + // now close any remaining streams. + + synchronized (closeables) { + Set 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; } /** diff -r dffe8439eb20 -r fb83b663cadc jdk/test/java/net/URLClassLoader/closetest/CloseTest.java --- 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= 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 ${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 ..