changeset 1320:ccd7a1d6f9d0

7901666: Reader/Writer leak in BenchmarkGenerator
author shade
date Tue, 03 May 2016 19:19:20 +0300
parents 39ed8b3c11ce
children 448bd18c85f6
files jmh-core/src/main/java/org/openjdk/jmh/generators/core/BenchmarkGenerator.java jmh-core/src/main/java/org/openjdk/jmh/util/FileUtils.java
diffstat 2 files changed, 82 insertions(+), 89 deletions(-) [+]
line wrap: on
line diff
--- a/jmh-core/src/main/java/org/openjdk/jmh/generators/core/BenchmarkGenerator.java	Fri Apr 01 10:01:30 2016 +0300
+++ b/jmh-core/src/main/java/org/openjdk/jmh/generators/core/BenchmarkGenerator.java	Tue May 03 19:19:20 2016 +0300
@@ -128,82 +128,96 @@
         compilerControl.finish(source, destination);
 
         // Processing completed, final round.
+        // Collect all benchmark entries here
+        Set<BenchmarkListEntry> entries = new HashSet<BenchmarkListEntry>();
+
+        // Try to read the benchmark entries from the previous generator sessions.
+        // Incremental compilation may add or remove @Benchmark entries. New entries
+        // are discovered and added from the current compilation session. It is harder
+        // to detect removed @Benchmark entries. To do so, we are overwriting all benchmark
+        // records that belong to a current compilation unit.
+        Multimap<String, BenchmarkListEntry> entriesByQName = new HashMultimap<String, BenchmarkListEntry>();
         try {
-            // Collect all benchmark entries here
-            Set<BenchmarkListEntry> entries = new HashSet<BenchmarkListEntry>();
+            for (String line : readBenchmarkList(destination)) {
+                BenchmarkListEntry br = new BenchmarkListEntry(line);
+                entries.add(br);
+                entriesByQName.put(br.getUserClassQName(), br);
+            }
+        } catch (UnsupportedOperationException e) {
+            destination.printWarning("Unable to read the existing benchmark list, because of UnsupportedOperationException. Run on JDK 7 or higher.");
+        }
 
