Skip to content

Commit f47c974

Browse files
author
AR Abdul Azeez
committed
Fix login race condition and JWT FAIL_UNAUTHORIZED blocking callers
1. Split LoginHelper into switchUser() + enqueueLogin() so the non-suspend login() can synchronously swap user models (instant, no network) while the backend create-user call fires in the background. This prevents ANRs on main thread while ensuring tags target the correct user after each login() call. loginSuspend() also calls switchUser() + enqueueLogin() directly (no wrapper method) for clarity. 2. Wake enqueueAndWait waiters on FAIL_UNAUTHORIZED and FAIL_PAUSE_OPREPO before re-queuing operations. Re-queued items use waiter=null so future retries don't reference stale waiters. This prevents loginSuspend from hanging forever when a JWT is invalid or the op repo pauses. Fixes SDK-4338 Made-with: Cursor
1 parent 258c8e6 commit f47c974

5 files changed

Lines changed: 154 additions & 94 deletions

File tree

OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/operations/impl/OperationRepo.kt

Lines changed: 16 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -288,8 +288,15 @@ internal class OperationRepo(
288288
if (externalId != null) {
289289
_jwtTokenStore.invalidateJwt(externalId)
290290
Logging.warn("Operation execution failed with 401 Unauthorized, JWT invalidated for user: $externalId. Operations re-queued.")
291+
// Unblock any enqueueAndWait callers so loginSuspend doesn't hang.
292+
ops.forEach { it.waiter?.wake(false) }
293+
// Re-queue with waiter = null: the operation is preserved for retry
294+
// (once a new JWT is provided via updateUserJwt), but the original
295+
// waiter is detached since it was already woken above.
291296
synchronized(queue) {
292-
ops.reversed().forEach { queue.add(0, it) }
297+
ops.reversed().forEach {
298+
queue.add(0, OperationQueueItem(it.operation, waiter = null, bucket = it.bucket, retries = it.retries))
299+
}
293300
}
294301
dispatchJwtInvalidatedToApp(externalId)
295302
} else {
@@ -331,9 +338,15 @@ internal class OperationRepo(
331338
Logging.error("Operation execution failed with eventual retry, pausing the operation repo: $operations")
332339
// keep the failed operation and pause the operation repo from executing
333340
paused = true
334-
// add back all operations to the front of the queue to be re-executed.
341+
// Unblock any enqueueAndWait callers so loginSuspend doesn't hang.
342+
ops.forEach { it.waiter?.wake(false) }
343+
// Re-queue with waiter = null: the operation is preserved for retry
344+
// on next cold start, but the original waiter is detached since it
345+
// was already woken above.
335346
synchronized(queue) {
336-
ops.reversed().forEach { queue.add(0, it) }
347+
ops.reversed().forEach {
348+
queue.add(0, OperationQueueItem(it.operation, waiter = null, bucket = it.bucket, retries = it.retries))
349+
}
337350
}
338351
}
339352
}

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

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -384,14 +384,20 @@ internal class OneSignalImp(
384384

385385
if (isBackgroundThreadingEnabled) {
386386
waitForInit(operationName = "login")
387-
suspendifyOnIO { loginHelper.login(externalId, jwtBearerToken) }
388387
} else {
389388
if (!isInitialized) {
390389
throw IllegalStateException("Must call 'initWithContext' before 'login'")
391390
}
391+
}
392+
393+
val context = loginHelper.switchUser(externalId, jwtBearerToken) ?: return
394+
395+
if (isBackgroundThreadingEnabled) {
396+
suspendifyOnIO { loginHelper.enqueueLogin(context) }
397+
} else {
392398
Thread {
393399
runBlocking(runtimeIoDispatcher) {
394-
loginHelper.login(externalId, jwtBearerToken)
400+
loginHelper.enqueueLogin(context)
395401
}
396402
}.start()
397403
}
@@ -695,7 +701,8 @@ internal class OneSignalImp(
695701
throw IllegalStateException("'initWithContext failed' before 'login'")
696702
}
697703

698-
loginHelper.login(externalId, jwtBearerToken)
704+
val context = loginHelper.switchUser(externalId, jwtBearerToken) ?: return@withContext
705+
loginHelper.enqueueLogin(context)
699706
}
700707

701708
override suspend fun logoutSuspend() =

OneSignalSDK/onesignal/core/src/main/java/com/onesignal/user/internal/LoginHelper.kt

Lines changed: 37 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -15,22 +15,30 @@ class LoginHelper(
1515
private val jwtTokenStore: JwtTokenStore,
1616
private val lock: Any,
1717
) {
18-
suspend fun login(
18+
internal data class LoginEnqueueContext(
19+
val appId: String,
20+
val newIdentityOneSignalId: String,
21+
val externalId: String,
22+
val existingOneSignalId: String?,
23+
)
24+
25+
/**
26+
* Synchronously switches local user models under the login/logout lock.
27+
* Returns context needed for [enqueueLogin], or null if the user was
28+
* already logged in with [externalId] (no switch needed).
29+
*/
30+
internal fun switchUser(
1931
externalId: String,
2032
jwtBearerToken: String? = null,
21-
) {
22-
var currentIdentityExternalId: String? = null
23-
var currentIdentityOneSignalId: String? = null
24-
var newIdentityOneSignalId: String = ""
25-
33+
): LoginEnqueueContext? {
2634
synchronized(lock) {
27-
currentIdentityExternalId = identityModelStore.model.externalId
28-
currentIdentityOneSignalId = identityModelStore.model.onesignalId
35+
val currentExternalId = identityModelStore.model.externalId
36+
val currentOneSignalId = identityModelStore.model.onesignalId
2937

30-
if (currentIdentityExternalId == externalId) {
38+
if (currentExternalId == externalId) {
3139
jwtTokenStore.putJwt(externalId, jwtBearerToken)
3240
operationRepo.forceExecuteOperations()
33-
return
41+
return null
3442
}
3543

3644
jwtTokenStore.putJwt(externalId, jwtBearerToken)
@@ -39,23 +47,30 @@ class LoginHelper(
3947
identityModel.externalId = externalId
4048
}
4149

42-
newIdentityOneSignalId = identityModelStore.model.onesignalId
43-
}
50+
val newOneSignalId = identityModelStore.model.onesignalId
4451

45-
val existingOneSignalId =
46-
if (configModel.useIdentityVerification == true) {
47-
null
48-
} else {
49-
if (currentIdentityExternalId == null) currentIdentityOneSignalId else null
50-
}
52+
val existingOneSignalId =
53+
if (configModel.useIdentityVerification == true) {
54+
null
55+
} else {
56+
if (currentExternalId == null) currentOneSignalId else null
57+
}
58+
59+
return LoginEnqueueContext(configModel.appId, newOneSignalId, externalId, existingOneSignalId)
60+
}
61+
}
5162

63+
/**
64+
* Enqueues the [LoginUserOperation] and suspends until it completes.
65+
*/
66+
internal suspend fun enqueueLogin(context: LoginEnqueueContext) {
5267
val result =
5368
operationRepo.enqueueAndWait(
5469
LoginUserOperation(
55-
configModel.appId,
56-
newIdentityOneSignalId,
57-
externalId,
58-
existingOneSignalId,
70+
context.appId,
71+
context.newIdentityOneSignalId,
72+
context.externalId,
73+
context.existingOneSignalId,
5974
),
6075
)
6176

OneSignalSDK/onesignal/core/src/test/java/com/onesignal/core/internal/operations/OperationRepoTests.kt

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -954,8 +954,9 @@ class OperationRepoTests : FunSpec({
954954
operationRepo.start()
955955
val response = operationRepo.enqueueAndWait(operation)
956956

957-
// Then
958-
response shouldBe true
957+
// Then – waiter is woken with false immediately on FAIL_UNAUTHORIZED
958+
// (operation is re-queued with waiter=null for retry when a new JWT is provided)
959+
response shouldBe false
959960
verify { jwtTokenStore.invalidateJwt("test-user") }
960961
handlerCalledWith shouldBe "test-user"
961962
}
@@ -1011,8 +1012,11 @@ class OperationRepoTests : FunSpec({
10111012
operationRepo.start()
10121013
val response = operationRepo.enqueueAndWait(operation)
10131014

1014-
response shouldBe true
1015+
// Waiter is woken with false immediately; operation re-queued with waiter=null
1016+
response shouldBe false
10151017
verify { jwtTokenStore.invalidateJwt("test-user") }
1018+
// The re-queued op (waiter=null) retries asynchronously; wait for it to complete
1019+
delay(3000)
10161020
coVerify(exactly = 2) { executor.execute(any()) }
10171021
}
10181022

0 commit comments

Comments
 (0)