Bug 514121: fixed EclipseFileManager leaks open URLClassLoaders

Change-Id: I8c0e3260c8ec12211fbbe28ff7c9e74cf6085a59
Signed-off-by: Igor Fedorenko <igor@ifedorenko.com>

3 files changed, 52 insertions, 18 deletions

diff --git a/org.eclipse.jdt.compiler.apt.tests/src/org/eclipse/jdt/compiler/apt/tests/FileManagerTests.java b/org.eclipse.jdt.compiler.apt.tests/src/org/eclipse/jdt/compiler/apt/tests/FileManagerTests.java
index f1af4b2ada..36b7079a58 100644
--- a/org.eclipse.jdt.compiler.apt.tests/src/org/eclipse/jdt/compiler/apt/tests/FileManagerTests.java
+++ b/org.eclipse.jdt.compiler.apt.tests/src/org/eclipse/jdt/compiler/apt/tests/FileManagerTests.java
@@ -16,6 +16,7 @@ import java.io.BufferedWriter;
 import java.io.File;
 import java.io.FileWriter;
 import java.io.IOException;
+import java.net.URLClassLoader;
 import java.nio.charset.Charset;
 import java.util.ArrayList;
 import java.util.HashSet;
@@ -198,4 +199,15 @@ public class FileManagerTests extends TestCase {
 		fileManager.close();
 	}
 
+	public void testBug514121_getClassloader_close() throws Exception {
+		EclipseFileManager fileManager = new EclipseFileManager(Locale.getDefault(), Charset.defaultCharset());
+		List<File> classpath = new ArrayList<>();
+		classpath.add(new File(BatchTestUtils.getPluginDirectoryPath(), "resources/targets/filemanager/dependency.zip"));
+		fileManager.setLocation(javax.tools.StandardLocation.ANNOTATION_PROCESSOR_PATH, classpath);
+		URLClassLoader loader = (URLClassLoader) fileManager
+				.getClassLoader(javax.tools.StandardLocation.ANNOTATION_PROCESSOR_PATH);
+		assertNotNull(loader.findResource("jarresource.txt")); // sanity check
+		fileManager.close();
+		assertNull(loader.findResource("jarresource.txt")); // assert the classloader is closed
+	}
 }
