Fix another race condition in the SnapshotStateObserver

Adding observed objects was not syncrhonized with removing objects causing a perioudic NPE. Fixes: 192677711 Test: ./gradlew :compose:r:r:tDUT Change-Id: I6d716afa248624b125119219fe892e02282c39d9
ggfan · Jul 12, 2021 · efca56e · efca56e
1 parent 0399731
commit efca56e
Show file tree

Hide file tree

Showing 2 changed files with 41 additions and 1 deletion.
diff --git a/...runtime/src/commonMain/kotlin/androidx/compose/runtime/snapshots/SnapshotStateObserver.kt b/...runtime/src/commonMain/kotlin/androidx/compose/runtime/snapshots/SnapshotStateObserver.kt
@@ -53,7 +53,9 @@ class SnapshotStateObserver(private val onChangedExecutor: (callback: () -> Unit
      */
     private val readObserver: (Any) -> Unit = { state ->
         if (!isPaused) {
-            currentMap!!.addValue(state)
+            synchronized(applyMaps) {
+                currentMap!!.addValue(state)
+            }
         }
     }
 

diff --git a/.../runtime/src/test/kotlin/androidx/compose/runtime/snapshots/SnapshotStateObserverTests.kt b/.../runtime/src/test/kotlin/androidx/compose/runtime/snapshots/SnapshotStateObserverTests.kt
@@ -324,6 +324,44 @@ class SnapshotStateObserverTests {
         assertNull(threadException)
     }
 
+    @Test // regression test for 192677711, second case
+    fun tryToReproduceSecondRaceCondtion() {
+        var running = true
+        var threadException: Exception? = null
+        try {
+            thread {
+                try {
+                    while (running) {
+                        Snapshot.sendApplyNotifications()
+                    }
+                } catch (e: Exception) {
+                    threadException = e
+                }
+            }
+
+            for (i in 1..10000) {
+                val state1 by mutableStateOf(0)
+                var state2 by mutableStateOf(true)
+                val observer = SnapshotStateObserver({}).apply {
+                    start()
+                }
+                observer.observeReads(Unit, {}) {
+                    repeat(1000) {
+                        @Suppress("UNUSED_EXPRESSION")
+                        state1
+                        if (state2) {
+                            state2 = false
+                        }
+                    }
+                }
+                assertNull(threadException)
+            }
+        } finally {
+            running = false
+        }
+        assertNull(threadException)
+    }
+
     private fun runSimpleTest(
         block: (modelObserver: SnapshotStateObserver, data: MutableState<Int>) -> Unit
     ) {