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) {