Bug 1066982 - Try to not launch processes on pre-JB devices because of Android bug. r=snorp, a=lmandel
authorJim Chen <nchen@mozilla.com>
Thu, 23 Oct 2014 16:32:27 -0400
changeset 225828 5a4dfee44717
parent 225827 15bafc2978d8
child 225829 22cfde2bf1ce
push id4029
push userryanvm@gmail.com
push date2014-10-27 14:38 +0000
treeherdermozilla-beta@22cfde2bf1ce [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssnorp, lmandel
bugs1066982
milestone34.0
Bug 1066982 - Try to not launch processes on pre-JB devices because of Android bug. r=snorp, a=lmandel On pre-JB Android, there's a bug in Bionic's fork implementation that can cause a hang when launching processes, so this patch makes us avoid doing that if we can.
mobile/android/base/ANRReporter.java
mobile/android/base/CrashReporter.java
--- a/mobile/android/base/ANRReporter.java
+++ b/mobile/android/base/ANRReporter.java
@@ -14,16 +14,17 @@ import java.io.FileReader;
 import java.io.IOException;
 import java.io.InputStreamReader;
 import java.io.OutputStream;
 import java.io.Reader;
 import java.util.UUID;
 import java.util.regex.Pattern;
 
 import org.json.JSONObject;
+import org.mozilla.gecko.AppConstants.Versions;
 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;
 import android.os.Looper;
@@ -129,54 +130,56 @@ public final class ANRReporter extends B
         }
     }
 
     private ANRReporter() {
     }
 
     // Return the "traces.txt" file, or null if there is no such file
     private static File getTracesFile() {
+        // Check most common location first.
+        File tracesFile = new File("/data/anr/traces.txt");
+        if (tracesFile.isFile() && tracesFile.canRead()) {
+            return tracesFile;
+        }
+
+        // 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();
             try {
                 BufferedReader buf = new BufferedReader(
                     new InputStreamReader(propProc.getInputStream()), 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
                 if (propVal != null && propVal.length() != 0) {
-                    File tracesFile = new File(propVal);
+                    tracesFile = new File(propVal);
                     if (tracesFile.isFile() && tracesFile.canRead()) {
                         return tracesFile;
                     } else if (DEBUG) {
                         Log.d(LOGTAG, "cannot access traces file");
                     }
                 } else if (DEBUG) {
                     Log.d(LOGTAG, "empty getprop result");
                 }
             } finally {
                 propProc.destroy();
             }
         } catch (IOException e) {
             Log.w(LOGTAG, e);
         } catch (ClassCastException e) {
             Log.w(LOGTAG, e); // Bug 975436
         }
-        // Check most common location one last time just in case
-        File tracesFile = new File("/data/anr/traces.txt");
-        if (tracesFile.isFile() && tracesFile.canRead()) {
-            return tracesFile;
-        }
         return null;
     }
 
     private static File getPingFile() {
         if (GeckoAppShell.getContext() == null) {
             return null;
         }
         GeckoProfile profile = GeckoAppShell.getGeckoInterface().getProfile();
@@ -401,24 +404,21 @@ public final class ANRReporter extends B
             if (endIndex > 0) {
                 // End pattern already found; return now
                 break;
             }
         }
         return total;
     }
 
-    private static void fillPingFooter(OutputStream ping,
-                                       boolean haveNativeStack)
-            throws IOException {
-
-        // We are at the end of ANR data
-
-        int total = writePingPayload(ping, ("\"," +
-                "\"androidLogcat\":\""));
+    private static void fillLogcat(final OutputStream ping) {
+        if (Versions.preJB) {
+            // Logcat retrieval is not supported on pre-JB devices.
+            return;
+        }
 
         try {
             // get the last 200 lines of logcat
             Process proc = (new ProcessBuilder())
                 .command("/system/bin/logcat", "-v", "threadtime", "-t", "200", "-d", "*:D")
                 .redirectErrorStream(true)
                 .start();
             try {
@@ -429,16 +429,27 @@ public final class ANRReporter extends B
                 }
             } finally {
                 proc.destroy();
             }
         } catch (IOException e) {
             // ignore because logcat is not essential
             Log.w(LOGTAG, e);
         }
+    }
+
+    private static void fillPingFooter(OutputStream ping,
+                                       boolean haveNativeStack)
+            throws IOException {
+
+        // We are at the end of ANR data
+
+        int total = writePingPayload(ping, ("\"," +
+                "\"androidLogcat\":\""));
+        fillLogcat(ping);
 
         if (haveNativeStack) {
             total += writePingPayload(ping, ("\"," +
                     "\"androidNativeStack\":"));
 
             String nativeStack = String.valueOf(getNativeStack());
             int size = writePingPayload(ping, nativeStack);
             if (DEBUG) {
--- a/mobile/android/base/CrashReporter.java
+++ b/mobile/android/base/CrashReporter.java
@@ -16,16 +16,18 @@ import java.io.InputStreamReader;
 import java.io.IOException;
 import java.io.OutputStream;
 import java.net.HttpURLConnection;
 import java.net.URL;
 import java.nio.channels.Channels;
 import java.nio.channels.FileChannel;
 import java.util.zip.GZIPOutputStream;
 
+import org.mozilla.gecko.AppConstants.Versions;
+
 import android.app.Activity;
 import android.app.AlertDialog;
 import android.app.ProgressDialog;
 import android.content.DialogInterface;
 import android.content.Intent;
 import android.content.SharedPreferences;
 import android.os.Build;
 import android.os.Bundle;
@@ -390,17 +392,17 @@ public class CrashReporter extends Activ
             sendPart(os, boundary, "Android_CPU_ABI", Build.CPU_ABI);
             try {
                 sendPart(os, boundary, "Android_CPU_ABI2", Build.CPU_ABI2);
                 sendPart(os, boundary, "Android_Hardware", Build.HARDWARE);
             } catch (Exception ex) {
                 Log.e(LOGTAG, "Exception while sending SDK version 8 keys", ex);
             }
             sendPart(os, boundary, "Android_Version",  Build.VERSION.SDK_INT + " (" + Build.VERSION.CODENAME + ")");
-            if (Build.VERSION.SDK_INT >= 16 && includeURLCheckbox.isChecked()) {
+            if (Versions.feature16Plus && includeURLCheckbox.isChecked()) {
                 sendPart(os, boundary, "Android_Logcat", readLogcat());
             }
 
             String comment = ((EditText) findViewById(R.id.comment)).getText().toString();
             if (!TextUtils.isEmpty(comment)) {
                 sendPart(os, boundary, "Comments", comment);
             }