Skip to content

Commit a74f95d

Browse files
committed
Change more loops with callback calls and add unit test
1 parent 0f23b90 commit a74f95d

File tree

2 files changed

+32
-5
lines changed

2 files changed

+32
-5
lines changed

OneSignalSDK/onesignal/core/src/main/java/com/onesignal/common/events/EventProducer.kt

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,8 @@ open class EventProducer<THandler> : IEventNotifier<THandler> {
6060
*/
6161
fun fireOnMain(callback: (THandler) -> Unit) {
6262
suspendifyOnMain {
63-
for (s in subscribers) {
63+
val localList = subscribers.toList()
64+
for (s in localList) {
6465
callback(s)
6566
}
6667
}
@@ -73,7 +74,8 @@ open class EventProducer<THandler> : IEventNotifier<THandler> {
7374
* @param callback The callback will be invoked for each subscribed handler, allowing you to call the handler.
7475
*/
7576
suspend fun suspendingFire(callback: suspend (THandler) -> Unit) {
76-
for (s in subscribers) {
77+
val localList = subscribers.toList()
78+
for (s in localList) {
7779
callback(s)
7880
}
7981
}
@@ -86,7 +88,8 @@ open class EventProducer<THandler> : IEventNotifier<THandler> {
8688
*/
8789
suspend fun suspendingFireOnMain(callback: suspend (THandler) -> Unit) {
8890
withContext(Dispatchers.Main) {
89-
for (s in subscribers) {
91+
val localList = subscribers.toList()
92+
for (s in localList) {
9093
callback(s)
9194
}
9295
}

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

Lines changed: 26 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import com.onesignal.mocks.MockPreferencesService
99
import com.onesignal.user.internal.subscriptions.SubscriptionModel
1010
import com.onesignal.user.internal.subscriptions.SubscriptionModelStore
1111
import io.kotest.core.spec.style.FunSpec
12+
import io.kotest.matchers.shouldBe
1213
import io.kotest.runner.junit4.KotestTestRunner
1314
import junit.framework.TestCase
1415
import org.junit.runner.RunWith
@@ -70,13 +71,11 @@ class ModelingTests : FunSpec({
7071
val t1 =
7172
Thread {
7273
// acquire "ModelStore.models", then trigger the onChanged event
73-
System.out.println("1")
7474
modelStore.add(newSubscriptionModel)
7575
}
7676

7777
val t2 =
7878
Thread {
79-
System.out.println("2")
8079
// acquire "model.data", then wait for "ModelStore.models"
8180
newSubscriptionModel.toJSON()
8281
}
@@ -116,4 +115,29 @@ class ModelingTests : FunSpec({
116115
// verify if the thread has been successfully terminated
117116
TestCase.assertEquals(Thread.State.TERMINATED, t2.state)
118117
}
118+
119+
test("Unsubscribing handler in change event may cause the concurrent modification exception") {
120+
// Given an arbitrary model
121+
val modelStore = MockHelper.configModelStore()
122+
val model = modelStore.model
123+
124+
// subscribe to a change handler
125+
model.subscribe(
126+
object : IModelChangedHandler {
127+
override fun onChanged(
128+
args: ModelChangedArgs,
129+
tag: String,
130+
) {
131+
// remove from "subscribers" while "subscribers" is being accessed
132+
model.unsubscribe(this)
133+
}
134+
},
135+
)
136+
137+
// this will trigger EventProducer.fire and loop through the list "subscribers"
138+
model.setOptAnyProperty("key1", "value1")
139+
140+
// ensure no concurrent modification exception is thrown and "subcribers" is clear
141+
model.hasSubscribers shouldBe false
142+
}
119143
})

0 commit comments

Comments
 (0)