Fix two StrictMode stack collection bugs.

am: 20db11cfda

Change-Id: Ie68764c9864e1c26f43c9d70c5697fe12d00e665

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

@@ -1474,9 +1474,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)) {
@@ -1973,18 +1970,14 @@ 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);
     }
@@ -1998,40 +1991,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);
@@ -2374,7 +2346,7 @@ public final class StrictMode {
      * @hide
      */
     public static class ViolationInfo {
-        public String message;
+        public final String message;
 
         /**
          * Stack and other stuff info.
@@ -2433,6 +2405,7 @@ public final class StrictMode {
          * Create an uninitialized instance of ViolationInfo
          */
         public ViolationInfo() {
+            message = null;
             crashInfo = null;
             policy = 0;
         }
@@ -2479,7 +2452,9 @@ public final class StrictMode {
         @Override
         public int hashCode() {
             int result = 17;
-            result = 37 * result + crashInfo.stackTrace.hashCode();
+            if (crashInfo != null) {
+                result = 37 * result + crashInfo.stackTrace.hashCode();
+            }
             if (numAnimationsRunning != 0) {
                 result *= 37;
             }
@@ -2509,7 +2484,11 @@ public final class StrictMode {
          */
         public ViolationInfo(Parcel in, boolean unsetGatheringBit) {
             message = in.readString();
-            crashInfo = new ApplicationErrorReport.CrashInfo(in);
+            if (in.readInt() != 0) {
+                crashInfo = new ApplicationErrorReport.CrashInfo(in);
+            } else {
+                crashInfo = null;
+            }
             int rawPolicy = in.readInt();
             if (unsetGatheringBit) {
                 policy = rawPolicy & ~PENALTY_GATHER;
@@ -2530,7 +2509,12 @@ public final class StrictMode {
          */
         public void writeToParcel(Parcel dest, int flags) {
             dest.writeString(message);
-            crashInfo.writeToParcel(dest, flags);
+            if (crashInfo != null) {
+                dest.writeInt(1);
+                crashInfo.writeToParcel(dest, flags);
+            } else {
+                dest.writeInt(0);
+            }
             int start = dest.dataPosition();
             dest.writeInt(policy);
             dest.writeInt(durationMillis);
@@ -2558,7 +2542,9 @@ public final class StrictMode {
          * Dump a ViolationInfo instance to a Printer.
          */
         public void dump(Printer pw, String prefix) {
-            crashInfo.dump(pw, prefix);
+            if (crashInfo != null) {
+                crashInfo.dump(pw, prefix);
+            }
             pw.println(prefix + "policy: " + policy);
             if (durationMillis != -1) {
                 pw.println(prefix + "durationMillis: " + durationMillis);