diff --git a/org.eclipse.jdt.compiler.apt/src/org/eclipse/jdt/internal/compiler/apt/util/EclipseFileManager.java b/org.eclipse.jdt.compiler.apt/src/org/eclipse/jdt/internal/compiler/apt/util/EclipseFileManager.java
index 2319696da3..59b628fc1a 100644
--- a/org.eclipse.jdt.compiler.apt/src/org/eclipse/jdt/internal/compiler/apt/util/EclipseFileManager.java
+++ b/org.eclipse.jdt.compiler.apt/src/org/eclipse/jdt/internal/compiler/apt/util/EclipseFileManager.java
@@ -62,6 +62,7 @@ public class EclipseFileManager implements StandardJavaFileManager {
 	Charset charset;
 	Locale locale;
 	Map<String, Iterable<? extends File>> locations;
+	final Map<Location, URLClassLoader> classloaders;
 	int flags;
 	public ResourceBundle bundle;
 	
@@ -69,6 +70,7 @@ public class EclipseFileManager implements StandardJavaFileManager {
 		this.locale = locale == null ? Locale.getDefault() : locale;
 		this.charset = charset == null ? Charset.defaultCharset() : charset;
 		this.locations = new HashMap<>();
+		this.classloaders = new HashMap<>();
 		this.archivesCache = new HashMap<>();
 		try {
 			this.setLocation(StandardLocation.PLATFORM_CLASS_PATH, getDefaultBootclasspath());
@@ -95,6 +97,10 @@ public class EclipseFileManager implements StandardJavaFileManager {
 			archive.close();
 		}
 		this.archivesCache.clear();
+		for (URLClassLoader cl : this.classloaders.values()) {
+			cl.close();
+		}
+		this.classloaders.clear();
 	}
 	
 	private void collectAllMatchingFiles(File file, String normalizedPackageName, Set<Kind> kinds, boolean recurse, ArrayList<JavaFileObject> collector) {
@@ -216,17 +222,22 @@ public class EclipseFileManager implements StandardJavaFileManager {
 			// location is unknown
 			return null;
 		}
-		ArrayList<URL> allURLs = new ArrayList<>();
-		for (File f : files) {
-			try {
-				allURLs.add(f.toURI().toURL());
-			} catch (MalformedURLException e) {
-				// the url is malformed - this should not happen
-				throw new RuntimeException(e);
+		URLClassLoader cl = this.classloaders.get(location);
+		if (cl == null) {
+			ArrayList<URL> allURLs = new ArrayList<>();
+			for (File f : files) {
+				try {
+					allURLs.add(f.toURI().toURL());
+				} catch (MalformedURLException e) {
+					// the url is malformed - this should not happen
+					throw new RuntimeException(e);
+				}
 			}
+			URL[] result = new URL[allURLs.size()];
+			cl = new URLClassLoader(allURLs.toArray(result), getClass().getClassLoader());
+			this.classloaders.put(location, cl);
 		}
-		URL[] result = new URL[allURLs.size()];
-		return new URLClassLoader(allURLs.toArray(result), getClass().getClassLoader());
+		return cl;
 	}
 
 	private Iterable<? extends File> getPathsFrom(String path) {
diff --git a/org.eclipse.jdt.compiler.tool/src/org/eclipse/jdt/internal/compiler/tool/EclipseFileManager.java b/org.eclipse.jdt.compiler.tool/src/org/eclipse/jdt/internal/compiler/tool/EclipseFileManager.java
index b47e4a2032..78099a49c5 100644
--- a/org.eclipse.jdt.compiler.tool/src/org/eclipse/jdt/internal/compiler/tool/EclipseFileManager.java
+++ b/org.eclipse.jdt.compiler.tool/src/org/eclipse/jdt/internal/compiler/tool/EclipseFileManager.java
@@ -62,6 +62,7 @@ public class EclipseFileManager implements StandardJavaFileManager {
 	Charset charset;
 	Locale locale;
 	Map<String, Iterable<? extends File>> locations;
+	final Map<Location, URLClassLoader> classloaders;
 	int flags;
 	public ResourceBundle bundle;
 	
@@ -69,6 +70,7 @@ public class EclipseFileManager implements StandardJavaFileManager {
 		this.locale = locale == null ? Locale.getDefault() : locale;
 		this.charset = charset == null ? Charset.defaultCharset() : charset;
 		this.locations = new HashMap<>();
+		this.classloaders = new HashMap<>();
 		this.archivesCache = new HashMap<>();
 		try {
 			this.setLocation(StandardLocation.PLATFORM_CLASS_PATH, getDefaultBootclasspath());
@@ -95,6 +97,10 @@ public class EclipseFileManager implements StandardJavaFileManager {
 			archive.close();
 		}
 		this.archivesCache.clear();
+		for (URLClassLoader cl : this.classloaders.values()) {
+			cl.close();
+		}
+		this.classloaders.clear();
 	}
 	
 	private void collectAllMatchingFiles(File file, String normalizedPackageName, Set<Kind> kinds, boolean recurse, ArrayList<JavaFileObject> collector) {
@@ -216,17 +222,22 @@ public class EclipseFileManager implements StandardJavaFileManager {
 			// location is unknown
 			return null;
 		}
-		ArrayList<URL> allURLs = new ArrayList<>();
-		for (File f : files) {
-			try {
-				allURLs.add(f.toURI().toURL());
-			} catch (MalformedURLException e) {
-				// the url is malformed - this should not happen
-				throw new RuntimeException(e);
+		URLClassLoader cl = this.classloaders.get(location);
+		if (cl == null) {
+			ArrayList<URL> allURLs = new ArrayList<>();
+			for (File f : files) {
+				try {
+					allURLs.add(f.toURI().toURL());
+				} catch (MalformedURLException e) {
+					// the url is malformed - this should not happen
+					throw new RuntimeException(e);
+				}
 			}
+			URL[] result = new URL[allURLs.size()];
+			cl = new URLClassLoader(allURLs.toArray(result), getClass().getClassLoader());
+			this.classloaders.put(location, cl);
 		}
-		URL[] result = new URL[allURLs.size()];
-		return new URLClassLoader(allURLs.toArray(result), getClass().getClassLoader());
+		return cl;
 	}
 
 	private Iterable<? extends File> getPathsFrom(String path) {