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

Commit 9de8f1bf authored by Hawkwood Glazier's avatar Hawkwood Glazier
Browse files

Guard off thread list mutations of the available clock map 

Bug: 274052672
Test: atest ClockRegistryTest
Change-Id: Ic892ab95c1aabe8283d413b2760140d459ddf8a5
parent b6d4f2ed

packages/SystemUI/customization/src/com/android/systemui/shared/clocks/ClockRegistry.kt

+18 −15
Original line number Diff line number Diff line
@@ -33,6 +33,7 @@ import com.android.systemui.plugins.PluginLifecycleManager 
import com.android.systemui.plugins.PluginListener

import com.android.systemui.plugins.PluginManager

import com.android.systemui.util.Assert

import java.util.concurrent.ConcurrentHashMap

import java.util.concurrent.atomic.AtomicBoolean

import kotlinx.coroutines.CoroutineDispatcher

import kotlinx.coroutines.CoroutineScope
@@ -41,6 +42,18 @@ import kotlinx.coroutines.launch 
private const val DEBUG = true

private val KEY_TIMESTAMP = "appliedTimestamp"



private fun <TKey, TVal> ConcurrentHashMap<TKey, TVal>.concurrentGetOrPut(

    key: TKey,

    value: TVal,

    onNew: () -> Unit

): TVal {

    val result = this.putIfAbsent(key, value)

    if (result == null) {

        onNew()

    }

    return result ?: value

}



/** ClockRegistry aggregates providers and plugins */

