Make ServiceProvider.getService thread safe

[jkasten2]

Description

One Line Summary

Make ServiceProvider thread safe, to avoid cases where it can unexpectedly return different instances for the same interface.

Details

Motivation

We are getting a number of crash reports with appContext being null and I believe ServiceProvider.getService not being thread safe is cause. (but unknown if the only one)

Scope

Only thread safely changes to ServiceProvider.kt.

SDK Internal Details

When investigating we found a case where two instances of IApplicationService could be created. Specifically if an instance of ReceiveReceiptWorker is created by the Android Operating System when OneSignal.initWithContext is called (by the app developer or internally by the SDK). This can result in two thread calling ServiceProvider.getService(), creating the two instances of a class unexpectedly.

What This May Fix

While this fix addresses a single known cause of OneSignal.initWithContext throwing on a null Context error, it is unclear if this is the only source of this, or by how much this fix will help lower these crashes.

Reproducing the Crash in an app

While I could not reproduce the issue in a real world scenario there is a unit test in this PR ServiceProviderTest. Also a integration style test can be done by running the example app in the crash-with-null-context-on-initWithContext branch, see aa32927 through for more details.

Crashes

This PR may address NullPointerException from initWithContext like this one:
#1990

Caused by java.lang.NullPointerException:
       at com.onesignal.core.internal.application.impl.ApplicationService.getAppContext(ApplicationService.kt:40)
       at com.onesignal.core.internal.preferences.impl.PreferencesService.getSharedPrefsByName(PreferencesService.kt:234)
       at com.onesignal.core.internal.preferences.impl.PreferencesService.get(PreferencesService.kt:134)
       at com.onesignal.core.internal.preferences.impl.PreferencesService.getString(PreferencesService.kt:42)
       at com.onesignal.common.modeling.ModelStore.load(ModelStore.kt:159)
       at com.onesignal.common.modeling.SimpleModelStore.<init>(SimpleModelStore.kt:23)
       at com.onesignal.core.internal.config.ConfigModelStore.<init>(ConfigModelStore.kt:8)
       at java.lang.reflect.Constructor.newInstance0(Constructor.java)
       at java.lang.reflect.Constructor.newInstance(Constructor.java:343)
       at com.onesignal.common.services.ServiceRegistrationReflection.resolve(ServiceRegistration.kt:89)
       at com.onesignal.common.services.ServiceProvider.getServiceOrNull(ServiceProvider.kt:79)
       at com.onesignal.common.services.ServiceProvider.getService(ServiceProvider.kt:67)
       at com.onesignal.common.services.ServiceProvider.getService$com_onesignal_core(ServiceProvider.kt:39)
       at com.onesignal.internal.OneSignalImp.initWithContext(OneSignalImp.kt:199)
       at com.onesignal.OneSignal.initWithContext(OneSignal.kt:135)
       at com.dwarfplanet.bundle.v5.data.manager.OneSignalManager.init(OneSignalManager.kt:31)
       at com.dwarfplanet.bundle.v5.app.BundleApplication.initOneSignal(BundleApplication.kt:514)
       at com.dwarfplanet.bundle.v5.app.BundleApplication.onCreate(BundleApplication.kt:63)
       at android.app.Instrumentation.callApplicationOnCreate(Instrumentation.java:1190)
       at android.app.ActivityThread.handleBindApplication(ActivityThread.java:6582)
       at android.app.ActivityThread.access$1400(ActivityThread.java:224)
       at android.app.ActivityThread$H.handleMessage(ActivityThread.java:1887)
       at android.os.Handler.dispatchMessage(Handler.java:107)
       at android.os.Looper.loop(Looper.java:224)
       at android.app.ActivityThread.main(ActivityThread.java:7562)
       at java.lang.reflect.Method.invoke(Method.java)
       at com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run(RuntimeInit.java:539)
       at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:950)

Testing

Unit testing

Added a new test

Created a new ServiceProviderTest test confirm that ServiceBuilder.getService() should return the same instance, even when accessed from two threads. To ensure the test is consistent we made the constructor slow so both threads end up on the same line of the production code we want to test.

• First commit, only new test added, confirmation of CI failure
• Second commit, fix added, confirmation of CI passing

Manual testing

Tested on an Android 6 and Android 14 emulator, ensuring notifications display.

Affected code checklist

• Notifications
  • Display
  • Open
  • Push Processing
  • Confirm Deliveries
• Outcomes
• Sessions
• In-App Messaging
• REST API requests
• Public API changes

Checklist

Overview

• I have filled out all REQUIRED sections above
• PR does one thing
• Any Public API changes are explained in the PR details and conform to existing APIs

Testing

• I have included test coverage for these changes, or explained why they are not needed
• All automated tests pass, or I explained why that is not possible
• I have personally tested this on my device, or explained why that is not possible

Final pass

• Code is as readable as possible.
• I have reviewed this PR myself, ensuring it meets each checklist item

---

This change is