Bug 1356693 - infer: fix RESOURCE_LEAK's in geckoview r?walkingice draft
authorAndrzej Hunt <ahunt@mozilla.com>
Wed, 26 Apr 2017 17:22:30 +0800
changeset 569898 3569248b3eed5eecd71f4a0cc5665289ffa21098
parent 569897 8b6cb491bcbcc59092ae07f6b2620dbe561dbca8
child 581374 776f424b87d9a549d872d7e838e8aa1550c3b9ad
push id56302
push userahunt@mozilla.com
push dateFri, 28 Apr 2017 00:45:18 +0000
reviewerswalkingice
bugs1356693
milestone55.0a1
Bug 1356693 - infer: fix RESOURCE_LEAK's in geckoview r?walkingice Similar to some previous commits, we prefer non-throwing constructors in order to ensure we never lose a reference to an unclosed stream. InpuStreamReader(..., Charset) is preferred over InputStreamReader(..., String) since the latter can throw when the String is not a valid Charset. (In reality we know that our Charset Strings are correct, but the compiler isn't able to determine that for itself. Moreover, using the preparsed Charset is more efficient.) MozReview-Commit-ID: 9z07G3hqPI3
mobile/android/geckoview/src/main/java/org/mozilla/gecko/GeckoAppShell.java
mobile/android/geckoview/src/main/java/org/mozilla/gecko/util/FileUtils.java
mobile/android/geckoview/src/main/java/org/mozilla/gecko/util/INIParser.java
mobile/android/geckoview/src/main/java/org/mozilla/gecko/util/publicsuffix/PublicSuffixPatterns.java
--- a/mobile/android/geckoview/src/main/java/org/mozilla/gecko/GeckoAppShell.java
+++ b/mobile/android/geckoview/src/main/java/org/mozilla/gecko/GeckoAppShell.java
@@ -18,31 +18,28 @@ import java.net.Proxy;
 import java.net.URLConnection;
 import java.nio.ByteBuffer;
 import java.util.ArrayList;
 import java.util.Iterator;
 import java.util.List;
 import java.util.Map;
 import java.util.StringTokenizer;
 import java.util.TreeMap;
-import java.util.concurrent.ConcurrentHashMap;
 
 import android.annotation.SuppressLint;
 import org.mozilla.gecko.annotation.JNITarget;
 import org.mozilla.gecko.annotation.RobocopTarget;
 import org.mozilla.gecko.annotation.WrapForJNI;
 import org.mozilla.gecko.gfx.BitmapUtils;
 import org.mozilla.gecko.gfx.LayerView;
-import org.mozilla.gecko.gfx.PanZoomController;
 import org.mozilla.gecko.permissions.Permissions;
 import org.mozilla.gecko.process.GeckoProcessManager;
-import org.mozilla.gecko.process.GeckoServiceChildProcess;
-import org.mozilla.gecko.util.EventCallback;
 import org.mozilla.gecko.util.HardwareCodecCapabilityUtils;
 import org.mozilla.gecko.util.HardwareUtils;
+import org.mozilla.gecko.util.IOUtils;
 import org.mozilla.gecko.util.ProxySelector;
 import org.mozilla.gecko.util.ThreadUtils;
 
 import android.Manifest;
 import android.app.Activity;
 import android.app.ActivityManager;
 import android.content.Context;
 import android.content.Intent;
