Skip to content

Commit 7570a43

Browse files
nan-liclaude
nan-li
and
claude
committed
fix: login race causes subsequent calls to target previous user
OneSignal.login() scheduled the user switch asynchronously via suspendifyOnIO, so the API call returned before the identity model was updated. Calls made right after login (e.g. addTag) applied to the OLD user because the switch hadn't happened yet. Split login into two phases: - switchUser(): synchronous under the login/logout lock. Swaps the local identity/properties/subscription models immediately so subsequent SDK calls see the new user. - enqueueLogin(): async. Enqueues the LoginUserOperation and awaits completion of the backend user creation. Because switchUser returns before enqueueLogin runs, there's a small window where RecoverFromDroppedLoginBug can observe the state (new externalId, local onesignalId, no LoginUserOperation in queue) and enqueue its own recovery login. When the real enqueueLogin then adds another login op, the executor crashes on a grouped batch containing two LoginUserOperations. Add a dedupe check in internalEnqueue: if a LoginUserOperation for the same onesignalId is already queued, drop the incoming one and wake its waiter optimistically with true so loginSuspend callers don't hang on enqueueAndWait. If the duplicate came from loadSavedOperations (addToStore = false), also remove it from the operation model store so we don't leave an orphan entry that lingers until the next cold start. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent 5f7187b commit 7570a43

4 files changed

Lines changed: 73 additions & 22 deletions

File tree

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

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import com.onesignal.core.internal.startup.IStartableService
1111
import com.onesignal.core.internal.time.ITime
1212
import com.onesignal.debug.LogLevel
1313
import com.onesignal.debug.internal.logging.Logging
14+
import com.onesignal.user.internal.operations.LoginUserOperation
1415
import com.onesignal.user.internal.operations.impl.states.NewRecordsState
1516
import kotlinx.coroutines.CompletableDeferred
1617
import kotlinx.coroutines.CoroutineScope
@@ -161,6 +162,27 @@ internal class OperationRepo(
161162
return
162163
}
163164

