From 88d1823c2e1e4fb8b647a92646ce716619b8956a Mon Sep 17 00:00:00 2001 From: Andreas Dangel Date: Mon, 25 Jul 2016 21:25:34 +0200 Subject: [PATCH 1/4] Fixes #1508 [core] [java] PMD is leaking file handles Fixes InputStreams and Readers --- .../main/java/net/sourceforge/pmd/PMD.java | 12 ++++++++- .../net/sourceforge/pmd/RuleSetFactory.java | 9 ++++++- .../pmd/benchmark/Benchmarker.java | 25 +++++++++++-------- .../pmd/dcd/graph/UsageGraphBuilder.java | 8 +++++- .../sourceforge/pmd/util/database/DBType.java | 12 +++++++-- .../pmd/util/designer/Designer.java | 18 ++++++++----- .../typeresolution/PMDASMClassLoader.java | 18 ++++++++++--- 7 files changed, 77 insertions(+), 25 deletions(-) diff --git a/pmd-core/src/main/java/net/sourceforge/pmd/PMD.java b/pmd-core/src/main/java/net/sourceforge/pmd/PMD.java index 0eda16cd36..9c03be6a87 100644 --- a/pmd-core/src/main/java/net/sourceforge/pmd/PMD.java +++ b/pmd-core/src/main/java/net/sourceforge/pmd/PMD.java @@ -22,12 +22,20 @@ import java.util.logging.Handler; import java.util.logging.Level; import java.util.logging.Logger; +import org.apache.commons.io.IOUtils; + import net.sourceforge.pmd.benchmark.Benchmark; import net.sourceforge.pmd.benchmark.Benchmarker; import net.sourceforge.pmd.benchmark.TextReport; import net.sourceforge.pmd.cli.PMDCommandLineInterface; import net.sourceforge.pmd.cli.PMDParameters; -import net.sourceforge.pmd.lang.*; +import net.sourceforge.pmd.lang.Language; +import net.sourceforge.pmd.lang.LanguageFilenameFilter; +import net.sourceforge.pmd.lang.LanguageVersion; +import net.sourceforge.pmd.lang.LanguageVersionDiscoverer; +import net.sourceforge.pmd.lang.LanguageVersionHandler; +import net.sourceforge.pmd.lang.Parser; +import net.sourceforge.pmd.lang.ParserOptions; import net.sourceforge.pmd.processor.MonoThreadProcessor; import net.sourceforge.pmd.processor.MultiThreadProcessor; import net.sourceforge.pmd.renderers.Renderer; @@ -500,6 +508,8 @@ public class PMD { pmdVersion = properties.getProperty("version"); } catch (IOException e) { LOG.log(Level.FINE, "Couldn't determine version of PMD", e); + } finally { + IOUtils.closeQuietly(stream); } } if (pmdVersion == null) { diff --git a/pmd-core/src/main/java/net/sourceforge/pmd/RuleSetFactory.java b/pmd-core/src/main/java/net/sourceforge/pmd/RuleSetFactory.java index 6694785f5d..c720051bf1 100644 --- a/pmd-core/src/main/java/net/sourceforge/pmd/RuleSetFactory.java +++ b/pmd-core/src/main/java/net/sourceforge/pmd/RuleSetFactory.java @@ -30,6 +30,7 @@ import net.sourceforge.pmd.lang.rule.properties.factories.PropertyDescriptorUtil import net.sourceforge.pmd.util.ResourceLoader; import net.sourceforge.pmd.util.StringUtil; +import org.apache.commons.io.IOUtils; import org.w3c.dom.Document; import org.w3c.dom.Element; import org.w3c.dom.Node; @@ -287,6 +288,8 @@ public class RuleSetFactory { return classNotFoundProblem(ioe); } catch (SAXException se) { return classNotFoundProblem(se); + } finally { + IOUtils.closeQuietly(inputStream); } } @@ -622,9 +625,11 @@ public class RuleSetFactory { */ private boolean containsRule(RuleSetReferenceId ruleSetReferenceId, String ruleName) { boolean found = false; + InputStream ruleset = null; try { + ruleset = ruleSetReferenceId.getInputStream(classLoader); DocumentBuilder builder = DocumentBuilderFactory.newInstance().newDocumentBuilder(); - Document document = builder.parse(ruleSetReferenceId.getInputStream(classLoader)); + Document document = builder.parse(ruleset); Element ruleSetElement = document.getDocumentElement(); NodeList rules = ruleSetElement.getElementsByTagName("rule"); @@ -639,6 +644,8 @@ public class RuleSetFactory { } } catch (Exception e) { throw new RuntimeException(e); + } finally { + IOUtils.closeQuietly(ruleset); } return found; diff --git a/pmd-core/src/main/java/net/sourceforge/pmd/benchmark/Benchmarker.java b/pmd-core/src/main/java/net/sourceforge/pmd/benchmark/Benchmarker.java index 34f4b79d86..343a8f749b 100644 --- a/pmd-core/src/main/java/net/sourceforge/pmd/benchmark/Benchmarker.java +++ b/pmd-core/src/main/java/net/sourceforge/pmd/benchmark/Benchmarker.java @@ -130,11 +130,12 @@ public class Benchmarker { long start = System.currentTimeMillis(); for (DataSource dataSource: dataSources) { - parser.parse( - dataSource.getNiceFileName(false, null), - new InputStreamReader(dataSource.getInputStream() - ) - ); + InputStreamReader reader = new InputStreamReader(dataSource.getInputStream()); + try { + parser.parse(dataSource.getNiceFileName(false, null), reader); + } finally { + IOUtils.closeQuietly(reader); + } } if (debug) { @@ -169,13 +170,15 @@ public class Benchmarker { RuleContext ctx = new RuleContext(); long start = System.currentTimeMillis(); - Reader reader = null; for (DataSource dataSource: dataSources) { - reader = new InputStreamReader(dataSource.getInputStream()); - ctx.setSourceCodeFilename(dataSource.getNiceFileName(false, null)); - new SourceCodeProcessor(config).processSourceCode(reader, ruleSets, ctx); - IOUtils.closeQuietly(reader); - } + Reader reader = new InputStreamReader(dataSource.getInputStream()); + try { + ctx.setSourceCodeFilename(dataSource.getNiceFileName(false, null)); + new SourceCodeProcessor(config).processSourceCode(reader, ruleSets, ctx); + } finally { + IOUtils.closeQuietly(reader); + } + } long end = System.currentTimeMillis(); long elapsed = end - start; results.add(new RuleDuration(elapsed, rule)); diff --git a/pmd-core/src/main/java/net/sourceforge/pmd/dcd/graph/UsageGraphBuilder.java b/pmd-core/src/main/java/net/sourceforge/pmd/dcd/graph/UsageGraphBuilder.java index 45080e0c9c..1c831359b3 100644 --- a/pmd-core/src/main/java/net/sourceforge/pmd/dcd/graph/UsageGraphBuilder.java +++ b/pmd-core/src/main/java/net/sourceforge/pmd/dcd/graph/UsageGraphBuilder.java @@ -8,6 +8,8 @@ import java.io.InputStream; import java.util.ArrayList; import java.util.Arrays; import java.util.List; + +import org.apache.commons.io.IOUtils; import org.objectweb.asm.AnnotationVisitor; import org.objectweb.asm.Attribute; import org.objectweb.asm.ClassReader; @@ -51,7 +53,11 @@ public class UsageGraphBuilder { InputStream inputStream = this.getClass().getClassLoader().getResourceAsStream( classResourceName + ".class"); ClassReader classReader = new ClassReader(inputStream); - classReader.accept(getNewClassVisitor(), 0); + try { + classReader.accept(getNewClassVisitor(), 0); + } finally { + IOUtils.closeQuietly(inputStream); + } } } } catch (IOException e) { diff --git a/pmd-core/src/main/java/net/sourceforge/pmd/util/database/DBType.java b/pmd-core/src/main/java/net/sourceforge/pmd/util/database/DBType.java index 3f3088833d..0e0a4cd3d7 100644 --- a/pmd-core/src/main/java/net/sourceforge/pmd/util/database/DBType.java +++ b/pmd-core/src/main/java/net/sourceforge/pmd/util/database/DBType.java @@ -7,12 +7,15 @@ import java.io.File; import java.io.FileInputStream; import java.io.FileNotFoundException; import java.io.IOException; +import java.io.InputStream; import java.util.Properties; import java.util.PropertyResourceBundle; import java.util.ResourceBundle; import java.util.logging.Level; import java.util.logging.Logger; +import org.apache.commons.io.IOUtils; + /** * Encapsulate the settings needed to access database source code. * @@ -155,6 +158,7 @@ public class DBType { LOGGER.entering(CLASS_NAME, matchString); // Locale locale = Control.g; ResourceBundle resourceBundle = null; + InputStream stream = null; if (LOGGER.isLoggable(Level.FINEST)) { LOGGER.finest("class_path+" + System.getProperty("java.class.path")); @@ -170,7 +174,8 @@ public class DBType { if (LOGGER.isLoggable(Level.FINEST)) { LOGGER.finest("Attempting File no file suffix: " + matchString); } - resourceBundle = new PropertyResourceBundle(new FileInputStream(propertiesFile)); + stream = new FileInputStream(propertiesFile); + resourceBundle = new PropertyResourceBundle(stream); propertiesSource = propertiesFile.getAbsolutePath(); LOGGER.finest("FileSystemWithoutExtension"); } catch (FileNotFoundException notFoundOnFilesystemWithoutExtension) { @@ -180,7 +185,8 @@ public class DBType { } try { File propertiesFile = new File(matchString + ".properties"); - resourceBundle = new PropertyResourceBundle(new FileInputStream(propertiesFile)); + stream = new FileInputStream(propertiesFile); + resourceBundle = new PropertyResourceBundle(stream); propertiesSource = propertiesFile.getAbsolutePath(); LOGGER.finest("FileSystemWithExtension"); } catch (FileNotFoundException notFoundOnFilesystemWithExtensionTackedOn) { @@ -207,6 +213,8 @@ public class DBType { } } } + } finally { + IOUtils.closeQuietly(stream); } // Properties in this matched resource diff --git a/pmd-core/src/main/java/net/sourceforge/pmd/util/designer/Designer.java b/pmd-core/src/main/java/net/sourceforge/pmd/util/designer/Designer.java index 10c93de09b..0d8cef6cb4 100644 --- a/pmd-core/src/main/java/net/sourceforge/pmd/util/designer/Designer.java +++ b/pmd-core/src/main/java/net/sourceforge/pmd/util/designer/Designer.java @@ -22,6 +22,7 @@ import java.io.File; import java.io.FileInputStream; import java.io.FileWriter; import java.io.IOException; +import java.io.InputStream; import java.io.StringReader; import java.io.StringWriter; import java.lang.reflect.Proxy; @@ -90,6 +91,12 @@ import javax.xml.transform.TransformerFactory; import javax.xml.transform.dom.DOMSource; import javax.xml.transform.stream.StreamResult; +import org.apache.commons.io.IOUtils; +import org.w3c.dom.Document; +import org.w3c.dom.Element; +import org.w3c.dom.Text; +import org.xml.sax.SAXException; + import net.sourceforge.pmd.PMD; import net.sourceforge.pmd.PMDConfiguration; import net.sourceforge.pmd.RuleContext; @@ -114,11 +121,6 @@ import net.sourceforge.pmd.lang.symboltable.ScopedNode; import net.sourceforge.pmd.lang.xpath.Initializer; import net.sourceforge.pmd.util.StringUtil; -import org.w3c.dom.Document; -import org.w3c.dom.Element; -import org.w3c.dom.Text; -import org.xml.sax.SAXException; - public class Designer implements ClipboardOwner { private int getDefaultLanguageVersionSelectionIndex() { @@ -953,11 +955,13 @@ public class Designer implements ClipboardOwner { + System.getProperty("file.separator") + ".pmd_designer.xml"; private void loadSettings() { + InputStream stream = null; try { File file = new File(SETTINGS_FILE_NAME); if (file.exists()) { DocumentBuilder builder = DocumentBuilderFactory.newInstance().newDocumentBuilder(); - Document document = builder.parse(new FileInputStream(file)); + stream = new FileInputStream(file); + Document document = builder.parse(stream); Element settingsElement = document.getDocumentElement(); Element codeElement = (Element) settingsElement.getElementsByTagName("code").item(0); Element xpathElement = (Element) settingsElement.getElementsByTagName("xpath").item(0); @@ -984,6 +988,8 @@ public class Designer implements ClipboardOwner { e.printStackTrace(); } catch (SAXException e) { e.printStackTrace(); + } finally { + IOUtils.closeQuietly(stream); } } diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/PMDASMClassLoader.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/PMDASMClassLoader.java index 8647325007..f6ff913d6e 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/PMDASMClassLoader.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/PMDASMClassLoader.java @@ -4,6 +4,7 @@ package net.sourceforge.pmd.lang.java.typeresolution; import java.io.IOException; +import java.io.InputStream; import java.util.ArrayList; import java.util.HashSet; import java.util.List; @@ -12,6 +13,7 @@ import java.util.Set; import net.sourceforge.pmd.lang.java.typeresolution.visitors.PMDASMVisitor; +import org.apache.commons.io.IOUtils; import org.objectweb.asm.ClassReader; /* @@ -80,8 +82,10 @@ public final class PMDASMClassLoader extends ClassLoader { if (dontBother.contains(name)) { throw new ClassNotFoundException(name); } + InputStream stream = null; try { - ClassReader reader = new ClassReader(getResourceAsStream(name.replace('.', '/') + ".class")); + stream = getResourceAsStream(name.replace('.', '/') + ".class"); + ClassReader reader = new ClassReader(stream); PMDASMVisitor asmVisitor = new PMDASMVisitor(); reader.accept(asmVisitor, 0); @@ -90,14 +94,22 @@ public final class PMDASMClassLoader extends ClassLoader { inner = new ArrayList(inner); // to avoid // ConcurrentModificationException for (String str : inner) { - reader = new ClassReader(getResourceAsStream(str.replace('.', '/') + ".class")); - reader.accept(asmVisitor, 0); + InputStream innerClassStream = null; + try { + innerClassStream = getResourceAsStream(str.replace('.', '/') + ".class"); + reader = new ClassReader(innerClassStream); + reader.accept(asmVisitor, 0); + } finally { + IOUtils.closeQuietly(innerClassStream); + } } } return asmVisitor.getPackages(); } catch (IOException e) { dontBother.add(name); throw new ClassNotFoundException(name, e); + } finally { + IOUtils.closeQuietly(stream); } } } \ No newline at end of file From 293731f009f5c0f274216d118487769ef0fd0a20 Mon Sep 17 00:00:00 2001 From: Andreas Dangel Date: Mon, 25 Jul 2016 21:34:40 +0200 Subject: [PATCH 2/4] Fixes #1508 [core] [java] PMD is leaking file handles Closes class loader after ant task is finished --- .../pmd/ant/internal/PMDTaskImpl.java | 24 ++++++++++++------- 1 file changed, 16 insertions(+), 8 deletions(-) diff --git a/pmd-core/src/main/java/net/sourceforge/pmd/ant/internal/PMDTaskImpl.java b/pmd-core/src/main/java/net/sourceforge/pmd/ant/internal/PMDTaskImpl.java index e5bd978549..0c91f74439 100644 --- a/pmd-core/src/main/java/net/sourceforge/pmd/ant/internal/PMDTaskImpl.java +++ b/pmd-core/src/main/java/net/sourceforge/pmd/ant/internal/PMDTaskImpl.java @@ -3,6 +3,7 @@ */ package net.sourceforge.pmd.ant.internal; +import java.io.Closeable; import java.io.File; import java.io.IOException; import java.io.PrintWriter; @@ -14,6 +15,14 @@ import java.util.concurrent.atomic.AtomicInteger; import java.util.logging.Handler; import java.util.logging.Level; +import org.apache.commons.io.IOUtils; +import org.apache.tools.ant.AntClassLoader; +import org.apache.tools.ant.BuildException; +import org.apache.tools.ant.DirectoryScanner; +import org.apache.tools.ant.Project; +import org.apache.tools.ant.types.FileSet; +import org.apache.tools.ant.types.Path; + import net.sourceforge.pmd.PMD; import net.sourceforge.pmd.PMDConfiguration; import net.sourceforge.pmd.Report; @@ -37,14 +46,6 @@ import net.sourceforge.pmd.util.datasource.FileDataSource; import net.sourceforge.pmd.util.log.AntLogHandler; import net.sourceforge.pmd.util.log.ScopedLogHandlersManager; -import org.apache.commons.io.IOUtils; -import org.apache.tools.ant.AntClassLoader; -import org.apache.tools.ant.BuildException; -import org.apache.tools.ant.DirectoryScanner; -import org.apache.tools.ant.Project; -import org.apache.tools.ant.types.FileSet; -import org.apache.tools.ant.types.Path; - public class PMDTaskImpl { private Path classpath; @@ -267,6 +268,13 @@ public class PMDTaskImpl { doTask(); } finally { logManager.close(); + tryCloseClassLoader(configuration.getClassLoader()); + } + } + + private void tryCloseClassLoader(ClassLoader classLoader) { + if (classLoader instanceof Closeable) { + IOUtils.closeQuietly((Closeable)classLoader); } } From d76b689c0714550dfd0daff2f156b33558bac499 Mon Sep 17 00:00:00 2001 From: Andreas Dangel Date: Mon, 25 Jul 2016 21:42:21 +0200 Subject: [PATCH 3/4] Fixes #1508 [core] [java] PMD is leaking file handles Closes class loader after PMD has processed all files --- pmd-core/src/main/java/net/sourceforge/pmd/PMD.java | 2 ++ .../net/sourceforge/pmd/ant/internal/PMDTaskImpl.java | 10 ++-------- .../src/main/java/net/sourceforge/pmd/util/IOUtil.java | 10 ++++++++++ 3 files changed, 14 insertions(+), 8 deletions(-) diff --git a/pmd-core/src/main/java/net/sourceforge/pmd/PMD.java b/pmd-core/src/main/java/net/sourceforge/pmd/PMD.java index 9c03be6a87..c0efe783ec 100644 --- a/pmd-core/src/main/java/net/sourceforge/pmd/PMD.java +++ b/pmd-core/src/main/java/net/sourceforge/pmd/PMD.java @@ -361,6 +361,8 @@ public class PMD { } else { new MonoThreadProcessor(configuration).processFiles(ruleSetFactory, files, ctx, renderers); } + + IOUtil.tryCloseClassLoader(configuration.getClassLoader()); } private static void sortFiles(final PMDConfiguration configuration, final List files) { diff --git a/pmd-core/src/main/java/net/sourceforge/pmd/ant/internal/PMDTaskImpl.java b/pmd-core/src/main/java/net/sourceforge/pmd/ant/internal/PMDTaskImpl.java index 0c91f74439..6ccb8e7008 100644 --- a/pmd-core/src/main/java/net/sourceforge/pmd/ant/internal/PMDTaskImpl.java +++ b/pmd-core/src/main/java/net/sourceforge/pmd/ant/internal/PMDTaskImpl.java @@ -3,7 +3,6 @@ */ package net.sourceforge.pmd.ant.internal; -import java.io.Closeable; import java.io.File; import java.io.IOException; import java.io.PrintWriter; @@ -40,6 +39,7 @@ import net.sourceforge.pmd.lang.LanguageRegistry; import net.sourceforge.pmd.lang.LanguageVersion; import net.sourceforge.pmd.renderers.AbstractRenderer; import net.sourceforge.pmd.renderers.Renderer; +import net.sourceforge.pmd.util.IOUtil; import net.sourceforge.pmd.util.StringUtil; import net.sourceforge.pmd.util.datasource.DataSource; import net.sourceforge.pmd.util.datasource.FileDataSource; @@ -268,13 +268,7 @@ public class PMDTaskImpl { doTask(); } finally { logManager.close(); - tryCloseClassLoader(configuration.getClassLoader()); - } - } - - private void tryCloseClassLoader(ClassLoader classLoader) { - if (classLoader instanceof Closeable) { - IOUtils.closeQuietly((Closeable)classLoader); + IOUtil.tryCloseClassLoader(configuration.getClassLoader()); } } diff --git a/pmd-core/src/main/java/net/sourceforge/pmd/util/IOUtil.java b/pmd-core/src/main/java/net/sourceforge/pmd/util/IOUtil.java index 37f6cb2705..e2d21037d2 100644 --- a/pmd-core/src/main/java/net/sourceforge/pmd/util/IOUtil.java +++ b/pmd-core/src/main/java/net/sourceforge/pmd/util/IOUtil.java @@ -5,12 +5,15 @@ package net.sourceforge.pmd.util; import java.io.BufferedReader; import java.io.BufferedWriter; +import java.io.Closeable; import java.io.FileWriter; import java.io.IOException; import java.io.OutputStreamWriter; import java.io.Reader; import java.io.Writer; +import org.apache.commons.io.IOUtils; + /** * * @author Brian Remedios @@ -45,4 +48,11 @@ public final class IOUtil { } return in; } + + public static void tryCloseClassLoader(ClassLoader classLoader) { + if (classLoader instanceof Closeable) { + IOUtils.closeQuietly((Closeable)classLoader); + } + } + } From 65708ef704c472afb526aa88b0db87be2e29685c Mon Sep 17 00:00:00 2001 From: Andreas Dangel Date: Tue, 26 Jul 2016 21:39:03 +0200 Subject: [PATCH 4/4] Update changelog --- src/site/markdown/overview/changelog.md | 1 + 1 file changed, 1 insertion(+) diff --git a/src/site/markdown/overview/changelog.md b/src/site/markdown/overview/changelog.md index 9134a03d26..80bdd77a2e 100644 --- a/src/site/markdown/overview/changelog.md +++ b/src/site/markdown/overview/changelog.md @@ -28,6 +28,7 @@ * General * [#1481](https://sourceforge.net/p/pmd/bugs/1481/): no problems found results in blank file instead of empty xml * [#1499](https://sourceforge.net/p/pmd/bugs/1499/): \[core] CPD test break PMD 5.5.1 build on Windows + * [#1508](https://sourceforge.net/p/pmd/bugs/1508/): \[core] \[java] PMD is leaking file handles **API Changes:**