Add unit test that simulates the deadlock scenario in SubscriptionMan…

…agerTests.kt

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();
+
+
+    }
 }

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>

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>
 

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

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 "[email protected]"
-                    it.optedIn shouldBe true
-                    it.status shouldBe SubscriptionStatus.SUBSCRIBED
-                },
+                    withArg {
+                        it.type shouldBe SubscriptionType.EMAIL
+                        it.address shouldBe "[email protected]"
+                        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
+                    },
             )
         }
     }

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({
+
+})