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

Commit b8a61212 authored by George Mount's avatar George Mount
Browse files

Make animation notifications thread safe. 

Fixes: 322266912

A system UI was calling pause() on an animator from a non-UI
thread. Previously, this was safe as long as the listener
list didn't change. Now, there is a possibility of a crash
because the cache is reused with the call.

This CL makes the access to the cache thread safe.

I tried to make a test to validate that this code works, but
it turns out to be extremely difficult to reproduce. The
pausing thread must interrupt between just a few instructions
and even running several hundred thousand times, I could not
reproduce it.

Test: ran existing tests
Change-Id: Ic0affd711ca0330edabd5f7f65f2731646ad8c77
parent 9bbc5ba4

core/java/android/animation/Animator.java

+6 −9
Original line number Original line Diff line number Diff line
@@ -26,6 +26,7 @@ import android.os.Build; 
import android.util.LongArray;

import android.util.LongArray;





import java.util.ArrayList;

import java.util.ArrayList;

import java.util.concurrent.atomic.AtomicReference;





/**

/**

 * This is the superclass for classes which provide basic support for animations which can be

 * This is the superclass for classes which provide basic support for animations which can be
@@ -76,7 +77,7 @@ public abstract class Animator implements Cloneable { 
     * of it in case the list is modified while iterating. The array can be reused to avoid

     * of it in case the list is modified while iterating. The array can be reused to avoid

     * allocation on every notification.

     * allocation on every notification.

     */

     */

    private Object[] mCachedList;

    private AtomicReference<Object[]> mCachedList = new AtomicReference<>();





    /**

    /**

     * Tracks whether we've notified listeners of the onAnimationStart() event. This can be

     * Tracks whether we've notified listeners of the onAnimationStart() event. This can be
@@ -452,7 +453,7 @@ public abstract class Animator implements Cloneable { 
            if (mPauseListeners != null) {

            if (mPauseListeners != null) {

                anim.mPauseListeners = new ArrayList<AnimatorPauseListener>(mPauseListeners);

                anim.mPauseListeners = new ArrayList<AnimatorPauseListener>(mPauseListeners);

            }

            }

            anim.mCachedList = null;

            anim.mCachedList.set(null);

            anim.mStartListenersCalled = false;

            anim.mStartListenersCalled = false;

            return anim;

            return anim;

        } catch (CloneNotSupportedException e) {

        } catch (CloneNotSupportedException e) {
@@ -654,13 +655,9 @@ public abstract class Animator implements Cloneable { 
        int size = list == null ? 0 : list.size();

        int size = list == null ? 0 : list.size();

        if (size > 0) {

        if (size > 0) {

            // Try to reuse mCacheList to store the items of list.

            // Try to reuse mCacheList to store the items of list.

            Object[] array;

            Object[] array = mCachedList.getAndSet(null);

            if (mCachedList == null || mCachedList.length < size) {

            if (array == null || array.length < size) {

                array = new Object[size];

                array = new Object[size];

            } else {

                array = mCachedList;

                // Clear it in case there is some reentrancy

                mCachedList = null;

            }

            }

            list.toArray(array);

            list.toArray(array);

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

            for (int i = 0; i < size; i++) {
@@ -670,7 +667,7 @@ public abstract class Animator implements Cloneable { 
                array[i] = null;

                array[i] = null;

            }

            }

            // Store it for the next call so we can reuse this array, if needed.

            // Store it for the next call so we can reuse this array, if needed.

            mCachedList = array;

            mCachedList.compareAndSet(null, array);

        }

        }

    }

    }