From c24f55fa63c3aefe93588d6d4aecbdd0d3d909ea Mon Sep 17 00:00:00 2001
From: jinliu9508 <jinliu9508@gmail.com>
Date: Wed, 13 Dec 2023 11:39:30 -0500
Subject: [PATCH 1/3] Add unit test that simulates the deadlock scenario in
 SubscriptionManagerTests.kt

---
 .../sdktest/model/MainActivityViewModel.java  |  46 ++++++++
 .../main/res/layout/main_activity_layout.xml  |  25 ++++
 .../app/src/main/res/values/strings.xml       |   1 +
 .../com/onesignal/common/modeling/Model.kt    |   5 +-
 .../subscriptions/SubscriptionManagerTests.kt | 111 ++++++++++++------
 .../com/onesignal/common/DeadlockTests.kt     |  14 +++
 6 files changed, 164 insertions(+), 38 deletions(-)
 create mode 100644 OneSignalSDK/onesignal/src/test/java/com/onesignal/common/DeadlockTests.kt

diff --git a/Examples/OneSignalDemo/app/src/main/java/com/onesignal/sdktest/model/MainActivityViewModel.java b/Examples/OneSignalDemo/app/src/main/java/com/onesignal/sdktest/model/MainActivityViewModel.java
index 58069a298c..9b4ec5e936 100644
--- a/Examples/OneSignalDemo/app/src/main/java/com/onesignal/sdktest/model/MainActivityViewModel.java
+++ b/Examples/OneSignalDemo/app/src/main/java/com/onesignal/sdktest/model/MainActivityViewModel.java
@@ -24,6 +24,7 @@
 import android.widget.TextView;
 import com.onesignal.Continue;
 import com.onesignal.OneSignal;
+import com.onesignal.core.internal.config.ConfigModelStore;
 import com.onesignal.sdktest.adapter.SubscriptionRecyclerViewAdapter;
 import com.onesignal.user.subscriptions.IPushSubscription;
 import com.onesignal.sdktest.R;
@@ -273,6 +274,11 @@ public ActivityViewModel onActivityCreated(Context context) {
             getActivity().startActivity(new Intent(getActivity(), SecondaryActivity.class));
         });
 
+        Button createDeadlock = getActivity().findViewById(R.id.main_activity_deadlock_button);
+        createDeadlock.setOnClickListener(v -> {
+            createDeadlock();
+        });
+
         aliasSet = new HashMap<>();
         aliasArrayList = new ArrayList<>();
 
@@ -931,4 +937,44 @@ private  void refreshState() {
         // triggers are not persisted, they are always "starting from scratch"
         refreshTriggerRecyclerView();
     }
+
+    private void createDeadlock() {
+
+        // register test observers
+        IPushSubscriptionObserver observer = state -> {
+            try {
+                Thread.sleep(1000);
+            } catch (InterruptedException e) {
+                throw new RuntimeException(e);
+            }
+            OneSignal.getUser().getPushSubscription().optIn();
+            System.out.println("JinTest optedout");
+        };
+        OneSignal.getUser().getPushSubscription().addObserver(observer);
+
+        // calling login will call setOptAnyProperty, which locks model.data
+        // then, any observer added will be fired, which will lock subscribers
+        Thread t1 = new Thread() {
+            @Override
+            public void run() {
+                OneSignal.getUser().getPushSubscription().optOut();
+                OneSignal.getUser().getPushSubscription().optIn();
+                System.out.println("JinTest optedin");
+            }
+        };
+
+        Thread t2 = new Thread() {
+            @Override
+            public void run() {
+                OneSignal.logout();
+                OneSignal.login("testJin");
+                System.out.println("JinTest login");
+            }
+        };
+
+        t1.start();
+        t2.start();
+
+
+    }
 }
diff --git a/Examples/OneSignalDemo/app/src/main/res/layout/main_activity_layout.xml b/Examples/OneSignalDemo/app/src/main/res/layout/main_activity_layout.xml
index ee4ce93cb5..5d1d29e28e 100644
--- a/Examples/OneSignalDemo/app/src/main/res/layout/main_activity_layout.xml
+++ b/Examples/OneSignalDemo/app/src/main/res/layout/main_activity_layout.xml
@@ -139,6 +139,7 @@
             android:orientation="vertical">
 
             <!-- Application -->
+
             <LinearLayout
                 android:id="@+id/main_activity_account_linear_layout"
                 android:layout_width="match_parent"
@@ -1268,6 +1269,30 @@
                   android:visibility="visible"/>
 
             </LinearLayout>
+
+            <LinearLayout
+                android:layout_width="match_parent"
+                android:layout_height="match_parent"
+                android:layout_gravity="center"
+                android:layout_marginStart="12dp"
+                android:layout_marginTop="4dp"
+                android:layout_marginEnd="12dp"
+                android:layout_marginBottom="12dp"
+                android:background="@color/colorPrimary"
+                android:gravity="center"
+                android:orientation="vertical">
+
+                <Button
+                    android:id="@+id/main_activity_deadlock_button"
+                    android:layout_width="match_parent"
+                    android:layout_height="match_parent"
+                    android:background="@drawable/ripple_selector_white_red"
+                    android:text="@string/create_deadlock"
+                    android:textColor="@android:color/white"
+                    android:textSize="19sp"
+                    android:visibility="visible" />
+
+            </LinearLayout>
         </LinearLayout>
 
     </androidx.core.widget.NestedScrollView>
diff --git a/Examples/OneSignalDemo/app/src/main/res/values/strings.xml b/Examples/OneSignalDemo/app/src/main/res/values/strings.xml
index 6fb52127e5..30ea85b09e 100644
--- a/Examples/OneSignalDemo/app/src/main/res/values/strings.xml
+++ b/Examples/OneSignalDemo/app/src/main/res/values/strings.xml
@@ -63,6 +63,7 @@
     <string name="revoke_consent">Revoke Consent</string>
 
     <string name="navigate_next_activity">Next activity</string>
