#190 make sure, InputStream is always closed after reading gc log file

chewiebug · Sep 8, 2017 · 60d19ae · 60d19ae
1 parent 31f0add
commit 60d19ae
Show file tree

Hide file tree

Showing 13 changed files with 72 additions and 111 deletions.
diff --git a/src/main/java/com/tagtraum/perf/gcviewer/imp/DataReaderFacade.java b/src/main/java/com/tagtraum/perf/gcviewer/imp/DataReaderFacade.java
@@ -1,14 +1,5 @@
 package com.tagtraum.perf.gcviewer.imp;
 
-import com.tagtraum.perf.gcviewer.ctrl.impl.GcSeriesLoader;
-import com.tagtraum.perf.gcviewer.model.GCModel;
-import com.tagtraum.perf.gcviewer.model.GCResource;
-import com.tagtraum.perf.gcviewer.model.GcResourceFile;
-import com.tagtraum.perf.gcviewer.model.GcResourceSeries;
-import com.tagtraum.perf.gcviewer.util.BuildInfoReader;
-import com.tagtraum.perf.gcviewer.util.HttpUrlConnectionHelper;
-import com.tagtraum.perf.gcviewer.util.LocalisationHelper;
-
 import java.beans.PropertyChangeListener;
 import java.io.File;
 import java.io.IOException;
@@ -21,6 +12,15 @@
 import java.util.concurrent.atomic.AtomicLong;
 import java.util.logging.Logger;
 
