Skip to content
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,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 {
Expand Down Expand Up @@ -263,7 +261,6 @@ internal class OneSignalImp(
suspendifyOnIO {
internalInit(context, appId)
}
initState = InitState.SUCCESS
return true
}

Expand Down Expand Up @@ -306,22 +303,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() }
}

Expand All @@ -333,33 +324,73 @@ 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")
}
}

/**
* Notifies both blocking and suspend callers that initialization is complete
*/
private fun notifyInitComplete() {
initAwaiter.complete()
}

private suspend fun suspendUntilInit() {
/**
* 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.
*
* @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) {
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
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")
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 -> {
Expand All @@ -377,23 +408,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()
}

Expand All @@ -407,8 +422,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)
}
}

Expand Down Expand Up @@ -508,7 +524,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'")
}
Expand All @@ -520,7 +536,7 @@ internal class OneSignalImp(
withContext(ioDispatcher) {
Logging.log(LogLevel.DEBUG, "logoutSuspend()")

suspendUntilInit()
suspendUntilInit(operationName = "logout")

if (!isInitialized) {
throw IllegalStateException("'initWithContext failed' before 'logout'")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,7 @@ class SDKInitTests : FunSpec({
// block SharedPreference before calling init
val trigger = CompletionAwaiter("Test")
val context = getApplicationContext<Context>()
val blockingPrefContext = BlockingPrefsContext(context, trigger, 1000)
val blockingPrefContext = BlockingPrefsContext(context, trigger, 2000)
val os = OneSignalImp()

// When
Expand All @@ -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
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: the wait mechanism for the initialization seems repetitive in the test. Can we clean this up?

while (!os.isInitialized && attempts < 50) {
Thread.sleep(20)
attempts++
}

os.isInitialized shouldBe true
}

Expand Down Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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
Expand All @@ -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
}
Expand Down
Loading