Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit d2fb7df2 authored by Hans Boehm's avatar Hans Boehm
Browse files

Only count uncleared ProxyMap refs when deciding to crash 

Add a method to compute the size of a ProxyMap without cleared
references. Use it in the crash decision. Report both counts, as
well as the count after a forced GC before actually crashing.

Rename the histogram generating function after a small refactoring
to make that easier.

When we crash, we now generate a message like the following
(generated here with reduced thresholds):

01-03 01:40:52.273  4793  4947 E JavaBinder: java.lang.AssertionError:
Binder ProxyMap has too many entries: 277 (total), 275 (uncleared),
257 (after GC). BinderProxy leak?

after the histogram. Unfortunately, the intervening GC may take some
time, and other intervening messages may sneak into the log between
them.

Experiments so far suggest that none of this greatly affects the
decision when to die. But this eliminates uncertainty as to whether
there was really a problem.

Bug: 71353150

Test: Tested with reduced thresholds, and then booted AOSP.
Change-Id: I53f24bae23eedcdb78a1c32296c65692b7bb2c42
parent 9c1279d2

core/java/android/os/Binder.java

+38 −11
Original line number Diff line number Diff line
@@ -792,7 +792,7 @@ final class BinderProxy implements IBinder { 
        /**

         * Return the total number of pairs in the map.

         */

        int size() {

        private int size() {

            int size = 0;

            for (ArrayList<WeakReference<BinderProxy>> a : mMainIndexValues) {

                if (a != null) {
@@ -802,6 +802,24 @@ final class BinderProxy implements IBinder { 
            return size;

        }



        /**

         * Return the total number of pairs in the map containing values that have

         * not been cleared. More expensive than the above size function.

         */

        private int unclearedSize() {

            int size = 0;

            for (ArrayList<WeakReference<BinderProxy>> a : mMainIndexValues) {

                if (a != null) {

                    for (WeakReference<BinderProxy> ref : a) {

                        if (ref.get() != null) {

                            ++size;

                        }

                    }

                }

            }

            return size;

        }



        /**

         * Remove ith entry from the hash bucket indicated by hash.

         */
@@ -895,17 +913,31 @@ final class BinderProxy implements IBinder { 
                Log.v(Binder.TAG, "BinderProxy map growth! bucket size = " + size

                        + " total = " + totalSize);

                mWarnBucketSize += WARN_INCREMENT;

                if (Build.IS_DEBUGGABLE && totalSize > CRASH_AT_SIZE) {

                    diagnosticCrash();

                if (Build.IS_DEBUGGABLE && totalSize >= CRASH_AT_SIZE) {

                    // Use the number of uncleared entries to determine whether we should

                    // really report a histogram and crash. We don't want to fundamentally

                    // change behavior for a debuggable process, so we GC only if we are

                    // about to crash.

                    final int totalUnclearedSize = unclearedSize();

                    if (totalUnclearedSize >= CRASH_AT_SIZE) {

                        dumpProxyInterfaceCounts();

                        Runtime.getRuntime().gc();

                        throw new AssertionError("Binder ProxyMap has too many entries: "

                                + totalSize + " (total), " + totalUnclearedSize + " (uncleared), "

                                + unclearedSize() + " (uncleared after GC). BinderProxy leak?");

                    } else if (totalSize > 3 * totalUnclearedSize / 2) {

                        Log.v(Binder.TAG, "BinderProxy map has many cleared entries: "

                                + (totalSize - totalUnclearedSize) + " of " + totalSize

                                + " are cleared");

                    }

                }

            }

        }



        /**

         * Dump a histogram to the logcat, then throw an assertion error. Used to diagnose

         * abnormally large proxy maps.

         * Dump a histogram to the logcat. Used to diagnose abnormally large proxy maps.

         */

        private void diagnosticCrash() {

        private void dumpProxyInterfaceCounts() {

            Map<String, Integer> counts = new HashMap<>();

            for (ArrayList<WeakReference<BinderProxy>> a : mMainIndexValues) {

                if (a != null) {
@@ -940,11 +972,6 @@ final class BinderProxy implements IBinder { 
                Log.v(Binder.TAG, " #" + (i + 1) + ": " + sorted[i].getKey() + " x"

                        + sorted[i].getValue());

            }



            // Now throw an assertion.

            final int totalSize = size();

            throw new AssertionError("Binder ProxyMap has too many entries: " + totalSize

                    + ". BinderProxy leak?");

        }



        // Corresponding ArrayLists in the following two arrays always have the same size.