Fix two StrictMode stack collection bugs.

When Binder calls are nested, we can quickly end up with a snowball
of stacktraces that can cause the original transaction to fail.  This
CL makes two specific changes to alleviate this pressure:

-- Consider a nested Binder call from PID A -> B -> C.  If both B and
C encounter dozens of StrictMode violations, then gatheredViolations
in B will end up with 10 ViolationInfo (5 from B and 5 from C).  This
problem only grows with each successive nested call.  To solve this,
always limit ourselves to only ever write out 3 ViolationInfo from
any given process.

-- CrashInfo already nicely truncates any large stack traces to 20kB,
but readAndHandleBinderCallViolations() blindly appends the entire
local trace, and never considers truncating again.  Similar to the
first problem above, nested calls can quickly cause the stackTrace
value to explode in size.  To solve this, we always re-truncate the
stackTrace value after appending our local stack.

Also fix some NPE bugs when missing crashInfo.

(cherry-picked from commit 58f27b5033)

Test: builds, boots
Bug: 32575987
Change-Id: Ie8373ca277296f920f2b1c564d419c702a8ee0f2

core/java/android/app/ApplicationErrorReport.java

@@ -378,6 +378,11 @@ public class ApplicationErrorReport implements Parcelable {
             exceptionMessage = sanitizeString(exceptionMessage);
         }
 
+        /** {@hide} */
+        public void appendStackTrace(String tr) {
+            stackTrace = sanitizeString(stackTrace + tr);
+        }
+
         /**
          * Ensure that the string is of reasonable size, truncating from the middle if needed.
          */

core/java/android/os/StrictMode.java

