Bug 1356693 - infer: fix RESOURCE_LEAK's in base
MozReview-Commit-ID: Gm9GqOk37UZ
--- a/mobile/android/base/java/org/mozilla/gecko/ANRReporter.java
+++ b/mobile/android/base/java/org/mozilla/gecko/ANRReporter.java
@@ -17,16 +17,17 @@ import java.io.OutputStream;
import java.io.Reader;
import java.util.Locale;
import java.util.UUID;
import java.util.regex.Pattern;
import org.json.JSONObject;
import org.mozilla.gecko.annotation.WrapForJNI;
import org.mozilla.gecko.AppConstants.Versions;
+import org.mozilla.gecko.util.IOUtils;
import org.mozilla.gecko.util.StringUtils;
import org.mozilla.gecko.util.ThreadUtils;
import android.content.BroadcastReceiver;
import android.content.Context;
import android.content.Intent;
import android.content.IntentFilter;
import android.os.Handler;
@@ -149,18 +150,20 @@ public final class ANRReporter extends B
// Find the traces file name if we can.
try {
// getprop [prop-name [default-value]]
Process propProc = (new ProcessBuilder())
.command("/system/bin/getprop", "dalvik.vm.stack-trace-file")
.redirectErrorStream(true)
.start();
+
+ BufferedReader buf = null;
try {
- BufferedReader buf = new BufferedReader(
+ buf = new BufferedReader(
new InputStreamReader(
propProc.getInputStream(), StringUtils.UTF_8), TRACES_LINE_SIZE);
String propVal = buf.readLine();
if (DEBUG) {
Log.d(LOGTAG, "getprop returned " + String.valueOf(propVal));
}
// getprop can return empty string when the prop value is empty
// or prop is undefined, treat both cases the same way
@@ -171,16 +174,18 @@ public final class ANRReporter extends B
} else if (DEBUG) {
Log.d(LOGTAG, "cannot access traces file");
}
} else if (DEBUG) {
Log.d(LOGTAG, "empty getprop result");
}
} finally {
propProc.destroy();
+
+ IOUtils.safeStreamClose(buf);
}
} catch (IOException e) {
Log.w(LOGTAG, e);
} catch (ClassCastException e) {
Log.w(LOGTAG, e); // Bug 975436
}
return null;
}
--- a/mobile/android/base/java/org/mozilla/gecko/db/SuggestedSites.java
+++ b/mobile/android/base/java/org/mozilla/gecko/db/SuggestedSites.java
@@ -36,17 +36,19 @@ import org.json.JSONException;
import org.json.JSONObject;
import org.mozilla.gecko.annotation.RobocopTarget;
import org.mozilla.gecko.GeckoSharedPrefs;
import org.mozilla.gecko.GeckoProfile;
import org.mozilla.gecko.Locales;
import org.mozilla.gecko.R;
import org.mozilla.gecko.distribution.Distribution;
import org.mozilla.gecko.restrictions.Restrictions;
+import org.mozilla.gecko.util.IOUtils;
import org.mozilla.gecko.util.RawResource;
+import org.mozilla.gecko.util.StringUtils;
import org.mozilla.gecko.util.ThreadUtils;
import org.mozilla.gecko.preferences.GeckoPreferences;
/**
* {@code SuggestedSites} provides API to get a list of locale-specific
* suggested sites to be used in Fennec's top sites panel. It provides
* only a single method to fetch the list as a {@code Cursor}. This cursor
* will then be wrapped by {@code TopSitesCursorWrapper} to blend top,
@@ -263,38 +265,32 @@ public class SuggestedSites {
*/
static void saveSites(File f, Map<String, Site> sites) {
ThreadUtils.assertNotOnUiThread();
if (sites == null || sites.isEmpty()) {
return;
}
- OutputStreamWriter osw = null;
+ OutputStreamWriter outputStreamWriter = null;
try {
final JSONArray jsonSites = new JSONArray();
for (Site site : sites.values()) {
jsonSites.put(site.toJSON());
}
- osw = new OutputStreamWriter(new FileOutputStream(f), "UTF-8");
+ outputStreamWriter = new OutputStreamWriter(new FileOutputStream(f), StringUtils.UTF_8);
final String jsonString = jsonSites.toString();
- osw.write(jsonString, 0, jsonString.length());
+ outputStreamWriter.write(jsonString, 0, jsonString.length());
} catch (Exception e) {
Log.e(LOGTAG, "Failed to save suggested sites", e);
} finally {
- if (osw != null) {
- try {
- osw.close();
- } catch (IOException e) {
- // Ignore.
- }
- }
+ IOUtils.safeStreamClose(outputStreamWriter);
}
}
private void maybeWaitForDistribution() {
if (distribution == null) {
return;
}
--- a/mobile/android/base/java/org/mozilla/gecko/distribution/Distribution.java
+++ b/mobile/android/base/java/org/mozilla/gecko/distribution/Distribution.java
@@ -644,17 +644,26 @@ public class Distribution {
final String contentType = connection.getContentType();
if (contentType == null || !contentType.startsWith(EXPECTED_CONTENT_TYPE)) {
Log.w(LOGTAG, "Malformed response: invalid Content-Type.");
Telemetry.addToHistogram(HISTOGRAM_CODE_CATEGORY, CODE_CATEGORY_FETCH_INVALID_CONTENT_TYPE);
return null;
}
- return new JarInputStream(new BufferedInputStream(connection.getInputStream()), true);
+ final BufferedInputStream bufferedInputStream = new BufferedInputStream(connection.getInputStream());
+ try {
+ return new JarInputStream(bufferedInputStream, true);
+ } catch (IOException e) {
+ // Thrown e.g. if JarInputStream can't parse the input as a valid Zip.
+ // In that case we need to ensure the bufferedInputStream gets closed since it won't
+ // be used anywhere (while still passing the Exception up the stack).
+ bufferedInputStream.close();
+ throw e;
+ }
}
private static void recordFetchTelemetry(final Exception exception) {
if (exception == null) {
// Should never happen.
Telemetry.addToHistogram(HISTOGRAM_CODE_CATEGORY, CODE_CATEGORY_FETCH_EXCEPTION);
return;
}
--- a/mobile/android/base/java/org/mozilla/gecko/switchboard/SwitchBoard.java
+++ b/mobile/android/base/java/org/mozilla/gecko/switchboard/SwitchBoard.java
@@ -29,16 +29,17 @@ import java.util.Locale;
import java.util.MissingResourceException;
import java.util.zip.CRC32;
import org.json.JSONException;
import org.json.JSONObject;
import org.json.JSONArray;
import org.mozilla.gecko.AppConstants;
import org.mozilla.gecko.util.HardwareUtils;
+import org.mozilla.gecko.util.IOUtils;
import org.mozilla.gecko.util.ProxySelector;
import android.content.Context;
import android.content.pm.PackageManager.NameNotFoundException;
import android.os.Build;
import android.support.annotation.NonNull;
import android.support.annotation.Nullable;
import android.text.TextUtils;
@@ -373,37 +374,46 @@ public class SwitchBoard {
}
/**
* Returns a String containing the server response from a GET request
* @param url URL for GET request.
* @return Returns String from server or null when failed.
*/
@Nullable private static String readFromUrlGET(URL url) {
+ HttpURLConnection connection = null;
+ InputStreamReader inputStreamReader = null;
+ BufferedReader bufferReader = null;
try {
- HttpURLConnection connection = (HttpURLConnection) ProxySelector.openConnectionWithProxy(url.toURI());
+ connection = (HttpURLConnection) ProxySelector.openConnectionWithProxy(url.toURI());
connection.setRequestProperty("User-Agent", HardwareUtils.isTablet() ?
AppConstants.USER_AGENT_FENNEC_TABLET :
AppConstants.USER_AGENT_FENNEC_MOBILE);
connection.setRequestMethod("GET");
connection.setUseCaches(false);
- InputStream is = connection.getInputStream();
- InputStreamReader inputStreamReader = new InputStreamReader(is);
- BufferedReader bufferReader = new BufferedReader(inputStreamReader, 8192);
+ // BufferedReader(Reader, int) can throw, hence we need to keep a separate reference
+ // to the InputStreamReader in order to always be able to close it:
+ inputStreamReader = new InputStreamReader(connection.getInputStream());
+ bufferReader = new BufferedReader(inputStreamReader, 8192);
String line;
StringBuilder resultContent = new StringBuilder();
while ((line = bufferReader.readLine()) != null) {
resultContent.append(line);
}
- bufferReader.close();
return resultContent.toString();
} catch (IOException | URISyntaxException e) {
e.printStackTrace();
+ } finally {
+ IOUtils.safeStreamClose(bufferReader);
+ IOUtils.safeStreamClose(inputStreamReader);
+ if (connection != null) {
+ connection.disconnect();
+ }
}
return null;
}
/**
* Return the bucket number of the user. There are 100 possible buckets.
*/
--- a/mobile/android/base/java/org/mozilla/gecko/updater/UpdateService.java
+++ b/mobile/android/base/java/org/mozilla/gecko/updater/UpdateService.java
@@ -7,16 +7,17 @@ package org.mozilla.gecko.updater;
import org.mozilla.gecko.AppConstants;
import org.mozilla.gecko.CrashHandler;
import org.mozilla.gecko.R;
import org.mozilla.apache.commons.codec.binary.Hex;
import org.mozilla.gecko.permissions.Permissions;
+import org.mozilla.gecko.util.IOUtils;
import org.mozilla.gecko.util.ProxySelector;
import org.w3c.dom.Document;
import org.w3c.dom.Node;
import org.w3c.dom.NodeList;
import android.Manifest;
import android.app.AlarmManager;
import android.app.IntentService;
@@ -43,16 +44,17 @@ import android.util.Log;
import java.io.BufferedInputStream;
import java.io.BufferedOutputStream;
import java.io.File;
import java.io.FileInputStream;
import java.io.FileOutputStream;
import java.io.InputStream;
import java.io.OutputStream;
+import java.net.HttpURLConnection;
import java.net.URI;
import java.net.URL;
import java.net.URLConnection;
import java.security.MessageDigest;
import java.util.Calendar;
import java.util.GregorianCalendar;
import java.util.List;
import java.util.TimeZone;
@@ -368,26 +370,28 @@ public class UpdateService extends Inten
builder.setContentText(getString(R.string.updater_apply_select));
builder.setContentIntent(contentIntent);
mNotificationManager.notify(NOTIFICATION_ID, builder.build());
}
}
private UpdateInfo findUpdate(boolean force) {
+ URLConnection conn = null;
try {
URI uri = getUpdateURI(force);
if (uri == null) {
Log.e(LOGTAG, "failed to get update URI");
return null;
}
DocumentBuilder builder = DocumentBuilderFactory.newInstance().newDocumentBuilder();
- Document dom = builder.parse(ProxySelector.openConnectionWithProxy(uri).getInputStream());
+ conn = ProxySelector.openConnectionWithProxy(uri);
+ Document dom = builder.parse(conn.getInputStream());
NodeList nodes = dom.getElementsByTagName("update");
if (nodes == null || nodes.getLength() == 0)
return null;
Node updateNode = nodes.item(0);
Node buildIdNode = updateNode.getAttributes().getNamedItem("buildID");
if (buildIdNode == null)
@@ -427,16 +431,24 @@ public class UpdateService extends Inten
Log.e(LOGTAG, "missing some required update information, have: " + info);
return null;
}
return info;
} catch (Exception e) {
Log.e(LOGTAG, "failed to check for update: ", e);
return null;
+ } finally {
+ // conn isn't guaranteed to be an HttpURLConnection, hence we don't want to cast earlier
+ // in this method. However in our current implementation it usually is, so we need to
+ // make sure we close it in that case:
+ final HttpURLConnection httpConn = (HttpURLConnection) conn;
+ if (httpConn != null) {
+ httpConn.disconnect();
+ }
}
}
private MessageDigest createMessageDigest(String hashFunction) {
String javaHashFunction = null;
if ("sha512".equalsIgnoreCase(hashFunction)) {
javaHashFunction = "SHA-512";
@@ -546,29 +558,30 @@ public class UpdateService extends Inten
deleteUpdatePackage(getLastFileName());
}
Log.i(LOGTAG, "downloading update package");
sendCheckUpdateResult(CheckUpdateResult.DOWNLOADING);
OutputStream output = null;
InputStream input = null;
+ URLConnection conn = null;
mDownloading = true;
mCancelDownload = false;
showDownloadNotification(downloadFile);
try {
NetworkInfo netInfo = mConnectivityManager.getActiveNetworkInfo();
if (netInfo != null && netInfo.isConnected() &&
netInfo.getType() == ConnectivityManager.TYPE_WIFI) {
mWifiLock.acquire();
}
- URLConnection conn = ProxySelector.openConnectionWithProxy(info.uri);
+ conn = ProxySelector.openConnectionWithProxy(info.uri);
int length = conn.getContentLength();
output = new BufferedOutputStream(new FileOutputStream(downloadFile));
input = new BufferedInputStream(conn.getInputStream());
byte[] buf = new byte[BUFSIZE];
int len = 0;
@@ -601,31 +614,32 @@ public class UpdateService extends Inten
}
} catch (Exception e) {
downloadFile.delete();
showDownloadFailure();
Log.e(LOGTAG, "failed to download update: ", e);
return null;
} finally {
- try {
- if (input != null)
- input.close();
- } catch (java.io.IOException e) { }
-
- try {
- if (output != null)
- output.close();
- } catch (java.io.IOException e) { }
+ IOUtils.safeStreamClose(input);
+ IOUtils.safeStreamClose(output);
mDownloading = false;
if (mWifiLock.isHeld()) {
mWifiLock.release();
}
+
+ // conn isn't guaranteed to be an HttpURLConnection, hence we don't want to cast earlier
+ // in this method. However in our current implementation it usually is, so we need to
+ // make sure we close it in that case:
+ final HttpURLConnection httpConn = (HttpURLConnection) conn;
+ if (httpConn != null) {
+ httpConn.disconnect();
+ }
}
}
private boolean verifyDownloadedPackage(File updateFile) {
MessageDigest digest = createMessageDigest(getLastHashFunction());
if (digest == null)
return false;