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

Commit 73387d2c authored by Luca Zuccarini's avatar Luca Zuccarini
Browse files

Fix concurrent modification issue with lifecycle listeners. 

I couldn't repro the bug locally as it is a flake that requires precise timing, but the problem is the list of listeners being updated during an animation, which calls `onTransitionAnimationProgress()` on each frame.

By using defensive copies in these lifecycle methods, we avoid the concurrent modification issue. The worst that will happen is that we will call a listener that has just been deregistered (or not call one that has just been registered), but since it's a race condition anyway there should be no expectation that this doesn't happen in the current frame.

Change-Id: I2ed33334ddcfcf7672fc1f94af7ccc49700f46ee
Fix: 386122979
Flag: EXEMPT bugfix
Test: atest ActivityTransitionAnimatorTest
parent 56bb04a0

packages/SystemUI/animation/src/com/android/systemui/animation/ActivityTransitionAnimator.kt

+6 −4
Original line number Diff line number Diff line
@@ -232,19 +232,21 @@ constructor( 
    private val lifecycleListener =

        object : Listener {

            override fun onTransitionAnimationStart() {

                listeners.forEach { it.onTransitionAnimationStart() }

                LinkedHashSet(listeners).forEach { it.onTransitionAnimationStart() }

            }



            override fun onTransitionAnimationEnd() {

                listeners.forEach { it.onTransitionAnimationEnd() }

                LinkedHashSet(listeners).forEach { it.onTransitionAnimationEnd() }

            }



            override fun onTransitionAnimationProgress(linearProgress: Float) {

                listeners.forEach { it.onTransitionAnimationProgress(linearProgress) }

                LinkedHashSet(listeners).forEach {

                     it.onTransitionAnimationProgress(linearProgress)

                }

            }



            override fun onTransitionAnimationCancelled() {

                listeners.forEach { it.onTransitionAnimationCancelled() }

                LinkedHashSet(listeners).forEach { it.onTransitionAnimationCancelled() }

            }

        }

packages/SystemUI/tests/src/com/android/systemui/animation/ActivityTransitionAnimatorTest.kt

+22 −0
Original line number Diff line number Diff line
@@ -466,6 +466,28 @@ class ActivityTransitionAnimatorTest : SysuiTestCase() { 
        assertNull(runner.delegate)

    }



    @Test

    fun concurrentListenerModification_doesNotThrow() {

        // Need a second listener to trigger the concurrent modification.

        activityTransitionAnimator.addListener(object : ActivityTransitionAnimator.Listener {})

        `when`(listener.onTransitionAnimationStart()).thenAnswer {

            activityTransitionAnimator.removeListener(listener)

            listener

        }



        val runner = activityTransitionAnimator.createEphemeralRunner(controller)

        runner.onAnimationStart(

            TRANSIT_NONE,

            arrayOf(fakeWindow()),

            emptyArray(),

            emptyArray(),

            iCallback,

        )



        waitForIdleSync()

        verify(listener).onTransitionAnimationStart()

    }



    private fun controllerFactory(

        cookie: ActivityTransitionAnimator.TransitionCookie =

            mock(ActivityTransitionAnimator.TransitionCookie::class.java),