Skip to content

Commit c24f55f

Browse files
jinliu9508jinliu9508
committed
Add unit test that simulates the deadlock scenario in SubscriptionManagerTests.kt
1 parent 6b5f77c commit c24f55f

File tree

6 files changed

+164
-38
lines changed

6 files changed

+164
-38
lines changed

‎Examples/OneSignalDemo/app/src/main/java/com/onesignal/sdktest/model/MainActivityViewModel.java

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@
2424
import android.widget.TextView;
2525
import com.onesignal.Continue;
2626
import com.onesignal.OneSignal;
27+
import com.onesignal.core.internal.config.ConfigModelStore;
2728
import com.onesignal.sdktest.adapter.SubscriptionRecyclerViewAdapter;
2829
import com.onesignal.user.subscriptions.IPushSubscription;
2930
import com.onesignal.sdktest.R;
@@ -273,6 +274,11 @@ public ActivityViewModel onActivityCreated(Context context) {
273274
getActivity().startActivity(new Intent(getActivity(), SecondaryActivity.class));
274275
});
275276

277+
Button createDeadlock = getActivity().findViewById(R.id.main_activity_deadlock_button);
278+
createDeadlock.setOnClickListener(v -> {
279+
createDeadlock();
280+
});
281+
276282
aliasSet = new HashMap<>();
277283
aliasArrayList = new ArrayList<>();
278284

@@ -931,4 +937,44 @@ private void refreshState() {
931937
// triggers are not persisted, they are always "starting from scratch"
932938
refreshTriggerRecyclerView();
933939
}
940+
941+
private void createDeadlock() {
942+
943+
// register test observers
944+
IPushSubscriptionObserver observer = state -> {
945+
try {
946+
Thread.sleep(1000);
947+
} catch (InterruptedException e) {
948+
throw new RuntimeException(e);
949+
}
950+
OneSignal.getUser().getPushSubscription().optIn();
951+
System.out.println("JinTest optedout");
952+
};
953+
OneSignal.getUser().getPushSubscription().addObserver(observer);
954+
955+
// calling login will call setOptAnyProperty, which locks model.data
956+
// then, any observer added will be fired, which will lock subscribers
957+
Thread t1 = new Thread() {
958+
@Override
959+
public void run() {
960+
OneSignal.getUser().getPushSubscription().optOut();
961+
OneSignal.getUser().getPushSubscription().optIn();
962+
System.out.println("JinTest optedin");
963+
}
964+
};
965+
966+
Thread t2 = new Thread() {
967+
@Override
968+
public void run() {
969+
OneSignal.logout();
970+
OneSignal.login("testJin");
971+
System.out.println("JinTest login");
972+
}
973+
};
974+
975+
t1.start();
976+
t2.start();
977+
978+
979+
}
934980
}

‎Examples/OneSignalDemo/app/src/main/res/layout/main_activity_layout.xml

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -139,6 +139,7 @@
139139
android:orientation="vertical">
140140

141141
<!-- Application -->
142+
142143
<LinearLayout
143144
android:id="@+id/main_activity_account_linear_layout"
144145
android:layout_width="match_parent"
@@ -1268,6 +1269,30 @@
12681269
android:visibility="visible"/>
12691270

12701271
</LinearLayout>
1272+
1273+
<LinearLayout
1274+
android:layout_width="match_parent"
1275+
android:layout_height="match_parent"
1276+
android:layout_gravity="center"
1277+
android:layout_marginStart="12dp"
1278+
android:layout_marginTop="4dp"
1279+
android:layout_marginEnd="12dp"
1280+
android:layout_marginBottom="12dp"
1281+
android:background="@color/colorPrimary"
1282+
android:gravity="center"
1283+
android:orientation="vertical">
1284+
1285+
<Button
1286+
android:id="@+id/main_activity_deadlock_button"
1287+
android:layout_width="match_parent"
1288+
android:layout_height="match_parent"
1289+
android:background="@drawable/ripple_selector_white_red"
1290+
android:text="@string/create_deadlock"
1291+
android:textColor="@android:color/white"
1292+
android:textSize="19sp"
1293+
android:visibility="visible" />
1294+
1295+
</LinearLayout>
12711296
</LinearLayout>
12721297

12731298
</androidx.core.widget.NestedScrollView>

‎Examples/OneSignalDemo/app/src/main/res/values/strings.xml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,7 @@
6363
<string name="revoke_consent">Revoke Consent</string>
6464