open class ClockRegistry(

    val context: Context,
@@ -64,7 +77,7 @@ open class ClockRegistry( 
        fun onAvailableClocksChanged() {}

    }



    private val availableClocks = mutableMapOf<ClockId, ClockInfo>()

    private val availableClocks = ConcurrentHashMap<ClockId, ClockInfo>()

    private val clockChangeListeners = mutableListOf<ClockChangeListener>()

    private val settingObserver =

        object : ContentObserver(null) {
@@ -92,14 +105,8 @@ open class ClockRegistry( 
                var isClockListChanged = false

                for (clock in plugin.getClocks()) {

                    val id = clock.clockId

                    var isNew = false

                    val info =

                        availableClocks.getOrPut(id) {

                            isNew = true

                            ClockInfo(clock, plugin, manager)

                        }



                    if (isNew) {

                        availableClocks.concurrentGetOrPut(id, ClockInfo(clock, plugin, manager)) {

                            isClockListChanged = true

                            onConnected(id)

                        }
@@ -254,10 +261,8 @@ open class ClockRegistry( 
            return

        }



        android.util.Log.e("HAWK", "triggerOnCurrentClockChanged")

        scope.launch(mainDispatcher) {

            assertMainThread()

            android.util.Log.e("HAWK", "isClockChanged")

            isClockChanged.set(false)

            clockChangeListeners.forEach { it.onCurrentClockChanged() }

        }
@@ -270,10 +275,8 @@ open class ClockRegistry( 
            return

        }



        android.util.Log.e("HAWK", "triggerOnAvailableClocksChanged")

        scope.launch(mainDispatcher) {

            assertMainThread()

            android.util.Log.e("HAWK", "isClockListChanged")

            isClockListChanged.set(false)

            clockChangeListeners.forEach { it.onAvailableClocksChanged() }

        }
@@ -356,7 +359,7 @@ open class ClockRegistry( 
    }



    private var isVerifying = AtomicBoolean(false)

    private fun verifyLoadedProviders() {

    fun verifyLoadedProviders() {

        val shouldSchedule = isVerifying.compareAndSet(false, true)

        if (!shouldSchedule) {

            return

packages/SystemUI/tests/src/com/android/systemui/shared/clocks/ClockRegistryTest.kt

+52 −5
Original line number Diff line number Diff line
@@ -122,7 +122,7 @@ class ClockRegistryTest : SysuiTestCase() { 
            isEnabled = true,

            handleAllUsers = true,

            defaultClockProvider = fakeDefaultProvider,

            keepAllLoaded = true,

            keepAllLoaded = false,

            subTag = "Test",

        ) {

            override fun querySettings() { }
@@ -154,8 +154,8 @@ class ClockRegistryTest : SysuiTestCase() { 
        pluginListener.onPluginLoaded(plugin2, mockContext, mockPluginLifecycle)

        val list = registry.getClocks()

        assertEquals(

            list,

            listOf(

            list.toSet(),

            setOf(

                ClockMetadata(DEFAULT_CLOCK_ID, DEFAULT_CLOCK_NAME),

                ClockMetadata("clock_1", "clock 1"),

                ClockMetadata("clock_2", "clock 2"),
@@ -187,8 +187,8 @@ class ClockRegistryTest : SysuiTestCase() { 
        pluginListener.onPluginLoaded(plugin2, mockContext, mockPluginLifecycle2)

        val list = registry.getClocks()

        assertEquals(

            list,

            listOf(

            list.toSet(),

            setOf(

                ClockMetadata(DEFAULT_CLOCK_ID, DEFAULT_CLOCK_NAME),

                ClockMetadata("clock_1", "clock 1"),

                ClockMetadata("clock_2", "clock 2")
@@ -293,6 +293,53 @@ class ClockRegistryTest : SysuiTestCase() { 
        assertEquals(4, listChangeCallCount)

    }



    @Test

    fun pluginAddRemove_concurrentModification() {

        val mockPluginLifecycle1 = mock<PluginLifecycleManager<ClockProviderPlugin>>()

        val mockPluginLifecycle2 = mock<PluginLifecycleManager<ClockProviderPlugin>>()

        val mockPluginLifecycle3 = mock<PluginLifecycleManager<ClockProviderPlugin>>()

        val mockPluginLifecycle4 = mock<PluginLifecycleManager<ClockProviderPlugin>>()

        val plugin1 = FakeClockPlugin().addClock("clock_1", "clock 1")

        val plugin2 = FakeClockPlugin().addClock("clock_2", "clock 2")

        val plugin3 = FakeClockPlugin().addClock("clock_3", "clock 3")

        val plugin4 = FakeClockPlugin().addClock("clock_4", "clock 4")

        whenever(mockPluginLifecycle1.isLoaded).thenReturn(true)

        whenever(mockPluginLifecycle2.isLoaded).thenReturn(true)

        whenever(mockPluginLifecycle3.isLoaded).thenReturn(true)

        whenever(mockPluginLifecycle4.isLoaded).thenReturn(true)



        // Set the current clock to the final clock to load

        registry.applySettings(ClockSettings("clock_4", null))

        scheduler.runCurrent()



        // When ClockRegistry attempts to unload a plugin, we at that point decide to load and

        // unload other plugins. This causes ClockRegistry to modify the list of available clock

        // plugins while it is being iterated over. In production this happens as a result of a

        // thread race, instead of synchronously like it does here.

        whenever(mockPluginLifecycle2.unloadPlugin()).then {

            pluginListener.onPluginDetached(mockPluginLifecycle1)

            pluginListener.onPluginLoaded(plugin4, mockContext, mockPluginLifecycle4)

        }



        // Load initial plugins

        pluginListener.onPluginLoaded(plugin1, mockContext, mockPluginLifecycle1)

        pluginListener.onPluginLoaded(plugin2, mockContext, mockPluginLifecycle2)

        pluginListener.onPluginLoaded(plugin3, mockContext, mockPluginLifecycle3)



        // Repeatedly verify the loaded providers to get final state

        registry.verifyLoadedProviders()

        scheduler.runCurrent()

        registry.verifyLoadedProviders()

        scheduler.runCurrent()



        // Verify all plugins were correctly loaded into the registry

        assertEquals(registry.getClocks().toSet(), setOf(

            ClockMetadata("DEFAULT", "Default Clock"),

            ClockMetadata("clock_2", "clock 2"),

            ClockMetadata("clock_3", "clock 3"),

            ClockMetadata("clock_4", "clock 4")

        ))

    }



    @Test

    fun jsonDeserialization_gotExpectedObject() {