165+
// Dedupe LoginUserOperation for the same user — prevents RecoverFromDroppedLoginBug
166+
// and the real login() call from both enqueuing a login op during the timing window.
167+
// Wake the waiter optimistically with `true`: the already-queued op will do the work,
168+
// but we don't await its real result (that would require waiter chaining). enqueueAndWait
169+
// callers like loginSuspend would otherwise hang forever. The only caller today is
170+
// LoginHelper.enqueueLogin which just logs on failure, so the lost signal is acceptable.
171+
// If the op came from loadSavedOperations (addToStore = false) it's also a stale
172+
// duplicate in the store; remove it to avoid an orphan entry that lingers until the
173+
// next cold start.
174+
val op = queueItem.operation
175+
if (op is LoginUserOperation &&
176+
queue.any { it.operation is LoginUserOperation && it.operation.onesignalId == op.onesignalId }
177+
) {
178+
Logging.debug("OperationRepo: internalEnqueue - LoginUserOperation for onesignalId: ${op.onesignalId} already exists in the queue.")
179+
if (!addToStore) {
180+
_operationModelStore.remove(queueItem.operation.id)
181+
}
182+
queueItem.waiter?.wake(true)
183+
return
184+
}
185+
164186
if (index != null) {
165187
queue.add(index, queueItem)
166188
} else {

‎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
@@ -378,14 +378,20 @@ internal class OneSignalImp(
378378

379379
if (isBackgroundThreadingEnabled) {
380380
waitForInit(operationName = "login")
381-
suspendifyOnIO { loginHelper.login(externalId, jwtBearerToken) }
382381
} else {
383382
if (!isInitialized) {
384383
throw IllegalStateException("Must call 'initWithContext' before 'login'")
385384
}
385+
}
386+
387+
val context = loginHelper.switchUser(externalId, jwtBearerToken) ?: return
388+
389+
if (isBackgroundThreadingEnabled) {
390+
suspendifyOnIO { loginHelper.enqueueLogin(context) }
391+
} else {
386392
Thread {
387393
runBlocking(runtimeIoDispatcher) {
388-
loginHelper.login(externalId, jwtBearerToken)
394+
loginHelper.enqueueLogin(context)
389395
}
390396
}.start()
391397
}
@@ -646,7 +652,8 @@ internal class OneSignalImp(
646652
throw IllegalStateException("'initWithContext failed' before 'login'")
647653
}
648654

649-
loginHelper.login(externalId, jwtBearerToken)
655+
val context = loginHelper.switchUser(externalId, jwtBearerToken) ?: return@withContext
656+
loginHelper.enqueueLogin(context)
650657
}
651658

652659
override suspend fun logoutSuspend() =

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

Lines changed: 33 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -13,37 +13,55 @@ class LoginHelper(
1313
private val configModel: ConfigModel,
1414
private val lock: Any,
1515
) {
16-
suspend fun login(
16+
internal data class LoginEnqueueContext(
17+
val appId: String,
18+
val newIdentityOneSignalId: String,
19+
val externalId: String,
20+
val existingOneSignalId: String?,
21+
)
22+
23+
/**
24+
* Synchronously switches local user models under the login/logout lock so subsequent
25+
* SDK calls (e.g. addTag) see the new user's identity immediately. Returns context
26+
* needed for [enqueueLogin], or null if the user was already logged in with [externalId]
27+
* (no switch needed).
28+
*/
29+
internal fun switchUser(
1730
externalId: String,
1831
jwtBearerToken: String? = null,
19-
) {
20-
var currentIdentityExternalId: String? = null
21-
var currentIdentityOneSignalId: String? = null
22-
var newIdentityOneSignalId: String = ""
23-
32+
): LoginEnqueueContext? {
2433
synchronized(lock) {
25-
currentIdentityExternalId = identityModelStore.model.externalId
26-
currentIdentityOneSignalId = identityModelStore.model.onesignalId
34+
val currentExternalId = identityModelStore.model.externalId
35+
val currentOneSignalId = identityModelStore.model.onesignalId
2736

28-
if (currentIdentityExternalId == externalId) {
29-
return
37+
if (currentExternalId == externalId) {
38+
return null
3039
}
3140

3241
// TODO: Set JWT Token for all future requests.
3342
userSwitcher.createAndSwitchToNewUser { identityModel, _ ->
3443
identityModel.externalId = externalId
3544
}
3645

37-
newIdentityOneSignalId = identityModelStore.model.onesignalId
46+
val newOneSignalId = identityModelStore.model.onesignalId
47+
val existingOneSignalId =
48+
if (currentExternalId == null) currentOneSignalId else null
49+
50+
return LoginEnqueueContext(configModel.appId, newOneSignalId, externalId, existingOneSignalId)
3851
}
52+
}
3953

54+
/**
55+
* Enqueues the [LoginUserOperation] and suspends until it completes.
56+
*/
57+
internal suspend fun enqueueLogin(context: LoginEnqueueContext) {
4058
val result =
4159
operationRepo.enqueueAndWait(
4260
LoginUserOperation(
43-
configModel.appId,
44-
newIdentityOneSignalId,
45-
externalId,
46-
if (currentIdentityExternalId == null) currentIdentityOneSignalId else null,
61+
context.appId,
62+
context.newIdentityOneSignalId,
63+
context.externalId,
64+
context.existingOneSignalId,
4765
),
4866
)
4967

‎OneSignalSDK/onesignal/core/src/test/java/com/onesignal/user/internal/LoginHelperTests.kt‎

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,8 @@ class LoginHelperTests : FunSpec({
6161

6262
// When
6363
runBlocking {
64-
loginHelper.login(currentExternalId)
64+
val context = loginHelper.switchUser(currentExternalId)
65+
if (context != null) loginHelper.enqueueLogin(context)
6566
}
6667

6768
// Then - should return early without any operations
@@ -113,7 +114,8 @@ class LoginHelperTests : FunSpec({
113114

114115
// When
115116
runBlocking {
116-
loginHelper.login(newExternalId)
117+
val context = loginHelper.switchUser(newExternalId)
118+
if (context != null) loginHelper.enqueueLogin(context)
117119
}
118120

119121
// Then - should switch users and enqueue login operation
@@ -178,7 +180,8 @@ class LoginHelperTests : FunSpec({
178180

179181
// When
180182
runBlocking {
181-
loginHelper.login(newExternalId)
183+
val context = loginHelper.switchUser(newExternalId)
184+
if (context != null) loginHelper.enqueueLogin(context)
182185
}
183186

184187
// Then - should provide existing OneSignal ID for anonymous user conversion
@@ -239,7 +242,8 @@ class LoginHelperTests : FunSpec({
239242

240243
// When
241244
runBlocking {
242-
loginHelper.login(newExternalId)
245+
val context = loginHelper.switchUser(newExternalId)
246+
if (context != null) loginHelper.enqueueLogin(context)
243247
}
244248

245249
// Then - should still switch users but operation fails

0 commit comments

Comments
 (0)