6565
<string name="navigate_next_activity">Next activity</string>
66+
<string name="create_deadlock">Create deadlock</string>
6667

6768
<string name="onesignal_app_id">0ba9731b-33bd-43f4-8b59-61172e27447d</string>
6869

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

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -438,7 +438,6 @@ open class Model(
438438
) {
439439
synchronized(data) {
440440
val oldValue = data[name]
441-
442441
if (oldValue == value && !forceChange) {
443442
return
444443
}
@@ -700,7 +699,9 @@ open class Model(
700699

701700
override fun subscribe(handler: IModelChangedHandler) = changeNotifier.subscribe(handler)
702701

703-
override fun unsubscribe(handler: IModelChangedHandler) = changeNotifier.unsubscribe(handler)
702+
override fun unsubscribe(handler: IModelChangedHandler) {
703+
changeNotifier.unsubscribe(handler)
704+
}
704705

705706
override val hasSubscribers: Boolean
706707
get() = changeNotifier.hasSubscribers

‎OneSignalSDK/onesignal/core/src/test/java/com/onesignal/user/internal/subscriptions/SubscriptionManagerTests.kt

Lines changed: 75 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,13 @@
11
package com.onesignal.user.internal.subscriptions
22

3+
import com.onesignal.common.modeling.IModelChangedHandler
4+
import com.onesignal.common.modeling.ISingletonModelStoreChangeHandler
35
import com.onesignal.common.modeling.ModelChangeTags
46
import com.onesignal.common.modeling.ModelChangedArgs
57
import com.onesignal.core.internal.application.IApplicationService
8+
import com.onesignal.core.internal.config.ConfigModel
9+
import com.onesignal.extensions.RobolectricTest
10+
import com.onesignal.mocks.MockHelper
611
import com.onesignal.session.internal.session.ISessionService
712
import com.onesignal.user.internal.subscriptions.impl.SubscriptionManager
813
import com.onesignal.user.subscriptions.ISmsSubscription
@@ -18,11 +23,45 @@ import io.mockk.mockk
1823
import io.mockk.runs
1924
import io.mockk.spyk
2025
import io.mockk.verify
26+
import junit.framework.TestCase
2127
import org.junit.runner.RunWith
2228

29+
@RobolectricTest
2330
@RunWith(KotestTestRunner::class)
2431
class SubscriptionManagerTests : FunSpec({
2532

33+
test("Deadlock related to Model.setOptAnyProperty") {
34+
// Given
35+
val modelStore = MockHelper.configModelStore()
36+
val model = modelStore.model
37+
38+
val t1 = Thread {
39+
// acquire "model.data", then trigger the onChanged event
40+
model.setOptAnyProperty("key1", "value1")
41+
}
42+
43+
val t2 = Thread {
44+
// acquire "model.initializationLock", then wait for "model.data" to be released
45+
model.initializeFromModel("", MockHelper.configModelStore().model)
46+
}
47+
48+
model.subscribe(object : IModelChangedHandler {
49+
// will be executed in t1
50+
override fun onChanged(args: ModelChangedArgs, tag: String) {
51+
Thread.sleep(200)
52+
// waiting for "model.initializationLock"
53+
model.toJSON()
54+
}
55+
})
56+
57+
t1.start()
58+
t2.start()
59+
// Give some time for the thread to complete the task
60+
Thread.sleep(1000)
61+
62+
TestCase.assertEquals(Thread.State.TERMINATED, t1.state)
63+
}
64+
2665
test("initializes subscriptions from model store") {
2766
// Given
2867
val mockSubscriptionModelStore = mockk<SubscriptionModelStore>()
@@ -91,12 +130,12 @@ class SubscriptionManagerTests : FunSpec({
91130
// Then
92131
verify {
93132
mockSubscriptionModelStore.add(
94-
withArg {
95-
it.type shouldBe SubscriptionType.EMAIL
96-
it.address shouldBe "name@company.com"
97-
it.optedIn shouldBe true
98-
it.status shouldBe SubscriptionStatus.SUBSCRIBED
99-
},
133+
withArg {
134+
it.type shouldBe SubscriptionType.EMAIL
135+
it.address shouldBe "name@company.com"
136+
it.optedIn shouldBe true
137+
it.status shouldBe SubscriptionStatus.SUBSCRIBED
138+
},
100139
)
101140
}
102141
}
@@ -120,12 +159,12 @@ class SubscriptionManagerTests : FunSpec({
120159
// Then
121160
verify {
122161
mockSubscriptionModelStore.add(
123-
withArg {
124-
it.type shouldBe SubscriptionType.SMS
125-
it.address shouldBe "+15558675309"
126-
it.optedIn shouldBe true
127-
it.status shouldBe SubscriptionStatus.SUBSCRIBED
128-
},
162+
withArg {
163+
it.type shouldBe SubscriptionType.SMS
164+
it.address shouldBe "+15558675309"
165+
it.optedIn shouldBe true
166+
it.status shouldBe SubscriptionStatus.SUBSCRIBED
167+
},
129168
)
130169
}
131170
}
@@ -149,12 +188,12 @@ class SubscriptionManagerTests : FunSpec({
149188
// Then
150189
verify {
151190
mockSubscriptionModelStore.add(
152-
withArg {
153-
it.type shouldBe SubscriptionType.PUSH
154-
it.address shouldBe "pushToken"
155-
it.optedIn shouldBe true
156-
it.status shouldBe SubscriptionStatus.SUBSCRIBED
157-
},
191+
withArg {
192+
it.type shouldBe SubscriptionType.PUSH
193+
it.address shouldBe "pushToken"
194+
it.optedIn shouldBe true
195+
it.status shouldBe SubscriptionStatus.SUBSCRIBED
196+
},
158197
)
159198
}
160199
}
@@ -279,11 +318,11 @@ class SubscriptionManagerTests : FunSpec({
279318
subscriptions.smss[0].number shouldBe smsSubscription.address
280319
verify(exactly = 1) {
281320
spySubscriptionChangedHandler.onSubscriptionAdded(
282-
withArg {
283-
it.id shouldBe smsSubscription.id
284-
it should beInstanceOf<ISmsSubscription>()
285-
(it as ISmsSubscription).number shouldBe smsSubscription.address
286-
},
321+
withArg {
322+
it.id shouldBe smsSubscription.id
323+
it should beInstanceOf<ISmsSubscription>()
324+
(it as ISmsSubscription).number shouldBe smsSubscription.address
325+
},
287326
)
288327
}
289328
}
@@ -313,14 +352,14 @@ class SubscriptionManagerTests : FunSpec({
313352
// When
314353
emailSubscription.address = "+15551234567"
315354
subscriptionManager.onModelUpdated(
316-
ModelChangedArgs(
317-
emailSubscription,
318-
SubscriptionModel::address.name,
319-
SubscriptionModel::address.name,
320-
"+15558675309",
321-
"+15551234567",
322-
),
323-
ModelChangeTags.NORMAL,
355+
ModelChangedArgs(
356+
emailSubscription,
357+
SubscriptionModel::address.name,
358+
SubscriptionModel::address.name,
359+
"+15558675309",
360+
"+15551234567",
361+
),
362+
ModelChangeTags.NORMAL,
324363
)
325364
val subscriptions = subscriptionManager.subscriptions
326365

@@ -361,11 +400,11 @@ class SubscriptionManagerTests : FunSpec({
361400
subscriptions.smss.count() shouldBe 0
362401
verify(exactly = 1) {
363402
spySubscriptionChangedHandler.onSubscriptionRemoved(
364-
withArg {
365-
it.id shouldBe smsSubscription.id
366-
it should beInstanceOf<ISmsSubscription>()
367-
(it as ISmsSubscription).number shouldBe smsSubscription.address
368-
},
403+
withArg {
404+
it.id shouldBe smsSubscription.id
405+
it should beInstanceOf<ISmsSubscription>()
406+
(it as ISmsSubscription).number shouldBe smsSubscription.address
407+
},
369408
)
370409
}
371410
}

‎OneSignalSDK/onesignal/src/test/java/com/onesignal/common/DeadlockTests.kt

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
package com.onesignal.common
2+
3+
import com.onesignal.common.modeling.IModelChangedHandler
4+
import com.onesignal.common.modeling.MapModel
5+
import com.onesignal.common.modeling.ModelChangedArgs
6+
import io.kotest.core.spec.style.FunSpec
7+
import io.kotest.runner.junit4.KotestTestRunner
8+
import junit.framework.TestCase
9+
import org.junit.runner.RunWith
10+
11+
@RunWith(KotestTestRunner::class)
12+
class DeadlockTests : FunSpec({
13+
14+
})

0 commit comments

Comments
 (0)