+    <string name="create_deadlock">Create deadlock</string>
 
     <string name="onesignal_app_id">0ba9731b-33bd-43f4-8b59-61172e27447d</string>
 
diff --git a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/common/modeling/Model.kt b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/common/modeling/Model.kt
index fe7cbaac77..7864872838 100644
--- a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/common/modeling/Model.kt
+++ b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/common/modeling/Model.kt
@@ -438,7 +438,6 @@ open class Model(
     ) {
         synchronized(data) {
             val oldValue = data[name]
-
             if (oldValue == value && !forceChange) {
                 return
             }
@@ -700,7 +699,9 @@ open class Model(
 
     override fun subscribe(handler: IModelChangedHandler) = changeNotifier.subscribe(handler)
 
-    override fun unsubscribe(handler: IModelChangedHandler) = changeNotifier.unsubscribe(handler)
+    override fun unsubscribe(handler: IModelChangedHandler) {
+        changeNotifier.unsubscribe(handler)
+    }
 
     override val hasSubscribers: Boolean
         get() = changeNotifier.hasSubscribers
diff --git a/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/user/internal/subscriptions/SubscriptionManagerTests.kt b/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/user/internal/subscriptions/SubscriptionManagerTests.kt
index 168c9c4cee..3b6a5459d5 100644
--- a/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/user/internal/subscriptions/SubscriptionManagerTests.kt
+++ b/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/user/internal/subscriptions/SubscriptionManagerTests.kt
@@ -1,8 +1,13 @@
 package com.onesignal.user.internal.subscriptions
 
+import com.onesignal.common.modeling.IModelChangedHandler
+import com.onesignal.common.modeling.ISingletonModelStoreChangeHandler
 import com.onesignal.common.modeling.ModelChangeTags
 import com.onesignal.common.modeling.ModelChangedArgs
 import com.onesignal.core.internal.application.IApplicationService
+import com.onesignal.core.internal.config.ConfigModel
+import com.onesignal.extensions.RobolectricTest
+import com.onesignal.mocks.MockHelper
 import com.onesignal.session.internal.session.ISessionService
 import com.onesignal.user.internal.subscriptions.impl.SubscriptionManager
 import com.onesignal.user.subscriptions.ISmsSubscription
@@ -18,11 +23,45 @@ import io.mockk.mockk
 import io.mockk.runs
 import io.mockk.spyk
 import io.mockk.verify
+import junit.framework.TestCase
 import org.junit.runner.RunWith
 
+@RobolectricTest
 @RunWith(KotestTestRunner::class)
 class SubscriptionManagerTests : FunSpec({
 
+    test("Deadlock related to Model.setOptAnyProperty") {
+        // Given
+        val modelStore = MockHelper.configModelStore()
+        val model = modelStore.model
+
+        val t1 = Thread {
+            // acquire "model.data", then trigger the onChanged event
+            model.setOptAnyProperty("key1", "value1")
+        }
+
+        val t2 = Thread {
+            // acquire "model.initializationLock", then wait for "model.data" to be released
+            model.initializeFromModel("", MockHelper.configModelStore().model)
+        }
+
+        model.subscribe(object : IModelChangedHandler {
+            // will be executed in t1
+            override fun onChanged(args: ModelChangedArgs, tag: String) {
+                Thread.sleep(200)
+                // waiting for "model.initializationLock"
+                model.toJSON()
+            }
+        })
+
+        t1.start()
+        t2.start()
+        // Give some time for the thread to complete the task
+        Thread.sleep(1000)
+
+        TestCase.assertEquals(Thread.State.TERMINATED, t1.state)
+    }
+
     test("initializes subscriptions from model store") {
         // Given
         val mockSubscriptionModelStore = mockk<SubscriptionModelStore>()
@@ -91,12 +130,12 @@ class SubscriptionManagerTests : FunSpec({
         // Then
         verify {
             mockSubscriptionModelStore.add(
-                withArg {
-                    it.type shouldBe SubscriptionType.EMAIL
-                    it.address shouldBe "name@company.com"
-                    it.optedIn shouldBe true
-                    it.status shouldBe SubscriptionStatus.SUBSCRIBED
-                },
+                    withArg {
+                        it.type shouldBe SubscriptionType.EMAIL
+                        it.address shouldBe "name@company.com"
+                        it.optedIn shouldBe true
+                        it.status shouldBe SubscriptionStatus.SUBSCRIBED
+                    },
             )
         }
     }
@@ -120,12 +159,12 @@ class SubscriptionManagerTests : FunSpec({
         // Then
         verify {
             mockSubscriptionModelStore.add(
-                withArg {
-                    it.type shouldBe SubscriptionType.SMS
-                    it.address shouldBe "+15558675309"
-                    it.optedIn shouldBe true
-                    it.status shouldBe SubscriptionStatus.SUBSCRIBED
-                },
+                    withArg {
+                        it.type shouldBe SubscriptionType.SMS
+                        it.address shouldBe "+15558675309"
+                        it.optedIn shouldBe true
+                        it.status shouldBe SubscriptionStatus.SUBSCRIBED
+                    },
             )
         }
     }
@@ -149,12 +188,12 @@ class SubscriptionManagerTests : FunSpec({
         // Then
         verify {
             mockSubscriptionModelStore.add(
-                withArg {
-                    it.type shouldBe SubscriptionType.PUSH
-                    it.address shouldBe "pushToken"
-                    it.optedIn shouldBe true
-                    it.status shouldBe SubscriptionStatus.SUBSCRIBED
-                },
+                    withArg {
+                        it.type shouldBe SubscriptionType.PUSH
+                        it.address shouldBe "pushToken"
+                        it.optedIn shouldBe true
+                        it.status shouldBe SubscriptionStatus.SUBSCRIBED
+                    },
             )
         }
     }
@@ -279,11 +318,11 @@ class SubscriptionManagerTests : FunSpec({
         subscriptions.smss[0].number shouldBe smsSubscription.address
         verify(exactly = 1) {
             spySubscriptionChangedHandler.onSubscriptionAdded(
-                withArg {
-                    it.id shouldBe smsSubscription.id
-                    it should beInstanceOf<ISmsSubscription>()
-                    (it as ISmsSubscription).number shouldBe smsSubscription.address
-                },
+                    withArg {
+                        it.id shouldBe smsSubscription.id
+                        it should beInstanceOf<ISmsSubscription>()
+                        (it as ISmsSubscription).number shouldBe smsSubscription.address
+                    },
             )
         }
     }
@@ -313,14 +352,14 @@ class SubscriptionManagerTests : FunSpec({
         // When
         emailSubscription.address = "+15551234567"
         subscriptionManager.onModelUpdated(
-            ModelChangedArgs(
-                emailSubscription,
-                SubscriptionModel::address.name,
-                SubscriptionModel::address.name,
-                "+15558675309",
-                "+15551234567",
-            ),
-            ModelChangeTags.NORMAL,
+                ModelChangedArgs(
+                        emailSubscription,
+                        SubscriptionModel::address.name,
+                        SubscriptionModel::address.name,
+                        "+15558675309",
+                        "+15551234567",
+                ),
+                ModelChangeTags.NORMAL,
         )
         val subscriptions = subscriptionManager.subscriptions
 
@@ -361,11 +400,11 @@ class SubscriptionManagerTests : FunSpec({
         subscriptions.smss.count() shouldBe 0
         verify(exactly = 1) {
             spySubscriptionChangedHandler.onSubscriptionRemoved(
-                withArg {
-                    it.id shouldBe smsSubscription.id
-                    it should beInstanceOf<ISmsSubscription>()
-                    (it as ISmsSubscription).number shouldBe smsSubscription.address
-                },
+                    withArg {
+                        it.id shouldBe smsSubscription.id
+                        it should beInstanceOf<ISmsSubscription>()
+                        (it as ISmsSubscription).number shouldBe smsSubscription.address
+                    },
             )
         }
     }
diff --git a/OneSignalSDK/onesignal/src/test/java/com/onesignal/common/DeadlockTests.kt b/OneSignalSDK/onesignal/src/test/java/com/onesignal/common/DeadlockTests.kt
new file mode 100644
index 0000000000..47bcdb3b30
--- /dev/null
+++ b/OneSignalSDK/onesignal/src/test/java/com/onesignal/common/DeadlockTests.kt
@@ -0,0 +1,14 @@
+package com.onesignal.common
+
+import com.onesignal.common.modeling.IModelChangedHandler
+import com.onesignal.common.modeling.MapModel
+import com.onesignal.common.modeling.ModelChangedArgs
+import io.kotest.core.spec.style.FunSpec
+import io.kotest.runner.junit4.KotestTestRunner
+import junit.framework.TestCase
+import org.junit.runner.RunWith
+
+@RunWith(KotestTestRunner::class)
+class DeadlockTests : FunSpec({
+
+})
\ No newline at end of file

From fbe9f74f3014c2293b4908f3d36f10ee4a8f0803 Mon Sep 17 00:00:00 2001
From: jinliu9508 <jinliu9508@gmail.com>
Date: Tue, 2 Jan 2024 15:48:08 -0500
Subject: [PATCH 2/3] Resolve the deadlock and pass the unit test

---
 .../com/onesignal/common/modeling/Model.kt    | 118 +++++++++++-------
 .../onesignal/common/modeling/ModelStore.kt   |   6 +-
 .../subscriptions/SubscriptionManagerTests.kt |   7 +-
 3 files changed, 76 insertions(+), 55 deletions(-)

diff --git a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/common/modeling/Model.kt b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/common/modeling/Model.kt
index 7864872838..1f335bf8d3 100644
--- a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/common/modeling/Model.kt
+++ b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/common/modeling/Model.kt
@@ -80,33 +80,52 @@ open class Model(
      * @param jsonObject The [JSONObject] to initialize this model from.
      */
     fun initializeFromJson(jsonObject: JSONObject) {
-        data.clear()
-        for (property in jsonObject.keys()) {
-            val jsonValue = jsonObject.get(property)
-            if (jsonValue is JSONObject) {
-                val childModel = createModelForProperty(property, jsonValue)
-                if (childModel != null) {
-                    data[property] = childModel
-                }
-            } else if (jsonValue is JSONArray) {
-                val listOfItems = createListForProperty(property, jsonValue)
-                if (listOfItems != null) {
-                    data[property] = listOfItems
-                }
-            } else {
-                val method = this.javaClass.methods.firstOrNull { it.returnType != Void::class.java && it.name.contains(property, true) }
-
-                if (method == null) {
-                    data[property] = jsonObject.get(property)
+        synchronized(data) {
+            data.clear()
+            for (property in jsonObject.keys()) {
+                val jsonValue = jsonObject.get(property)
+                if (jsonValue is JSONObject) {
+                    val childModel = createModelForProperty(property, jsonValue)
+                    if (childModel != null) {
+                        data[property] = childModel
+                    }
+                } else if (jsonValue is JSONArray) {
+                    val listOfItems = createListForProperty(property, jsonValue)
+                    if (listOfItems != null) {
+                        data[property] = listOfItems
+                    }
                 } else {
-                    when (method.returnType) {
-                        Double::class.java, java.lang.Double::class.java -> data[property] = jsonObject.getDouble(property)
-                        Long::class.java, java.lang.Long::class.java -> data[property] = jsonObject.getLong(property)
-                        Float::class.java, java.lang.Float::class.java -> data[property] = jsonObject.getDouble(property).toFloat()
-                        Int::class.java, java.lang.Integer::class.java -> data[property] = jsonObject.getInt(property)
-                        Boolean::class.java, java.lang.Boolean::class.java -> data[property] = jsonObject.getBoolean(property)
-                        String::class.java, java.lang.String::class.java -> data[property] = jsonObject.getString(property)
-                        else -> data[property] = jsonObject.get(property)
+                    val method = this.javaClass.methods.firstOrNull {
+                        it.returnType != Void::class.java && it.name.contains(
+                            property,
+                            true
+                        )
+                    }
+
+                    if (method == null) {
+                        data[property] = jsonObject.get(property)
+                    } else {
+                        when (method.returnType) {
+                            Double::class.java, java.lang.Double::class.java -> data[property] =
+                                jsonObject.getDouble(property)
+
+                            Long::class.java, java.lang.Long::class.java -> data[property] =
+                                jsonObject.getLong(property)
+
+                            Float::class.java, java.lang.Float::class.java -> data[property] =
+                                jsonObject.getDouble(property).toFloat()
+
+                            Int::class.java, java.lang.Integer::class.java -> data[property] =
+                                jsonObject.getInt(property)
+
+                            Boolean::class.java, java.lang.Boolean::class.java -> data[property] =
+                                jsonObject.getBoolean(property)
+
+                            String::class.java, java.lang.String::class.java -> data[property] =
+                                jsonObject.getString(property)
+
+                            else -> data[property] = jsonObject.get(property)
+                        }
                     }
                 }
             }
@@ -141,8 +160,10 @@ open class Model(
         }
 
         synchronized(initializationLock) {
-            data.clear()
-            data.putAll(newData)
+            synchronized(data) {
+                data.clear()
+                data.putAll(newData)
+            }
         }
     }
 
@@ -436,8 +457,8 @@ open class Model(
         tag: String = ModelChangeTags.NORMAL,
         forceChange: Boolean = false,
     ) {
+        val oldValue = data[name]
         synchronized(data) {
-            val oldValue = data[name]
             if (oldValue == value && !forceChange) {
                 return
             }
@@ -447,9 +468,8 @@ open class Model(
             } else if (data.containsKey(name)) {
                 data.remove(name)
             }
-
-            notifyChanged(name, name, tag, oldValue, value)
         }
+        notifyChanged(name, name, tag, oldValue, value)
     }
 
     /**
@@ -672,24 +692,28 @@ open class Model(
     fun toJSON(): JSONObject {
         synchronized(initializationLock) {
             val jsonObject = JSONObject()
-            for (kvp in data) {
-                when (val value = kvp.value) {
-                    is Model -> {
-                        jsonObject.put(kvp.key, value.toJSON())
-                    }
-                    is List<*> -> {
-                        val jsonArray = JSONArray()
-                        for (arrayItem in value) {
-                            if (arrayItem is Model) {
-                                jsonArray.put(arrayItem.toJSON())
-                            } else {
-                                jsonArray.put(arrayItem)
+            synchronized(data) {
+                for (kvp in data) {
+                    when (val value = kvp.value) {
+                        is Model -> {
+                            jsonObject.put(kvp.key, value.toJSON())
+                        }
+
+                        is List<*> -> {
+                            val jsonArray = JSONArray()
+                            for (arrayItem in value) {
+                                if (arrayItem is Model) {
+                                    jsonArray.put(arrayItem.toJSON())
+                                } else {
+                                    jsonArray.put(arrayItem)
+                                }
                             }
+                            jsonObject.put(kvp.key, jsonArray)
+                        }
+
+                        else -> {
+                            jsonObject.put(kvp.key, value)
                         }
-                        jsonObject.put(kvp.key, jsonArray)
-                    }
-                    else -> {
-                        jsonObject.put(kvp.key, value)
                     }
                 }
             }
diff --git a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/common/modeling/ModelStore.kt b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/common/modeling/ModelStore.kt
index e39679d1b0..782ab203a1 100644
--- a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/common/modeling/ModelStore.kt
+++ b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/common/modeling/ModelStore.kt
@@ -84,7 +84,6 @@ abstract class ModelStore<TModel>(
         tag: String,
     ) {
         persist()
-
         changeSubscription.fire { it.onModelUpdated(args, tag) }
     }
 
@@ -102,10 +101,11 @@ abstract class ModelStore<TModel>(
 
     override fun clear(tag: String) {
         val localList = models.toList()
-        models.clear()
+        synchronized(models) {
+            models.clear()
 
             persist()
-
+        }
         for (item in localList) {
             // no longer listen for changes to this model
             item.unsubscribe(this)
diff --git a/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/user/internal/subscriptions/SubscriptionManagerTests.kt b/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/user/internal/subscriptions/SubscriptionManagerTests.kt
index 3b6a5459d5..68375146a9 100644
--- a/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/user/internal/subscriptions/SubscriptionManagerTests.kt
+++ b/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/user/internal/subscriptions/SubscriptionManagerTests.kt
@@ -1,12 +1,9 @@
 package com.onesignal.user.internal.subscriptions
 
 import com.onesignal.common.modeling.IModelChangedHandler
-import com.onesignal.common.modeling.ISingletonModelStoreChangeHandler
 import com.onesignal.common.modeling.ModelChangeTags
 import com.onesignal.common.modeling.ModelChangedArgs
 import com.onesignal.core.internal.application.IApplicationService
-import com.onesignal.core.internal.config.ConfigModel
-import com.onesignal.extensions.RobolectricTest
 import com.onesignal.mocks.MockHelper
 import com.onesignal.session.internal.session.ISessionService
 import com.onesignal.user.internal.subscriptions.impl.SubscriptionManager
@@ -26,7 +23,6 @@ import io.mockk.verify
 import junit.framework.TestCase
 import org.junit.runner.RunWith
 
-@RobolectricTest
 @RunWith(KotestTestRunner::class)
 class SubscriptionManagerTests : FunSpec({
 
@@ -56,9 +52,10 @@ class SubscriptionManagerTests : FunSpec({
 
         t1.start()
         t2.start()
+
         // Give some time for the thread to complete the task
         Thread.sleep(1000)
-
+        // verify if the thread has been successfully terminated
         TestCase.assertEquals(Thread.State.TERMINATED, t1.state)
     }
 

From 3d34cf008a17a0d794a320471b3a75622261b216 Mon Sep 17 00:00:00 2001
From: jinliu9508 <jinliu9508@gmail.com>
Date: Tue, 2 Jan 2024 16:27:27 -0500
Subject: [PATCH 3/3] Remove unused code and fix format

---
 .../sdktest/model/MainActivityViewModel.java  |  46 -------
 .../main/res/layout/main_activity_layout.xml  |  25 ----
 .../app/src/main/res/values/strings.xml       |   1 -
 .../com/onesignal/common/modeling/Model.kt    |  92 ++++++--------
 .../onesignal/common/modeling/ModelStore.kt   |   4 +-
 .../com/onesignal/common/ModelingTests.kt     | 119 ++++++++++++++++++
 .../subscriptions/SubscriptionManagerTests.kt | 108 ++++++----------
 .../com/onesignal/common/DeadlockTests.kt     |  14 ---
 8 files changed, 195 insertions(+), 214 deletions(-)
 create mode 100644 OneSignalSDK/onesignal/core/src/test/java/com/onesignal/common/ModelingTests.kt
 delete mode 100644 OneSignalSDK/onesignal/src/test/java/com/onesignal/common/DeadlockTests.kt

diff --git a/Examples/OneSignalDemo/app/src/main/java/com/onesignal/sdktest/model/MainActivityViewModel.java b/Examples/OneSignalDemo/app/src/main/java/com/onesignal/sdktest/model/MainActivityViewModel.java
index 9b4ec5e936..58069a298c 100644
--- a/Examples/OneSignalDemo/app/src/main/java/com/onesignal/sdktest/model/MainActivityViewModel.java
+++ b/Examples/OneSignalDemo/app/src/main/java/com/onesignal/sdktest/model/MainActivityViewModel.java
@@ -24,7 +24,6 @@
 import android.widget.TextView;
 import com.onesignal.Continue;
 import com.onesignal.OneSignal;
-import com.onesignal.core.internal.config.ConfigModelStore;
 import com.onesignal.sdktest.adapter.SubscriptionRecyclerViewAdapter;
 import com.onesignal.user.subscriptions.IPushSubscription;
 import com.onesignal.sdktest.R;
@@ -274,11 +273,6 @@ public ActivityViewModel onActivityCreated(Context context) {
             getActivity().startActivity(new Intent(getActivity(), SecondaryActivity.class));
         });
 
-        Button createDeadlock = getActivity().findViewById(R.id.main_activity_deadlock_button);
-        createDeadlock.setOnClickListener(v -> {
-            createDeadlock();
-        });
-
         aliasSet = new HashMap<>();
         aliasArrayList = new ArrayList<>();
 
@@ -937,44 +931,4 @@ private  void refreshState() {
         // triggers are not persisted, they are always "starting from scratch"
         refreshTriggerRecyclerView();
     }
-
-    private void createDeadlock() {
-
-        // register test observers
-        IPushSubscriptionObserver observer = state -> {
-            try {
-                Thread.sleep(1000);
-            } catch (InterruptedException e) {
-                throw new RuntimeException(e);
-            }
-            OneSignal.getUser().getPushSubscription().optIn();
-            System.out.println("JinTest optedout");
-        };
-        OneSignal.getUser().getPushSubscription().addObserver(observer);
-
-        // calling login will call setOptAnyProperty, which locks model.data
-        // then, any observer added will be fired, which will lock subscribers
-        Thread t1 = new Thread() {
-            @Override
-            public void run() {
-                OneSignal.getUser().getPushSubscription().optOut();
-                OneSignal.getUser().getPushSubscription().optIn();
-                System.out.println("JinTest optedin");
-            }
-        };
-
-        Thread t2 = new Thread() {
-            @Override
-            public void run() {
-                OneSignal.logout();
-                OneSignal.login("testJin");
-                System.out.println("JinTest login");
-            }
-        };
-
-        t1.start();
-        t2.start();
-
-
-    }
 }
diff --git a/Examples/OneSignalDemo/app/src/main/res/layout/main_activity_layout.xml b/Examples/OneSignalDemo/app/src/main/res/layout/main_activity_layout.xml
index 5d1d29e28e..ee4ce93cb5 100644
--- a/Examples/OneSignalDemo/app/src/main/res/layout/main_activity_layout.xml
+++ b/Examples/OneSignalDemo/app/src/main/res/layout/main_activity_layout.xml
@@ -139,7 +139,6 @@
             android:orientation="vertical">
 
             <!-- Application -->
-
             <LinearLayout
                 android:id="@+id/main_activity_account_linear_layout"
                 android:layout_width="match_parent"
@@ -1269,30 +1268,6 @@
                   android:visibility="visible"/>
 
             </LinearLayout>
-
-            <LinearLayout
-                android:layout_width="match_parent"
-                android:layout_height="match_parent"
-                android:layout_gravity="center"
-                android:layout_marginStart="12dp"
-                android:layout_marginTop="4dp"
-                android:layout_marginEnd="12dp"
-                android:layout_marginBottom="12dp"
-                android:background="@color/colorPrimary"
-                android:gravity="center"
-                android:orientation="vertical">
-
-                <Button
-                    android:id="@+id/main_activity_deadlock_button"
-                    android:layout_width="match_parent"
-                    android:layout_height="match_parent"
-                    android:background="@drawable/ripple_selector_white_red"
-                    android:text="@string/create_deadlock"
-                    android:textColor="@android:color/white"
-                    android:textSize="19sp"
-                    android:visibility="visible" />
-
-            </LinearLayout>
         </LinearLayout>
 
     </androidx.core.widget.NestedScrollView>
diff --git a/Examples/OneSignalDemo/app/src/main/res/values/strings.xml b/Examples/OneSignalDemo/app/src/main/res/values/strings.xml
index 30ea85b09e..6fb52127e5 100644
--- a/Examples/OneSignalDemo/app/src/main/res/values/strings.xml
+++ b/Examples/OneSignalDemo/app/src/main/res/values/strings.xml
@@ -63,7 +63,6 @@
     <string name="revoke_consent">Revoke Consent</string>
 
     <string name="navigate_next_activity">Next activity</string>
-    <string name="create_deadlock">Create deadlock</string>
 
     <string name="onesignal_app_id">0ba9731b-33bd-43f4-8b59-61172e27447d</string>
 
diff --git a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/common/modeling/Model.kt b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/common/modeling/Model.kt
index 1f335bf8d3..92344eebc3 100644
--- a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/common/modeling/Model.kt
+++ b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/common/modeling/Model.kt
@@ -51,7 +51,6 @@ open class Model(
      * specified, must also specify [_parentModel]
      */
     private val _parentProperty: String? = null,
-    private val initializationLock: Any = Any(),
 ) : IEventNotifier<IModelChangedHandler> {
     /**
      * A unique identifier for this model.
@@ -95,35 +94,26 @@ open class Model(
                         data[property] = listOfItems
                     }
                 } else {
-                    val method = this.javaClass.methods.firstOrNull {
-                        it.returnType != Void::class.java && it.name.contains(
-                            property,
-                            true
-                        )
-                    }
+                    val method =
+                        this.javaClass.methods.firstOrNull {
+                            it.returnType !=
+                                Void::class.java &&
+                                it.name.contains(
+                                    property,
+                                    true,
+                                )
+                        }
 
                     if (method == null) {
                         data[property] = jsonObject.get(property)
                     } else {
                         when (method.returnType) {
-                            Double::class.java, java.lang.Double::class.java -> data[property] =
-                                jsonObject.getDouble(property)
-
-                            Long::class.java, java.lang.Long::class.java -> data[property] =
-                                jsonObject.getLong(property)
-
-                            Float::class.java, java.lang.Float::class.java -> data[property] =
-                                jsonObject.getDouble(property).toFloat()
-
-                            Int::class.java, java.lang.Integer::class.java -> data[property] =
-                                jsonObject.getInt(property)
-
-                            Boolean::class.java, java.lang.Boolean::class.java -> data[property] =
-                                jsonObject.getBoolean(property)
-
-                            String::class.java, java.lang.String::class.java -> data[property] =
-                                jsonObject.getString(property)
-
+                            Double::class.java, java.lang.Double::class.java -> data[property] = jsonObject.getDouble(property)
+                            Long::class.java, java.lang.Long::class.java -> data[property] = jsonObject.getLong(property)
+                            Float::class.java, java.lang.Float::class.java -> data[property] = jsonObject.getDouble(property).toFloat()
+                            Int::class.java, java.lang.Integer::class.java -> data[property] = jsonObject.getInt(property)
+                            Boolean::class.java, java.lang.Boolean::class.java -> data[property] = jsonObject.getBoolean(property)
+                            String::class.java, java.lang.String::class.java -> data[property] = jsonObject.getString(property)
                             else -> data[property] = jsonObject.get(property)
                         }
                     }
@@ -159,11 +149,9 @@ open class Model(
             newData[::id.name] = id
         }
 
-        synchronized(initializationLock) {
-            synchronized(data) {
-                data.clear()
-                data.putAll(newData)
-            }
+        synchronized(data) {
+            data.clear()
+            data.putAll(newData)
         }
     }
 
@@ -690,42 +678,38 @@ open class Model(
      * @return The resulting [JSONObject].
      */
     fun toJSON(): JSONObject {
-        synchronized(initializationLock) {
-            val jsonObject = JSONObject()
-            synchronized(data) {
-                for (kvp in data) {
-                    when (val value = kvp.value) {
-                        is Model -> {
-                            jsonObject.put(kvp.key, value.toJSON())
-                        }
+        val jsonObject = JSONObject()
+        synchronized(data) {
+            for (kvp in data) {
+                when (val value = kvp.value) {
+                    is Model -> {
+                        jsonObject.put(kvp.key, value.toJSON())
+                    }
 
-                        is List<*> -> {
-                            val jsonArray = JSONArray()
-                            for (arrayItem in value) {
-                                if (arrayItem is Model) {
-                                    jsonArray.put(arrayItem.toJSON())
-                                } else {
-                                    jsonArray.put(arrayItem)
-                                }
+                    is List<*> -> {
+                        val jsonArray = JSONArray()
+                        for (arrayItem in value) {
+                            if (arrayItem is Model) {
+                                jsonArray.put(arrayItem.toJSON())
+                            } else {
+                                jsonArray.put(arrayItem)
                             }
-                            jsonObject.put(kvp.key, jsonArray)
                         }
+                        jsonObject.put(kvp.key, jsonArray)
+                    }
 
-                        else -> {
-                            jsonObject.put(kvp.key, value)
-                        }
+                    else -> {
+                        jsonObject.put(kvp.key, value)
                     }
                 }
             }
-            return jsonObject
         }
+        return jsonObject
     }
 
     override fun subscribe(handler: IModelChangedHandler) = changeNotifier.subscribe(handler)
 
-    override fun unsubscribe(handler: IModelChangedHandler) {
-        changeNotifier.unsubscribe(handler)
-    }
+    override fun unsubscribe(handler: IModelChangedHandler) = changeNotifier.unsubscribe(handler)
 
     override val hasSubscribers: Boolean
         get() = changeNotifier.hasSubscribers
diff --git a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/common/modeling/ModelStore.kt b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/common/modeling/ModelStore.kt
index 782ab203a1..083afccbe0 100644
--- a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/common/modeling/ModelStore.kt
+++ b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/common/modeling/ModelStore.kt
@@ -128,7 +128,7 @@ abstract class ModelStore<TModel>(
             model.subscribe(this)
 
             persist()
-
+        }
         changeSubscription.fire { it.onModelAdded(model, tag) }
     }
 
@@ -142,7 +142,7 @@ abstract class ModelStore<TModel>(
             model.unsubscribe(this)
 
             persist()
-
+        }
         changeSubscription.fire { it.onModelRemoved(model, tag) }
     }
 
diff --git a/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/common/ModelingTests.kt b/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/common/ModelingTests.kt
new file mode 100644
index 0000000000..16db95d981
--- /dev/null
+++ b/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/common/ModelingTests.kt
@@ -0,0 +1,119 @@
+package com.onesignal.common
+
+import com.onesignal.common.events.EventProducer
+import com.onesignal.common.modeling.IModelChangedHandler
+import com.onesignal.common.modeling.IModelStoreChangeHandler
+import com.onesignal.common.modeling.ModelChangedArgs
+import com.onesignal.mocks.MockHelper
+import com.onesignal.mocks.MockPreferencesService
+import com.onesignal.user.internal.subscriptions.SubscriptionModel
+import com.onesignal.user.internal.subscriptions.SubscriptionModelStore
+import io.kotest.core.spec.style.FunSpec
+import io.kotest.runner.junit4.KotestTestRunner
+import junit.framework.TestCase
+import org.junit.runner.RunWith
+
+@RunWith(KotestTestRunner::class)
+class ModelingTests : FunSpec({
+
+    test("Deadlock related to Model.setOptAnyProperty") {
+        // Given
+        val modelStore = MockHelper.configModelStore()
+        val model = modelStore.model
+
+        val t1 =
+            Thread {
+                // acquire "model.data", then trigger the onChanged event
+                model.setOptAnyProperty("key1", "value1")
+            }
+
+        val t2 =
+            Thread {
+                // acquire "model.initializationLock", then wait for "model.data" to be released
+                model.initializeFromModel("", MockHelper.configModelStore().model)
+            }
+
+        model.subscribe(
+            object : IModelChangedHandler {
+                // will be executed in t1
+                override fun onChanged(
+                    args: ModelChangedArgs,
+                    tag: String,
+                ) {
+                    Thread.sleep(200)
+                    // waiting for "model.initializationLock"
+                    model.toJSON()
+                }
+            },
+        )
+
+        t1.start()
+        t2.start()
+
+        // Set 1s timeout for t2 to complete the task
+        t2.join(1000)
+
+        // verify if the thread has been successfully terminated
+        TestCase.assertEquals(Thread.State.TERMINATED, t2.state)
+    }
+
+    test("Deadlock related to ModelSstore add() or remove()") {
+        // Given
+        val modelStore = SubscriptionModelStore(MockPreferencesService())
+        val event = EventProducer<SubscriptionModel>()
+        val oldSubscriptionModel = SubscriptionModel()
+        val newSubscriptionModel = SubscriptionModel()
+        oldSubscriptionModel.id = "oldModel"
+        newSubscriptionModel.id = "newModel"
+        modelStore.add(oldSubscriptionModel)
+
+        val t1 =
+            Thread {
+                // acquire "ModelStore.models", then trigger the onChanged event
+                System.out.println("1")
+                modelStore.add(newSubscriptionModel)
+            }
+
+        val t2 =
+            Thread {
+                System.out.println("2")
+                // acquire "model.data", then wait for "ModelStore.models"
+                newSubscriptionModel.toJSON()
+            }
+
+        modelStore.subscribe(
+            object : IModelStoreChangeHandler<SubscriptionModel> {
+                override fun onModelAdded(
+                    model: SubscriptionModel,
+                    tag: String,
+                ) {
+                    // waiting for "model.data"
+                    model.initializeFromModel("", MockHelper.configModelStore().model)
+                }
+
+                override fun onModelUpdated(
+                    args: ModelChangedArgs,
+                    tag: String,
+                ) {
+                    // left empty in purpose
+                }
+
+                override fun onModelRemoved(
+                    model: SubscriptionModel,
+                    tag: String,
+                ) {
+                    // left empty in purpose
+                }
+            },
+        )
+
+        t1.start()
+        t2.start()
+
+        // Set 1s timeout for t2 to complete the task
+        t2.join(1000)
+
+        // verify if the thread has been successfully terminated
+        TestCase.assertEquals(Thread.State.TERMINATED, t2.state)
+    }
+})
diff --git a/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/user/internal/subscriptions/SubscriptionManagerTests.kt b/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/user/internal/subscriptions/SubscriptionManagerTests.kt
index 68375146a9..168c9c4cee 100644
--- a/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/user/internal/subscriptions/SubscriptionManagerTests.kt
+++ b/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/user/internal/subscriptions/SubscriptionManagerTests.kt
@@ -1,10 +1,8 @@
 package com.onesignal.user.internal.subscriptions
 
-import com.onesignal.common.modeling.IModelChangedHandler
 import com.onesignal.common.modeling.ModelChangeTags
 import com.onesignal.common.modeling.ModelChangedArgs
 import com.onesignal.core.internal.application.IApplicationService
-import com.onesignal.mocks.MockHelper
 import com.onesignal.session.internal.session.ISessionService
 import com.onesignal.user.internal.subscriptions.impl.SubscriptionManager
 import com.onesignal.user.subscriptions.ISmsSubscription
@@ -20,45 +18,11 @@ import io.mockk.mockk
 import io.mockk.runs
 import io.mockk.spyk
 import io.mockk.verify
-import junit.framework.TestCase
 import org.junit.runner.RunWith
 
 @RunWith(KotestTestRunner::class)
 class SubscriptionManagerTests : FunSpec({
 
-    test("Deadlock related to Model.setOptAnyProperty") {
-        // Given
-        val modelStore = MockHelper.configModelStore()
-        val model = modelStore.model
-
-        val t1 = Thread {
-            // acquire "model.data", then trigger the onChanged event
-            model.setOptAnyProperty("key1", "value1")
-        }
-
-        val t2 = Thread {
-            // acquire "model.initializationLock", then wait for "model.data" to be released
-            model.initializeFromModel("", MockHelper.configModelStore().model)
-        }
-
-        model.subscribe(object : IModelChangedHandler {
-            // will be executed in t1
-            override fun onChanged(args: ModelChangedArgs, tag: String) {
-                Thread.sleep(200)
-                // waiting for "model.initializationLock"
-                model.toJSON()
-            }
-        })
-
-        t1.start()
-        t2.start()
-
-        // Give some time for the thread to complete the task
-        Thread.sleep(1000)
-        // verify if the thread has been successfully terminated
-        TestCase.assertEquals(Thread.State.TERMINATED, t1.state)
-    }
-
     test("initializes subscriptions from model store") {
         // Given
         val mockSubscriptionModelStore = mockk<SubscriptionModelStore>()
@@ -127,12 +91,12 @@ class SubscriptionManagerTests : FunSpec({
         // Then
         verify {
             mockSubscriptionModelStore.add(
-                    withArg {
-                        it.type shouldBe SubscriptionType.EMAIL
-                        it.address shouldBe "name@company.com"
-                        it.optedIn shouldBe true
-                        it.status shouldBe SubscriptionStatus.SUBSCRIBED
-                    },
+                withArg {
+                    it.type shouldBe SubscriptionType.EMAIL
+                    it.address shouldBe "name@company.com"
+                    it.optedIn shouldBe true
+                    it.status shouldBe SubscriptionStatus.SUBSCRIBED
+                },
             )
         }
     }
@@ -156,12 +120,12 @@ class SubscriptionManagerTests : FunSpec({
         // Then
         verify {
             mockSubscriptionModelStore.add(
-                    withArg {
-                        it.type shouldBe SubscriptionType.SMS
-                        it.address shouldBe "+15558675309"
-                        it.optedIn shouldBe true
-                        it.status shouldBe SubscriptionStatus.SUBSCRIBED
-                    },
+                withArg {
+                    it.type shouldBe SubscriptionType.SMS
+                    it.address shouldBe "+15558675309"
+                    it.optedIn shouldBe true
+                    it.status shouldBe SubscriptionStatus.SUBSCRIBED
+                },
             )
         }
     }
@@ -185,12 +149,12 @@ class SubscriptionManagerTests : FunSpec({
         // Then
         verify {
             mockSubscriptionModelStore.add(
-                    withArg {
-                        it.type shouldBe SubscriptionType.PUSH
-                        it.address shouldBe "pushToken"
-                        it.optedIn shouldBe true
-                        it.status shouldBe SubscriptionStatus.SUBSCRIBED
-                    },
+                withArg {
+                    it.type shouldBe SubscriptionType.PUSH
+                    it.address shouldBe "pushToken"
+                    it.optedIn shouldBe true
+                    it.status shouldBe SubscriptionStatus.SUBSCRIBED
+                },
             )
         }
     }
@@ -315,11 +279,11 @@ class SubscriptionManagerTests : FunSpec({
         subscriptions.smss[0].number shouldBe smsSubscription.address
         verify(exactly = 1) {
             spySubscriptionChangedHandler.onSubscriptionAdded(
-                    withArg {
-                        it.id shouldBe smsSubscription.id
-                        it should beInstanceOf<ISmsSubscription>()
-                        (it as ISmsSubscription).number shouldBe smsSubscription.address
-                    },
+                withArg {
+                    it.id shouldBe smsSubscription.id
+                    it should beInstanceOf<ISmsSubscription>()
+                    (it as ISmsSubscription).number shouldBe smsSubscription.address
+                },
             )
         }
     }
@@ -349,14 +313,14 @@ class SubscriptionManagerTests : FunSpec({
         // When
         emailSubscription.address = "+15551234567"
         subscriptionManager.onModelUpdated(
-                ModelChangedArgs(
-                        emailSubscription,
-                        SubscriptionModel::address.name,
-                        SubscriptionModel::address.name,
-                        "+15558675309",
-                        "+15551234567",
-                ),
-                ModelChangeTags.NORMAL,
+            ModelChangedArgs(
+                emailSubscription,
+                SubscriptionModel::address.name,
+                SubscriptionModel::address.name,
+                "+15558675309",
+                "+15551234567",
+            ),
+            ModelChangeTags.NORMAL,
         )
         val subscriptions = subscriptionManager.subscriptions
 
@@ -397,11 +361,11 @@ class SubscriptionManagerTests : FunSpec({
         subscriptions.smss.count() shouldBe 0
         verify(exactly = 1) {
             spySubscriptionChangedHandler.onSubscriptionRemoved(
-                    withArg {
-                        it.id shouldBe smsSubscription.id
-                        it should beInstanceOf<ISmsSubscription>()
-                        (it as ISmsSubscription).number shouldBe smsSubscription.address
-                    },
+                withArg {
+                    it.id shouldBe smsSubscription.id
+                    it should beInstanceOf<ISmsSubscription>()
+                    (it as ISmsSubscription).number shouldBe smsSubscription.address
+                },
             )
         }
     }
diff --git a/OneSignalSDK/onesignal/src/test/java/com/onesignal/common/DeadlockTests.kt b/OneSignalSDK/onesignal/src/test/java/com/onesignal/common/DeadlockTests.kt
deleted file mode 100644
index 47bcdb3b30..0000000000
--- a/OneSignalSDK/onesignal/src/test/java/com/onesignal/common/DeadlockTests.kt
+++ /dev/null
@@ -1,14 +0,0 @@
-package com.onesignal.common
-
-import com.onesignal.common.modeling.IModelChangedHandler
-import com.onesignal.common.modeling.MapModel
-import com.onesignal.common.modeling.ModelChangedArgs
-import io.kotest.core.spec.style.FunSpec
-import io.kotest.runner.junit4.KotestTestRunner
-import junit.framework.TestCase
-import org.junit.runner.RunWith
-
-@RunWith(KotestTestRunner::class)
-class DeadlockTests : FunSpec({
-
-})
\ No newline at end of file