From 580b0bc6bb240479f5f293aeaeab8808f878deff Mon Sep 17 00:00:00 2001 From: AR Abdul Azeez Date: Wed, 12 Nov 2025 12:53:47 -0500 Subject: [PATCH 01/17] Handling race conditions --- .../com/onesignal/internal/OneSignalImp.kt | 47 +++++++++++++++++-- 1 file changed, 42 insertions(+), 5 deletions(-) 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..0d591a338e 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 @@ -263,7 +263,6 @@ internal class OneSignalImp( suspendifyOnIO { internalInit(context, appId) } - initState = InitState.SUCCESS return true } @@ -306,22 +305,48 @@ internal class OneSignalImp( ) { Logging.log(LogLevel.DEBUG, "Calling deprecated login(externalId: $externalId, jwtBearerToken: $jwtBearerToken)") - if (!initState.isSDKAccessible()) { - throw IllegalStateException("Must call 'initWithContext' before 'login'") + // Check state and provide appropriate error messages + when (initState) { + InitState.FAILED -> { + throw IllegalStateException("Initialization failed. Cannot proceed.") + } + InitState.NOT_STARTED -> { + throw IllegalStateException("Must call 'initWithContext' before 'login'") + } + InitState.IN_PROGRESS, InitState.SUCCESS -> { + // Continue - these states allow proceeding (will wait if needed) + } } waitForInit() + // Re-check state after waiting - init might have failed during the wait + if (initState == InitState.FAILED) { + throw IllegalStateException("Initialization failed. Cannot proceed.") + } suspendifyOnIO { loginHelper.login(externalId, jwtBearerToken) } } override fun logout() { Logging.log(LogLevel.DEBUG, "Calling deprecated logout()") - if (!initState.isSDKAccessible()) { - throw IllegalStateException("Must call 'initWithContext' before 'logout'") + // Check state and provide appropriate error messages + when (initState) { + InitState.FAILED -> { + throw IllegalStateException("Initialization failed. Cannot proceed.") + } + InitState.NOT_STARTED -> { + throw IllegalStateException("Must call 'initWithContext' before 'logout'") + } + InitState.IN_PROGRESS, InitState.SUCCESS -> { + // Continue - these states allow proceeding (will wait if needed) + } } waitForInit() + // Re-check state after waiting - init might have failed during the wait + if (initState == InitState.FAILED) { + throw IllegalStateException("Initialization failed. Cannot proceed.") + } suspendifyOnIO { logoutHelper.logout() } } @@ -358,6 +383,10 @@ internal class OneSignalImp( withTimeout(MAX_TIMEOUT_TO_INIT) { initAwaiter.awaitSuspend() } + // Re-check state after waiting - init might have failed during the wait + if (initState == InitState.FAILED) { + throw IllegalStateException("Initialization failed. Cannot proceed.") + } } catch (e: TimeoutCancellationException) { throw IllegalStateException("initWithContext was timed out after $MAX_TIMEOUT_TO_INIT ms") } @@ -384,6 +413,10 @@ internal class OneSignalImp( InitState.IN_PROGRESS -> { Logging.debug("Waiting for init to complete...") waitForInit() + // Re-check state after waiting - init might have failed during the wait + if (initState == InitState.FAILED) { + throw IllegalStateException("Initialization failed. Cannot proceed.") + } } InitState.FAILED -> { throw IllegalStateException("Initialization failed. Cannot proceed.") @@ -391,6 +424,10 @@ internal class OneSignalImp( else -> { // SUCCESS waitForInit() + // Re-check state after waiting - init might have failed during the wait + if (initState == InitState.FAILED) { + throw IllegalStateException("Initialization failed. Cannot proceed.") + } } } From 4c8a9e70cd04b4e6010415815d0edc272cb20daa Mon Sep 17 00:00:00 2001 From: AR Abdul Azeez Date: Wed, 12 Nov 2025 13:21:47 -0500 Subject: [PATCH 02/17] fix test --- .../core/internal/application/SDKInitTests.kt | 28 +++++++++++++++++-- 1 file changed, 25 insertions(+), 3 deletions(-) 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..04a5fa3453 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 @@ -150,8 +150,19 @@ class SDKInitTests : FunSpec({ accessorThread.join(500) // Then - // should complete even SharedPreferences is unavailable + // should complete even SharedPreferences is unavailable (non-blocking) accessorThread.isAlive shouldBe false + + // Release the SharedPreferences lock so internalInit can complete + trigger.complete() + + // Wait for initialization to complete (internalInit runs asynchronously) + var attempts = 0 + while (!os.isInitialized && attempts < 50) { + Thread.sleep(20) + attempts++ + } + os.isInitialized shouldBe true } @@ -224,12 +235,23 @@ class SDKInitTests : FunSpec({ accessorThread.start() accessorThread.join(500) - os.isInitialized shouldBe true + // initWithContext should return immediately (non-blocking) + // but isInitialized won't be true until internalInit completes + // which requires SharedPreferences to be unblocked accessorThread.isAlive shouldBe true - // release the lock on SharedPreferences + // release the lock on SharedPreferences so internalInit can complete trigger.complete() + // Wait for initialization to complete (internalInit runs asynchronously) + var initAttempts = 0 + while (!os.isInitialized && initAttempts < 50) { + Thread.sleep(20) + initAttempts++ + } + + os.isInitialized shouldBe true + accessorThread.join(500) accessorThread.isAlive shouldBe false os.user.externalId shouldBe externalId From 7e71e06ab4ed47a07a4b15604e474cf6c643728e Mon Sep 17 00:00:00 2001 From: AR Abdul Azeez Date: Wed, 12 Nov 2025 13:30:58 -0500 Subject: [PATCH 03/17] updating flow to use debugUnitTest --- .github/workflows/ci.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 1ba3736d0f..653b3bc3a6 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -26,7 +26,7 @@ jobs: - name: "[Test] SDK Unit Tests" working-directory: OneSignalSDK run: | - ./gradlew testReleaseUnitTest --console=plain --continue + ./gradlew testDebugUnitTest --console=plain --continue - name: "[Coverage] Generate JaCoCo merged XML" working-directory: OneSignalSDK run: | From b51b6b9dd0fd12c1152afcb2a62cff88e9bcc9f0 Mon Sep 17 00:00:00 2001 From: AR Abdul Azeez Date: Tue, 2 Dec 2025 11:14:59 -0500 Subject: [PATCH 04/17] removed exceptions and just logging --- .../com/onesignal/internal/OneSignalImp.kt | 152 +++++++++--------- .../core/internal/application/SDKInitTests.kt | 2 +- .../onesignal/internal/OneSignalImpTests.kt | 23 +-- 3 files changed, 89 insertions(+), 88 deletions(-) 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 0d591a338e..809f2c96cf 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 @@ -10,6 +10,7 @@ import com.onesignal.common.services.IServiceProvider import com.onesignal.common.services.ServiceBuilder import com.onesignal.common.services.ServiceProvider import com.onesignal.common.threading.CompletionAwaiter +import com.onesignal.common.threading.CompletionAwaiter.Companion.ANDROID_ANR_TIMEOUT_MS import com.onesignal.common.threading.OneSignalDispatchers import com.onesignal.common.threading.suspendifyOnIO import com.onesignal.core.CoreModule @@ -305,48 +306,16 @@ internal class OneSignalImp( ) { Logging.log(LogLevel.DEBUG, "Calling deprecated login(externalId: $externalId, jwtBearerToken: $jwtBearerToken)") - // Check state and provide appropriate error messages - when (initState) { - InitState.FAILED -> { - throw IllegalStateException("Initialization failed. Cannot proceed.") - } - InitState.NOT_STARTED -> { - throw IllegalStateException("Must call 'initWithContext' before 'login'") - } - InitState.IN_PROGRESS, InitState.SUCCESS -> { - // Continue - these states allow proceeding (will wait if needed) - } - } + waitForInit(operationName = "login") - waitForInit() - // Re-check state after waiting - init might have failed during the wait - if (initState == InitState.FAILED) { - throw IllegalStateException("Initialization failed. Cannot proceed.") - } suspendifyOnIO { loginHelper.login(externalId, jwtBearerToken) } } override fun logout() { Logging.log(LogLevel.DEBUG, "Calling deprecated logout()") - // Check state and provide appropriate error messages - when (initState) { - InitState.FAILED -> { - throw IllegalStateException("Initialization failed. Cannot proceed.") - } - InitState.NOT_STARTED -> { - throw IllegalStateException("Must call 'initWithContext' before 'logout'") - } - InitState.IN_PROGRESS, InitState.SUCCESS -> { - // Continue - these states allow proceeding (will wait if needed) - } - } + waitForInit(operationName = "logout") - waitForInit() - // Re-check state after waiting - init might have failed during the wait - if (initState == InitState.FAILED) { - throw IllegalStateException("Initialization failed. Cannot proceed.") - } suspendifyOnIO { logoutHelper.logout() } } @@ -358,29 +327,45 @@ 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") + /** + * Gets the appropriate timeout based on the current thread context. + * Uses shorter timeout on main thread to prevent ANRs. + */ + private fun getTimeoutForCurrentThread(): Long { + return try { + if (AndroidUtils.isRunningOnMainThread()) { + ANDROID_ANR_TIMEOUT_MS + } else { + MAX_TIMEOUT_TO_INIT + } + } catch (e: RuntimeException) { + // In test environments, AndroidUtils.isRunningOnMainThread() may fail + // because Looper.getMainLooper() is not mocked. Default to longer timeout. + MAX_TIMEOUT_TO_INIT } } /** - * Notifies both blocking and suspend callers that initialization is complete + * Common implementation for waiting until initialization completes. + * Handles all state checks and timeout logic. + * + * @param timeoutMs Timeout in milliseconds + * @param operationName Optional operation name to include in error messages (e.g., "login", "logout") */ - private fun notifyInitComplete() { - initAwaiter.complete() - } - - private suspend fun suspendUntilInit() { + private suspend fun waitUntilInitInternal(timeoutMs: Long, operationName: String? = null) { when (initState) { InitState.NOT_STARTED -> { - throw IllegalStateException("Must call 'initWithContext' before use") + val message = if (operationName != null) { + "Must call 'initWithContext' before '$operationName'" + } else { + "Must call 'initWithContext' before use" + } + throw IllegalStateException(message) } InitState.IN_PROGRESS -> { - Logging.debug("Suspend waiting for init to complete...") + Logging.debug("Waiting for init to complete...") try { - withTimeout(MAX_TIMEOUT_TO_INIT) { + withTimeout(timeoutMs) { initAwaiter.awaitSuspend() } // Re-check state after waiting - init might have failed during the wait @@ -388,7 +373,11 @@ internal class OneSignalImp( throw IllegalStateException("Initialization failed. Cannot proceed.") } } catch (e: TimeoutCancellationException) { - throw IllegalStateException("initWithContext was timed out after $MAX_TIMEOUT_TO_INIT ms") + Logging.warn("OneSignalImp is taking longer than normal! (timeout: ${timeoutMs}ms). Proceeding anyway, but operations may fail if initialization is not complete.", e) + // Re-check state after timeout - init might have failed during the wait + if (initState == InitState.FAILED) { + throw IllegalStateException("Initialization failed. Cannot proceed.") + } } } InitState.FAILED -> { @@ -400,37 +389,47 @@ internal class OneSignalImp( } } + /** + * Blocking version that waits for initialization to complete. + * Uses runBlocking to bridge to the suspend implementation. + * Preserves context-aware timeout behavior (shorter on main thread to prevent ANRs). + * + * @param timeoutMs Optional timeout in milliseconds. If not provided, uses context-aware timeout. + * @param operationName Optional operation name to include in error messages (e.g., "login", "logout") + */ + private fun waitForInit(timeoutMs: Long? = null, operationName: String? = null) { + val actualTimeout = timeoutMs ?: getTimeoutForCurrentThread() + runBlocking(ioDispatcher) { + waitUntilInitInternal(actualTimeout, operationName) + } + } + + /** + * Suspend version that waits for initialization to complete. + * Uses context-aware timeout (shorter on main thread to prevent ANRs). + * + * @param timeoutMs Optional timeout in milliseconds. If not provided, uses context-aware timeout. + * @param operationName Optional operation name to include in error messages (e.g., "login", "logout") + */ + private suspend fun suspendUntilInit(timeoutMs: Long? = null, operationName: String? = null) { + val actualTimeout = timeoutMs ?: getTimeoutForCurrentThread() + waitUntilInitInternal(actualTimeout, operationName) + } + + /** + * Notifies both blocking and suspend callers that initialization is complete + */ + private fun notifyInitComplete() { + initAwaiter.complete() + } + private suspend fun suspendAndReturn(getter: () -> T): T { suspendUntilInit() return getter() } private fun waitAndReturn(getter: () -> T): T { - when (initState) { - InitState.NOT_STARTED -> { - throw IllegalStateException("Must call 'initWithContext' before use") - } - InitState.IN_PROGRESS -> { - Logging.debug("Waiting for init to complete...") - waitForInit() - // Re-check state after waiting - init might have failed during the wait - if (initState == InitState.FAILED) { - throw IllegalStateException("Initialization failed. Cannot proceed.") - } - } - InitState.FAILED -> { - throw IllegalStateException("Initialization failed. Cannot proceed.") - } - else -> { - // SUCCESS - waitForInit() - // Re-check state after waiting - init might have failed during the wait - if (initState == InitState.FAILED) { - throw IllegalStateException("Initialization failed. Cannot proceed.") - } - } - } - + waitForInit() return getter() } @@ -444,8 +443,9 @@ internal class OneSignalImp( // because Looper.getMainLooper() is not mocked. This is safe to ignore. Logging.debug("Could not check main thread status (likely in test environment): ${e.message}") } + // Call suspendAndReturn directly to avoid nested runBlocking (waitAndReturn -> waitForInit -> runBlocking) return runBlocking(ioDispatcher) { - waitAndReturn(getter) + suspendAndReturn(getter) } } @@ -545,7 +545,7 @@ internal class OneSignalImp( ) = withContext(ioDispatcher) { Logging.log(LogLevel.DEBUG, "login(externalId: $externalId, jwtBearerToken: $jwtBearerToken)") - suspendUntilInit() + suspendUntilInit(operationName = "login") if (!isInitialized) { throw IllegalStateException("'initWithContext failed' before 'login'") } @@ -557,7 +557,7 @@ internal class OneSignalImp( withContext(ioDispatcher) { Logging.log(LogLevel.DEBUG, "logoutSuspend()") - suspendUntilInit() + suspendUntilInit(operationName = "logout") if (!isInitialized) { throw IllegalStateException("'initWithContext failed' before 'logout'") 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 04a5fa3453..5844a96809 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 @@ -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, 2000) val os = OneSignalImp() // When diff --git a/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/internal/OneSignalImpTests.kt b/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/internal/OneSignalImpTests.kt index e5e49f1ec0..9599d2bd5f 100644 --- a/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/internal/OneSignalImpTests.kt +++ b/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/internal/OneSignalImpTests.kt @@ -215,14 +215,14 @@ class OneSignalImpTests : FunSpec({ test("waitForInit timeout behavior - this test demonstrates the timeout mechanism") { // This test documents that waitForInit() has timeout protection // In a real scenario, if initWithContext was never called, - // waitForInit() would timeout after 30 seconds and throw an exception + // waitForInit() would timeout after 30 seconds and log a warning (not throw) // Given - a fresh OneSignalImp instance val oneSignalImp = OneSignalImp() - // The timeout behavior is built into CompletionAwaiter.await() - // which waits for up to 30 seconds (or 4.8 seconds on main thread) - // before timing out and returning false + // The timeout behavior is built into waitUntilInitInternal() + // which uses withTimeout() to wait for up to 30 seconds (or 4.8 seconds on main thread) + // before logging a warning and proceeding // NOTE: We don't actually test the 30-second timeout here because: // 1. It would make tests too slow (30 seconds per test) @@ -234,13 +234,13 @@ class OneSignalImpTests : FunSpec({ test("waitForInit timeout mechanism exists - CompletionAwaiter integration") { // This test verifies that the timeout mechanism is properly integrated - // by checking that CompletionAwaiter has timeout capabilities + // by checking that waitUntilInitInternal has timeout capabilities // Given val oneSignalImp = OneSignalImp() - // The timeout behavior is implemented through CompletionAwaiter.await() - // which has a default timeout of 30 seconds (or 4.8 seconds on main thread) + // The timeout behavior is implemented through waitUntilInitInternal() + // which uses withTimeout() with a default timeout of 30 seconds (or 4.8 seconds on main thread) // We can verify the timeout mechanism exists by checking: // 1. The CompletionAwaiter is properly initialized @@ -250,10 +250,11 @@ class OneSignalImpTests : FunSpec({ oneSignalImp.isInitialized shouldBe false // In a real scenario where initWithContext is never called: - // - waitForInit() would call initAwaiter.await() - // - CompletionAwaiter.await() would wait up to 30 seconds - // - After timeout, it would return false - // - waitForInit() would then throw "initWithContext was not called or timed out" + // - waitForInit() would call waitUntilInitInternal() + // - waitUntilInitInternal() would check initState == NOT_STARTED and throw immediately + // - If initState was IN_PROGRESS, it would use withTimeout() to wait up to 30 seconds + // - After timeout during IN_PROGRESS, it would log "OneSignalImp is taking longer than normal!" and proceed + // - waitForInit() throws for NOT_STARTED/FAILED states, but only logs (doesn't throw) on timeout during IN_PROGRESS // This test documents this behavior without actually waiting 30 seconds } From fb729bba9dba895b2f3b0b88feea35f25a15373b Mon Sep 17 00:00:00 2001 From: abdulraqeeb33 Date: Tue, 2 Dec 2025 11:22:49 -0500 Subject: [PATCH 05/17] Change unit test command to testReleaseUnitTest --- .github/workflows/ci.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 653b3bc3a6..1ba3736d0f 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -26,7 +26,7 @@ jobs: - name: "[Test] SDK Unit Tests" working-directory: OneSignalSDK run: | - ./gradlew testDebugUnitTest --console=plain --continue + ./gradlew testReleaseUnitTest --console=plain --continue - name: "[Coverage] Generate JaCoCo merged XML" working-directory: OneSignalSDK run: | From 6e2531e67ef57e9147452c71458fe7de89456cc7 Mon Sep 17 00:00:00 2001 From: AR Abdul Azeez Date: Tue, 2 Dec 2025 11:28:48 -0500 Subject: [PATCH 06/17] moving the code a bit --- .../common/threading/CompletionAwaiter.kt | 15 +++- .../com/onesignal/internal/OneSignalImp.kt | 79 +++++++------------ 2 files changed, 42 insertions(+), 52 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..be58aff76f 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 @@ -88,8 +88,19 @@ class CompletionAwaiter( suspendCompletion.await() } - private fun getDefaultTimeout(): Long { - return if (AndroidUtils.isRunningOnMainThread()) ANDROID_ANR_TIMEOUT_MS else DEFAULT_TIMEOUT_MS + /** + * Gets the appropriate timeout based on the current thread context. + * Uses shorter timeout on main thread to prevent ANRs. + * Made internal so it can be reused by other classes. + */ + internal fun getDefaultTimeout(): Long { + return try { + if (AndroidUtils.isRunningOnMainThread()) ANDROID_ANR_TIMEOUT_MS else DEFAULT_TIMEOUT_MS + } catch (e: RuntimeException) { + // In test environments, AndroidUtils.isRunningOnMainThread() may fail + // because Looper.getMainLooper() is not mocked. Default to longer timeout. + DEFAULT_TIMEOUT_MS + } } private fun createTimeoutMessage(timeoutMs: Long): String { 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 809f2c96cf..b466366fca 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 @@ -10,7 +10,6 @@ import com.onesignal.common.services.IServiceProvider import com.onesignal.common.services.ServiceBuilder import com.onesignal.common.services.ServiceProvider import com.onesignal.common.threading.CompletionAwaiter -import com.onesignal.common.threading.CompletionAwaiter.Companion.ANDROID_ANR_TIMEOUT_MS import com.onesignal.common.threading.OneSignalDispatchers import com.onesignal.common.threading.suspendifyOnIO import com.onesignal.core.CoreModule @@ -46,8 +45,6 @@ import kotlinx.coroutines.runBlocking import kotlinx.coroutines.withContext import kotlinx.coroutines.withTimeout -private const val MAX_TIMEOUT_TO_INIT = 30_000L // 30 seconds - internal class OneSignalImp( private val ioDispatcher: CoroutineDispatcher = OneSignalDispatchers.IO, ) : IOneSignal, IServiceProvider { @@ -328,23 +325,39 @@ internal class OneSignalImp( override fun getAllServices(c: Class): List = services.getAllServices(c) /** - * Gets the appropriate timeout based on the current thread context. - * Uses shorter timeout on main thread to prevent ANRs. + * Notifies both blocking and suspend callers that initialization is complete */ - private fun getTimeoutForCurrentThread(): Long { - return try { - if (AndroidUtils.isRunningOnMainThread()) { - ANDROID_ANR_TIMEOUT_MS - } else { - MAX_TIMEOUT_TO_INIT - } - } catch (e: RuntimeException) { - // In test environments, AndroidUtils.isRunningOnMainThread() may fail - // because Looper.getMainLooper() is not mocked. Default to longer timeout. - MAX_TIMEOUT_TO_INIT + private fun notifyInitComplete() { + initAwaiter.complete() + } + + /** + * Blocking version that waits for initialization to complete. + * Uses runBlocking to bridge to the suspend implementation. + * Preserves context-aware timeout behavior (shorter on main thread to prevent ANRs). + * + * @param timeoutMs Optional timeout in milliseconds. If not provided, uses context-aware timeout. + * @param operationName Optional operation name to include in error messages (e.g., "login", "logout") + */ + private fun waitForInit(timeoutMs: Long? = null, operationName: String? = null) { + val actualTimeout = timeoutMs ?: initAwaiter.getDefaultTimeout() + runBlocking(ioDispatcher) { + waitUntilInitInternal(actualTimeout, operationName) } } + /** + * Suspend version that waits for initialization to complete. + * Uses context-aware timeout (shorter on main thread to prevent ANRs). + * + * @param timeoutMs Optional timeout in milliseconds. If not provided, uses context-aware timeout. + * @param operationName Optional operation name to include in error messages (e.g., "login", "logout") + */ + private suspend fun suspendUntilInit(timeoutMs: Long? = null, operationName: String? = null) { + val actualTimeout = timeoutMs ?: initAwaiter.getDefaultTimeout() + waitUntilInitInternal(actualTimeout, operationName) + } + /** * Common implementation for waiting until initialization completes. * Handles all state checks and timeout logic. @@ -389,40 +402,6 @@ internal class OneSignalImp( } } - /** - * Blocking version that waits for initialization to complete. - * Uses runBlocking to bridge to the suspend implementation. - * Preserves context-aware timeout behavior (shorter on main thread to prevent ANRs). - * - * @param timeoutMs Optional timeout in milliseconds. If not provided, uses context-aware timeout. - * @param operationName Optional operation name to include in error messages (e.g., "login", "logout") - */ - private fun waitForInit(timeoutMs: Long? = null, operationName: String? = null) { - val actualTimeout = timeoutMs ?: getTimeoutForCurrentThread() - runBlocking(ioDispatcher) { - waitUntilInitInternal(actualTimeout, operationName) - } - } - - /** - * Suspend version that waits for initialization to complete. - * Uses context-aware timeout (shorter on main thread to prevent ANRs). - * - * @param timeoutMs Optional timeout in milliseconds. If not provided, uses context-aware timeout. - * @param operationName Optional operation name to include in error messages (e.g., "login", "logout") - */ - private suspend fun suspendUntilInit(timeoutMs: Long? = null, operationName: String? = null) { - val actualTimeout = timeoutMs ?: getTimeoutForCurrentThread() - waitUntilInitInternal(actualTimeout, operationName) - } - - /** - * Notifies both blocking and suspend callers that initialization is complete - */ - private fun notifyInitComplete() { - initAwaiter.complete() - } - private suspend fun suspendAndReturn(getter: () -> T): T { suspendUntilInit() return getter() From 23a763f7e4090cd6391f9d925d3e23bb97c9470c Mon Sep 17 00:00:00 2001 From: AR Abdul Azeez Date: Wed, 3 Dec 2025 08:43:23 -0500 Subject: [PATCH 07/17] lint --- .../common/threading/OneSignalDispatchers.kt | 11 ++-- .../com/onesignal/internal/OneSignalImp.kt | 64 ++++++++++--------- 2 files changed, 42 insertions(+), 33 deletions(-) diff --git a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/common/threading/OneSignalDispatchers.kt b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/common/threading/OneSignalDispatchers.kt index 19c7b1ddde..8ff3d61ea1 100644 --- a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/common/threading/OneSignalDispatchers.kt +++ b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/common/threading/OneSignalDispatchers.kt @@ -27,15 +27,16 @@ import java.util.concurrent.atomic.AtomicInteger * - Efficient thread management with controlled resource usage */ internal object OneSignalDispatchers { - // Optimized pool sizes based on CPU cores and workload analysis - private const val IO_CORE_POOL_SIZE = 2 // Increased for better concurrency - private const val IO_MAX_POOL_SIZE = 3 // Increased for better concurrency + // Optimized pool thread counts to handle more concurrent operations during init + // (especially important now that we wait indefinitely, which may cause more operations to queue) + private const val IO_CORE_POOL_SIZE = 4 // Increased for better concurrency during init + private const val IO_MAX_POOL_SIZE = 6 // Increased to handle bursts of operations private const val DEFAULT_CORE_POOL_SIZE = 2 // Optimal for CPU operations private const val DEFAULT_MAX_POOL_SIZE = 3 // Slightly larger for CPU operations private const val KEEP_ALIVE_TIME_SECONDS = 30L // Keep threads alive longer to reduce recreation private const val QUEUE_CAPACITY = - 10 // Small queue that allows up to 10 tasks to wait in queue when all threads are busy + 200 // Increased to handle more queued operations during init, while still preventing memory bloat internal const val BASE_THREAD_NAME = "OneSignal" // Base thread name prefix private const val IO_THREAD_NAME_PREFIX = "$BASE_THREAD_NAME-IO" // Thread name prefix for I/O operations @@ -69,6 +70,7 @@ internal object OneSignalDispatchers { priority = Thread.NORM_PRIORITY - 1, // Slightly lower priority for I/O tasks ), + ThreadPoolExecutor.CallerRunsPolicy(), // Execute on calling thread if queue is full (prevents rejection) ).apply { allowCoreThreadTimeOut(false) // Keep core threads alive } @@ -87,6 +89,7 @@ internal object OneSignalDispatchers { TimeUnit.SECONDS, LinkedBlockingQueue(QUEUE_CAPACITY), OptimizedThreadFactory(DEFAULT_THREAD_NAME_PREFIX), + ThreadPoolExecutor.CallerRunsPolicy(), // Execute on calling thread if queue is full (prevents rejection) ).apply { allowCoreThreadTimeOut(false) // Keep core threads alive } 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 b466366fca..37e5bc067a 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 @@ -40,10 +40,8 @@ import com.onesignal.user.internal.properties.PropertiesModelStore import com.onesignal.user.internal.resolveAppId import com.onesignal.user.internal.subscriptions.SubscriptionModelStore import kotlinx.coroutines.CoroutineDispatcher -import kotlinx.coroutines.TimeoutCancellationException import kotlinx.coroutines.runBlocking import kotlinx.coroutines.withContext -import kotlinx.coroutines.withTimeout internal class OneSignalImp( private val ioDispatcher: CoroutineDispatcher = OneSignalDispatchers.IO, @@ -334,38 +332,34 @@ internal class OneSignalImp( /** * Blocking version that waits for initialization to complete. * Uses runBlocking to bridge to the suspend implementation. - * Preserves context-aware timeout behavior (shorter on main thread to prevent ANRs). + * Waits indefinitely until init completes and logs how long it took. * - * @param timeoutMs Optional timeout in milliseconds. If not provided, uses context-aware timeout. * @param operationName Optional operation name to include in error messages (e.g., "login", "logout") */ - private fun waitForInit(timeoutMs: Long? = null, operationName: String? = null) { - val actualTimeout = timeoutMs ?: initAwaiter.getDefaultTimeout() + private fun waitForInit(operationName: String? = null) { runBlocking(ioDispatcher) { - waitUntilInitInternal(actualTimeout, operationName) + waitUntilInitInternal(operationName) } } /** * Suspend version that waits for initialization to complete. - * Uses context-aware timeout (shorter on main thread to prevent ANRs). + * Waits indefinitely until init completes and logs how long it took. * - * @param timeoutMs Optional timeout in milliseconds. If not provided, uses context-aware timeout. * @param operationName Optional operation name to include in error messages (e.g., "login", "logout") */ - private suspend fun suspendUntilInit(timeoutMs: Long? = null, operationName: String? = null) { - val actualTimeout = timeoutMs ?: initAwaiter.getDefaultTimeout() - waitUntilInitInternal(actualTimeout, operationName) + private suspend fun suspendUntilInit(operationName: String? = null) { + waitUntilInitInternal(operationName) } /** * Common implementation for waiting until initialization completes. - * Handles all state checks and timeout logic. + * Waits indefinitely until init completes (SUCCESS or FAILED) to ensure consistent state. + * Logs how long initialization took when it completes. * - * @param timeoutMs Timeout in milliseconds * @param operationName Optional operation name to include in error messages (e.g., "login", "logout") */ - private suspend fun waitUntilInitInternal(timeoutMs: Long, operationName: String? = null) { + private suspend fun waitUntilInitInternal(operationName: String? = null) { when (initState) { InitState.NOT_STARTED -> { val message = if (operationName != null) { @@ -377,21 +371,33 @@ internal class OneSignalImp( } InitState.IN_PROGRESS -> { Logging.debug("Waiting for init to complete...") - try { - withTimeout(timeoutMs) { - initAwaiter.awaitSuspend() - } - // Re-check state after waiting - init might have failed during the wait - if (initState == InitState.FAILED) { - throw IllegalStateException("Initialization failed. Cannot proceed.") - } - } catch (e: TimeoutCancellationException) { - Logging.warn("OneSignalImp is taking longer than normal! (timeout: ${timeoutMs}ms). Proceeding anyway, but operations may fail if initialization is not complete.", e) - // Re-check state after timeout - init might have failed during the wait - if (initState == InitState.FAILED) { - throw IllegalStateException("Initialization failed. Cannot proceed.") - } + + val startTime = System.currentTimeMillis() + + // Wait indefinitely until init actually completes - ensures consistent state + // Function only returns when initState is SUCCESS or FAILED + // NOTE: This is a suspend function, so it's non-blocking when called from coroutines. + // However, if waitForInit() (which uses runBlocking) is called from the main thread, + // it will block the main thread indefinitely until init completes, which can cause ANRs. + // This is intentional per PR #2412: "ANR is the lesser of two evils and the app can recover, + // where an uncaught throw it can not." To avoid ANRs, call SDK methods from background threads + // or use the suspend API from coroutines. + initAwaiter.awaitSuspend() + + // Log how long initialization took + val elapsed = System.currentTimeMillis() - startTime + val message = if (operationName != null) { + "OneSignalImp initialization completed before '$operationName' (took ${elapsed}ms)" + } else { + "OneSignalImp initialization completed (took ${elapsed}ms)" + } + Logging.debug(message) + + // Re-check state after waiting - init might have failed during the wait + if (initState == InitState.FAILED) { + throw IllegalStateException("Initialization failed. Cannot proceed.") } + // initState is guaranteed to be SUCCESS here - consistent state } InitState.FAILED -> { throw IllegalStateException("Initialization failed. Cannot proceed.") From 124f849a9b47a8cdfb516c826f16343779d07718 Mon Sep 17 00:00:00 2001 From: AR Abdul Azeez Date: Wed, 3 Dec 2025 09:18:39 -0500 Subject: [PATCH 08/17] making the awaiter return consistent state --- .../common/threading/CompletionAwaiter.kt | 65 +++++----------- .../com/onesignal/internal/OneSignalImp.kt | 1 + .../threading/CompletionAwaiterTests.kt | 76 ++----------------- .../application/SDKInitSuspendTests.kt | 4 +- .../core/internal/application/SDKInitTests.kt | 2 +- 5 files changed, 31 insertions(+), 117 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 be58aff76f..1118eac1a3 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 @@ -1,11 +1,9 @@ package com.onesignal.common.threading -import com.onesignal.common.AndroidUtils import com.onesignal.common.threading.OneSignalDispatchers.BASE_THREAD_NAME import com.onesignal.debug.internal.logging.Logging import kotlinx.coroutines.CompletableDeferred import java.util.concurrent.CountDownLatch -import java.util.concurrent.TimeUnit /** * A unified completion awaiter that supports both blocking and suspend-based waiting. @@ -40,10 +38,6 @@ import java.util.concurrent.TimeUnit class CompletionAwaiter( private val componentName: String = "Component", ) { - companion object { - const val DEFAULT_TIMEOUT_MS = 30_000L // 30 seconds - const val ANDROID_ANR_TIMEOUT_MS = 4_800L // Conservative ANR threshold - } private val latch = CountDownLatch(1) private val suspendCompletion = CompletableDeferred() @@ -57,27 +51,32 @@ class CompletionAwaiter( } /** - * Wait for completion using blocking approach with an optional timeout. + * Wait for completion using blocking approach. + * Waits indefinitely until completion to ensure consistent state. * - * @param timeoutMs Timeout in milliseconds, defaults to context-appropriate timeout - * @return true if completed before timeout, false otherwise. + * @return Always returns true when completion occurs (never times out). */ - fun await(timeoutMs: Long = getDefaultTimeout()): Boolean { - val completed = - try { - latch.await(timeoutMs, TimeUnit.MILLISECONDS) - } catch (e: InterruptedException) { + fun await(): Boolean { + // Wait indefinitely until completion - ensures consistent state + // This can cause ANRs if called from main thread, but that's acceptable + // as it's better than returning with inconsistent state + try { + latch.await() + } catch (e: InterruptedException) { + // Check if the latch was actually completed before interruption + // If completed, return true to maintain consistent state + // If not completed, re-throw to indicate interruption + if (latch.count == 0L) { + // Latch was completed, return true even though we were interrupted + return true + } else { + // Latch was not completed, re-throw to indicate interruption Logging.warn("Interrupted while waiting for $componentName", e) logAllThreads() - false + throw e } - - if (!completed) { - val message = createTimeoutMessage(timeoutMs) - Logging.warn(message) } - - return completed + return true } /** @@ -88,30 +87,6 @@ class CompletionAwaiter( suspendCompletion.await() } - /** - * Gets the appropriate timeout based on the current thread context. - * Uses shorter timeout on main thread to prevent ANRs. - * Made internal so it can be reused by other classes. - */ - internal fun getDefaultTimeout(): Long { - return try { - if (AndroidUtils.isRunningOnMainThread()) ANDROID_ANR_TIMEOUT_MS else DEFAULT_TIMEOUT_MS - } catch (e: RuntimeException) { - // In test environments, AndroidUtils.isRunningOnMainThread() may fail - // because Looper.getMainLooper() is not mocked. Default to longer timeout. - DEFAULT_TIMEOUT_MS - } - } - - private fun createTimeoutMessage(timeoutMs: Long): String { - return 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 37e5bc067a..e5182ca10c 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 @@ -531,6 +531,7 @@ internal class OneSignalImp( Logging.log(LogLevel.DEBUG, "login(externalId: $externalId, jwtBearerToken: $jwtBearerToken)") suspendUntilInit(operationName = "login") + if (!isInitialized) { throw IllegalStateException("'initWithContext failed' before 'login'") } 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..50ec129a3f 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 @@ -7,8 +7,6 @@ import io.kotest.core.spec.style.FunSpec import io.kotest.matchers.longs.shouldBeGreaterThan import io.kotest.matchers.longs.shouldBeLessThan import io.kotest.matchers.shouldBe -import io.mockk.every -import io.mockk.mockkObject import io.mockk.unmockkObject import kotlinx.coroutines.Job import kotlinx.coroutines.async @@ -39,7 +37,7 @@ class CompletionAwaiterTests : FunSpec({ // When val startTime = System.currentTimeMillis() - val completed = awaiter.await(1000) + val completed = awaiter.await() val duration = System.currentTimeMillis() - startTime // Then @@ -49,7 +47,6 @@ class CompletionAwaiterTests : FunSpec({ test("await waits for delayed completion") { val completionDelay = 300L - val timeoutMs = 2000L val startTime = System.currentTimeMillis() @@ -59,7 +56,7 @@ class CompletionAwaiterTests : FunSpec({ awaiter.complete() } - val result = awaiter.await(timeoutMs) + val result = awaiter.await() val duration = System.currentTimeMillis() - startTime result shouldBe true @@ -67,36 +64,6 @@ class CompletionAwaiterTests : FunSpec({ duration shouldBeLessThan (completionDelay + 150) // buffer } - test("await returns false when timeout expires") { - 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) - } - - 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 startTime = System.currentTimeMillis() - val completed = awaiter.await(0) - val duration = System.currentTimeMillis() - startTime - - completed shouldBe false - duration shouldBeLessThan 20L - - unmockkObject(AndroidUtils) - } - test("multiple blocking callers are all unblocked") { val numCallers = 5 val results = mutableListOf() @@ -106,7 +73,7 @@ class CompletionAwaiterTests : FunSpec({ repeat(numCallers) { index -> val thread = Thread { - val result = awaiter.await(2000) + val result = awaiter.await() synchronized(results) { results.add(result) } @@ -249,7 +216,7 @@ class CompletionAwaiterTests : FunSpec({ val blockingThreads = (1..2).map { index -> Thread { - val result = awaiter.await(2000) + val result = awaiter.await() synchronized(blockingResults) { blockingResults.add(result) } @@ -280,7 +247,7 @@ class CompletionAwaiterTests : FunSpec({ awaiter.complete() // Should still work normally - val completed = awaiter.await(100) + val completed = awaiter.await() completed shouldBe true } @@ -329,35 +296,6 @@ class CompletionAwaiterTests : FunSpec({ } } - context("timeout behavior") { - - test("uses shorter timeout on main thread") { - 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 - } - - test("uses longer timeout on background thread") { - 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: Timeout behavior tests removed - await() now waits indefinitely per PR #2412 + // to ensure consistent state. Logging of slow operations is handled by callers. }) diff --git a/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/core/internal/application/SDKInitSuspendTests.kt b/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/core/internal/application/SDKInitSuspendTests.kt index 07fce3358c..462ea7a746 100644 --- a/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/core/internal/application/SDKInitSuspendTests.kt +++ b/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/core/internal/application/SDKInitSuspendTests.kt @@ -287,7 +287,7 @@ class SDKInitSuspendTests : FunSpec({ } // Should throw immediately because isInitialized is false - exception.message shouldBe "Must call 'initWithContext' before use" + exception.message shouldBe "Must call 'initWithContext' before 'login'" } } @@ -303,7 +303,7 @@ class SDKInitSuspendTests : FunSpec({ } // Should throw immediately because isInitialized is false - exception.message shouldBe "Must call 'initWithContext' before use" + exception.message shouldBe "Must call 'initWithContext' before 'logout'" } } }) 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 5844a96809..e75068cc3f 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 @@ -467,7 +467,7 @@ class BlockingPrefsContext( mode: Int, ): SharedPreferences { try { - unblockTrigger.await(timeoutInMillis) + unblockTrigger.await() } catch (e: InterruptedException) { throw e } catch (e: TimeoutCancellationException) { From 23ac7f8733e80038f612cb21332956fc5bdc1f0f Mon Sep 17 00:00:00 2001 From: AR Abdul Azeez Date: Wed, 3 Dec 2025 09:24:21 -0500 Subject: [PATCH 09/17] fixing repetitive code --- .../core/internal/application/SDKInitTests.kt | 37 +++++++++---------- 1 file changed, 18 insertions(+), 19 deletions(-) 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 e75068cc3f..021c72ee86 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 @@ -21,6 +21,21 @@ import kotlinx.coroutines.runBlocking @RobolectricTest class SDKInitTests : FunSpec({ + /** + * Helper function to wait for OneSignal initialization to complete. + * @param oneSignalImp The OneSignalImp instance to wait for + * @param maxAttempts Maximum number of attempts (default: 100) + * @param sleepMs Sleep duration between attempts in milliseconds (default: 20) + */ + fun waitForInitialization(oneSignalImp: OneSignalImp, maxAttempts: Int = 100, sleepMs: Long = 20) { + var attempts = 0 + while (!oneSignalImp.isInitialized && attempts < maxAttempts) { + Thread.sleep(sleepMs) + attempts++ + } + oneSignalImp.isInitialized shouldBe true + } + beforeAny { Logging.logLevel = LogLevel.NONE @@ -157,13 +172,7 @@ class SDKInitTests : FunSpec({ trigger.complete() // Wait for initialization to complete (internalInit runs asynchronously) - var attempts = 0 - while (!os.isInitialized && attempts < 50) { - Thread.sleep(20) - attempts++ - } - - os.isInitialized shouldBe true + waitForInitialization(os, maxAttempts = 50) } test("accessors will be blocked if call too early after initWithContext with appId") { @@ -329,12 +338,7 @@ class SDKInitTests : FunSpec({ os.initWithContext(context, "appId") // Wait for initialization to complete before accessing user - var attempts = 0 - while (!os.isInitialized && attempts < 100) { - Thread.sleep(20) - attempts++ - } - os.isInitialized shouldBe true + waitForInitialization(os) // Give additional time for coroutines to settle, especially in CI/CD Thread.sleep(50) @@ -345,12 +349,7 @@ class SDKInitTests : FunSpec({ os.initWithContext(context) // Wait for second initialization to complete - attempts = 0 - while (!os.isInitialized && attempts < 100) { - Thread.sleep(20) - attempts++ - } - os.isInitialized shouldBe true + waitForInitialization(os) // Give additional time for coroutines to settle after second init Thread.sleep(50) From fd7f6c0dd1e082d9de21d96984fbee3def749aa2 Mon Sep 17 00:00:00 2001 From: AR Abdul Azeez Date: Wed, 3 Dec 2025 15:28:28 -0500 Subject: [PATCH 10/17] Addressed comments --- .../com/onesignal/common/threading/OneSignalDispatchers.kt | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/common/threading/OneSignalDispatchers.kt b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/common/threading/OneSignalDispatchers.kt index 8ff3d61ea1..a309622e52 100644 --- a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/common/threading/OneSignalDispatchers.kt +++ b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/common/threading/OneSignalDispatchers.kt @@ -29,8 +29,8 @@ import java.util.concurrent.atomic.AtomicInteger internal object OneSignalDispatchers { // Optimized pool thread counts to handle more concurrent operations during init // (especially important now that we wait indefinitely, which may cause more operations to queue) - private const val IO_CORE_POOL_SIZE = 4 // Increased for better concurrency during init - private const val IO_MAX_POOL_SIZE = 6 // Increased to handle bursts of operations + private const val IO_CORE_POOL_SIZE = 2 // Increased for better concurrency during init + private const val IO_MAX_POOL_SIZE = 3 // Increased to handle bursts of operations private const val DEFAULT_CORE_POOL_SIZE = 2 // Optimal for CPU operations private const val DEFAULT_MAX_POOL_SIZE = 3 // Slightly larger for CPU operations private const val KEEP_ALIVE_TIME_SECONDS = @@ -70,7 +70,6 @@ internal object OneSignalDispatchers { priority = Thread.NORM_PRIORITY - 1, // Slightly lower priority for I/O tasks ), - ThreadPoolExecutor.CallerRunsPolicy(), // Execute on calling thread if queue is full (prevents rejection) ).apply { allowCoreThreadTimeOut(false) // Keep core threads alive } @@ -89,7 +88,6 @@ internal object OneSignalDispatchers { TimeUnit.SECONDS, LinkedBlockingQueue(QUEUE_CAPACITY), OptimizedThreadFactory(DEFAULT_THREAD_NAME_PREFIX), - ThreadPoolExecutor.CallerRunsPolicy(), // Execute on calling thread if queue is full (prevents rejection) ).apply { allowCoreThreadTimeOut(false) // Keep core threads alive } From dddaa6737f639095ad5de9b09189d76c073d8212 Mon Sep 17 00:00:00 2001 From: AR Abdul Azeez Date: Wed, 3 Dec 2025 15:30:50 -0500 Subject: [PATCH 11/17] moving the code around to make it easily readable --- .../common/threading/OneSignalDispatchers.kt | 4 ++-- .../java/com/onesignal/internal/OneSignalImp.kt | 14 +++++++------- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/common/threading/OneSignalDispatchers.kt b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/common/threading/OneSignalDispatchers.kt index a309622e52..8ac259d1f3 100644 --- a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/common/threading/OneSignalDispatchers.kt +++ b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/common/threading/OneSignalDispatchers.kt @@ -29,8 +29,8 @@ import java.util.concurrent.atomic.AtomicInteger internal object OneSignalDispatchers { // Optimized pool thread counts to handle more concurrent operations during init // (especially important now that we wait indefinitely, which may cause more operations to queue) - private const val IO_CORE_POOL_SIZE = 2 // Increased for better concurrency during init - private const val IO_MAX_POOL_SIZE = 3 // Increased to handle bursts of operations + private const val IO_CORE_POOL_SIZE = 2 // Increased for better concurrency + private const val IO_MAX_POOL_SIZE = 3 // Increased for better concurrency private const val DEFAULT_CORE_POOL_SIZE = 2 // Optimal for CPU operations private const val DEFAULT_MAX_POOL_SIZE = 3 // Slightly larger for CPU operations private const val KEEP_ALIVE_TIME_SECONDS = 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 e5182ca10c..27c3dace79 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 @@ -322,13 +322,6 @@ internal class OneSignalImp( override fun getAllServices(c: Class): List = services.getAllServices(c) - /** - * Notifies both blocking and suspend callers that initialization is complete - */ - private fun notifyInitComplete() { - initAwaiter.complete() - } - /** * Blocking version that waits for initialization to complete. * Uses runBlocking to bridge to the suspend implementation. @@ -342,6 +335,13 @@ internal class OneSignalImp( } } + /** + * Notifies both blocking and suspend callers that initialization is complete + */ + private fun notifyInitComplete() { + initAwaiter.complete() + } + /** * Suspend version that waits for initialization to complete. * Waits indefinitely until init completes and logs how long it took. From 920189ad9722ea73890225ac42cad1817471110e Mon Sep 17 00:00:00 2001 From: AR Abdul Azeez Date: Wed, 3 Dec 2025 15:32:20 -0500 Subject: [PATCH 12/17] moving the code around to make it easily readable --- .../com/onesignal/common/threading/OneSignalDispatchers.kt | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/common/threading/OneSignalDispatchers.kt b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/common/threading/OneSignalDispatchers.kt index 8ac259d1f3..9e9a307a24 100644 --- a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/common/threading/OneSignalDispatchers.kt +++ b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/common/threading/OneSignalDispatchers.kt @@ -27,8 +27,7 @@ import java.util.concurrent.atomic.AtomicInteger * - Efficient thread management with controlled resource usage */ internal object OneSignalDispatchers { - // Optimized pool thread counts to handle more concurrent operations during init - // (especially important now that we wait indefinitely, which may cause more operations to queue) + // Optimized pool sizes based on CPU cores and workload analysis private const val IO_CORE_POOL_SIZE = 2 // Increased for better concurrency private const val IO_MAX_POOL_SIZE = 3 // Increased for better concurrency private const val DEFAULT_CORE_POOL_SIZE = 2 // Optimal for CPU operations From 6f27c460f4cc8f8a7810c1ed04017212be1f3cfb Mon Sep 17 00:00:00 2001 From: AR Abdul Azeez Date: Fri, 5 Dec 2025 16:11:10 -0500 Subject: [PATCH 13/17] cleanup code further --- .../common/threading/CompletionAwaiter.kt | 96 +---------- .../threading/CompletionAwaiterTests.kt | 149 ++---------------- 2 files changed, 16 insertions(+), 229 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 1118eac1a3..6ddad18600 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 @@ -1,121 +1,39 @@ package com.onesignal.common.threading -import com.onesignal.common.threading.OneSignalDispatchers.BASE_THREAD_NAME -import com.onesignal.debug.internal.logging.Logging import kotlinx.coroutines.CompletableDeferred -import java.util.concurrent.CountDownLatch /** - * A unified completion awaiter that supports both blocking and suspend-based waiting. - * This class allows both legacy blocking code and modern coroutines to wait for the same event. - * - * It is designed for scenarios where certain tasks, such as SDK initialization, must finish - * before continuing. When used on the main/UI thread for blocking operations, it applies a - * shorter timeout and logs warnings to prevent ANR errors. - * - * PERFORMANCE NOTE: Having both blocking (CountDownLatch) and suspend (Channel) mechanisms - * in place is very low cost and should not hurt performance. The overhead is minimal: - * - CountDownLatch: ~32 bytes, optimized for blocking threads - * - Channel: ~64 bytes, optimized for coroutine suspension - * - Total overhead: <100 bytes per awaiter instance - * - Notification cost: Two simple operations (countDown + trySend) - * - * This dual approach provides optimal performance for each use case rather than forcing - * a one-size-fits-all solution that would be suboptimal for both scenarios. + * A completion awaiter for suspend-based waiting. + * This class allows coroutines to wait for an event (such as SDK initialization) to complete. * * Usage: * val awaiter = CompletionAwaiter("OneSignal SDK Init") * - * // For blocking code: - * awaiter.await() - * - * // For suspend code: + * // Wait for completion (suspend): * awaiter.awaitSuspend() * - * // When complete: + * // Signal completion: * awaiter.complete() */ class CompletionAwaiter( private val componentName: String = "Component", ) { - private val latch = CountDownLatch(1) private val suspendCompletion = CompletableDeferred() /** - * Completes the awaiter, unblocking both blocking and suspend callers. + * Completes the awaiter, unblocking all suspend callers. */ fun complete() { - latch.countDown() suspendCompletion.complete(Unit) } - /** - * Wait for completion using blocking approach. - * Waits indefinitely until completion to ensure consistent state. - * - * @return Always returns true when completion occurs (never times out). - */ - fun await(): Boolean { - // Wait indefinitely until completion - ensures consistent state - // This can cause ANRs if called from main thread, but that's acceptable - // as it's better than returning with inconsistent state - try { - latch.await() - } catch (e: InterruptedException) { - // Check if the latch was actually completed before interruption - // If completed, return true to maintain consistent state - // If not completed, re-throw to indicate interruption - if (latch.count == 0L) { - // Latch was completed, return true even though we were interrupted - return true - } else { - // Latch was not completed, re-throw to indicate interruption - Logging.warn("Interrupted while waiting for $componentName", e) - logAllThreads() - throw e - } - } - return true - } - /** * Wait for completion using suspend approach (non-blocking for coroutines). - * This method will suspend the current coroutine until completion is signaled. + * Suspends the current coroutine until completion is signaled. + * Will wait indefinitely until complete() is called. */ suspend fun awaitSuspend() { suspendCompletion.await() } - - private fun logAllThreads(): String { - val sb = StringBuilder() - - // Add OneSignal dispatcher status first (fast) - sb.append("=== OneSignal Dispatchers Status ===\n") - sb.append(OneSignalDispatchers.getStatus()) - sb.append("=== OneSignal Dispatchers Performance ===\n") - sb.append(OneSignalDispatchers.getPerformanceMetrics()) - sb.append("\n\n") - - // Add lightweight thread info (fast) - sb.append("=== All Threads Summary ===\n") - val threads = Thread.getAllStackTraces().keys - for (thread in threads) { - sb.append("Thread: ${thread.name} [${thread.state}] ${if (thread.isDaemon) "(daemon)" else ""}\n") - } - - // Only add full stack traces for OneSignal threads (much faster) - sb.append("\n=== OneSignal Thread Details ===\n") - for ((thread, stack) in Thread.getAllStackTraces()) { - if (thread.name.startsWith(BASE_THREAD_NAME)) { - sb.append("Thread: ${thread.name} [${thread.state}]\n") - for (element in stack.take(10)) { // Limit to first 10 frames - sb.append("\tat $element\n") - } - sb.append("\n") - } - } - - return sb.toString() - } } 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 50ec129a3f..5c74b945f1 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 @@ -4,8 +4,6 @@ import com.onesignal.common.AndroidUtils import com.onesignal.debug.LogLevel import com.onesignal.debug.internal.logging.Logging import io.kotest.core.spec.style.FunSpec -import io.kotest.matchers.longs.shouldBeGreaterThan -import io.kotest.matchers.longs.shouldBeLessThan import io.kotest.matchers.shouldBe import io.mockk.unmockkObject import kotlinx.coroutines.Job @@ -29,74 +27,6 @@ class CompletionAwaiterTests : FunSpec({ unmockkObject(AndroidUtils) } - context("blocking await functionality") { - - test("await completes immediately when already completed") { - // Given - awaiter.complete() - - // When - val startTime = System.currentTimeMillis() - val completed = awaiter.await() - val duration = System.currentTimeMillis() - startTime - - // Then - completed shouldBe true - duration shouldBeLessThan 50L // Should be very fast - } - - test("await waits for delayed completion") { - val completionDelay = 300L - - val startTime = System.currentTimeMillis() - - // Simulate delayed completion from another thread - suspendifyOnIO { - delay(completionDelay) - awaiter.complete() - } - - val result = awaiter.await() - val duration = System.currentTimeMillis() - startTime - - result shouldBe true - duration shouldBeGreaterThan (completionDelay - 50) - duration shouldBeLessThan (completionDelay + 150) // buffer - } - - test("multiple blocking callers are all unblocked") { - val numCallers = 5 - val results = mutableListOf() - val jobs = mutableListOf() - - // Start multiple blocking callers - repeat(numCallers) { index -> - val thread = - Thread { - val result = awaiter.await() - synchronized(results) { - results.add(result) - } - } - thread.start() - jobs.add(thread) - } - - // Wait a bit to ensure all threads are waiting - Thread.sleep(100) - - // Complete the awaiter - 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 - } - } - context("suspend await functionality") { test("awaitSuspend completes immediately when already completed") { @@ -176,79 +106,18 @@ class CompletionAwaiterTests : FunSpec({ } } - context("mixed blocking and suspend callers") { - - test("completion unblocks both blocking and suspend callers") { - // This test verifies the dual mechanism works - // We'll test blocking and suspend separately since mixing them in runTest is problematic + context("edge cases and safety") { - // Test suspend callers first + test("multiple complete calls are safe") { runBlocking { - val suspendResults = mutableListOf() - - // Start suspend callers - val suspendJobs = - (1..2).map { index -> - async { - awaiter.awaitSuspend() - suspendResults.add("suspend-$index") - } - } - - // Wait a bit to ensure all are waiting - delay(50) - - // Complete the awaiter + // Complete multiple times + awaiter.complete() + awaiter.complete() awaiter.complete() - // Wait for all to complete - suspendJobs.awaitAll() - - // All should have completed - suspendResults.size shouldBe 2 + // Should still work normally + awaiter.awaitSuspend() } - - // Reset for blocking test - awaiter = CompletionAwaiter("TestComponent") - - // Test blocking callers - val blockingResults = mutableListOf() - val blockingThreads = - (1..2).map { index -> - Thread { - val result = awaiter.await() - synchronized(blockingResults) { - blockingResults.add(result) - } - } - } - blockingThreads.forEach { it.start() } - - // Wait a bit to ensure all are waiting - Thread.sleep(100) - - // Complete the awaiter - awaiter.complete() - - // Wait for all to complete - blockingThreads.forEach { it.join(1000) } - - // All should have completed - blockingResults shouldBe arrayOf(true, true) - } - } - - context("edge cases and safety") { - - test("multiple complete calls are safe") { - // Complete multiple times - awaiter.complete() - awaiter.complete() - awaiter.complete() - - // Should still work normally - val completed = awaiter.await() - completed shouldBe true } test("waiting after completion returns immediately") { @@ -296,6 +165,6 @@ class CompletionAwaiterTests : FunSpec({ } } - // Note: Timeout behavior tests removed - await() now waits indefinitely per PR #2412 - // to ensure consistent state. Logging of slow operations is handled by callers. + // Note: Blocking await() method removed - only suspend-based awaitSuspend() is supported. + // This ensures the SDK uses modern coroutine patterns for all async operations. }) From 900abf96c05480377cffe05d1d5ac8ac5b74647a Mon Sep 17 00:00:00 2001 From: AR Abdul Azeez Date: Fri, 5 Dec 2025 16:37:02 -0500 Subject: [PATCH 14/17] cleaned up more code --- .../common/threading/CompletionAwaiter.kt | 39 ---- .../com/onesignal/internal/OneSignalImp.kt | 10 +- .../threading/CompletionAwaiterTests.kt | 170 ------------------ .../core/internal/application/SDKInitTests.kt | 38 ++-- .../onesignal/internal/OneSignalImpTests.kt | 29 ++- 5 files changed, 32 insertions(+), 254 deletions(-) delete mode 100644 OneSignalSDK/onesignal/core/src/main/java/com/onesignal/common/threading/CompletionAwaiter.kt delete mode 100644 OneSignalSDK/onesignal/core/src/test/java/com/onesignal/common/threading/CompletionAwaiterTests.kt 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 deleted file mode 100644 index 6ddad18600..0000000000 --- a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/common/threading/CompletionAwaiter.kt +++ /dev/null @@ -1,39 +0,0 @@ -package com.onesignal.common.threading - -import kotlinx.coroutines.CompletableDeferred - -/** - * A completion awaiter for suspend-based waiting. - * This class allows coroutines to wait for an event (such as SDK initialization) to complete. - * - * Usage: - * val awaiter = CompletionAwaiter("OneSignal SDK Init") - * - * // Wait for completion (suspend): - * awaiter.awaitSuspend() - * - * // Signal completion: - * awaiter.complete() - */ -class CompletionAwaiter( - private val componentName: String = "Component", -) { - - private val suspendCompletion = CompletableDeferred() - - /** - * Completes the awaiter, unblocking all suspend callers. - */ - fun complete() { - suspendCompletion.complete(Unit) - } - - /** - * Wait for completion using suspend approach (non-blocking for coroutines). - * Suspends the current coroutine until completion is signaled. - * Will wait indefinitely until complete() is called. - */ - suspend fun awaitSuspend() { - suspendCompletion.await() - } -} 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 27c3dace79..7b6ee4041e 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 @@ -9,7 +9,6 @@ import com.onesignal.common.modules.IModule import com.onesignal.common.services.IServiceProvider import com.onesignal.common.services.ServiceBuilder import com.onesignal.common.services.ServiceProvider -import com.onesignal.common.threading.CompletionAwaiter import com.onesignal.common.threading.OneSignalDispatchers import com.onesignal.common.threading.suspendifyOnIO import com.onesignal.core.CoreModule @@ -39,6 +38,7 @@ import com.onesignal.user.internal.identity.IdentityModelStore import com.onesignal.user.internal.properties.PropertiesModelStore import com.onesignal.user.internal.resolveAppId import com.onesignal.user.internal.subscriptions.SubscriptionModelStore +import kotlinx.coroutines.CompletableDeferred import kotlinx.coroutines.CoroutineDispatcher import kotlinx.coroutines.runBlocking import kotlinx.coroutines.withContext @@ -46,8 +46,8 @@ import kotlinx.coroutines.withContext internal class OneSignalImp( private val ioDispatcher: CoroutineDispatcher = OneSignalDispatchers.IO, ) : IOneSignal, IServiceProvider { - @Volatile - private var initAwaiter = CompletionAwaiter("OneSignalImp") + + private val suspendCompletion = CompletableDeferred() @Volatile private var initState: InitState = InitState.NOT_STARTED @@ -339,7 +339,7 @@ internal class OneSignalImp( * Notifies both blocking and suspend callers that initialization is complete */ private fun notifyInitComplete() { - initAwaiter.complete() + suspendCompletion.complete(Unit) } /** @@ -382,7 +382,7 @@ internal class OneSignalImp( // This is intentional per PR #2412: "ANR is the lesser of two evils and the app can recover, // where an uncaught throw it can not." To avoid ANRs, call SDK methods from background threads // or use the suspend API from coroutines. - initAwaiter.awaitSuspend() + suspendCompletion.await() // Log how long initialization took val elapsed = System.currentTimeMillis() - startTime 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 deleted file mode 100644 index 5c74b945f1..0000000000 --- a/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/common/threading/CompletionAwaiterTests.kt +++ /dev/null @@ -1,170 +0,0 @@ -package com.onesignal.common.threading - -import com.onesignal.common.AndroidUtils -import com.onesignal.debug.LogLevel -import com.onesignal.debug.internal.logging.Logging -import io.kotest.core.spec.style.FunSpec -import io.kotest.matchers.shouldBe -import io.mockk.unmockkObject -import kotlinx.coroutines.Job -import kotlinx.coroutines.async -import kotlinx.coroutines.awaitAll -import kotlinx.coroutines.delay -import kotlinx.coroutines.joinAll -import kotlinx.coroutines.launch -import kotlinx.coroutines.runBlocking - -class CompletionAwaiterTests : FunSpec({ - - lateinit var awaiter: CompletionAwaiter - - beforeEach { - Logging.logLevel = LogLevel.NONE - awaiter = CompletionAwaiter("TestComponent") - } - - afterEach { - unmockkObject(AndroidUtils) - } - - context("suspend await functionality") { - - test("awaitSuspend completes immediately when already completed") { - runBlocking { - // Given - awaiter.complete() - - // When - should complete immediately without hanging - awaiter.awaitSuspend() - - // Then - if we get here, it completed successfully - // No timing assertions needed in test environment - } - } - - test("awaitSuspend waits for delayed completion") { - runBlocking { - val completionDelay = 100L - - // Start delayed completion - val completionJob = - launch { - delay(completionDelay) - awaiter.complete() - } - - // Wait for completion - awaiter.awaitSuspend() - - // In test environment, we just verify it completed without hanging - completionJob.join() - } - } - - test("multiple suspend callers are all unblocked") { - runBlocking { - val numCallers = 5 - val results = mutableListOf() - - // Start multiple suspend callers - val jobs = - (1..numCallers).map { index -> - async { - awaiter.awaitSuspend() - results.add("caller-$index") - } - } - - // Wait a bit to ensure all coroutines are suspended - delay(50) - - // Complete the awaiter - awaiter.complete() - - // Wait for all callers to complete - jobs.awaitAll() - - // All should have completed - results.size shouldBe numCallers - } - } - - test("awaitSuspend can be cancelled") { - runBlocking { - val job = - launch { - awaiter.awaitSuspend() - } - - // Wait a bit then cancel - delay(50) - job.cancel() - - // Job should be cancelled - job.isCancelled shouldBe true - } - } - } - - context("edge cases and safety") { - - test("multiple complete calls are safe") { - runBlocking { - // Complete multiple times - awaiter.complete() - awaiter.complete() - awaiter.complete() - - // Should still work normally - awaiter.awaitSuspend() - } - } - - test("waiting after completion returns immediately") { - runBlocking { - // Complete first - awaiter.complete() - - // Then wait - should return immediately without hanging - awaiter.awaitSuspend() - - // Multiple calls should also work immediately - awaiter.awaitSuspend() - awaiter.awaitSuspend() - } - } - - test("concurrent access is safe") { - runBlocking { - val numOperations = 10 // Reduced for test stability - val jobs = mutableListOf() - - // Start some waiters first - repeat(numOperations / 2) { index -> - jobs.add( - async { - awaiter.awaitSuspend() - }, - ) - } - - // Wait a bit for them to start waiting - delay(10) - - // Then complete multiple times concurrently - repeat(numOperations / 2) { index -> - jobs.add(launch { awaiter.complete() }) - } - - // Wait for all operations - jobs.joinAll() - - // Final wait should work immediately - awaiter.awaitSuspend() - } - } - } - - // Note: Blocking await() method removed - only suspend-based awaitSuspend() is supported. - // This ensures the SDK uses modern coroutine patterns for all async operations. -}) 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 021c72ee86..013899f8c8 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 @@ -5,7 +5,6 @@ import android.content.ContextWrapper import android.content.SharedPreferences import androidx.test.core.app.ApplicationProvider.getApplicationContext import br.com.colman.kotest.android.extensions.robolectric.RobolectricTest -import com.onesignal.common.threading.CompletionAwaiter import com.onesignal.core.internal.preferences.PreferenceOneSignalKeys.PREFS_LEGACY_APP_ID import com.onesignal.debug.LogLevel import com.onesignal.debug.internal.logging.Logging @@ -15,8 +14,8 @@ import io.kotest.core.spec.style.FunSpec import io.kotest.matchers.maps.shouldContain import io.kotest.matchers.shouldBe import io.kotest.matchers.shouldNotBe -import kotlinx.coroutines.TimeoutCancellationException import kotlinx.coroutines.runBlocking +import java.util.concurrent.CountDownLatch @RobolectricTest class SDKInitTests : FunSpec({ @@ -104,9 +103,9 @@ class SDKInitTests : FunSpec({ test("initWithContext with no appId succeeds when configModel has appId") { // Given // block SharedPreference before calling init - val trigger = CompletionAwaiter("Test") + val trigger = CountDownLatch(1) val context = getApplicationContext() - val blockingPrefContext = BlockingPrefsContext(context, trigger, 2000) + val blockingPrefContext = BlockingPrefsContext(context, trigger) val os = OneSignalImp() var initSuccess = true @@ -137,7 +136,7 @@ class SDKInitTests : FunSpec({ accessorThread.isAlive shouldBe true // release SharedPreferences - trigger.complete() + trigger.countDown() accessorThread.join(500) accessorThread.isAlive shouldBe false @@ -150,9 +149,9 @@ class SDKInitTests : FunSpec({ test("initWithContext with appId does not block") { // Given // block SharedPreference before calling init - val trigger = CompletionAwaiter("Test") + val trigger = CountDownLatch(1) val context = getApplicationContext() - val blockingPrefContext = BlockingPrefsContext(context, trigger, 2000) + val blockingPrefContext = BlockingPrefsContext(context, trigger) val os = OneSignalImp() // When @@ -169,7 +168,7 @@ class SDKInitTests : FunSpec({ accessorThread.isAlive shouldBe false // Release the SharedPreferences lock so internalInit can complete - trigger.complete() + trigger.countDown() // Wait for initialization to complete (internalInit runs asynchronously) waitForInitialization(os, maxAttempts = 50) @@ -178,9 +177,9 @@ class SDKInitTests : FunSpec({ test("accessors will be blocked if call too early after initWithContext with appId") { // Given // block SharedPreference before calling init - val trigger = CompletionAwaiter("Test") + val trigger = CountDownLatch(1) val context = getApplicationContext() - val blockingPrefContext = BlockingPrefsContext(context, trigger, 2000) + val blockingPrefContext = BlockingPrefsContext(context, trigger) val os = OneSignalImp() val accessorThread = @@ -195,7 +194,7 @@ class SDKInitTests : FunSpec({ accessorThread.isAlive shouldBe true // release the lock on SharedPreferences - trigger.complete() + trigger.countDown() accessorThread.join(1000) accessorThread.isAlive shouldBe false @@ -222,9 +221,9 @@ class SDKInitTests : FunSpec({ test("ensure login called right after initWithContext can set externalId correctly") { // Given // block SharedPreference before calling init - val trigger = CompletionAwaiter("Test") + val trigger = CountDownLatch(1) val context = getApplicationContext() - val blockingPrefContext = BlockingPrefsContext(context, trigger, 2000) + val blockingPrefContext = BlockingPrefsContext(context, trigger) val os = OneSignalImp() val externalId = "testUser" @@ -250,7 +249,7 @@ class SDKInitTests : FunSpec({ accessorThread.isAlive shouldBe true // release the lock on SharedPreferences so internalInit can complete - trigger.complete() + trigger.countDown() // Wait for initialization to complete (internalInit runs asynchronously) var initAttempts = 0 @@ -458,20 +457,13 @@ class SDKInitTests : FunSpec({ */ class BlockingPrefsContext( context: Context, - private val unblockTrigger: CompletionAwaiter, - private val timeoutInMillis: Long, + private val unblockTrigger: CountDownLatch, ) : ContextWrapper(context) { override fun getSharedPreferences( name: String, mode: Int, ): SharedPreferences { - try { - unblockTrigger.await() - } catch (e: InterruptedException) { - throw e - } catch (e: TimeoutCancellationException) { - throw e - } + unblockTrigger.await() return super.getSharedPreferences(name, mode) } diff --git a/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/internal/OneSignalImpTests.kt b/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/internal/OneSignalImpTests.kt index 9599d2bd5f..d660fa2525 100644 --- a/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/internal/OneSignalImpTests.kt +++ b/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/internal/OneSignalImpTests.kt @@ -224,27 +224,23 @@ class OneSignalImpTests : FunSpec({ // which uses withTimeout() to wait for up to 30 seconds (or 4.8 seconds on main thread) // before logging a warning and proceeding - // NOTE: We don't actually test the 30-second timeout here because: - // 1. It would make tests too slow (30 seconds per test) - // 2. The timeout is tested in CompletionAwaiterTests - // 3. This test documents the behavior for developers + // NOTE: We don't test waiting indefinitely here because: + // 1. It would make tests hang forever + // 2. This test documents the behavior for developers oneSignalImp.isInitialized shouldBe false } - test("waitForInit timeout mechanism exists - CompletionAwaiter integration") { - // This test verifies that the timeout mechanism is properly integrated - // by checking that waitUntilInitInternal has timeout capabilities + test("waitForInit waits indefinitely until init completes") { + // This test verifies that waitUntilInitInternal waits indefinitely + // until initialization completes (per PR #2412) // Given val oneSignalImp = OneSignalImp() - // The timeout behavior is implemented through waitUntilInitInternal() - // which uses withTimeout() with a default timeout of 30 seconds (or 4.8 seconds on main thread) - - // We can verify the timeout mechanism exists by checking: - // 1. The CompletionAwaiter is properly initialized - // 2. The initState is NOT_STARTED (which would trigger timeout) + // We can verify the wait behavior by checking: + // 1. The suspendCompletion (CompletableDeferred) is properly initialized + // 2. The initState is NOT_STARTED (which would throw immediately) // 3. The isInitialized property correctly reflects the state oneSignalImp.isInitialized shouldBe false @@ -252,10 +248,9 @@ class OneSignalImpTests : FunSpec({ // In a real scenario where initWithContext is never called: // - waitForInit() would call waitUntilInitInternal() // - waitUntilInitInternal() would check initState == NOT_STARTED and throw immediately - // - If initState was IN_PROGRESS, it would use withTimeout() to wait up to 30 seconds - // - After timeout during IN_PROGRESS, it would log "OneSignalImp is taking longer than normal!" and proceed - // - waitForInit() throws for NOT_STARTED/FAILED states, but only logs (doesn't throw) on timeout during IN_PROGRESS + // - If initState was IN_PROGRESS, it would wait indefinitely using suspendCompletion.await() + // - waitForInit() throws for NOT_STARTED/FAILED states, waits indefinitely for IN_PROGRESS - // This test documents this behavior without actually waiting 30 seconds + // This test documents this behavior without actually waiting indefinitely } }) From 854c277cfceaed9f07db337caa92966c056975bc Mon Sep 17 00:00:00 2001 From: AR Abdul Azeez Date: Fri, 5 Dec 2025 17:01:58 -0500 Subject: [PATCH 15/17] verifying if calls are made async --- .../internal/startup/StartupServiceTests.kt | 33 +++++++++++++------ 1 file changed, 23 insertions(+), 10 deletions(-) diff --git a/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/core/internal/startup/StartupServiceTests.kt b/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/core/internal/startup/StartupServiceTests.kt index a5d254cce6..3d3e24862b 100644 --- a/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/core/internal/startup/StartupServiceTests.kt +++ b/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/core/internal/startup/StartupServiceTests.kt @@ -6,8 +6,8 @@ import com.onesignal.debug.LogLevel import com.onesignal.debug.internal.logging.Logging import io.kotest.assertions.throwables.shouldThrowUnit import io.kotest.core.spec.style.FunSpec +import io.kotest.matchers.comparables.shouldBeLessThan import io.kotest.matchers.shouldBe -import io.mockk.coVerifyOrder import io.mockk.every import io.mockk.mockk import io.mockk.spyk @@ -101,17 +101,30 @@ class StartupServiceTests : FunSpec({ val startupService = StartupService(setupServiceProvider(listOf(), listOf(mockStartableService1, mockStartableService2))) - // When + // When - scheduleStart() is async, so it doesn't block + val startTime = System.currentTimeMillis() startupService.scheduleStart() + val scheduleTime = System.currentTimeMillis() - startTime + + // This should execute immediately since scheduleStart() doesn't block mockStartableService3.start() + val immediateTime = System.currentTimeMillis() - startTime - // Then - Thread.sleep(10) - coVerifyOrder { - // service3 will call start() first even though service1 and service2 are scheduled first - mockStartableService3.start() - mockStartableService1.start() - mockStartableService2.start() - } + // Then - verify scheduleStart() returned quickly (non-blocking) + // Should return in < 50ms (proving it doesn't wait for services to start) + scheduleTime shouldBeLessThan 50L + immediateTime shouldBeLessThan 50L + + // Wait for async execution to complete + Thread.sleep(100) + + // Verify all services were called + verify(exactly = 1) { mockStartableService1.start() } + verify(exactly = 1) { mockStartableService2.start() } + verify(exactly = 1) { mockStartableService3.start() } + + // The key assertion: scheduleStart() returned immediately without blocking, + // allowing service3.start() to be called synchronously. All services eventually + // get started, proving scheduleStart() is non-blocking. } }) From 28bf106a57dc7959f5ba0079b9919d5e455301ce Mon Sep 17 00:00:00 2001 From: AR Abdul Azeez Date: Fri, 5 Dec 2025 17:33:01 -0500 Subject: [PATCH 16/17] spotless cleanup --- .../internal/startup/StartupServiceTests.kt | 25 ++++++++++++------- 1 file changed, 16 insertions(+), 9 deletions(-) diff --git a/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/core/internal/startup/StartupServiceTests.kt b/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/core/internal/startup/StartupServiceTests.kt index bfab1efb62..549600a5aa 100644 --- a/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/core/internal/startup/StartupServiceTests.kt +++ b/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/core/internal/startup/StartupServiceTests.kt @@ -5,7 +5,6 @@ import com.onesignal.common.services.ServiceProvider import com.onesignal.debug.LogLevel import com.onesignal.debug.internal.logging.Logging import com.onesignal.mocks.IOMockHelper -import com.onesignal.mocks.IOMockHelper.awaitIO import io.kotest.assertions.throwables.shouldThrowUnit import io.kotest.core.spec.style.FunSpec import io.kotest.matchers.comparables.shouldBeLessThan @@ -102,9 +101,11 @@ class StartupServiceTests : FunSpec({ // Given val mockStartableService1 = spyk() val mockStartableService2 = spyk() - val startupService = StartupService(setupServiceProvider(listOf(), listOf(mockStartableService1))) + val mockStartableService3 = spyk() + // Only service1 and service2 are scheduled - service3 is NOT scheduled + val startupService = StartupService(setupServiceProvider(listOf(), listOf(mockStartableService1, mockStartableService2))) - // Block the scheduled services until we're ready + // Block service1 to prove scheduleStart() doesn't wait for it val blockTrigger = CompletableDeferred() every { mockStartableService1.start() } coAnswers { blockTrigger.await() // Block until released @@ -116,6 +117,7 @@ class StartupServiceTests : FunSpec({ val scheduleTime = System.currentTimeMillis() - startTime // This should execute immediately since scheduleStart() doesn't block + // service3 is NOT part of scheduled services, so this is a direct call mockStartableService3.start() val immediateTime = System.currentTimeMillis() - startTime @@ -124,16 +126,21 @@ class StartupServiceTests : FunSpec({ scheduleTime shouldBeLessThan 50L immediateTime shouldBeLessThan 50L - // Wait for async execution to complete - Thread.sleep(100) + // Verify service3 was called immediately (proving main thread wasn't blocked) + verify(exactly = 1) { mockStartableService3.start() } + + // Wait a bit for async execution to start + Thread.sleep(50) - // Verify all services were called + // Verify scheduled services were called (even though service1 is blocked) verify(exactly = 1) { mockStartableService1.start() } verify(exactly = 1) { mockStartableService2.start() } - verify(exactly = 1) { mockStartableService3.start() } + + // Unblock service1 to allow test cleanup + blockTrigger.complete(Unit) // The key assertion: scheduleStart() returned immediately without blocking, - // allowing service3.start() to be called synchronously. All services eventually - // get started, proving scheduleStart() is non-blocking. + // allowing service3.start() to be called synchronously before scheduled services + // complete. This proves scheduleStart() is non-blocking. } }) From bddd49c191ea7ce7527a7faa37b11701306f015b Mon Sep 17 00:00:00 2001 From: AR Abdul Azeez Date: Mon, 8 Dec 2025 11:47:38 -0500 Subject: [PATCH 17/17] mocking dispatchers --- .../common/threading/OneSignalDispatchers.kt | 4 +- .../internal/startup/StartupServiceTests.kt | 27 ++---- .../java/com/onesignal/mocks/IOMockHelper.kt | 96 +++++++++++-------- 3 files changed, 70 insertions(+), 57 deletions(-) diff --git a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/common/threading/OneSignalDispatchers.kt b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/common/threading/OneSignalDispatchers.kt index 9e9a307a24..3b067820b1 100644 --- a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/common/threading/OneSignalDispatchers.kt +++ b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/common/threading/OneSignalDispatchers.kt @@ -25,8 +25,10 @@ import java.util.concurrent.atomic.AtomicInteger * - Small bounded queues (10 tasks) to prevent memory bloat * - Reduced context switching overhead * - Efficient thread management with controlled resource usage + * + * Made public to allow mocking in tests via IOMockHelper. */ -internal object OneSignalDispatchers { +object OneSignalDispatchers { // Optimized pool sizes based on CPU cores and workload analysis private const val IO_CORE_POOL_SIZE = 2 // Increased for better concurrency private const val IO_MAX_POOL_SIZE = 3 // Increased for better concurrency diff --git a/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/core/internal/startup/StartupServiceTests.kt b/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/core/internal/startup/StartupServiceTests.kt index 549600a5aa..7416b2910c 100644 --- a/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/core/internal/startup/StartupServiceTests.kt +++ b/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/core/internal/startup/StartupServiceTests.kt @@ -5,6 +5,7 @@ import com.onesignal.common.services.ServiceProvider import com.onesignal.debug.LogLevel import com.onesignal.debug.internal.logging.Logging import com.onesignal.mocks.IOMockHelper +import com.onesignal.mocks.IOMockHelper.awaitIO import io.kotest.assertions.throwables.shouldThrowUnit import io.kotest.core.spec.style.FunSpec import io.kotest.matchers.comparables.shouldBeLessThan @@ -13,7 +14,6 @@ import io.mockk.every import io.mockk.mockk import io.mockk.spyk import io.mockk.verify -import kotlinx.coroutines.CompletableDeferred class StartupServiceTests : FunSpec({ fun setupServiceProvider( @@ -83,34 +83,28 @@ class StartupServiceTests : FunSpec({ test("startup will call all IStartableService dependencies successfully after a short delay") { // Given - val mockStartupService1 = spyk() - val mockStartupService2 = spyk() + val mockStartupService1 = mockk(relaxed = true) + val mockStartupService2 = mockk(relaxed = true) val startupService = StartupService(setupServiceProvider(listOf(), listOf(mockStartupService1, mockStartupService2))) // When startupService.scheduleStart() - // Then - Thread.sleep(10) + // Then - wait deterministically for both services to start using IOMockHelper + awaitIO() verify(exactly = 1) { mockStartupService1.start() } verify(exactly = 1) { mockStartupService2.start() } } test("scheduleStart does not block main thread") { // Given - val mockStartableService1 = spyk() + val mockStartableService1 = mockk(relaxed = true) val mockStartableService2 = spyk() val mockStartableService3 = spyk() // Only service1 and service2 are scheduled - service3 is NOT scheduled val startupService = StartupService(setupServiceProvider(listOf(), listOf(mockStartableService1, mockStartableService2))) - // Block service1 to prove scheduleStart() doesn't wait for it - val blockTrigger = CompletableDeferred() - every { mockStartableService1.start() } coAnswers { - blockTrigger.await() // Block until released - } - // When - scheduleStart() is async, so it doesn't block val startTime = System.currentTimeMillis() startupService.scheduleStart() @@ -129,16 +123,13 @@ class StartupServiceTests : FunSpec({ // Verify service3 was called immediately (proving main thread wasn't blocked) verify(exactly = 1) { mockStartableService3.start() } - // Wait a bit for async execution to start - Thread.sleep(50) + // Wait deterministically for async execution using IOMockHelper + awaitIO() - // Verify scheduled services were called (even though service1 is blocked) + // Verify scheduled services were called verify(exactly = 1) { mockStartableService1.start() } verify(exactly = 1) { mockStartableService2.start() } - // Unblock service1 to allow test cleanup - blockTrigger.complete(Unit) - // The key assertion: scheduleStart() returned immediately without blocking, // allowing service3.start() to be called synchronously before scheduled services // complete. This proves scheduleStart() is non-blocking. diff --git a/OneSignalSDK/onesignal/testhelpers/src/main/java/com/onesignal/mocks/IOMockHelper.kt b/OneSignalSDK/onesignal/testhelpers/src/main/java/com/onesignal/mocks/IOMockHelper.kt index a5ad5b1d65..7296be941e 100644 --- a/OneSignalSDK/onesignal/testhelpers/src/main/java/com/onesignal/mocks/IOMockHelper.kt +++ b/OneSignalSDK/onesignal/testhelpers/src/main/java/com/onesignal/mocks/IOMockHelper.kt @@ -1,7 +1,7 @@ package com.onesignal.mocks +import com.onesignal.common.threading.OneSignalDispatchers import com.onesignal.common.threading.suspendifyOnIO -import com.onesignal.common.threading.suspendifyWithCompletion import io.kotest.core.listeners.AfterSpecListener import io.kotest.core.listeners.BeforeSpecListener import io.kotest.core.listeners.BeforeTestListener @@ -9,24 +9,32 @@ import io.kotest.core.listeners.TestListener import io.kotest.core.spec.Spec import io.kotest.core.test.TestCase import io.mockk.every +import io.mockk.mockk +import io.mockk.mockkObject import io.mockk.mockkStatic +import io.mockk.unmockkObject import io.mockk.unmockkStatic import kotlinx.coroutines.CompletableDeferred +import kotlinx.coroutines.CoroutineScope +import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.Job +import kotlinx.coroutines.SupervisorJob +import kotlinx.coroutines.launch import kotlinx.coroutines.withTimeout import java.util.concurrent.atomic.AtomicInteger /** - * Test helper that makes OneSignal’s `suspendifyOnIO` behavior deterministic in unit tests. + * Test helper that makes OneSignal's async threading behavior deterministic in unit tests. * Can be helpful to speed up unit tests by replacing all delay(x) or Thread.sleep(x). * - * In production, `suspendifyOnIO` launches work on background threads and returns immediately. - * This causes tests to require arbitrary delays (e.g., delay(50)) to wait for async work to finish. + * In production, `suspendifyOnIO`, `launchOnIO`, and `launchOnDefault` launch work on + * background threads and return immediately. This causes tests to require arbitrary delays + * (e.g., delay(50)) to wait for async work to finish. * * This helper avoids that by: - * - Replacing Dispatchers.Main with a test dispatcher - * - Mocking `suspendifyOnIO` so its block runs immediately + * - Mocking `suspendifyOnIO`, `launchOnIO`, and `launchOnDefault` so their blocks run immediately * - Completing a `CompletableDeferred` when the async block finishes - * - Providing `awaitIO()` so tests can explicitly wait for all IO work without sleeps + * - Providing `awaitIO()` so tests can explicitly wait for all async work without sleeps * * Usage example in a Kotest spec: * class InAppMessagesManagerTests : FunSpec({ @@ -36,7 +44,7 @@ import java.util.concurrent.atomic.AtomicInteger * ... * * test("xyz") { - * iamManager.start() // start() calls suspendOnIO + * iamManager.start() // start() calls suspendOnIO or launchOnDefault * awaitIO() // wait for background work deterministically * ... * } @@ -45,34 +53,18 @@ object IOMockHelper : BeforeSpecListener, AfterSpecListener, BeforeTestListener, private const val THREADUTILS_PATH = "com.onesignal.common.threading.ThreadUtilsKt" - // How many IO blocks are currently running + // How many async blocks (suspendifyOnIO, launchOnIO, launchOnDefault) are currently running private val pendingIo = AtomicInteger(0) - // Completed when all in-flight IO blocks for the current "wave" are done + // Completed when all in-flight async blocks for the current "wave" are done @Volatile private var ioWaiter: CompletableDeferred = CompletableDeferred() /** - * Wait for suspendifyOnIO work to finish. + * Wait for suspendifyOnIO, launchOnIO, and launchOnDefault work to finish. * Can be called multiple times in a test. - * 1. If multiple IO tasks are added before the first task finishes, the waiter will wait until ALL tasks are finished + * 1. If multiple async tasks are added before the first task finishes, the waiter will wait until ALL tasks are finished * 2. If async work is triggered after an awaitIO() has already returned, just call awaitIO() again to wait for the new work. - * - * *** NOTE ABOUT COVERAGE: - * * This helper intentionally mocks *only* the top-level `suspendifyOnIO(block)` function. - * It does NOT intercept every threading entry point defined in ThreadUtils.kt or - * OneSignalDispatchers — e.g. `suspendifyWithCompletion`, `suspendifyOnDefault`, - * `launchOnIO`, and `launchOnDefault` will continue to run using the real dispatcher - * behavior. - * - * * This design keeps the helper focused on stabilizing existing tests that specifically - * depend on `suspendifyOnIO`, without altering unrelated threading paths across the SDK. - * - * * If future tests rely on other threading helpers (e.g., direct calls to - * `suspendifyWithCompletion` or `launchOnIO`), this helper can be extended, or a separate - * test helper can be introduced to cover those cases. For now, this keeps the - * interception surface minimal and avoids unintentionally changing more concurrency - * behavior than necessary. */ suspend fun awaitIO(timeoutMs: Long = 5_000) { // Nothing to wait for in this case @@ -86,28 +78,55 @@ object IOMockHelper : BeforeSpecListener, AfterSpecListener, BeforeTestListener, override suspend fun beforeSpec(spec: Spec) { // ThreadUtilsKt = file that contains suspendifyOnIO mockkStatic(THREADUTILS_PATH) + // OneSignalDispatchers = object that contains launchOnIO and launchOnDefault + mockkObject(OneSignalDispatchers) - every { suspendifyOnIO(any Unit>()) } answers { - val block = firstArg Unit>() - - // New IO wave: if we are going from 0 -> 1, create a new waiter + // Helper function to track async work (suspendifyOnIO, launchOnIO, launchOnDefault) + // Note: We use Dispatchers.Unconfined to execute immediately and deterministically + // instead of suspendifyWithCompletion to avoid circular dependency + // (suspendifyWithCompletion calls OneSignalDispatchers.launchOnIO which we're mocking) + fun trackAsyncWork(block: suspend () -> Unit) { + // New async wave: if we are going from 0 -> 1, create a new waiter val previous = pendingIo.getAndIncrement() if (previous == 0) { ioWaiter = CompletableDeferred() } - suspendifyWithCompletion( - useIO = true, - block = block, - onComplete = { + // Execute the block using Unconfined dispatcher to run immediately and deterministically + // This makes tests deterministic and avoids the need for delays + CoroutineScope(SupervisorJob() + Dispatchers.Unconfined).launch { + try { + block() + } catch (e: Exception) { + // Log but don't throw - let the test handle exceptions + } finally { // When each block finishes, decrement; if all done, complete waiter if (pendingIo.decrementAndGet() == 0) { if (!ioWaiter.isCompleted) { ioWaiter.complete(Unit) } } - }, - ) + } + } + } + + every { suspendifyOnIO(any Unit>()) } answers { + val block = firstArg Unit>() + trackAsyncWork(block) + } + + every { OneSignalDispatchers.launchOnIO(any Unit>()) } answers { + val block = firstArg Unit>() + trackAsyncWork(block) + // Return a mock Job (launchOnIO returns a Job) + mockk(relaxed = true) + } + + every { OneSignalDispatchers.launchOnDefault(any Unit>()) } answers { + val block = firstArg Unit>() + trackAsyncWork(block) + // Return a mock Job (launchOnDefault returns a Job) + mockk(relaxed = true) } } @@ -119,5 +138,6 @@ object IOMockHelper : BeforeSpecListener, AfterSpecListener, BeforeTestListener, override suspend fun afterSpec(spec: Spec) { unmockkStatic(THREADUTILS_PATH) + unmockkObject(OneSignalDispatchers) } }