@@ -1201,21 +1198,24 @@ public class GeckoAppShell
     interface GeckoProcessesVisitor {
         boolean callback(int pid);
     }
 
     private static void EnumerateGeckoProcesses(GeckoProcessesVisitor visiter) {
         int pidColumn = -1;
         int userColumn = -1;
 
+        Process ps = null;
+        InputStreamReader inputStreamReader = null;
+        BufferedReader in = null;
         try {
             // run ps and parse its output
-            java.lang.Process ps = Runtime.getRuntime().exec("ps");
-            BufferedReader in = new BufferedReader(new InputStreamReader(ps.getInputStream()),
-                                                   2048);
+            ps = Runtime.getRuntime().exec("ps");
+            inputStreamReader = new InputStreamReader(ps.getInputStream());
+            in = new BufferedReader(inputStreamReader, 2048);
 
             String headerOutput = in.readLine();
 
             // figure out the column offsets.  We only care about the pid and user fields
             StringTokenizer st = new StringTokenizer(headerOutput);
 
             int tokenSoFar = 0;
             while (st.hasMoreTokens()) {
@@ -1237,54 +1237,58 @@ public class GeckoAppShell
                 if (uid == android.os.Process.myUid() &&
                     !split[split.length - 1].equalsIgnoreCase("ps")) {
                     int pid = Integer.parseInt(split[pidColumn]);
                     boolean keepGoing = visiter.callback(pid);
                     if (keepGoing == false)
                         break;
                 }
             }
-            in.close();
-        }
-        catch (Exception e) {
+        } catch (Exception e) {
             Log.w(LOGTAG, "Failed to enumerate Gecko processes.",  e);
+        } finally {
+            IOUtils.safeStreamClose(in);
+            IOUtils.safeStreamClose(inputStreamReader);
+            if (ps != null) {
+                ps.destroy();
+            }
         }
     }
 
     public static String getAppNameByPID(int pid) {
         BufferedReader cmdlineReader = null;
         String path = "/proc/" + pid + "/cmdline";
         try {
             File cmdlineFile = new File(path);
             if (!cmdlineFile.exists())
                 return "";
             cmdlineReader = new BufferedReader(new FileReader(cmdlineFile));
             return cmdlineReader.readLine().trim();
         } catch (Exception ex) {
             return "";
         } finally {
-            if (null != cmdlineReader) {
-                try {
-                    cmdlineReader.close();
-                } catch (Exception e) { }
-            }
+            IOUtils.safeStreamClose(cmdlineReader);
         }
     }
 
     public static void listOfOpenFiles() {
         int pidColumn = -1;
         int nameColumn = -1;
 
+        // run lsof and parse its output
+        Process process = null;
+        InputStreamReader inputStreamReader = null;
+        BufferedReader in = null;
         try {
             String filter = GeckoProfile.get(getApplicationContext()).getDir().toString();
             Log.d(LOGTAG, "[OPENFILE] Filter: " + filter);
 
-            // run lsof and parse its output
-            java.lang.Process lsof = Runtime.getRuntime().exec("lsof");
-            BufferedReader in = new BufferedReader(new InputStreamReader(lsof.getInputStream()), 2048);
+            process = Runtime.getRuntime().exec("lsof");
+            inputStreamReader = new InputStreamReader(process.getInputStream());
+            in = new BufferedReader(inputStreamReader, 2048);
 
             String headerOutput = in.readLine();
             StringTokenizer st = new StringTokenizer(headerOutput);
             int token = 0;
             while (st.hasMoreTokens()) {
                 String next = st.nextToken();
                 if (next.equalsIgnoreCase("PID"))
                     pidColumn = token;
@@ -1305,18 +1309,24 @@ public class GeckoAppShell
                 if (name == null) {
                     name = getAppNameByPID(pid.intValue());
                     pidNameMap.put(pid, name);
                 }
                 String file = split[nameColumn];
                 if (!TextUtils.isEmpty(name) && !TextUtils.isEmpty(file) && file.startsWith(filter))
                     Log.d(LOGTAG, "[OPENFILE] " + name + "(" + split[pidColumn] + ") : " + file);
             }
-            in.close();
-        } catch (Exception e) { }
+        } catch (Exception e) {
+        } finally {
+            IOUtils.safeStreamClose(in);
+            IOUtils.safeStreamClose(inputStreamReader);
+            if (process != null) {
+                process.destroy();
+            }
+        }
     }
 
     @WrapForJNI(calledFrom = "gecko")
     private static byte[] getIconForExtension(String aExt, int iconSize) {
         try {
             if (iconSize <= 0)
                 iconSize = 16;
 
@@ -2082,24 +2092,26 @@ public class GeckoAppShell
             public synchronized int read(byte[] buffer, int byteOffset, int byteCount)
                                     throws IOException {
                 if (mHaveConnected) {
                     return super.read(buffer, byteOffset, byteCount);
                 }
 
                 final PipedOutputStream output = new PipedOutputStream();
                 connect(output);
+
                 ThreadUtils.postToBackgroundThread(
                     new Runnable() {
                         @Override
                         public void run() {
                             try {
                                 bitmap.compress(Bitmap.CompressFormat.PNG, 100, output);
-                                output.close();
-                            } catch (IOException ioe) { }
+                            } finally {
+                                IOUtils.safeStreamClose(output);
+                            }
                         }
                     });
                 mHaveConnected = true;
                 return super.read(buffer, byteOffset, byteCount);
             }
         }
     }
 