@@ -1432,9 +1432,6 @@ public final class StrictMode {
                 if (violations == null) {
                     violations = new ArrayList<ViolationInfo>(1);
                     gatheredViolations.set(violations);
-                } else if (violations.size() >= 5) {
-                    // Too many.  In a loop or something?  Don't gather them all.
-                    return;
                 }
                 for (ViolationInfo previous : violations) {
                     if (info.crashInfo.stackTrace.equals(previous.crashInfo.stackTrace)) {
@@ -1931,19 +1928,15 @@ public final class StrictMode {
         if (violations == null) {
             p.writeInt(0);
         } else {
-            p.writeInt(violations.size());
+            // To avoid taking up too much transaction space, only include
-            for (int i = 0; i < violations.size(); ++i) {
+            // details for the first 3 violations. Deep inside, CrashInfo
-                int start = p.dataPosition();
+            // will truncate each stack trace to ~20kB.
-                violations.get(i).writeToParcel(p, 0 /* unused flags? */);
+            final int size = Math.min(violations.size(), 3);
-                int size = p.dataPosition()-start;
+            p.writeInt(size);
-                if (size > 10*1024) {
+            for (int i = 0; i < size; i++) {
-                    Slog.d(TAG, "Wrote violation #" + i + " of " + violations.size() + ": "
+                violations.get(i).writeToParcel(p, 0);
-                            + (p.dataPosition()-start) + " bytes");
             }
         }
-            if (LOG_V) Log.d(TAG, "wrote violations to response parcel; num=" + violations.size());
-            violations.clear(); // somewhat redundant, as we're about to null the threadlocal
-        }
         gatheredViolations.set(null);
     }
 
@@ -1956,40 +1949,19 @@ public final class StrictMode {
     /* package */ static void readAndHandleBinderCallViolations(Parcel p) {
         // Our own stack trace to append
         StringWriter sw = new StringWriter();
+        sw.append("# via Binder call with stack:\n");
         PrintWriter pw = new FastPrintWriter(sw, false, 256);
         new LogStackTrace().printStackTrace(pw);
         pw.flush();
         String ourStack = sw.toString();
 
-        int policyMask = getThreadPolicyMask();
+        final int policyMask = getThreadPolicyMask();
-        boolean currentlyGathering = (policyMask & PENALTY_GATHER) != 0;
+        final boolean currentlyGathering = (policyMask & PENALTY_GATHER) != 0;
 
-        int numViolations = p.readInt();
+        final int size = p.readInt();
-        for (int i = 0; i < numViolations; ++i) {
+        for (int i = 0; i < size; i++) {
-            if (LOG_V) Log.d(TAG, "strict mode violation stacks read from binder call.  i=" + i);
+            final ViolationInfo info = new ViolationInfo(p, !currentlyGathering);
-            ViolationInfo info = new ViolationInfo(p, !currentlyGathering);
+            info.crashInfo.appendStackTrace(ourStack);
-            if (info.crashInfo.stackTrace != null && info.crashInfo.stackTrace.length() > 30000) {
-                String front = info.crashInfo.stackTrace.substring(0, 256);
-                // 30000 characters is way too large for this to be any sane kind of
-                // strict mode collection of stacks.  We've had a problem where we leave
-                // strict mode violations associated with the thread, and it keeps tacking
-                // more and more stacks on to the violations.  Looks like we're in this casse,
-                // so we'll report it and bail on all of the current strict mode violations
-                // we currently are maintaining for this thread.
-                // First, drain the remaining violations from the parcel.
-                i++;  // Skip the current entry.
-                for (; i < numViolations; i++) {
-                    info = new ViolationInfo(p, !currentlyGathering);
-                }
-                // Next clear out all gathered violations.
-                clearGatheredViolations();
-                // Now report the problem.
-                Slog.wtfStack(TAG, "Stack is too large: numViolations=" + numViolations
-                        + " policy=#" + Integer.toHexString(policyMask)
-                        + " front=" + front);
-                return;
-            }
-            info.crashInfo.stackTrace += "# via Binder call with stack:\n" + ourStack;
             BlockGuard.Policy policy = BlockGuard.getThreadPolicy();
             if (policy instanceof AndroidBlockGuardPolicy) {
                 ((AndroidBlockGuardPolicy) policy).handleViolationWithTimingAttempt(info);
@@ -2320,7 +2292,7 @@ public final class StrictMode {
      * @hide
      */
     public static class ViolationInfo {
-        public String message;
+        public final String message;
 
         /**
          * Stack and other stuff info.
@@ -2379,6 +2351,7 @@ public final class StrictMode {
          * Create an uninitialized instance of ViolationInfo
          */
         public ViolationInfo() {
+            message = null;
             crashInfo = null;
             policy = 0;
         }
@@ -2425,7 +2398,9 @@ public final class StrictMode {
         @Override
         public int hashCode() {
             int result = 17;
+            if (crashInfo != null) {
                 result = 37 * result + crashInfo.stackTrace.hashCode();
+            }
             if (numAnimationsRunning != 0) {
                 result *= 37;
             }
@@ -2455,7 +2430,11 @@ public final class StrictMode {
          */
         public ViolationInfo(Parcel in, boolean unsetGatheringBit) {
             message = in.readString();
+            if (in.readInt() != 0) {
                 crashInfo = new ApplicationErrorReport.CrashInfo(in);
+            } else {
+                crashInfo = null;
+            }
             int rawPolicy = in.readInt();
             if (unsetGatheringBit) {
                 policy = rawPolicy & ~PENALTY_GATHER;
@@ -2476,7 +2455,12 @@ public final class StrictMode {
          */
         public void writeToParcel(Parcel dest, int flags) {
             dest.writeString(message);
+            if (crashInfo != null) {
+                dest.writeInt(1);
                 crashInfo.writeToParcel(dest, flags);
+            } else {
+                dest.writeInt(0);
+            }
             int start = dest.dataPosition();
             dest.writeInt(policy);
             dest.writeInt(durationMillis);
@@ -2504,7 +2488,9 @@ public final class StrictMode {
          * Dump a ViolationInfo instance to a Printer.
          */
         public void dump(Printer pw, String prefix) {
+            if (crashInfo != null) {
                 crashInfo.dump(pw, prefix);
+            }
             pw.println(prefix + "policy: " + policy);
             if (durationMillis != -1) {
                 pw.println(prefix + "durationMillis: " + durationMillis);