From efca56ef5287dd73ff56aa78536761a93947a3e3 Mon Sep 17 00:00:00 2001
From: Chuck Jazdzewski <chuckj@google.com>
Date: Mon, 12 Jul 2021 09:17:58 -0700
Subject: [PATCH] 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
---
 .../snapshots/SnapshotStateObserver.kt        |  4 +-
 .../snapshots/SnapshotStateObserverTests.kt   | 38 +++++++++++++++++++
 2 files changed, 41 insertions(+), 1 deletion(-)

diff --git a/compose/runtime/runtime/src/commonMain/kotlin/androidx/compose/runtime/snapshots/SnapshotStateObserver.kt b/compose/runtime/runtime/src/commonMain/kotlin/androidx/compose/runtime/snapshots/SnapshotStateObserver.kt
index aec9166f8b32d..8affa829be594 100644
--- a/compose/runtime/runtime/src/commonMain/kotlin/androidx/compose/runtime/snapshots/SnapshotStateObserver.kt
+++ b/compose/runtime/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/compose/runtime/runtime/src/test/kotlin/androidx/compose/runtime/snapshots/SnapshotStateObserverTests.kt b/compose/runtime/runtime/src/test/kotlin/androidx/compose/runtime/snapshots/SnapshotStateObserverTests.kt
index 66b607b9a9546..af482386b69ed 100644
--- a/compose/runtime/runtime/src/test/kotlin/androidx/compose/runtime/snapshots/SnapshotStateObserverTests.kt
+++ b/compose/runtime/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
     ) {