Merge "Fix memory leak in StrictMode violation throttling" into rvc-dev am: 471ef84e33

Original change: https://googleplex-android-review.googlesource.com/c/platform/frameworks/base/+/11902484

Change-Id: Ie1fdf6c77d35c113b8a22502e41b18c2cd963934

core/java/android/os/StrictMode.java

@@ -84,6 +84,8 @@ import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Deque;
 import java.util.HashMap;
+import java.util.Iterator;
+import java.util.Map;
 import java.util.concurrent.Executor;
 import java.util.concurrent.RejectedExecutionException;
 import java.util.concurrent.atomic.AtomicInteger;
@@ -189,6 +191,9 @@ public final class StrictMode {
     // Only show an annoying dialog at most every 30 seconds
     private static final long MIN_DIALOG_INTERVAL_MS = 30000;
 
+    // Only log a dropbox entry at most every 30 seconds
+    private static final long MIN_DROPBOX_INTERVAL_MS = 3000;
+
     // How many Span tags (e.g. animations) to report.
     private static final int MAX_SPAN_TAGS = 20;
 
@@ -1752,16 +1757,20 @@ public final class StrictMode {
             // Not perfect, but fast and good enough for dup suppression.
             Integer crashFingerprint = info.hashCode();
             long lastViolationTime = 0;
-            if (mLastViolationTime != null) {
-                Long vtime = mLastViolationTime.get(crashFingerprint);
-                if (vtime != null) {
-                    lastViolationTime = vtime;
-                }
-            } else {
-                mLastViolationTime = new ArrayMap<>(1);
-            }
             long now = SystemClock.uptimeMillis();
-            mLastViolationTime.put(crashFingerprint, now);
+            if (sLogger == LOGCAT_LOGGER) { // Don't throttle it if there is a non-default logger
+                if (mLastViolationTime != null) {
+                    Long vtime = mLastViolationTime.get(crashFingerprint);
+                    if (vtime != null) {
+                        lastViolationTime = vtime;
+                    }
+                    clampViolationTimeMap(mLastViolationTime, Math.max(MIN_LOG_INTERVAL_MS,
+                                Math.max(MIN_DIALOG_INTERVAL_MS, MIN_DROPBOX_INTERVAL_MS)));
+                } else {
+                    mLastViolationTime = new ArrayMap<>(1);
+                }
+                mLastViolationTime.put(crashFingerprint, now);
+            }
             long timeSinceLastViolationMillis =
                     lastViolationTime == 0 ? Long.MAX_VALUE : (now - lastViolationTime);
 
@@ -1780,7 +1789,8 @@ public final class StrictMode {
                 penaltyMask |= PENALTY_DIALOG;
             }
 
-            if (info.penaltyEnabled(PENALTY_DROPBOX) && lastViolationTime == 0) {
+            if (info.penaltyEnabled(PENALTY_DROPBOX)
+                    && timeSinceLastViolationMillis > MIN_DROPBOX_INTERVAL_MS) {
                 penaltyMask |= PENALTY_DROPBOX;
             }
 
@@ -2215,6 +2225,23 @@ public final class StrictMode {
     @UnsupportedAppUsage
     private static final HashMap<Integer, Long> sLastVmViolationTime = new HashMap<>();
 
+    /**
+     * Clamp the given map by removing elements with timestamp older than the given retainSince.
+     */
+    private static void clampViolationTimeMap(final @NonNull Map<Integer, Long> violationTime,
+            final long retainSince) {
+        final Iterator<Map.Entry<Integer, Long>> iterator = violationTime.entrySet().iterator();
+        while (iterator.hasNext()) {
+            Map.Entry<Integer, Long> e = iterator.next();
+            if (e.getValue() < retainSince) {
+                // Remove stale entries
+                iterator.remove();
+            }
+        }
+        // Ideally we'd cap the total size of the map, though it'll involve quickselect of topK,
+        // seems not worth it (saving some space immediately but they will be obsoleted soon anyway)
+    }
+
     /** @hide */
     public static void onVmPolicyViolation(Violation originStack) {
         onVmPolicyViolation(originStack, false);
@@ -2238,13 +2265,17 @@ public final class StrictMode {
         final long now = SystemClock.uptimeMillis();
         long lastViolationTime;
         long timeSinceLastViolationMillis = Long.MAX_VALUE;
-        synchronized (sLastVmViolationTime) {
+        if (sLogger == LOGCAT_LOGGER) { // Don't throttle it if there is a non-default logger
-            if (sLastVmViolationTime.containsKey(fingerprint)) {
+            synchronized (sLastVmViolationTime) {
-                lastViolationTime = sLastVmViolationTime.get(fingerprint);
+                if (sLastVmViolationTime.containsKey(fingerprint)) {
-                timeSinceLastViolationMillis = now - lastViolationTime;
+                    lastViolationTime = sLastVmViolationTime.get(fingerprint);
-            }
+                    timeSinceLastViolationMillis = now - lastViolationTime;
-            if (timeSinceLastViolationMillis > MIN_VM_INTERVAL_MS) {
+                }
-                sLastVmViolationTime.put(fingerprint, now);
+                if (timeSinceLastViolationMillis > MIN_VM_INTERVAL_MS) {
+                    sLastVmViolationTime.put(fingerprint, now);
+                }
+                clampViolationTimeMap(sLastVmViolationTime,
+                        now - Math.max(MIN_VM_INTERVAL_MS, MIN_LOG_INTERVAL_MS));
             }
         }
         if (timeSinceLastViolationMillis <= MIN_VM_INTERVAL_MS) {

core/java/android/os/strictmode/Violation.java

@@ -18,7 +18,58 @@ package android.os.strictmode;
 
 /** Root class for all StrictMode violations. */
 public abstract class Violation extends Throwable {
+    private int mHashCode;
+    private boolean mHashCodeValid;
+
     Violation(String message) {
         super(message);
     }
+
+    @Override
+    public int hashCode() {
+        synchronized (this) {
+            if (mHashCodeValid) {
+                return mHashCode;
+            }
+            final String message = getMessage();
+            final Throwable cause = getCause();
+            int hashCode = message != null ? message.hashCode() : getClass().hashCode();
+            hashCode = hashCode * 37 + calcStackTraceHashCode(getStackTrace());
+            hashCode = hashCode * 37 + (cause != null ? cause.toString().hashCode() : 0);
+            mHashCodeValid = true;
+            return mHashCode = hashCode;
+        }
+    }
+
+    @Override
+    public synchronized Throwable initCause(Throwable cause) {
+        mHashCodeValid = false;
+        return super.initCause(cause);
+    }
+
+    @Override
+    public void setStackTrace(StackTraceElement[] stackTrace) {
+        super.setStackTrace(stackTrace);
+        synchronized (this) {
+            mHashCodeValid = false;
+        }
+    }
+
+    @Override
+    public synchronized Throwable fillInStackTrace() {
+        mHashCodeValid = false;
+        return super.fillInStackTrace();
+    }
+
+    private static int calcStackTraceHashCode(final StackTraceElement[] stackTrace) {
+        int hashCode = 17;
+        if (stackTrace != null) {
+            for (int i = 0; i < stackTrace.length; i++) {
+                if (stackTrace[i] != null) {
+                    hashCode = hashCode * 37 + stackTrace[i].hashCode();
+                }
+            }
+        }
+        return hashCode;
+    }
 }