bug: Handling race condition

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<Unit>()
@@ -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,19 +87,6 @@ class CompletionAwaiter(
         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()
 

OneSignalSDK/onesignal/core/src/main/java/com/onesignal/common/threading/OneSignalDispatchers.kt

@@ -35,7 +35,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

OneSignalSDK/onesignal/core/src/main/java/com/onesignal/internal/OneSignalImp.kt

@@ -40,12 +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
-
-private const val MAX_TIMEOUT_TO_INIT = 30_000L // 30 seconds
 
 internal class OneSignalImp(
     private val ioDispatcher: CoroutineDispatcher = OneSignalDispatchers.IO,
@@ -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 <T> getAllServices(c: Class<T>): List<T> = 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)
         }
     }
 
@@ -347,20 +342,62 @@ internal class OneSignalImp(
         initAwaiter.complete()
     }
 
-    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.
+                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.")
@@ -377,23 +414,7 @@ internal class OneSignalImp(
     }
 
     private fun <T> 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'")