--- a/mobile/android/geckoview/src/main/java/org/mozilla/gecko/util/FileUtils.java
+++ b/mobile/android/geckoview/src/main/java/org/mozilla/gecko/util/FileUtils.java
@@ -140,35 +140,36 @@ public class FileUtils {
      * For a higher-level method, see {@link #readStringFromFile(File)}.
      *
      * Since this is generic, it may not be the most performant for your use case.
      *
      * @param bufferSize Size of the underlying buffer for read optimizations - must be > 0.
      */
     public static String readStringFromInputStreamAndCloseStream(final InputStream inputStream, final int bufferSize)
             throws IOException {
-        if (bufferSize <= 0) {
-            // Safe close: it's more important to alert the programmer of
-            // their error than to let them catch and continue on their way.
-            IOUtils.safeStreamClose(inputStream);
-            throw new IllegalArgumentException("Expected buffer size larger than 0. Got: " + bufferSize);
-        }
+        InputStreamReader reader = null;
+        try {
+            if (bufferSize <= 0) {
+                throw new IllegalArgumentException("Expected buffer size larger than 0. Got: " + bufferSize);
+            }
 
-        final StringBuilder stringBuilder = new StringBuilder(bufferSize);
-        final InputStreamReader reader = new InputStreamReader(inputStream, Charset.forName("UTF-8"));
-        try {
+            final StringBuilder stringBuilder = new StringBuilder(bufferSize);
+            reader = new InputStreamReader(inputStream, StringUtils.UTF_8);
+
             int charsRead;
             final char[] buffer = new char[bufferSize];
             while ((charsRead = reader.read(buffer, 0, bufferSize)) != -1) {
                 stringBuilder.append(buffer, 0, charsRead);
             }
+
+            return stringBuilder.toString();
         } finally {
-            reader.close();
+            IOUtils.safeStreamClose(reader);
+            IOUtils.safeStreamClose(inputStream);
         }
-        return stringBuilder.toString();
     }
 
     /**
      * A generic solution to write a JSONObject to a file.
      * See {@link #writeStringToFile(File, String)} for more details.
      */
     public static void writeJSONObjectToFile(final File file, final JSONObject obj) throws IOException {
         writeStringToFile(file, obj.toString());
--- a/mobile/android/geckoview/src/main/java/org/mozilla/gecko/util/INIParser.java
+++ b/mobile/android/geckoview/src/main/java/org/mozilla/gecko/util/INIParser.java
@@ -40,22 +40,23 @@ public final class INIParser extends INI
 
         FileWriter outputStream = null;
         try {
             outputStream = new FileWriter(f);
         } catch (IOException e1) {
             e1.printStackTrace();
         }
 
-        BufferedWriter writer = new BufferedWriter(outputStream);
+        final BufferedWriter writer = new BufferedWriter(outputStream);
         try {
             write(writer);
-            writer.close();
         } catch (IOException e) {
             e.printStackTrace();
+        } finally {
+            IOUtils.safeStreamClose(writer);
         }
     }
 
     @Override
     public void write(BufferedWriter writer) throws IOException {
         super.write(writer);
 
         if (mSections != null) {
--- a/mobile/android/geckoview/src/main/java/org/mozilla/gecko/util/publicsuffix/PublicSuffixPatterns.java
+++ b/mobile/android/geckoview/src/main/java/org/mozilla/gecko/util/publicsuffix/PublicSuffixPatterns.java
@@ -23,32 +23,30 @@ class PublicSuffixPatterns {
 
     static synchronized Set<String> getExactSet(Context context) {
         if (EXACT != null) {
             return EXACT;
         }
 
         EXACT = new HashSet<>();
 
-        InputStream stream = null;
-
+        BufferedReader reader = null;
         try {
-            stream = context.getAssets().open("publicsuffixlist");
-            BufferedReader reader = new BufferedReader(new InputStreamReader(
-                    new BufferedInputStream(stream)));
+            reader = new BufferedReader(new InputStreamReader(
+                    new BufferedInputStream(context.getAssets().open("publicsuffixlist"))));
 
             String line;
             while ((line = reader.readLine()) != null) {
                 EXACT.add(line);
             }
 
         } catch (IOException e) {
             Log.e("Patterns", "IOException during loading public suffix list");
         } finally {
-            IOUtils.safeStreamClose(stream);
+            IOUtils.safeStreamClose(reader);
         }
 
         return EXACT;
     }
 
 
     /**
      * If a hostname is not a key in the EXCLUDE map, and if removing its