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 880556393b..0000000000 --- a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/common/threading/CompletionAwaiter.kt +++ /dev/null @@ -1,135 +0,0 @@ -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. - * 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. - * - * Usage: - * val awaiter = CompletionAwaiter("OneSignal SDK Init") - * - * // For blocking code: - * awaiter.await() - * - * // For suspend code: - * awaiter.awaitSuspend() - * - * // When complete: - * awaiter.complete() - */ -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() - - /** - * Completes the awaiter, unblocking both blocking and suspend callers. - */ - fun complete() { - latch.countDown() - suspendCompletion.complete(Unit) - } - - /** - * Wait for completion using blocking approach with an optional timeout. - * - * @param timeoutMs Timeout in milliseconds, defaults to context-appropriate timeout - * @return true if completed before timeout, false otherwise. - */ - fun await(timeoutMs: Long = getDefaultTimeout()): Boolean { - val completed = - try { - latch.await(timeoutMs, TimeUnit.MILLISECONDS) - } catch (e: InterruptedException) { - Logging.warn("Interrupted while waiting for $componentName", e) - logAllThreads() - false - } - - if (!completed) { - val message = createTimeoutMessage(timeoutMs) - Logging.warn(message) - } - - return completed - } - - /** - * Wait for completion using suspend approach (non-blocking for coroutines). - * This method will suspend the current coroutine until completion is signaled. - */ - suspend fun awaitSuspend() { - suspendCompletion.await() - } - - private fun getDefaultTimeout(): Long { - return if (AndroidUtils.isRunningOnMainThread()) ANDROID_ANR_TIMEOUT_MS else 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() - - // 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/main/java/com/onesignal/common/threading/OneSignalDispatchers.kt b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/common/threading/OneSignalDispatchers.kt index 19c7b1ddde..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 @@ -35,7 +37,7 @@ internal object OneSignalDispatchers { 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 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..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,19 +38,16 @@ 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.TimeoutCancellationException 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 { - @Volatile - private var initAwaiter = CompletionAwaiter("OneSignalImp") + + private val suspendCompletion = CompletableDeferred() @Volatile private var initState: InitState = InitState.NOT_STARTED @@ -263,7 +259,6 @@ internal class OneSignalImp( suspendifyOnIO { internalInit(context, appId) } - initState = InitState.SUCCESS return true } @@ -306,22 +301,16 @@ internal class OneSignalImp( ) { Logging.log(LogLevel.DEBUG, "Calling deprecated login(externalId: $externalId, jwtBearerToken: $jwtBearerToken)") - if (!initState.isSDKAccessible()) { - throw IllegalStateException("Must call 'initWithContext' before 'login'") - } + waitForInit(operationName = "login") - waitForInit() 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'") - } + waitForInit(operationName = "logout") - waitForInit() suspendifyOnIO { logoutHelper.logout() } } @@ -333,10 +322,16 @@ 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") + /** + * Blocking version that waits for initialization to complete. + * Uses runBlocking to bridge to the suspend implementation. + * Waits indefinitely until init completes and logs how long it took. + * + * @param operationName Optional operation name to include in error messages (e.g., "login", "logout") + */ + private fun waitForInit(operationName: String? = null) { + runBlocking(ioDispatcher) { + waitUntilInitInternal(operationName) } } @@ -344,23 +339,65 @@ internal class OneSignalImp( * Notifies both blocking and suspend callers that initialization is complete */ private fun notifyInitComplete() { - initAwaiter.complete() + suspendCompletion.complete(Unit) } - private suspend fun suspendUntilInit() { + /** + * Suspend version that waits for initialization to complete. + * Waits indefinitely until init completes and logs how long it took. + * + * @param operationName Optional operation name to include in error messages (e.g., "login", "logout") + */ + private suspend fun suspendUntilInit(operationName: String? = null) { + waitUntilInitInternal(operationName) + } + + /** + * Common implementation for waiting until initialization completes. + * Waits indefinitely until init completes (SUCCESS or FAILED) to ensure consistent state. + * Logs how long initialization took when it completes. + * + * @param operationName Optional operation name to include in error messages (e.g., "login", "logout") + */ + private suspend fun waitUntilInitInternal(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...") - try { - withTimeout(MAX_TIMEOUT_TO_INIT) { - initAwaiter.awaitSuspend() - } - } catch (e: TimeoutCancellationException) { - throw IllegalStateException("initWithContext was timed out after $MAX_TIMEOUT_TO_INIT ms") + Logging.debug("Waiting for init to complete...") + + 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. + suspendCompletion.await() + + // 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.") @@ -377,23 +414,7 @@ internal class OneSignalImp( } 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() - } - InitState.FAILED -> { - throw IllegalStateException("Initialization failed. Cannot proceed.") - } - else -> { - // SUCCESS - waitForInit() - } - } - + waitForInit() return getter() } @@ -407,8 +428,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) } } @@ -508,7 +530,8 @@ 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'") } @@ -520,7 +543,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/common/threading/CompletionAwaiterTests.kt b/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/common/threading/CompletionAwaiterTests.kt deleted file mode 100644 index 37f239ead3..0000000000 --- a/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/common/threading/CompletionAwaiterTests.kt +++ /dev/null @@ -1,363 +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.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 -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("blocking await functionality") { - - test("await completes immediately when already completed") { - // Given - awaiter.complete() - - // When - val startTime = System.currentTimeMillis() - val completed = awaiter.await(1000) - 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 timeoutMs = 2000L - - val startTime = System.currentTimeMillis() - - // Simulate delayed completion from another thread - suspendifyOnIO { - delay(completionDelay) - awaiter.complete() - } - - val result = awaiter.await(timeoutMs) - val duration = System.currentTimeMillis() - startTime - - result shouldBe true - duration shouldBeGreaterThan (completionDelay - 50) - duration shouldBeLessThan (completionDelay + 150) // buffer - } - - test("await returns false when timeout expires") { - 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() - val jobs = mutableListOf() - - // Start multiple blocking callers - repeat(numCallers) { index -> - val thread = - Thread { - val result = awaiter.await(2000) - 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") { - 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("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 - - // Test suspend callers first - 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 - awaiter.complete() - - // Wait for all to complete - suspendJobs.awaitAll() - - // All should have completed - suspendResults.size shouldBe 2 - } - - // Reset for blocking test - awaiter = CompletionAwaiter("TestComponent") - - // Test blocking callers - val blockingResults = mutableListOf() - val blockingThreads = - (1..2).map { index -> - Thread { - val result = awaiter.await(2000) - 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(100) - completed shouldBe true - } - - 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() - } - } - } - - 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 - } - } -}) 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 318f6cb1c1..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,12 +14,27 @@ 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({ + /** + * 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 @@ -89,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 @@ -122,7 +136,7 @@ class SDKInitTests : FunSpec({ accessorThread.isAlive shouldBe true // release SharedPreferences - trigger.complete() + trigger.countDown() accessorThread.join(500) accessorThread.isAlive shouldBe false @@ -135,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, 1000) + val blockingPrefContext = BlockingPrefsContext(context, trigger) val os = OneSignalImp() // When @@ -150,17 +164,22 @@ 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 - os.isInitialized shouldBe true + + // Release the SharedPreferences lock so internalInit can complete + trigger.countDown() + + // Wait for initialization to complete (internalInit runs asynchronously) + waitForInitialization(os, maxAttempts = 50) } 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 = @@ -175,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 @@ -202,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" @@ -224,11 +243,22 @@ 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 - trigger.complete() + // release the lock on SharedPreferences so internalInit can complete + trigger.countDown() + + // 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 @@ -307,12 +337,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) @@ -323,12 +348,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) @@ -437,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(timeoutInMillis) - } 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/core/internal/startup/StartupServiceTests.kt b/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/core/internal/startup/StartupServiceTests.kt index 93fa9e6b11..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 @@ -8,12 +8,12 @@ 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 import io.kotest.matchers.shouldBe 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,47 +83,55 @@ 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 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 - 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() + val scheduleTime = System.currentTimeMillis() - startTime - // When - val thread = - Thread { - startupService.scheduleStart() - mockStartableService2.start() - } - thread.start() + // 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 - // Then - // service2 does not block even though service1 is blocked - verify(exactly = 1) { 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 + + // Verify service3 was called immediately (proving main thread wasn't blocked) + verify(exactly = 1) { mockStartableService3.start() } - // unblock the trigger and wait for scheduled service to complete - blockTrigger.complete(Unit) + // Wait deterministically for async execution using IOMockHelper awaitIO() - verify { mockStartableService1.start() } + + // Verify scheduled services were called + verify(exactly = 1) { mockStartableService1.start() } + verify(exactly = 1) { mockStartableService2.start() } + + // 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/core/src/test/java/com/onesignal/internal/OneSignalImpTests.kt b/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/internal/OneSignalImpTests.kt index e5e49f1ec0..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 @@ -215,46 +215,42 @@ 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) - // 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 CompletionAwaiter 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 CompletionAwaiter.await() - // which has 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 // 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 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 } }) 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) } }