From 00d33806b50f05695e01be3329a969c4fb7e548a Mon Sep 17 00:00:00 2001 From: Josh Kasten Date: Mon, 17 Nov 2025 20:10:14 -0500 Subject: [PATCH] fix: remove throwing on init timeout Starting in 5.4.0 calls such as OneSignal.Notification or OneSignal.Location would throw if they waited on the SDK into to long. This comment changes this to wait forever until init is complete, and log instead if the time is taking longer than expected. If these calls were done from the main thread an ANR is the better of two evils and the app can recover, where an uncaught throw it can not. This commit will bring the behavior the similar behavior in 5.1.x. The only difference is you will now never see an ANR with initWithContext, however if you call other parts of the SDK from the main thread, such as OneSignal.Notification, you will continue to see ANRs, just at a different stacktrace. To avoid these ANRs completely call all other SDK methods outside of the main thread. --- .../common/threading/CompletionAwaiter.kt | 34 ++++--- .../com/onesignal/internal/OneSignalImp.kt | 5 +- .../threading/CompletionAwaiterTests.kt | 89 +++++-------------- .../core/internal/application/SDKInitTests.kt | 11 ++- 4 files changed, 43 insertions(+), 96 deletions(-) diff --git a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/common/threading/CompletionAwaiter.kt b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/common/threading/CompletionAwaiter.kt index 880556393b..0efe4c5b9c 100644 --- a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/common/threading/CompletionAwaiter.kt +++ b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/common/threading/CompletionAwaiter.kt @@ -57,27 +57,26 @@ class CompletionAwaiter( } /** - * Wait for completion using blocking approach with an optional timeout. + * Wait for completion using blocking approach * - * @param timeoutMs Timeout in milliseconds, defaults to context-appropriate timeout - * @return true if completed before timeout, false otherwise. + * @param timeoutToLog Timeout in milliseconds to log a message if not + * completed in time. Does NOT have any effect on logic. */ - fun await(timeoutMs: Long = getDefaultTimeout()): Boolean { - val completed = + fun awaitAndLogIfOverTimeout(timeoutToLog: Long = getDefaultTimeout()) { + logIfOverTimeout(timeoutToLog) + latch.await() + } + + private fun logIfOverTimeout(timeoutMs: Long) { + launchOnDefault { try { latch.await(timeoutMs, TimeUnit.MILLISECONDS) } catch (e: InterruptedException) { - Logging.warn("Interrupted while waiting for $componentName", e) + Logging.warn("$componentName is taking longer that normal!", e) logAllThreads() - false + Logging.warn(createTimeoutMessage(timeoutMs)) } - - if (!completed) { - val message = createTimeoutMessage(timeoutMs) - Logging.warn(message) } - - return completed } /** @@ -88,18 +87,15 @@ class CompletionAwaiter( suspendCompletion.await() } - private fun getDefaultTimeout(): Long { - return if (AndroidUtils.isRunningOnMainThread()) ANDROID_ANR_TIMEOUT_MS else DEFAULT_TIMEOUT_MS - } + private fun getDefaultTimeout(): Long = if (AndroidUtils.isRunningOnMainThread()) ANDROID_ANR_TIMEOUT_MS else DEFAULT_TIMEOUT_MS - private fun createTimeoutMessage(timeoutMs: Long): String { - return if (AndroidUtils.isRunningOnMainThread()) { + private fun createTimeoutMessage(timeoutMs: Long): String = + if (AndroidUtils.isRunningOnMainThread()) { "Timeout waiting for $componentName after ${timeoutMs}ms on the main thread. " + "This can cause ANRs. Consider calling from a background thread." } else { "Timeout waiting for $componentName after ${timeoutMs}ms." } - } private fun logAllThreads(): String { val sb = StringBuilder() diff --git a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/internal/OneSignalImp.kt b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/internal/OneSignalImp.kt index 99024c3da4..e06c84cdd9 100644 --- a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/internal/OneSignalImp.kt +++ b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/internal/OneSignalImp.kt @@ -334,10 +334,7 @@ internal class OneSignalImp( override fun getAllServices(c: Class): List = services.getAllServices(c) private fun waitForInit() { - val completed = initAwaiter.await() - if (!completed) { - throw IllegalStateException("initWithContext was not called or timed out") - } + initAwaiter.awaitAndLogIfOverTimeout() } /** diff --git a/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/common/threading/CompletionAwaiterTests.kt b/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/common/threading/CompletionAwaiterTests.kt index 37f239ead3..728cee9fa2 100644 --- a/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/common/threading/CompletionAwaiterTests.kt +++ b/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/common/threading/CompletionAwaiterTests.kt @@ -17,6 +17,7 @@ import kotlinx.coroutines.delay import kotlinx.coroutines.joinAll import kotlinx.coroutines.launch import kotlinx.coroutines.runBlocking +import kotlin.concurrent.thread class CompletionAwaiterTests : FunSpec({ @@ -39,11 +40,10 @@ class CompletionAwaiterTests : FunSpec({ // When val startTime = System.currentTimeMillis() - val completed = awaiter.await(1000) + awaiter.awaitAndLogIfOverTimeout(1000) val duration = System.currentTimeMillis() - startTime // Then - completed shouldBe true duration shouldBeLessThan 50L // Should be very fast } @@ -59,57 +59,37 @@ class CompletionAwaiterTests : FunSpec({ awaiter.complete() } - val result = awaiter.await(timeoutMs) + awaiter.awaitAndLogIfOverTimeout(timeoutMs) val duration = System.currentTimeMillis() - startTime - result shouldBe true duration shouldBeGreaterThan (completionDelay - 50) duration shouldBeLessThan (completionDelay + 150) // buffer } - test("await returns false when timeout expires") { + test("timeout input should only effect logging") { mockkObject(AndroidUtils) every { AndroidUtils.isRunningOnMainThread() } returns false - val timeoutMs = 200L - val startTime = System.currentTimeMillis() - - val completed = awaiter.await(timeoutMs) - val duration = System.currentTimeMillis() - startTime - - completed shouldBe false - duration shouldBeGreaterThan (timeoutMs - 50) - duration shouldBeLessThan (timeoutMs + 150) - } + val timeoutMs = 1L - test("await timeout of 0 returns false immediately when not completed") { - // Mock AndroidUtils to avoid Looper.getMainLooper() issues - mockkObject(AndroidUtils) - every { AndroidUtils.isRunningOnMainThread() } returns false + val thread = thread { awaiter.awaitAndLogIfOverTimeout(timeoutMs) } - val startTime = System.currentTimeMillis() - val completed = awaiter.await(0) - val duration = System.currentTimeMillis() - startTime + delay(100) - completed shouldBe false - duration shouldBeLessThan 20L + thread.isAlive shouldBe true - unmockkObject(AndroidUtils) + thread.interrupt() } test("multiple blocking callers are all unblocked") { val numCallers = 5 - val results = mutableListOf() val jobs = mutableListOf() // Start multiple blocking callers - repeat(numCallers) { index -> + repeat(numCallers) { val thread = Thread { - val result = awaiter.await(2000) - synchronized(results) { - results.add(result) - } + awaiter.awaitAndLogIfOverTimeout(2000) } thread.start() jobs.add(thread) @@ -122,11 +102,8 @@ class CompletionAwaiterTests : FunSpec({ awaiter.complete() // Wait for all threads to complete - jobs.forEach { it.join(1000) } - - // All should have completed successfully - results.size shouldBe numCallers - results.all { it } shouldBe true + // If this stalls on this test then it is considered failed + jobs.forEach { it.join() } } } @@ -245,14 +222,10 @@ class CompletionAwaiterTests : FunSpec({ awaiter = CompletionAwaiter("TestComponent") // Test blocking callers - val blockingResults = mutableListOf() val blockingThreads = - (1..2).map { index -> + (1..2).map { Thread { - val result = awaiter.await(2000) - synchronized(blockingResults) { - blockingResults.add(result) - } + awaiter.awaitAndLogIfOverTimeout(2000) } } blockingThreads.forEach { it.start() } @@ -264,10 +237,8 @@ class CompletionAwaiterTests : FunSpec({ awaiter.complete() // Wait for all to complete - blockingThreads.forEach { it.join(1000) } - - // All should have completed - blockingResults shouldBe arrayOf(true, true) + // Test considered failed if we hang here forever + blockingThreads.forEach { it.join() } } } @@ -280,8 +251,7 @@ class CompletionAwaiterTests : FunSpec({ awaiter.complete() // Should still work normally - val completed = awaiter.await(100) - completed shouldBe true + awaiter.awaitAndLogIfOverTimeout(100) } test("waiting after completion returns immediately") { @@ -331,33 +301,18 @@ class CompletionAwaiterTests : FunSpec({ context("timeout behavior") { - test("uses shorter timeout on main thread") { + test("uses shorter timeout on main thread").config(enabled = false) { mockkObject(AndroidUtils) every { AndroidUtils.isRunningOnMainThread() } returns true - val startTime = System.currentTimeMillis() - val completed = awaiter.await() // Default timeout - val duration = System.currentTimeMillis() - startTime - - completed shouldBe false - // Should use ANDROID_ANR_TIMEOUT_MS (4800ms) instead of DEFAULT_TIMEOUT_MS (30000ms) - duration shouldBeLessThan 6000L // Much less than 30 seconds - duration shouldBeGreaterThan 4000L // But around 4.8 seconds + // NOTE: Reintroduce test once we use Otel with non-fatal errors } - test("uses longer timeout on background thread") { + test("uses longer timeout on background thread").config(enabled = false) { mockkObject(AndroidUtils) every { AndroidUtils.isRunningOnMainThread() } returns false - // We can't actually wait 30 seconds in a test, so just verify it would use the longer timeout - // by checking the timeout logic doesn't kick in quickly - val startTime = System.currentTimeMillis() - val completed = awaiter.await(1000) // Force shorter timeout for test - val duration = System.currentTimeMillis() - startTime - - completed shouldBe false - duration shouldBeGreaterThan 900L - duration shouldBeLessThan 1200L + // NOTE: Reintroduce test once we use Otel with non-fatal errors } } }) diff --git a/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/core/internal/application/SDKInitTests.kt b/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/core/internal/application/SDKInitTests.kt index 318f6cb1c1..6e1a907a05 100644 --- a/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/core/internal/application/SDKInitTests.kt +++ b/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/core/internal/application/SDKInitTests.kt @@ -91,7 +91,7 @@ class SDKInitTests : FunSpec({ // block SharedPreference before calling init val trigger = CompletionAwaiter("Test") val context = getApplicationContext() - val blockingPrefContext = BlockingPrefsContext(context, trigger, 2000) + val blockingPrefContext = BlockingPrefsContext(context, trigger) val os = OneSignalImp() var initSuccess = true @@ -137,7 +137,7 @@ class SDKInitTests : FunSpec({ // block SharedPreference before calling init val trigger = CompletionAwaiter("Test") val context = getApplicationContext() - val blockingPrefContext = BlockingPrefsContext(context, trigger, 1000) + val blockingPrefContext = BlockingPrefsContext(context, trigger) val os = OneSignalImp() // When @@ -160,7 +160,7 @@ class SDKInitTests : FunSpec({ // block SharedPreference before calling init val trigger = CompletionAwaiter("Test") val context = getApplicationContext() - val blockingPrefContext = BlockingPrefsContext(context, trigger, 2000) + val blockingPrefContext = BlockingPrefsContext(context, trigger) val os = OneSignalImp() val accessorThread = @@ -204,7 +204,7 @@ class SDKInitTests : FunSpec({ // block SharedPreference before calling init val trigger = CompletionAwaiter("Test") val context = getApplicationContext() - val blockingPrefContext = BlockingPrefsContext(context, trigger, 2000) + val blockingPrefContext = BlockingPrefsContext(context, trigger) val os = OneSignalImp() val externalId = "testUser" @@ -438,14 +438,13 @@ class SDKInitTests : FunSpec({ class BlockingPrefsContext( context: Context, private val unblockTrigger: CompletionAwaiter, - private val timeoutInMillis: Long, ) : ContextWrapper(context) { override fun getSharedPreferences( name: String, mode: Int, ): SharedPreferences { try { - unblockTrigger.await(timeoutInMillis) + unblockTrigger.awaitAndLogIfOverTimeout() } catch (e: InterruptedException) { throw e } catch (e: TimeoutCancellationException) {