Skip to content

Commit 0e613e3

Browse files
committed
make ServiceProvider's serviceMap thread safe
Add synchronized around all access to ServiceProvider's serviceMap. This ensures nothing can modify this map, resulting in multiple instance of a class being return unexpectedly. Another critical path is the call to resolve(this), where this logic could also take enough time that could case threading issues that is now wrapped in this new lock. This most likely will fix the real world problem we have been seeing where IApplicationService.appContext is null unexpectedly. One discovered path is ReceiveReceiptWorker is created from the Android OS on a background thread and it calls getService() without first calling OneSignal.initWithContext, resulting in a race condition where two instance of IApplicationService can be created. ReceiveReceiptWorker probably should call OneSignal.initWithContext first, but changing this is out of scope for this commit.
1 parent 893381b commit 0e613e3

File tree

1 file changed

+19
-15
lines changed

1 file changed

+19
-15
lines changed

OneSignalSDK/onesignal/core/src/main/java/com/onesignal/common/services/ServiceProvider.kt

Lines changed: 19 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ class ServiceProvider(
99
registrations: List<ServiceRegistration<*>>,
1010
) : IServiceProvider {
1111
private var serviceMap: Map<Class<*>, List<ServiceRegistration<*>>>
12+
private val serviceMapLock = Any()
1213

1314
init {
1415
val serviceMap = mutableMapOf<Class<*>, MutableList<ServiceRegistration<*>>>()
@@ -44,23 +45,27 @@ class ServiceProvider(
4445
}
4546

4647
override fun <T> hasService(c: Class<T>): Boolean {
47-
return serviceMap.containsKey(c)
48+
synchronized(serviceMapLock) {
49+
return serviceMap.containsKey(c)
50+
}
4851
}
4952

5053
override fun <T> getAllServices(c: Class<T>): List<T> {
51-
val listOfServices: MutableList<T> = mutableListOf()
54+
synchronized(serviceMapLock) {
55+
val listOfServices: MutableList<T> = mutableListOf()
5256

53-
if (serviceMap.containsKey(c)) {
54-
for (serviceReg in serviceMap!![c]!!) {
55-
val service =
56-
serviceReg.resolve(this) as T?
57-
?: throw Exception("Could not instantiate service: $serviceReg")
57+
if (serviceMap.containsKey(c)) {
58+
for (serviceReg in serviceMap!![c]!!) {
59+
val service =
60+
serviceReg.resolve(this) as T?
61+
?: throw Exception("Could not instantiate service: $serviceReg")
5862

59-
listOfServices.add(service)
63+
listOfServices.add(service)
64+
}
6065
}
61-
}
6266

63-
return listOfServices
67+
return listOfServices
68+
}
6469
}
6570

6671
override fun <T> getService(c: Class<T>): T {
@@ -74,11 +79,10 @@ class ServiceProvider(
7479
}
7580

7681
override fun <T> getServiceOrNull(c: Class<T>): T? {
77-
Logging.debug("${indent}Retrieving service $c")
78-
// indent += " "
79-
val service = serviceMap[c]?.last()?.resolve(this) as T?
80-
// indent = indent.substring(0, indent.length-2)
81-
return service
82+
synchronized(serviceMapLock) {
83+
Logging.debug("${indent}Retrieving service $c")
84+
return serviceMap[c]?.last()?.resolve(this) as T?
85+
}
8286
}
8387

8488
companion object {

0 commit comments

Comments
 (0)