fix: SDK-4504 ConcurrentModificationException in Model.initializeFromModel

OneSignalSDK/onesignal/core/src/main/java/com/onesignal/common/modeling/Model.kt

@@ -129,22 +129,33 @@ open class Model(
         id: String?,
         model: Model,
     ) {
-        val newData = Collections.synchronizedMap(mutableMapOf<String, Any?>())
+        // Snapshot the source map under its own monitor. `model.data` is a
+        // Collections.synchronizedMap, which only makes individual operations
+        // thread-safe; iterating its entry set requires holding the map's
+        // monitor or a ConcurrentModificationException can be thrown if
+        // another thread mutates it (e.g. via setOptAnyProperty).
+        val sourceSnapshot =
+            synchronized(model.data) {
+                model.data.toMap()
+            }
 
-        for (item in model.data) {
-            if (item.value is Model) {
-                val childModel = item.value as Model
-                childModel._parentModel = this
-                newData[item.key] = childModel
+        val newData = mutableMapOf<String, Any?>()
+        for ((key, value) in sourceSnapshot) {
+            if (value is Model) {
+                value._parentModel = this
+                newData[key] = value
             } else {
-                newData[item.key] = item.value
+                newData[key] = value
             }
         }
 
         if (id != null) {
             newData[::id.name] = id
         }
 
+        // Acquire `this.data` only after releasing `model.data` so the two
+        // monitors are never held simultaneously, avoiding any lock-ordering
+        // deadlocks between concurrent initializeFromModel calls.
         synchronized(data) {
             data.clear()
             data.putAll(newData)

OneSignalSDK/onesignal/core/src/test/java/com/onesignal/common/ModelingTests.kt

@@ -20,6 +20,50 @@ import java.util.UUID
 
 class ModelingTests : FunSpec({
 
+    test("initializeFromModel does not throw ConcurrentModificationException when source model is mutated concurrently") {
+        // Given a source model that is being mutated continuously on one thread...
+        val sourceModel = MockHelper.configModelStore().model
+        val destinationModel = MockHelper.configModelStore().model
+
+        val stop = java.util.concurrent.atomic.AtomicBoolean(false)
+        val failure = java.util.concurrent.atomic.AtomicReference<Throwable?>(null)
+
+        val mutator =
+            Thread {
+                var i = 0
+                while (!stop.get()) {
+                    sourceModel.setOptAnyProperty("k${i % 32}", "v$i")
+                    i++
+                }
+            }
+
+        val initializer =
+            Thread {
+                try {
+                    repeat(500) {
+                        destinationModel.initializeFromModel(null, sourceModel)
+                    }
+                } catch (t: Throwable) {
+                    failure.set(t)
+                } finally {
+                    stop.set(true)
+                }
+            }
+
+        // When both threads run concurrently
+        mutator.start()
+        initializer.start()
+
+        initializer.join(10_000)
+        stop.set(true)
+        mutator.join(10_000)
+
+        // Then initializeFromModel must not throw a ConcurrentModificationException
+        failure.get() shouldBe null
+        initializer.state shouldBe Thread.State.TERMINATED
+        mutator.state shouldBe Thread.State.TERMINATED
+    }
+
     test("Deadlock related to Model.setOptAnyProperty") {
         // Given
         val modelStore = MockHelper.configModelStore()