-            // Try to read the benchmark entries from the previous generator sessions.
-            // Incremental compilation may add or remove @Benchmark entries. New entries
-            // are discovered and added from the current compilation session. It is harder
-            // to detect removed @Benchmark entries. To do so, we are overwriting all benchmark
-            // records that belong to a current compilation unit.
-            Multimap<String, BenchmarkListEntry> entriesByQName = new HashMultimap<String, BenchmarkListEntry>();
+        // Generate new benchmark entries
+        for (BenchmarkInfo info : benchmarkInfos) {
             try {
-                Reader reader = destination.getResource(BenchmarkList.BENCHMARK_LIST.substring(1));
-                Collection<String> existingLines = FileUtils.readAllLines(reader);
-                for (String line : existingLines) {
-                    BenchmarkListEntry br = new BenchmarkListEntry(line);
+                MethodGroup group = info.methodGroup;
+                for (Mode m : group.getModes()) {
+                    BenchmarkListEntry br = new BenchmarkListEntry(
+                            info.userClassQName,
+                            info.generatedClassQName,
+                            group.getName(),
+                            m,
+                            group.getThreads(),
+                            group.getTotalThreadCount(),
+                            group.getWarmupIterations(),
+                            group.getWarmupTime(),
+                            group.getWarmupBatchSize(),
+                            group.getMeasurementIterations(),
+                            group.getMeasurementTime(),
+                            group.getMeasurementBatchSize(),
+                            group.getForks(),
+                            group.getWarmupForks(),
+                            group.getJvm(),
+                            group.getJvmArgs(),
+                            group.getJvmArgsPrepend(),
+                            group.getJvmArgsAppend(),
+                            group.getParams(),
+                            group.getOutputTimeUnit(),
+                            group.getOperationsPerInvocation(),
+                            group.getTimeout()
+                    );
+
+                    if (entriesByQName.keys().contains(info.userClassQName)) {
+                        destination.printNote("Benchmark entries for " + info.userClassQName + " already exist, overwriting");
+                        entries.removeAll(entriesByQName.get(info.userClassQName));
+                        entriesByQName.remove(info.userClassQName);
+                    }
+
                     entries.add(br);
-                    entriesByQName.put(br.getUserClassQName(), br);
                 }
-            } catch (IOException e) {
-                // Expected in most cases, move on.
-            } catch (UnsupportedOperationException e) {
-                destination.printWarning("Unable to read the existing benchmark list, because of UnsupportedOperationException. Run on JDK 7 or higher.");
+            } catch (GenerationException ge) {
+                destination.printError(ge.getMessage(), ge.getElement());
             }
+        }
 
-            // Generate new benchmark entries
-            for (BenchmarkInfo info : benchmarkInfos) {
-                try {
-                    MethodGroup group = info.methodGroup;
-                    for (Mode m : group.getModes()) {
-                        BenchmarkListEntry br = new BenchmarkListEntry(
-                                info.userClassQName,
-                                info.generatedClassQName,
-                                group.getName(),
-                                m,
-                                group.getThreads(),
-                                group.getTotalThreadCount(),
-                                group.getWarmupIterations(),
-                                group.getWarmupTime(),
-                                group.getWarmupBatchSize(),
-                                group.getMeasurementIterations(),
-                                group.getMeasurementTime(),
-                                group.getMeasurementBatchSize(),
-                                group.getForks(),
-                                group.getWarmupForks(),
-                                group.getJvm(),
-                                group.getJvmArgs(),
-                                group.getJvmArgsPrepend(),
-                                group.getJvmArgsAppend(),
-                                group.getParams(),
-                                group.getOutputTimeUnit(),
-                                group.getOperationsPerInvocation(),
-                                group.getTimeout()
-                        );
+        writeBenchmarkList(destination, entries);
+    }
 
-                        if (entriesByQName.keys().contains(info.userClassQName)) {
-                            destination.printNote("Benchmark entries for " + info.userClassQName + " already exist, overwriting");
-                            entries.removeAll(entriesByQName.get(info.userClassQName));
-                            entriesByQName.remove(info.userClassQName);
-                        }
+    private Collection<String> readBenchmarkList(GeneratorDestination destination) {
+        Reader reader = null;
+        try {
+            reader = destination.getResource(BenchmarkList.BENCHMARK_LIST.substring(1));
+            return FileUtils.readAllLines(reader);
+        } catch (IOException e) {
+            // okay, move on
+        } finally {
+            FileUtils.safelyClose(reader);
+        }
+        return Collections.emptyList();
+    }
 
-                        entries.add(br);
-                    }
-                } catch (GenerationException ge) {
-                    destination.printError(ge.getMessage(), ge.getElement());
-                }
-            }
-
+    private void writeBenchmarkList(GeneratorDestination destination, Collection<BenchmarkListEntry> entries) {
+        PrintWriter writer = null;
+        try {
             // Write out the complete benchmark list
-            PrintWriter writer = new PrintWriter(destination.newResource(BenchmarkList.BENCHMARK_LIST.substring(1)));
+            writer = new PrintWriter(destination.newResource(BenchmarkList.BENCHMARK_LIST.substring(1)));
             for (BenchmarkListEntry entry : entries) {
                 writer.println(entry.toLine());
             }
-            writer.close();
-
         } catch (IOException ex) {
             destination.printError("Error writing benchmark list", ex);
+        } finally {
+            FileUtils.safelyClose(writer);
         }
     }
 
--- a/jmh-core/src/main/java/org/openjdk/jmh/util/FileUtils.java	Fri Apr 01 10:01:30 2016 +0300
+++ b/jmh-core/src/main/java/org/openjdk/jmh/util/FileUtils.java	Tue May 03 19:19:20 2016 +0300
@@ -25,7 +25,6 @@
 package org.openjdk.jmh.util;
 
 import java.io.*;
-import java.nio.channels.Channel;
 import java.util.ArrayList;
 import java.util.Collection;
 import java.util.LinkedList;
@@ -183,45 +182,25 @@
         }
     }
 
-    public static void safelyClose(OutputStream out) {
-        if (out != null) {
+    public static <T extends Flushable & Closeable> void safelyClose(T obj) {
+        if (obj != null) {
             try {
-                out.flush();
+                obj.flush();
             } catch (IOException e) {
                 // ignore
             }
             try {
-                out.close();
+                obj.close();
             } catch (IOException e) {
                 // ignore
             }
         }
     }
 
-    public static void safelyClose(InputStream in) {
-        if (in != null) {
+    public static <T extends Closeable> void safelyClose(T obj) {
+        if (obj != null) {
             try {
-                in.close();
-            } catch (IOException e) {
-                // ignore
-            }
-        }
-    }
-
-    public static void safelyClose(Channel channel) {
-        if (channel != null) {
-            try {
-                channel.close();
-            } catch (IOException e) {
-                // do nothing
-            }
-        }
-    }
-
-    public static void safelyClose(Reader reader) {
-        if (reader != null) {
-            try {
-                reader.close();
+                obj.close();
             } catch (IOException e) {
                 // do nothing
             }