Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix ANR caused by operationRepo.enqueue while loading is in progress #2233

Merged
merged 4 commits into from
Dec 16, 2024
Merged
Show file tree
Hide file tree
Changes from all 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
@@ -0,0 +1,19 @@
package com.onesignal.common.threading

import kotlinx.coroutines.CoroutineScope
import kotlinx.coroutines.launch
import kotlinx.coroutines.newSingleThreadContext

object OSPrimaryCoroutineScope {
// CoroutineScope tied to the main thread
private val mainScope = CoroutineScope(newSingleThreadContext(name = "OSPrimaryCoroutineScope"))

/**
* Executes the given [block] on the OS primary coroutine scope.
*/
fun execute(block: suspend () -> Unit) {
mainScope.launch {
block()
}
}
}
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package com.onesignal.session.internal.session.impl

import com.onesignal.common.threading.OSPrimaryCoroutineScope
import com.onesignal.common.threading.suspendifyOnThread
import com.onesignal.core.internal.config.ConfigModelStore
import com.onesignal.core.internal.operations.IOperationRepo
Expand Down Expand Up @@ -40,7 +41,10 @@ internal class SessionListener(
}

override fun onSessionStarted() {
_operationRepo.enqueue(TrackSessionStartOperation(_configModelStore.model.appId, _identityModelStore.model.onesignalId), true)
// enqueue the operation in background
OSPrimaryCoroutineScope.execute {
_operationRepo.enqueue(TrackSessionStartOperation(_configModelStore.model.appId, _identityModelStore.model.onesignalId), true)
}
}

override fun onSessionActive() {
Expand All @@ -54,9 +58,12 @@ internal class SessionListener(
Logging.error("SessionListener.onSessionEnded sending duration of $durationInSeconds seconds")
}

_operationRepo.enqueue(
TrackSessionEndOperation(_configModelStore.model.appId, _identityModelStore.model.onesignalId, durationInSeconds),
)
// enqueue the operation in background
OSPrimaryCoroutineScope.execute {
_operationRepo.enqueue(
TrackSessionEndOperation(_configModelStore.model.appId, _identityModelStore.model.onesignalId, durationInSeconds),
)
}

suspendifyOnThread {
_outcomeEventsController.sendSessionEndOutcomeEvent(durationInSeconds)
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package com.onesignal.user.internal.service

import com.onesignal.common.IDManager
import com.onesignal.common.threading.OSPrimaryCoroutineScope
import com.onesignal.core.internal.application.IApplicationService
import com.onesignal.core.internal.config.ConfigModelStore
import com.onesignal.core.internal.operations.IOperationRepo
Expand Down Expand Up @@ -28,12 +29,14 @@ class UserRefreshService(
return
}

_operationRepo.enqueue(
RefreshUserOperation(
_configModelStore.model.appId,
_identityModelStore.model.onesignalId,
),
)
OSPrimaryCoroutineScope.execute {
_operationRepo.enqueue(
RefreshUserOperation(
_configModelStore.model.appId,
_identityModelStore.model.onesignalId,
),
)
}
}

override fun start() = _sessionService.subscribe(this)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,15 @@ import com.onesignal.common.threading.WaiterWithValue
import com.onesignal.core.internal.operations.impl.OperationModelStore
import com.onesignal.core.internal.operations.impl.OperationRepo
import com.onesignal.core.internal.operations.impl.OperationRepo.OperationQueueItem
import com.onesignal.core.internal.preferences.PreferenceOneSignalKeys
import com.onesignal.core.internal.preferences.PreferenceStores
import com.onesignal.core.internal.time.impl.Time
import com.onesignal.debug.LogLevel
import com.onesignal.debug.internal.logging.Logging
import com.onesignal.mocks.MockHelper
import com.onesignal.mocks.MockPreferencesService
import com.onesignal.user.internal.operations.ExecutorMocks.Companion.getNewRecordState
import com.onesignal.user.internal.operations.LoginUserOperation
import io.kotest.core.spec.style.FunSpec
import io.kotest.matchers.shouldBe
import io.mockk.CapturingSlot
Expand All @@ -28,6 +32,7 @@ import kotlinx.coroutines.launch
import kotlinx.coroutines.withTimeout
import kotlinx.coroutines.withTimeoutOrNull
import kotlinx.coroutines.yield
import org.json.JSONArray
import java.util.UUID

// Mocks used by every test in this file
Expand Down Expand Up @@ -76,6 +81,65 @@ class OperationRepoTests : FunSpec({
Logging.logLevel = LogLevel.NONE
}

test("ensure loading in the background thread does not block enqueue") {
// Given
val prefs = MockPreferencesService()
val mocks = Mocks()
val operationModelStore: OperationModelStore = spyk(OperationModelStore(prefs))
val operationRepo =
spyk(
OperationRepo(
listOf(mocks.executor),
operationModelStore,
mocks.configModelStore,
Time(),
getNewRecordState(mocks.configModelStore),
),
)

val cachedOperation = LoginUserOperation()
val newOperation = LoginUserOperation()
val jsonArray = JSONArray()

// cache the operation
jsonArray.put(cachedOperation.toJSON())
prefs.saveString(PreferenceStores.ONESIGNAL, PreferenceOneSignalKeys.MODEL_STORE_PREFIX + "operations", jsonArray.toString())

cachedOperation.id = UUID.randomUUID().toString()
newOperation.id = UUID.randomUUID().toString()
every { operationModelStore.create(any()) } answers {
// simulate a prolonged loading from cache
Thread.sleep(1000)
cachedOperation
}

// simulate a background thread to load operations
val backgroundThread =
Thread {
operationRepo.loadSavedOperations()
}

val mainThread =
Thread {
operationRepo.enqueue(newOperation)
}

// When
backgroundThread.start()
mainThread.start()

// Then
// insertion from the main thread is done without blocking
mainThread.join(500)
operationRepo.queue.size shouldBe 1
mainThread.state shouldBe Thread.State.TERMINATED

// after loading is completed, the cached operation should be at the beginning of the queue
backgroundThread.join()
operationRepo.queue.size shouldBe 2
operationRepo.queue.first().operation shouldBe cachedOperation
}

test("containsInstanceOf") {
// Given
val operationRepo = Mocks().operationRepo
Expand Down
Loading