+import com.tagtraum.perf.gcviewer.ctrl.impl.GcSeriesLoader;
+import com.tagtraum.perf.gcviewer.model.GCModel;
+import com.tagtraum.perf.gcviewer.model.GCResource;
+import com.tagtraum.perf.gcviewer.model.GcResourceFile;
+import com.tagtraum.perf.gcviewer.model.GcResourceSeries;
+import com.tagtraum.perf.gcviewer.util.BuildInfoReader;
+import com.tagtraum.perf.gcviewer.util.HttpUrlConnectionHelper;
+import com.tagtraum.perf.gcviewer.util.LocalisationHelper;
+
 /**
  * DataReaderFacade is a helper class providing a simple interface to read a gc log file
  * including standard error handling.
@@ -103,34 +103,46 @@ private GCModel readModel(GcResourceFile gcResource) throws IOException {
         URL url = gcResource.getResourceNameAsUrl();
         DataReaderFactory factory = new DataReaderFactory();
         long contentLength = 0L;
-        InputStream in;
-        if (url.getProtocol().startsWith("http")) {
-            AtomicLong cl = new AtomicLong();
-            URLConnection conn = url.openConnection();
-            in = HttpUrlConnectionHelper.openInputStream((HttpURLConnection)conn, HttpUrlConnectionHelper.GZIP, cl);
-            contentLength = cl.get();
-        }
-        else {
-            in = url.openStream();
-            if (url.getProtocol().startsWith("file")) {
-                File file = new File(url.getFile());
-                if (file.exists()) {
-                contentLength = file.length();
+        InputStream in = null;
+        try {
+            if (url.getProtocol().startsWith("http")) {
+                AtomicLong cl = new AtomicLong();
+                URLConnection conn = url.openConnection();
+                in = HttpUrlConnectionHelper.openInputStream((HttpURLConnection) conn,
+                        HttpUrlConnectionHelper.GZIP,
+                        cl);
+                contentLength = cl.get();
+            }
+            else {
+                in = url.openStream();
+                if (url.getProtocol().startsWith("file")) {
+                    File file = new File(url.getFile());
+                    if (file.exists()) {
+                        contentLength = file.length();
+                    }
                 }
             }
-        }
-        if (contentLength > 100L) {
-            in = new MonitoredBufferedInputStream(in, DataReaderFactory.FOUR_KB, contentLength);
-            for (PropertyChangeListener listener : propertyChangeListeners) {
-                ((MonitoredBufferedInputStream)in).addPropertyChangeListener(listener);
+            if (contentLength > 100L) {
+                in = new MonitoredBufferedInputStream(in, DataReaderFactory.FOUR_KB, contentLength);
+                for (PropertyChangeListener listener : propertyChangeListeners) {
+                    ((MonitoredBufferedInputStream) in).addPropertyChangeListener(listener);
+                }
             }
-        }
 
-        DataReader reader = factory.getDataReader(gcResource, in);
-        GCModel model = reader.read();
-        model.setURL(url);
+            DataReader reader = factory.getDataReader(gcResource, in);
+            GCModel model = reader.read();
+            model.setURL(url);
 
-        return model;
+            return model;
+        } finally {
+            if (in != null) {
+                try {
+                    in.close();
+                } catch (IOException e) {
+                    gcResource.getLogger().warning("A problem occurred trying to close the InputStream: " + e.toString());
+                }
+            }
+        }
     }
 
 }
diff --git a/src/main/java/com/tagtraum/perf/gcviewer/imp/DataReaderHPUX1_2.java b/src/main/java/com/tagtraum/perf/gcviewer/imp/DataReaderHPUX1_2.java
@@ -148,12 +148,6 @@ public GCModel read() throws IOException {
             return model;
         }
         finally {
-            if (in != null)
-                try {
-                    in.close();
-                }
-                catch (IOException ioe) {
-                }
             if (getLogger().isLoggable(Level.INFO)) getLogger().info("Reading done.");
         }
     }

diff --git a/src/main/java/com/tagtraum/perf/gcviewer/imp/DataReaderHPUX1_4_1.java b/src/main/java/com/tagtraum/perf/gcviewer/imp/DataReaderHPUX1_4_1.java
@@ -204,12 +204,6 @@ If the heap area holding the reflection objects (representing classes and method
             return model;
         }
         finally {
-            if (in != null)
-                try {
-                    in.close();
-                }
-                catch (IOException ioe) {
-                }
             if (getLogger().isLoggable(Level.INFO)) getLogger().info("Reading done.");
         }
     }

diff --git a/src/main/java/com/tagtraum/perf/gcviewer/imp/DataReaderIBM1_3_0.java b/src/main/java/com/tagtraum/perf/gcviewer/imp/DataReaderIBM1_3_0.java
@@ -81,12 +81,6 @@ public GCModel read() throws IOException {
             return model;
         }
         finally {
-            if (in != null)
-                try {
-                    in.close();
-                }
-                catch (IOException ioe) {
-                }
             if (getLogger().isLoggable(Level.INFO)) getLogger().info("Reading done.");
         }
     }

diff --git a/src/main/java/com/tagtraum/perf/gcviewer/imp/DataReaderIBM1_3_1.java b/src/main/java/com/tagtraum/perf/gcviewer/imp/DataReaderIBM1_3_1.java
@@ -111,12 +111,6 @@ else if (line.indexOf("managing allocation failure, action=3") != -1) {
             return model;
         }
         finally {
-            if (in != null)
-                try {
-                    in.close();
-                }
-                catch (IOException ioe) {
-                }
             if (getLogger().isLoggable(Level.INFO)) getLogger().info("Done reading.");
         }
     }

diff --git a/src/main/java/com/tagtraum/perf/gcviewer/imp/DataReaderIBM1_4_2.java b/src/main/java/com/tagtraum/perf/gcviewer/imp/DataReaderIBM1_4_2.java
@@ -119,12 +119,6 @@ else if (line.indexOf("managing allocation failure, action=3") != -1) {
             return model;
         }
         finally {
-            if (in != null)
-                try {
-                    in.close();
-                }
-                catch (IOException ioe) {
-                }
             if (getLogger().isLoggable(Level.INFO)) getLogger().info("Done reading.");
         }
     }

diff --git a/src/main/java/com/tagtraum/perf/gcviewer/imp/DataReaderIBMi5OS1_4_2.java b/src/main/java/com/tagtraum/perf/gcviewer/imp/DataReaderIBMi5OS1_4_2.java
@@ -130,12 +130,6 @@ else if (line.indexOf("collection ending") != -1) {
             return model;
         }
         finally {
-            if (in != null)
-                try {
-                    in.close();
-                }
-                catch (IOException ioe) {
-                }
             if (getLogger().isLoggable(Level.INFO)) getLogger().info("Done reading.");
         }
     }

diff --git a/src/main/java/com/tagtraum/perf/gcviewer/imp/DataReaderJRockit1_4_2.java b/src/main/java/com/tagtraum/perf/gcviewer/imp/DataReaderJRockit1_4_2.java
@@ -144,12 +144,6 @@ else if (line.substring(startTimeIndex).startsWith("<")) {
             return model;
         }
         finally {
-            if (in != null)
-                try {
-                    in.close();
-                }
-                catch (IOException ioe) {
-                }
             if (getLogger().isLoggable(Level.INFO)) getLogger().info("Reading done.");
         }
     }

diff --git a/src/main/java/com/tagtraum/perf/gcviewer/imp/DataReaderJRockit1_5_0.java b/src/main/java/com/tagtraum/perf/gcviewer/imp/DataReaderJRockit1_5_0.java
@@ -185,12 +185,6 @@ else if (line.indexOf("->") == -1){
             return model;
         }
         finally {
-            if (in != null)
-                try {
-                    in.close();
-                }
-                catch (IOException ioe) {
-                }
             if (getLogger().isLoggable(Level.INFO)) getLogger().info("Reading done.");
         }
     }

diff --git a/src/main/java/com/tagtraum/perf/gcviewer/imp/DataReaderJRockit1_6_0.java b/src/main/java/com/tagtraum/perf/gcviewer/imp/DataReaderJRockit1_6_0.java
@@ -180,12 +180,6 @@ else if ((line.indexOf("C#") == -1) || (line.indexOf("->") == -1)){
             return model;
         }
         finally {
-            if (in != null)
-                try {
-                    in.close();
-                }
-                catch (IOException ioe) {
-                }
             if (getLogger().isLoggable(Level.INFO)) getLogger().info("Reading done.");
         }
     }

diff --git a/src/main/java/com/tagtraum/perf/gcviewer/imp/DataReaderShenandoah.java b/src/main/java/com/tagtraum/perf/gcviewer/imp/DataReaderShenandoah.java
@@ -7,6 +7,7 @@
 import java.util.Arrays;
 import java.util.List;
 import java.util.Objects;
+import java.util.logging.Level;
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
 import java.util.stream.Stream;
@@ -97,15 +98,20 @@ protected DataReaderShenandoah(GCResource gcResource, InputStream in) throws Uns
     public GCModel read() throws IOException {
         getLogger().info("Reading Shenandoah format...");
 
-        GCModel model = new GCModel();
-        model.setFormat(GCModel.Format.RED_HAT_SHENANDOAH_GC);
+        try {
+            GCModel model = new GCModel();
+            model.setFormat(GCModel.Format.RED_HAT_SHENANDOAH_GC);
 
-        Stream<String> lines = in.lines();
-        lines.filter(this::lineNotInExcludedStrings)
-                .map(this::parseShenandoahEvent)
-                .filter(Objects::nonNull)
-                .forEach(model::add);
-        return model;
+            Stream<String> lines = in.lines();
+            lines.filter(this::lineNotInExcludedStrings)
+                    .map(this::parseShenandoahEvent)
+                    .filter(Objects::nonNull)
+                    .forEach(model::add);
+
+            return model;
+        } finally {
+            if (getLogger().isLoggable(Level.INFO)) getLogger().info("Reading done.");
+        }
     }
 
     private AbstractGCEvent<?> parseShenandoahEvent(String line) {

diff --git a/src/main/java/com/tagtraum/perf/gcviewer/imp/DataReaderSun1_2_2.java b/src/main/java/com/tagtraum/perf/gcviewer/imp/DataReaderSun1_2_2.java
@@ -103,12 +103,6 @@ else if (line.indexOf(" freed ") != -1 && line.indexOf(" objects, ") != -1) {
             return model;
         }
         finally {
-            if (in != null)
-                try {
-                    in.close();
-                }
-                catch (IOException ioe) {
-                }
             if (getLogger().isLoggable(Level.INFO)) getLogger().info("Done reading.");
         }
     }

diff --git a/src/test/java/com/tagtraum/perf/gcviewer/imp/TestDataReaderShenandoah.java b/src/test/java/com/tagtraum/perf/gcviewer/imp/TestDataReaderShenandoah.java
@@ -1,16 +1,21 @@
 package com.tagtraum.perf.gcviewer.imp;
 
+import static org.hamcrest.Matchers.closeTo;
 import static org.hamcrest.Matchers.is;
 import static org.junit.Assert.assertThat;
-import static org.hamcrest.Matchers.closeTo;
 
 import java.io.IOException;
 import java.io.InputStream;
 import java.time.ZonedDateTime;
 import java.util.logging.Level;
 
 import com.tagtraum.perf.gcviewer.UnittestHelper;
-import com.tagtraum.perf.gcviewer.model.*;
+import com.tagtraum.perf.gcviewer.model.AbstractGCEvent;
+import com.tagtraum.perf.gcviewer.model.ConcurrentGCEvent;
+import com.tagtraum.perf.gcviewer.model.GCEvent;
+import com.tagtraum.perf.gcviewer.model.GCModel;
+import com.tagtraum.perf.gcviewer.model.GCResource;
+import com.tagtraum.perf.gcviewer.model.GcResourceFile;
 import org.junit.Test;
 
 /**
@@ -21,10 +26,6 @@ private InputStream getInputStream(String fileName) throws IOException {
         return UnittestHelper.getResourceAsStream(UnittestHelper.FOLDER_OPENJDK, fileName);
     }
 
-    private DataReader getDataReader(GCResource gcResource) throws IOException {
-        return new DataReaderShenandoah(gcResource, getInputStream(gcResource.getResourceName()));
-    }
-
     @Test
     public void parseBasicEvent() throws Exception {
         GCModel model = getGCModelFromLogFile("SampleShenandoahBasic.txt");
@@ -171,10 +172,12 @@ private GCModel getGCModelFromLogFile(String fileName) throws IOException {
         GCResource gcResource = new GcResourceFile(fileName);
         gcResource.getLogger().addHandler(handler);
 
-        DataReader reader = getDataReader(gcResource);
-        GCModel model = reader.read();
-        assertThat("model format", model.getFormat(), is(GCModel.Format.RED_HAT_SHENANDOAH_GC));
-        assertThat("number of errors", handler.getCount(), is(0));
-        return model;
+        try (InputStream in = getInputStream(gcResource.getResourceName())) {
+            DataReader reader = new DataReaderShenandoah(gcResource, in);
+            GCModel model = reader.read();
+            assertThat("model format", model.getFormat(), is(GCModel.Format.RED_HAT_SHENANDOAH_GC));
+            assertThat("number of errors", handler.getCount(), is(0));
+            return model;
+        }
     }
 }