From 33a31d93afc61e0ea2775b7aa49d2817833d417c Mon Sep 17 00:00:00 2001
From: Austin Wang <wangaustin@google.com>
Date: Fri, 19 May 2023 15:10:01 -0700
Subject: [PATCH] Fix race condition when accessing ClockChangeListener

Make sure clients register and unregister listeners on the main thread.

Only switch to background thread for `mutateSetting`, so mutate consecutively happened in order. The call site decides if they need a new job.

Bug: 274403822
Test: set clock options via WPP
Change-Id: I7c14ae912c551a78cee664793eef043e961e75a1
---
 .../systemui/shared/clocks/ClockRegistry.kt   | 27 ++++++++++++++-----
 1 file changed, 21 insertions(+), 6 deletions(-)

diff --git a/packages/SystemUI/customization/src/com/android/systemui/shared/clocks/ClockRegistry.kt b/packages/SystemUI/customization/src/com/android/systemui/shared/clocks/ClockRegistry.kt
index 7c76281e4bdd1..0e20444347fd2 100644
--- a/packages/SystemUI/customization/src/com/android/systemui/shared/clocks/ClockRegistry.kt
+++ b/packages/SystemUI/customization/src/com/android/systemui/shared/clocks/ClockRegistry.kt
@@ -45,6 +45,7 @@ import java.util.concurrent.atomic.AtomicBoolean
 import kotlinx.coroutines.CoroutineDispatcher
 import kotlinx.coroutines.CoroutineScope
 import kotlinx.coroutines.launch
+import kotlinx.coroutines.withContext
 
 private val KEY_TIMESTAMP = "appliedTimestamp"
 
@@ -320,20 +321,20 @@ open class ClockRegistry(
         }
     }
 
-    public fun mutateSetting(mutator: (ClockSettings) -> ClockSettings) {
-        scope.launch(bgDispatcher) { applySettings(mutator(settings ?: ClockSettings())) }
+    public suspend fun mutateSetting(mutator: (ClockSettings) -> ClockSettings) {
+        withContext(bgDispatcher) { applySettings(mutator(settings ?: ClockSettings())) }
     }
 
     var currentClockId: ClockId
         get() = settings?.clockId ?: fallbackClockId
         set(value) {
-            mutateSetting { it.copy(clockId = value) }
+            scope.launch(bgDispatcher) { mutateSetting { it.copy(clockId = value) } }
         }
 
     var seedColor: Int?
         get() = settings?.seedColor
         set(value) {
-            mutateSetting { it.copy(seedColor = value) }
+            scope.launch(bgDispatcher) { mutateSetting { it.copy(seedColor = value) } }
         }
 
     init {
@@ -501,11 +502,25 @@ open class ClockRegistry(
 
     fun createExampleClock(clockId: ClockId): ClockController? = createClock(clockId)
 
-    fun registerClockChangeListener(listener: ClockChangeListener) =
+    /**
+     * Adds [listener] to receive future clock changes.
+     *
+     * Calling from main thread to make sure the access is thread safe.
+     */
+    fun registerClockChangeListener(listener: ClockChangeListener) {
+        assertMainThread()
         clockChangeListeners.add(listener)
+    }
 
-    fun unregisterClockChangeListener(listener: ClockChangeListener) =
+    /**
+     * Removes [listener] from future clock changes.
+     *
+     * Calling from main thread to make sure the access is thread safe.
+     */
+    fun unregisterClockChangeListener(listener: ClockChangeListener) {
+        assertMainThread()
         clockChangeListeners.remove(listener)
+    }
 
     fun createCurrentClock(): ClockController {
         val clockId = currentClockId