From e931e7bad1b82f459c34b7de1ba191d269322d3b Mon Sep 17 00:00:00 2001 From: Stefan Bodewig Date: Fri, 4 Sep 2009 15:02:39 +0000 Subject: [PATCH] plug some more classloader leaks and mark those places where a leak remains and I don't see an obvious way to fix it git-svn-id: https://svn.apache.org/repos/asf/ant/core/trunk@811435 13f79535-47bb-0310-9956-ffa450edef68 --- .../apache/tools/ant/taskdefs/Property.java | 9 +++++-- .../org/apache/tools/ant/taskdefs/Rmic.java | 19 +++++++++++++- .../tools/ant/taskdefs/WhichResource.java | 10 ++++++-- .../compilers/CompilerAdapterFactory.java | 1 + .../ant/taskdefs/condition/HasMethod.java | 13 +++++++--- .../taskdefs/optional/XMLValidateTask.java | 25 ++++++++++++++++--- .../optional/ejb/GenericDeploymentTool.java | 1 + .../optional/ejb/JonasDeploymentTool.java | 10 +++++++- .../optional/ejb/WebsphereDeploymentTool.java | 14 +++++++---- .../ant/taskdefs/optional/javacc/JavaCC.java | 9 ++++++- .../optional/javah/JavahAdapterFactory.java | 1 + .../tools/ant/taskdefs/optional/jsp/JspC.java | 10 +++++++- .../compilers/JspCompilerAdapterFactory.java | 1 + .../taskdefs/optional/junit/JUnitTask.java | 9 ++++++- .../Native2AsciiAdapterFactory.java | 1 + .../ant/taskdefs/rmic/RmicAdapterFactory.java | 1 + .../org/apache/tools/ant/types/Mapper.java | 1 + .../apache/tools/ant/types/XMLCatalog.java | 2 +- .../ant/types/selectors/ExtendSelector.java | 1 + .../modifiedselector/ModifiedSelector.java | 1 + 20 files changed, 116 insertions(+), 23 deletions(-) diff --git a/src/main/org/apache/tools/ant/taskdefs/Property.java b/src/main/org/apache/tools/ant/taskdefs/Property.java index ad8393c26..4892aa5a8 100644 --- a/src/main/org/apache/tools/ant/taskdefs/Property.java +++ b/src/main/org/apache/tools/ant/taskdefs/Property.java @@ -30,6 +30,7 @@ import java.util.Map; import java.util.Properties; import java.util.Vector; +import org.apache.tools.ant.AntClassLoader; import org.apache.tools.ant.BuildException; import org.apache.tools.ant.Project; import org.apache.tools.ant.PropertyHelper; @@ -558,10 +559,11 @@ public class Property extends Task { Properties props = new Properties(); log("Resource Loading " + name, Project.MSG_VERBOSE); InputStream is = null; + ClassLoader cL = null; + boolean cleanup = false; try { - ClassLoader cL = null; - if (classpath != null) { + cleanup = true; cL = getProject().createClassLoader(classpath); } else { cL = this.getClass().getClassLoader(); @@ -589,6 +591,9 @@ public class Property extends Task { // ignore } } + if (cleanup && cL != null) { + ((AntClassLoader) cL).cleanup(); + } } } diff --git a/src/main/org/apache/tools/ant/taskdefs/Rmic.java b/src/main/org/apache/tools/ant/taskdefs/Rmic.java index edac71518..f52729a99 100644 --- a/src/main/org/apache/tools/ant/taskdefs/Rmic.java +++ b/src/main/org/apache/tools/ant/taskdefs/Rmic.java @@ -21,6 +21,7 @@ import java.io.File; import java.io.IOException; import java.rmi.Remote; import java.util.Vector; +import org.apache.tools.ant.AntClassLoader; import org.apache.tools.ant.BuildException; import org.apache.tools.ant.DirectoryScanner; import org.apache.tools.ant.Project; @@ -104,7 +105,7 @@ public class Rmic extends MatchingTask { private Vector compileList = new Vector(); - private ClassLoader loader = null; + private AntClassLoader loader = null; private FacadeTaskHelper facade; /** unable to verify message */ @@ -581,6 +582,7 @@ public class Rmic extends MatchingTask { * if there's a problem with baseDir or RMIC */ public void execute() throws BuildException { + try { compileList.clear(); File outputDir = getOutputDir(); @@ -659,6 +661,21 @@ public class Rmic extends MatchingTask { } } } + } finally { + cleanup(); + } + } + + /** + * Cleans up resources. + * + * @since Ant 1.8.0 + */ + protected void cleanup() { + if (loader != null) { + loader.cleanup(); + loader = null; + } } /** diff --git a/src/main/org/apache/tools/ant/taskdefs/WhichResource.java b/src/main/org/apache/tools/ant/taskdefs/WhichResource.java index e666914d4..89a131bce 100644 --- a/src/main/org/apache/tools/ant/taskdefs/WhichResource.java +++ b/src/main/org/apache/tools/ant/taskdefs/WhichResource.java @@ -126,15 +126,16 @@ public class WhichResource extends Task { public void execute() throws BuildException { validate(); if (classpath != null) { + classpath = classpath.concatSystemClasspath("ignore"); getProject().log("using user supplied classpath: " + classpath, Project.MSG_DEBUG); - classpath = classpath.concatSystemClasspath("ignore"); } else { classpath = new Path(getProject()); classpath = classpath.concatSystemClasspath("only"); getProject().log("using system classpath: " + classpath, Project.MSG_DEBUG); } - AntClassLoader loader; + AntClassLoader loader = null; + try { loader = AntClassLoader.newAntClassLoader(getProject().getCoreLoader(), getProject(), classpath, false); @@ -160,6 +161,11 @@ public class WhichResource extends Task { loc = url.toExternalForm(); getProject().setNewProperty(property, loc); } + } finally { + if (loader != null) { + loader.cleanup(); + } + } } /** diff --git a/src/main/org/apache/tools/ant/taskdefs/compilers/CompilerAdapterFactory.java b/src/main/org/apache/tools/ant/taskdefs/compilers/CompilerAdapterFactory.java index 83d8c9626..ffd20aee0 100644 --- a/src/main/org/apache/tools/ant/taskdefs/compilers/CompilerAdapterFactory.java +++ b/src/main/org/apache/tools/ant/taskdefs/compilers/CompilerAdapterFactory.java @@ -152,6 +152,7 @@ public final class CompilerAdapterFactory { return new Sj(); } return resolveClassName(compilerType, + // Memory-Leak in line below task.getProject().createClassLoader(classpath)); } diff --git a/src/main/org/apache/tools/ant/taskdefs/condition/HasMethod.java b/src/main/org/apache/tools/ant/taskdefs/condition/HasMethod.java index ddc73d749..b239e94c1 100644 --- a/src/main/org/apache/tools/ant/taskdefs/condition/HasMethod.java +++ b/src/main/org/apache/tools/ant/taskdefs/condition/HasMethod.java @@ -110,7 +110,6 @@ public class HasMethod extends ProjectComponent implements Condition { loader = getProject().createClassLoader(classpath); loader.setParentFirst(false); loader.addJavaLibraries(); - if (loader != null) { try { return loader.findClass(classname); } catch (SecurityException se) { @@ -118,11 +117,9 @@ public class HasMethod extends ProjectComponent implements Condition { // actually the case we're looking for in JDK 1.3+, // so catch the exception and return return null; - } - } else { - return null; } } else if (loader != null) { + // How do we ever get here? return loader.loadClass(classname); } else { ClassLoader l = this.getClass().getClassLoader(); @@ -148,6 +145,8 @@ public class HasMethod extends ProjectComponent implements Condition { if (classname == null) { throw new BuildException("No classname defined"); } + ClassLoader preLoadClass = loader; + try { Class clazz = loadClass(classname); if (method != null) { return isMethodFound(clazz); @@ -156,6 +155,12 @@ public class HasMethod extends ProjectComponent implements Condition { return isFieldFound(clazz); } throw new BuildException("Neither method nor field defined"); + } finally { + if (preLoadClass != loader && loader != null) { + loader.cleanup(); + loader = null; + } + } } private boolean isFieldFound(Class clazz) { diff --git a/src/main/org/apache/tools/ant/taskdefs/optional/XMLValidateTask.java b/src/main/org/apache/tools/ant/taskdefs/optional/XMLValidateTask.java index 962ba7524..f9ef0632d 100644 --- a/src/main/org/apache/tools/ant/taskdefs/optional/XMLValidateTask.java +++ b/src/main/org/apache/tools/ant/taskdefs/optional/XMLValidateTask.java @@ -103,6 +103,8 @@ public class XMLValidateTask extends Task { public static final String MESSAGE_FILES_VALIDATED = " file(s) have been successfully validated."; + private AntClassLoader readerLoader = null; + /** * Specify how parser error are to be handled. * Optional, default is true. @@ -285,7 +287,7 @@ public class XMLValidateTask extends Task { * @throws BuildException if failonerror is true and an error happens */ public void execute() throws BuildException { - + try { int fileProcessed = 0; if (file == null && (filesets.size() == 0)) { throw new BuildException( @@ -321,6 +323,9 @@ public class XMLValidateTask extends Task { } } onSuccessfulValidation(fileProcessed); + } finally { + cleanup(); + } } /** @@ -389,9 +394,9 @@ public class XMLValidateTask extends Task { try { // load the parser class if (classpath != null) { - AntClassLoader loader = - getProject().createClassLoader(classpath); - readerClass = Class.forName(readerClassName, true, loader); + readerLoader = getProject().createClassLoader(classpath); + readerClass = Class.forName(readerClassName, true, + readerLoader); } else { readerClass = Class.forName(readerClassName); } @@ -431,6 +436,18 @@ public class XMLValidateTask extends Task { return newReader; } + /** + * Cleans up resources. + * + * @since Ant 1.8.0 + */ + protected void cleanup() { + if (readerLoader != null) { + readerLoader.cleanup(); + readerLoader = null; + } + } + /** * * @return diff --git a/src/main/org/apache/tools/ant/taskdefs/optional/ejb/GenericDeploymentTool.java b/src/main/org/apache/tools/ant/taskdefs/optional/ejb/GenericDeploymentTool.java index ec2006c31..2cb67e36f 100644 --- a/src/main/org/apache/tools/ant/taskdefs/optional/ejb/GenericDeploymentTool.java +++ b/src/main/org/apache/tools/ant/taskdefs/optional/ejb/GenericDeploymentTool.java @@ -929,6 +929,7 @@ public class GenericDeploymentTool implements EJBDeploymentTool { if (combinedClasspath == null) { classpathLoader = getClass().getClassLoader(); } else { + // Memory leak in line below classpathLoader = getTask().getProject().createClassLoader(combinedClasspath); } diff --git a/src/main/org/apache/tools/ant/taskdefs/optional/ejb/JonasDeploymentTool.java b/src/main/org/apache/tools/ant/taskdefs/optional/ejb/JonasDeploymentTool.java index c8c1f6252..bc81c75bc 100644 --- a/src/main/org/apache/tools/ant/taskdefs/optional/ejb/JonasDeploymentTool.java +++ b/src/main/org/apache/tools/ant/taskdefs/optional/ejb/JonasDeploymentTool.java @@ -679,7 +679,10 @@ public class JonasDeploymentTool extends GenericDeploymentTool { log("Looking for GenIC class in classpath: " + classpath.toString(), Project.MSG_VERBOSE); - AntClassLoader cl = classpath.getProject().createClassLoader(classpath); + AntClassLoader cl = null; + + try { + cl = classpath.getProject().createClassLoader(classpath); try { cl.loadClass(JonasDeploymentTool.GENIC_CLASS); @@ -716,6 +719,11 @@ public class JonasDeploymentTool extends GenericDeploymentTool { + "' not found in classpath.", Project.MSG_VERBOSE); } + } finally { + if (cl != null) { + cl.cleanup(); + } + } return null; } diff --git a/src/main/org/apache/tools/ant/taskdefs/optional/ejb/WebsphereDeploymentTool.java b/src/main/org/apache/tools/ant/taskdefs/optional/ejb/WebsphereDeploymentTool.java index 10f1712b0..ec721b556 100644 --- a/src/main/org/apache/tools/ant/taskdefs/optional/ejb/WebsphereDeploymentTool.java +++ b/src/main/org/apache/tools/ant/taskdefs/optional/ejb/WebsphereDeploymentTool.java @@ -27,6 +27,7 @@ import java.util.Iterator; import java.util.jar.JarEntry; import java.util.jar.JarFile; import java.util.jar.JarOutputStream; +import org.apache.tools.ant.AntClassLoader; import org.apache.tools.ant.BuildException; import org.apache.tools.ant.Project; import org.apache.tools.ant.taskdefs.Java; @@ -589,11 +590,9 @@ public class WebsphereDeploymentTool extends GenericDeploymentTool { classpath = getCombinedClasspath(); } + javaTask.setFork(true); if (classpath != null) { javaTask.setClasspath(classpath); - javaTask.setFork(true); - } else { - javaTask.setFork(true); } log("Calling websphere.ejbdeploy for " + sourceJar.toString(), @@ -684,6 +683,7 @@ public class WebsphereDeploymentTool extends GenericDeploymentTool { JarFile wasJar = null; File newwasJarFile = null; JarOutputStream newJarStream = null; + ClassLoader genericLoader = null; try { log("Checking if websphere Jar needs to be rebuilt for jar " @@ -713,7 +713,7 @@ public class WebsphereDeploymentTool extends GenericDeploymentTool { } //Cycle Through generic and make sure its in websphere - ClassLoader genericLoader = getClassLoaderFromJar(genericJarFile); + genericLoader = getClassLoaderFromJar(genericJarFile); for (Enumeration e = genericEntries.keys(); e.hasMoreElements();) { String filepath = (String) e.nextElement(); @@ -861,6 +861,11 @@ public class WebsphereDeploymentTool extends GenericDeploymentTool { rebuild = true; } } + if (genericLoader != null + && genericLoader instanceof AntClassLoader) { + AntClassLoader loader = (AntClassLoader) genericLoader; + loader.cleanup(); + } } return rebuild; @@ -889,4 +894,3 @@ public class WebsphereDeploymentTool extends GenericDeploymentTool { return getTask().getProject().createClassLoader(lookupPath); } } - diff --git a/src/main/org/apache/tools/ant/taskdefs/optional/javacc/JavaCC.java b/src/main/org/apache/tools/ant/taskdefs/optional/javacc/JavaCC.java index 22d25b544..427e1ff76 100644 --- a/src/main/org/apache/tools/ant/taskdefs/optional/javacc/JavaCC.java +++ b/src/main/org/apache/tools/ant/taskdefs/optional/javacc/JavaCC.java @@ -411,7 +411,9 @@ public class JavaCC extends Task { String packagePrefix = null; String mainClass = null; - AntClassLoader l = + AntClassLoader l = null; + try { + l = AntClassLoader.newAntClassLoader(null, null, path .concatSystemClasspath("ignore"), @@ -483,6 +485,11 @@ public class JavaCC extends Task { throw new BuildException("unknown task type " + type); } return packagePrefix + mainClass; + } finally { + if (l != null) { + l.cleanup(); + } + } } /** diff --git a/src/main/org/apache/tools/ant/taskdefs/optional/javah/JavahAdapterFactory.java b/src/main/org/apache/tools/ant/taskdefs/optional/javah/JavahAdapterFactory.java index 8fc55f0f4..bbb2edaa3 100644 --- a/src/main/org/apache/tools/ant/taskdefs/optional/javah/JavahAdapterFactory.java +++ b/src/main/org/apache/tools/ant/taskdefs/optional/javah/JavahAdapterFactory.java @@ -86,6 +86,7 @@ public class JavahAdapterFactory { return new SunJavah(); } else if (choice != null) { return resolveClassName(choice, + // Memory leak in line below log.getProject() .createClassLoader(classpath)); } diff --git a/src/main/org/apache/tools/ant/taskdefs/optional/jsp/JspC.java b/src/main/org/apache/tools/ant/taskdefs/optional/jsp/JspC.java index e570ad8e6..8e4819355 100644 --- a/src/main/org/apache/tools/ant/taskdefs/optional/jsp/JspC.java +++ b/src/main/org/apache/tools/ant/taskdefs/optional/jsp/JspC.java @@ -22,6 +22,7 @@ import java.io.File; import java.util.Date; import java.util.Enumeration; import java.util.Vector; +import org.apache.tools.ant.AntClassLoader; import org.apache.tools.ant.BuildException; import org.apache.tools.ant.DirectoryScanner; import org.apache.tools.ant.Project; @@ -436,10 +437,12 @@ public class JspC extends MatchingTask { File dest = getActualDestDir(); + AntClassLoader al = null; + try { //bind to a compiler JspCompilerAdapter compiler = JspCompilerAdapterFactory.getCompiler(compilerName, this, - getProject().createClassLoader(compilerClasspath)); + al = getProject().createClassLoader(compilerClasspath)); //if we are a webapp, hand off to the compiler, which had better handle it if (webApp != null) { @@ -503,6 +506,11 @@ public class JspC extends MatchingTask { log("all files are up to date", Project.MSG_VERBOSE); } } + } finally { + if (al != null) { + al.cleanup(); + } + } } /** diff --git a/src/main/org/apache/tools/ant/taskdefs/optional/jsp/compilers/JspCompilerAdapterFactory.java b/src/main/org/apache/tools/ant/taskdefs/optional/jsp/compilers/JspCompilerAdapterFactory.java index 071a9d54c..896a7a8d8 100644 --- a/src/main/org/apache/tools/ant/taskdefs/optional/jsp/compilers/JspCompilerAdapterFactory.java +++ b/src/main/org/apache/tools/ant/taskdefs/optional/jsp/compilers/JspCompilerAdapterFactory.java @@ -54,6 +54,7 @@ public final class JspCompilerAdapterFactory { public static JspCompilerAdapter getCompiler(String compilerType, Task task) throws BuildException { return getCompiler(compilerType, task, + // Memory-Leak in line below task.getProject().createClassLoader(null)); } diff --git a/src/main/org/apache/tools/ant/taskdefs/optional/junit/JUnitTask.java b/src/main/org/apache/tools/ant/taskdefs/optional/junit/JUnitTask.java index 33c182152..de4b924e5 100644 --- a/src/main/org/apache/tools/ant/taskdefs/optional/junit/JUnitTask.java +++ b/src/main/org/apache/tools/ant/taskdefs/optional/junit/JUnitTask.java @@ -1108,7 +1108,9 @@ public class JUnitTask extends Task { if (!cmd.haveClasspath()) { return; } - AntClassLoader loader = AntClassLoader.newAntClassLoader(null, + AntClassLoader loader = null; + try { + loader = AntClassLoader.newAntClassLoader(null, getProject(), cmd.createClasspath(getProject()), true); String projectResourceName = LoaderUtils.classNameToResource( @@ -1131,6 +1133,11 @@ public class JUnitTask extends Task { } catch (Exception ex) { // Ignore exception } + } finally { + if (loader != null) { + loader.cleanup(); + } + } } /** diff --git a/src/main/org/apache/tools/ant/taskdefs/optional/native2ascii/Native2AsciiAdapterFactory.java b/src/main/org/apache/tools/ant/taskdefs/optional/native2ascii/Native2AsciiAdapterFactory.java index e19ce7f36..e60a4b0c8 100644 --- a/src/main/org/apache/tools/ant/taskdefs/optional/native2ascii/Native2AsciiAdapterFactory.java +++ b/src/main/org/apache/tools/ant/taskdefs/optional/native2ascii/Native2AsciiAdapterFactory.java @@ -86,6 +86,7 @@ public class Native2AsciiAdapterFactory { return new SunNative2Ascii(); } else if (choice != null) { return resolveClassName(choice, + // Memory leak in line below log.getProject() .createClassLoader(classpath)); } diff --git a/src/main/org/apache/tools/ant/taskdefs/rmic/RmicAdapterFactory.java b/src/main/org/apache/tools/ant/taskdefs/rmic/RmicAdapterFactory.java index fc625b70f..8be90f937 100644 --- a/src/main/org/apache/tools/ant/taskdefs/rmic/RmicAdapterFactory.java +++ b/src/main/org/apache/tools/ant/taskdefs/rmic/RmicAdapterFactory.java @@ -118,6 +118,7 @@ public final class RmicAdapterFactory { } //no match? ask for the non-lower-cased type return resolveClassName(rmicType, + // Memory leak in line below task.getProject().createClassLoader(classpath)); } diff --git a/src/main/org/apache/tools/ant/types/Mapper.java b/src/main/org/apache/tools/ant/types/Mapper.java index ce2a9cf83..90396271f 100644 --- a/src/main/org/apache/tools/ant/types/Mapper.java +++ b/src/main/org/apache/tools/ant/types/Mapper.java @@ -261,6 +261,7 @@ public class Mapper extends DataType implements Cloneable { ClassLoader loader = (classpath == null) ? getClass().getClassLoader() + // Memory leak in line below : getProject().createClassLoader(classpath); return Class.forName(cName, true, loader); diff --git a/src/main/org/apache/tools/ant/types/XMLCatalog.java b/src/main/org/apache/tools/ant/types/XMLCatalog.java index 1d055712e..77a90d11e 100644 --- a/src/main/org/apache/tools/ant/types/XMLCatalog.java +++ b/src/main/org/apache/tools/ant/types/XMLCatalog.java @@ -500,7 +500,7 @@ public class XMLCatalog extends DataType if (catalogResolver == null) { AntClassLoader loader = null; - + // Memory-Leak in line below loader = getProject().createClassLoader(Path.systemClasspath); try { diff --git a/src/main/org/apache/tools/ant/types/selectors/ExtendSelector.java b/src/main/org/apache/tools/ant/types/selectors/ExtendSelector.java index 05bc265b7..6c29abe62 100644 --- a/src/main/org/apache/tools/ant/types/selectors/ExtendSelector.java +++ b/src/main/org/apache/tools/ant/types/selectors/ExtendSelector.java @@ -65,6 +65,7 @@ public class ExtendSelector extends BaseSelector { if (classpath == null) { c = Class.forName(classname); } else { + // Memory-Leak in line below AntClassLoader al = getProject().createClassLoader(classpath); c = Class.forName(classname, true, al); diff --git a/src/main/org/apache/tools/ant/types/selectors/modifiedselector/ModifiedSelector.java b/src/main/org/apache/tools/ant/types/selectors/modifiedselector/ModifiedSelector.java index 049e127e1..bb8ea42d5 100644 --- a/src/main/org/apache/tools/ant/types/selectors/modifiedselector/ModifiedSelector.java +++ b/src/main/org/apache/tools/ant/types/selectors/modifiedselector/ModifiedSelector.java @@ -646,6 +646,7 @@ public class ModifiedSelector extends BaseExtendSelector // the usual classloader ? getClass().getClassLoader() // additional use the provided classpath + // Memory leak in line below : getProject().createClassLoader(classpath); } return myClassLoader;