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

Commit 4873e34e authored by Chet Haase's avatar Chet Haase
Browse files

Fix activity leak bug 

It is possible for ViewRootImpl.doDie() to be called prior to one of the
other calls which tells the system that the window is going to the
foreground. This race condition can cause an activity leak since the
ViewRootImpl in question is stored in a collection of "requestors" which
determine when to pause/resume animators for background/foreground apps.

The fix is two-fold:
- Only cause an item to be added to the "requestor" list in
  AnimationHandler when doDie() has not yet been called (marked by
  the mReoved" flag)
- Store the requestor objects as WeakReferences, rather than the objects
  themselves. Thus if the requestor list is the only referent to the
  object, it can be collected anyway, removing the leak potential.

Bug: 258616235, 258616235
Test: AnimatorLeakTest passes
Change-Id: I7dfebac90aa43f89433f49e0ed60b3ff57bcc497
parent 814729fe

core/java/android/animation/AnimationHandler.java

+39 −20
Original line number Diff line number Diff line
@@ -19,10 +19,10 @@ package android.animation; 
import android.os.SystemClock;

import android.os.SystemProperties;

import android.util.ArrayMap;

import android.util.ArraySet;

import android.util.Log;

import android.view.Choreographer;



import java.lang.ref.WeakReference;

import java.util.ArrayList;



/**
@@ -78,7 +78,7 @@ public class AnimationHandler { 
     * store visible (foreground) requestors; if the set size reaches zero, there are no

     * objects in the foreground and it is time to pause animators.

     */

    private final ArraySet<Object> mAnimatorRequestors = new ArraySet<>();

    private final ArrayList<WeakReference<Object>> mAnimatorRequestors = new ArrayList<>();



    private final Choreographer.FrameCallback mFrameCallback = new Choreographer.FrameCallback() {

        @Override
@@ -141,19 +141,9 @@ public class AnimationHandler { 
     * tracking obsolete+enabled requestors.

     */

    public static void removeRequestor(Object requestor) {

        getInstance().removeRequestorImpl(requestor);

    }



    private void removeRequestorImpl(Object requestor) {

        // Also request disablement, in case that requestor was the sole object keeping

        // animators un-paused

        requestAnimatorsEnabled(false, requestor);

        mAnimatorRequestors.remove(requestor);

        getInstance().requestAnimatorsEnabledImpl(false, requestor);

        if (LOCAL_LOGV) {

            Log.v(TAG, "removeRequestorImpl for " + requestor);

            for (int i = 0; i < mAnimatorRequestors.size(); ++i) {

                Log.v(TAG, "animatorRequesters " + i + " = " + mAnimatorRequestors.valueAt(i));

            }

            Log.v(TAG, "removeRequestor for " + requestor);

        }

    }
@@ -173,10 +163,36 @@ public class AnimationHandler { 
    private void requestAnimatorsEnabledImpl(boolean enable, Object requestor) {

        boolean wasEmpty = mAnimatorRequestors.isEmpty();

        setAnimatorPausingEnabled(isPauseBgAnimationsEnabledInSystemProperties());

        synchronized (mAnimatorRequestors) {

            // Only store WeakRef objects to avoid leaks

            if (enable) {

            mAnimatorRequestors.add(requestor);

                // First, check whether such a reference is already on the list

                WeakReference<Object> weakRef = null;

                for (int i = mAnimatorRequestors.size() - 1; i >= 0; --i) {

                    WeakReference<Object> ref = mAnimatorRequestors.get(i);

                    Object referent = ref.get();

                    if (referent == requestor) {

                        weakRef = ref;

                    } else if (referent == null) {

                        // Remove any reference that has been cleared

                        mAnimatorRequestors.remove(i);

                    }

                }

                if (weakRef == null) {

                    weakRef = new WeakReference<>(requestor);

                    mAnimatorRequestors.add(weakRef);

                }

            } else {

            mAnimatorRequestors.remove(requestor);

                for (int i = mAnimatorRequestors.size() - 1; i >= 0; --i) {

                    WeakReference<Object> ref = mAnimatorRequestors.get(i);

                    Object referent = ref.get();

                    if (referent == requestor || referent == null) {

                        // remove requested item or item that has been cleared

                        mAnimatorRequestors.remove(i);

                    }

                }

                // If a reference to the requestor wasn't in the list, nothing to remove

            }

        }

        if (!sAnimatorPausingEnabled) {

            // Resume any animators that have been paused in the meantime, otherwise noop
@@ -198,9 +214,12 @@ public class AnimationHandler { 
            }

        }

        if (LOCAL_LOGV) {

            Log.v(TAG, enable ? "enable" : "disable" + " animators for " + requestor);

            Log.v(TAG, (enable ? "enable" : "disable") + " animators for " + requestor

                    + " with pauseDelay of " + Animator.getBackgroundPauseDelay());

            for (int i = 0; i < mAnimatorRequestors.size(); ++i) {

                Log.v(TAG, "animatorRequesters " + i + " = " + mAnimatorRequestors.valueAt(i));

                Log.v(TAG, "animatorRequestors " + i + " = "

                        + mAnimatorRequestors.get(i) + " with referent "

                        + mAnimatorRequestors.get(i).get());

            }

        }

    }

core/java/android/view/ViewRootImpl.java

+11 −2
Original line number Diff line number Diff line
@@ -1408,7 +1408,11 @@ public final class ViewRootImpl implements ViewParent, 
                mFirstPostImeInputStage = earlyPostImeStage;

                mPendingInputEventQueueLengthCounterName = "aq:pending:" + counterSuffix;



                if (!mRemoved || !mAppVisible) {

                    AnimationHandler.requestAnimatorsEnabled(mAppVisible, this);

                } else if (LOCAL_LOGV) {

                    Log.v(mTag, "setView() enabling visibility when removed");

                }

            }

        }

    }
@@ -1759,7 +1763,12 @@ public final class ViewRootImpl implements ViewParent, 
                mAppVisibilityChanged = true;

                scheduleTraversals();

            }

            // Only enable if the window is not already removed (via earlier call to doDie())

            if (!mRemoved || !mAppVisible) {

                AnimationHandler.requestAnimatorsEnabled(mAppVisible, this);

            } else if (LOCAL_LOGV) {

                Log.v(mTag, "handleAppVisibility() enabling visibility when removed");

            }

        }

    }