From eb5cc0097ef36bfe62ed5db00dce8fa34971bcfc Mon Sep 17 00:00:00 2001 From: "Jason P. Leasure" Date: Thu, 2 Jul 2020 18:28:42 -0400 Subject: [PATCH] fix leaks caused by Files.list Stream not being closed --- .../plugin/core/osgi/GhidraSourceBundle.java | 34 ++++++++++++------- .../ghidra/app/script/GhidraScriptUtil.java | 7 ++-- .../AbstractGhidraScriptMgrPluginTest.java | 34 +++++++++++-------- 3 files changed, 45 insertions(+), 30 deletions(-) diff --git a/Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/osgi/GhidraSourceBundle.java b/Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/osgi/GhidraSourceBundle.java index 8fc63a807b..20726640f1 100644 --- a/Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/osgi/GhidraSourceBundle.java +++ b/Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/osgi/GhidraSourceBundle.java @@ -27,6 +27,7 @@ import java.util.jar.Attributes; import java.util.jar.Manifest; import java.util.regex.Pattern; import java.util.stream.Collectors; +import java.util.stream.Stream; import javax.tools.*; import javax.tools.JavaFileObject.Kind; @@ -952,19 +953,26 @@ public class GhidraSourceBundle extends GhidraBundle { * @throws IOException if there's a problem listing files */ ClassMapper(Path directory) throws IOException { - classToClassFilesMap = Files.exists(directory) ? Files.list(directory) - .filter(f -> Files.isRegularFile(f) && - f.getFileName().toString().endsWith(".class")) - .collect(groupingBy(f -> { - String fileName = f.getFileName().toString(); - // if f is the class file of an inner class, use the class name - int money = fileName.indexOf('$'); - if (money >= 0) { - return fileName.substring(0, money); - } - // drop ".class" - return fileName.substring(0, fileName.length() - 6); - })) : Collections.emptyMap(); + if (Files.exists(directory)) { + try (Stream pathStream = Files.list(directory)) { + classToClassFilesMap = pathStream + .filter(f -> Files.isRegularFile(f) && + f.getFileName().toString().endsWith(".class")) + .collect(groupingBy(f -> { + String fileName = f.getFileName().toString(); + // if f is the class file of an inner class, use the class name + int money = fileName.indexOf('$'); + if (money >= 0) { + return fileName.substring(0, money); + } + // drop ".class" + return fileName.substring(0, fileName.length() - 6); + })); + } + } + else { + classToClassFilesMap = Collections.emptyMap(); + } } List findAndRemove(ResourceFile sourceFile) { diff --git a/Ghidra/Features/Base/src/main/java/ghidra/app/script/GhidraScriptUtil.java b/Ghidra/Features/Base/src/main/java/ghidra/app/script/GhidraScriptUtil.java index 87ad50c74c..7986946cbc 100644 --- a/Ghidra/Features/Base/src/main/java/ghidra/app/script/GhidraScriptUtil.java +++ b/Ghidra/Features/Base/src/main/java/ghidra/app/script/GhidraScriptUtil.java @@ -18,9 +18,11 @@ package ghidra.app.script; import java.io.File; import java.io.IOException; import java.nio.file.Files; +import java.nio.file.Path; import java.util.*; import java.util.concurrent.atomic.AtomicInteger; import java.util.stream.Collectors; +import java.util.stream.Stream; import generic.jar.ResourceFile; import ghidra.app.plugin.core.osgi.BundleHost; @@ -230,9 +232,8 @@ public class GhidraScriptUtil { */ @Deprecated public static List getExplodedCompiledSourceBundlePaths() { - try { - return Files.list(BundleHost.getOsgiDir()) - .filter(Files::isDirectory) + try (Stream pathStream = Files.list(BundleHost.getOsgiDir())) { + return pathStream.filter(Files::isDirectory) .map(x -> new ResourceFile(x.toFile())) .collect(Collectors.toList()); } diff --git a/Ghidra/Features/Base/src/test.slow/java/ghidra/app/plugin/core/script/AbstractGhidraScriptMgrPluginTest.java b/Ghidra/Features/Base/src/test.slow/java/ghidra/app/plugin/core/script/AbstractGhidraScriptMgrPluginTest.java index e717fe449d..010963ace4 100644 --- a/Ghidra/Features/Base/src/test.slow/java/ghidra/app/plugin/core/script/AbstractGhidraScriptMgrPluginTest.java +++ b/Ghidra/Features/Base/src/test.slow/java/ghidra/app/plugin/core/script/AbstractGhidraScriptMgrPluginTest.java @@ -27,6 +27,7 @@ import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicInteger; import java.util.concurrent.atomic.AtomicReference; import java.util.stream.Collectors; +import java.util.stream.Stream; import javax.swing.*; import javax.swing.table.TableModel; @@ -181,7 +182,7 @@ public abstract class AbstractGhidraScriptMgrPluginTest protected static void wipe(Path path) throws IOException { if (Files.exists(path)) { for (Path p : (Iterable) Files.walk(path) - .sorted(Comparator.reverseOrder())::iterator) { + .sorted(Comparator.reverseOrder())::iterator) { Files.deleteIfExists(p); } } @@ -189,8 +190,10 @@ public abstract class AbstractGhidraScriptMgrPluginTest protected void wipeUserScripts() throws IOException { Path userScriptDir = java.nio.file.Paths.get(GhidraScriptUtil.USER_SCRIPTS_DIR); - for (Path p : (Iterable) Files.list(userScriptDir)::iterator) { - wipe(p); + try (Stream pathStream = Files.list(userScriptDir)) { + for (Path p : (Iterable) pathStream::iterator) { + wipe(p); + } } } @@ -392,7 +395,8 @@ public abstract class AbstractGhidraScriptMgrPluginTest assertNotNull(editor); - editorTextArea = (JTextArea) findComponentByName(editor.getComponent(), GhidraScriptEditorComponentProvider.EDITOR_COMPONENT_NAME); + editorTextArea = (JTextArea) findComponentByName(editor.getComponent(), + GhidraScriptEditorComponentProvider.EDITOR_COMPONENT_NAME); assertNotNull(editorTextArea); buffer = new StringBuffer(editorTextArea.getText()); @@ -411,7 +415,8 @@ public abstract class AbstractGhidraScriptMgrPluginTest // initialize our editor variable to the newly opened editor editor = waitForComponentProvider(GhidraScriptEditorComponentProvider.class); - editorTextArea = (JTextArea) findComponentByName(editor.getComponent(), GhidraScriptEditorComponentProvider.EDITOR_COMPONENT_NAME); + editorTextArea = (JTextArea) findComponentByName(editor.getComponent(), + GhidraScriptEditorComponentProvider.EDITOR_COMPONENT_NAME); waitForSwing(); @@ -686,8 +691,8 @@ public abstract class AbstractGhidraScriptMgrPluginTest "Contents of file on disk do not match that of the editor after performing " + "a save operation: " + file); printChars(expectedContents, fileText); - Assert - .fail("Contents of file on disk do not match that of the editor after performing " + + Assert.fail( + "Contents of file on disk do not match that of the editor after performing " + "a save operation: " + file); } // @@ -849,8 +854,8 @@ public abstract class AbstractGhidraScriptMgrPluginTest Map editorMap = provider.getEditorMap(); GhidraScriptEditorComponentProvider fileEditor = editorMap.get(file); - final JTextArea textArea = - (JTextArea) findComponentByName(fileEditor.getComponent(), GhidraScriptEditorComponentProvider.EDITOR_COMPONENT_NAME); + final JTextArea textArea = (JTextArea) findComponentByName(fileEditor.getComponent(), + GhidraScriptEditorComponentProvider.EDITOR_COMPONENT_NAME); assertNotNull(textArea); final String[] box = new String[1]; @@ -985,10 +990,10 @@ public abstract class AbstractGhidraScriptMgrPluginTest // destroy any NewScriptxxx files...and Temp ones too List paths = provider.getBundleHost() - .getBundleFiles() - .stream() - .filter(ResourceFile::isDirectory) - .collect(Collectors.toList()); + .getBundleFiles() + .stream() + .filter(ResourceFile::isDirectory) + .collect(Collectors.toList()); for (ResourceFile path : paths) { File file = path.getFile(false); @@ -1494,7 +1499,8 @@ public abstract class AbstractGhidraScriptMgrPluginTest protected JTextComponent grabScriptEditorTextArea() { GhidraScriptEditorComponentProvider scriptEditor = grabScriptEditor(); - JTextArea textArea = (JTextArea) findComponentByName(scriptEditor.getComponent(), GhidraScriptEditorComponentProvider.EDITOR_COMPONENT_NAME); + JTextArea textArea = (JTextArea) findComponentByName(scriptEditor.getComponent(), + GhidraScriptEditorComponentProvider.EDITOR_COMPONENT_NAME); assertNotNull(textArea); return textArea; }