Skip to content
Merged
Show file tree
Hide file tree
Changes from 4 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
1 change: 1 addition & 0 deletions changelog.d/8482.bugfix
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
fix: Make some crypto calls suspendable to avoid reported ANR
Original file line number Diff line number Diff line change
Expand Up @@ -256,7 +256,7 @@ internal class DefaultCryptoService @Inject constructor(
return if (longFormat) olmManager.getDetailedVersion(context) else olmManager.version
}

override fun getMyCryptoDevice(): CryptoDeviceInfo {
override suspend fun getMyCryptoDevice(): CryptoDeviceInfo {
return myDeviceInfoHolder.get().myDevice
}

Expand Down Expand Up @@ -536,7 +536,7 @@ internal class DefaultCryptoService @Inject constructor(
// .executeBy(taskExecutor)
// }

override fun getCryptoDeviceInfo(userId: String): List<CryptoDeviceInfo> {
override suspend fun getCryptoDeviceInfo(userId: String): List<CryptoDeviceInfo> {
return cryptoStore.getUserDeviceList(userId).orEmpty()
}
//
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ interface CryptoService {

suspend fun getUserDevices(userId: String): List<CryptoDeviceInfo>

fun getMyCryptoDevice(): CryptoDeviceInfo
suspend fun getMyCryptoDevice(): CryptoDeviceInfo

fun getGlobalBlacklistUnverifiedDevices(): Boolean

Expand Down Expand Up @@ -130,7 +130,7 @@ interface CryptoService {

suspend fun getCryptoDeviceInfo(userId: String, deviceId: String?): CryptoDeviceInfo?

fun getCryptoDeviceInfo(userId: String): List<CryptoDeviceInfo>
suspend fun getCryptoDeviceInfo(userId: String): List<CryptoDeviceInfo>

// fun getCryptoDeviceInfoFlow(userId: String): Flow<List<CryptoDeviceInfo>>

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@ import com.zhuinden.monarchy.Monarchy
import dagger.assisted.Assisted
import dagger.assisted.AssistedFactory
import dagger.assisted.AssistedInject
import kotlinx.coroutines.withContext
import org.matrix.android.sdk.api.MatrixCoroutineDispatchers
import org.matrix.android.sdk.api.session.room.model.ReadReceipt
import org.matrix.android.sdk.api.session.room.read.ReadService
import org.matrix.android.sdk.api.util.Optional
Expand All @@ -43,7 +45,8 @@ internal class DefaultReadService @AssistedInject constructor(
private val setReadMarkersTask: SetReadMarkersTask,
private val readReceiptsSummaryMapper: ReadReceiptsSummaryMapper,
@UserId private val userId: String,
private val homeServerCapabilitiesDataSource: HomeServerCapabilitiesDataSource
private val homeServerCapabilitiesDataSource: HomeServerCapabilitiesDataSource,
private val matrixCoroutineDispatchers: MatrixCoroutineDispatchers,
) : ReadService {

@AssistedFactory
Expand All @@ -66,7 +69,7 @@ internal class DefaultReadService @AssistedInject constructor(
setReadMarkersTask.execute(taskParams)
}

override suspend fun setReadReceipt(eventId: String, threadId: String) {
override suspend fun setReadReceipt(eventId: String, threadId: String) = withContext(matrixCoroutineDispatchers.io) {
val readReceiptThreadId = if (homeServerCapabilitiesDataSource.getHomeServerCapabilities()?.canUseThreadReadReceiptsAndNotifications == true) {
threadId
} else {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@ import kotlinx.coroutines.CoroutineScope
import kotlinx.coroutines.NonCancellable
import kotlinx.coroutines.cancelChildren
import kotlinx.coroutines.launch
import kotlinx.coroutines.runBlocking
import kotlinx.coroutines.withContext
import org.matrix.android.sdk.api.MatrixConfiguration
import org.matrix.android.sdk.api.MatrixCoroutineDispatchers
Expand Down Expand Up @@ -188,8 +187,8 @@ internal class RustCryptoService @Inject constructor(
return if (longFormat) "Rust SDK $version, Vodozemac $vodozemac" else version
}

override fun getMyCryptoDevice(): CryptoDeviceInfo {
return runBlocking { olmMachine.ownDevice() }
override suspend fun getMyCryptoDevice(): CryptoDeviceInfo = withContext(coroutineDispatchers.io) {
olmMachine.ownDevice()
}

override suspend fun fetchDevicesList(): List<DeviceInfo> {
Expand Down Expand Up @@ -341,11 +340,11 @@ internal class RustCryptoService @Inject constructor(
*/
override suspend fun getCryptoDeviceInfo(userId: String, deviceId: String?): CryptoDeviceInfo? {
if (userId.isEmpty() || deviceId.isNullOrEmpty()) return null
return olmMachine.getCryptoDeviceInfo(userId, deviceId)
return withContext(coroutineDispatchers.io) { olmMachine.getCryptoDeviceInfo(userId, deviceId) }
}

override fun getCryptoDeviceInfo(userId: String): List<CryptoDeviceInfo> {
return runBlocking {
override suspend fun getCryptoDeviceInfo(userId: String): List<CryptoDeviceInfo> {
return withContext(coroutineDispatchers.io) {
olmMachine.getCryptoDeviceInfo(userId)
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ package im.vector.app.gplay.features.settings.troubleshoot

import androidx.fragment.app.FragmentActivity
import androidx.lifecycle.Observer
import androidx.lifecycle.lifecycleScope
import androidx.work.WorkInfo
import androidx.work.WorkManager
import im.vector.app.R
Expand All @@ -25,6 +26,8 @@ import im.vector.app.core.pushers.FcmHelper
import im.vector.app.core.pushers.PushersManager
import im.vector.app.core.resources.StringProvider
import im.vector.app.features.settings.troubleshoot.TroubleshootTest
import kotlinx.coroutines.Dispatchers
import kotlinx.coroutines.launch
import org.matrix.android.sdk.api.session.pushers.PusherState
import javax.inject.Inject

Expand Down Expand Up @@ -60,16 +63,18 @@ class TestTokenRegistration @Inject constructor(
)
quickFix = object : TroubleshootQuickFix(R.string.settings_troubleshoot_test_token_registration_quick_fix) {
override fun doFix() {
val workId = pushersManager.enqueueRegisterPusherWithFcmKey(fcmToken)
WorkManager.getInstance(context).getWorkInfoByIdLiveData(workId).observe(context, Observer { workInfo ->
if (workInfo != null) {
if (workInfo.state == WorkInfo.State.SUCCEEDED) {
manager?.retry(testParameters)
} else if (workInfo.state == WorkInfo.State.FAILED) {
manager?.retry(testParameters)
context.lifecycleScope.launch(Dispatchers.IO) {
val workId = pushersManager.enqueueRegisterPusherWithFcmKey(fcmToken)
WorkManager.getInstance(context).getWorkInfoByIdLiveData(workId).observe(context, Observer { workInfo ->
if (workInfo != null) {
if (workInfo.state == WorkInfo.State.SUCCEEDED) {
manager?.retry(testParameters)
} else if (workInfo.state == WorkInfo.State.FAILED) {
manager?.retry(testParameters)
}
}
}
})
})
}
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,11 @@ import dagger.hilt.android.qualifiers.ApplicationContext
import im.vector.app.R
import im.vector.app.core.di.ActiveSessionHolder
import im.vector.app.core.di.DefaultPreferences
import im.vector.app.core.dispatchers.CoroutineDispatchers
import im.vector.app.core.pushers.FcmHelper
import im.vector.app.core.pushers.PushersManager
import kotlinx.coroutines.CoroutineScope
import kotlinx.coroutines.launch
import timber.log.Timber
import javax.inject.Inject

Expand All @@ -38,7 +41,12 @@ import javax.inject.Inject
class GoogleFcmHelper @Inject constructor(
@ApplicationContext private val context: Context,
@DefaultPreferences private val sharedPrefs: SharedPreferences,
appScope: CoroutineScope,
private val coroutineDispatchers: CoroutineDispatchers
) : FcmHelper {

private val scope = CoroutineScope(appScope.coroutineContext + coroutineDispatchers.io)

companion object {
private const val PREFS_KEY_FCM_TOKEN = "FCM_TOKEN"
}
Expand All @@ -64,7 +72,9 @@ class GoogleFcmHelper @Inject constructor(
.addOnSuccessListener { token ->
storeFcmToken(token)
if (registerPusher) {
pushersManager.enqueueRegisterPusherWithFcmKey(token)
scope.launch {
pushersManager.enqueueRegisterPusherWithFcmKey(token)
}
}
}
.addOnFailureListener { e ->
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,10 @@ import im.vector.app.core.pushers.PushersManager
import im.vector.app.core.pushers.UnifiedPushHelper
import im.vector.app.core.pushers.VectorPushHandler
import im.vector.app.features.settings.VectorPreferences
import kotlinx.coroutines.CoroutineScope
import kotlinx.coroutines.SupervisorJob
import kotlinx.coroutines.cancel
import kotlinx.coroutines.launch
import org.matrix.android.sdk.api.logger.LoggerTag
import timber.log.Timber
import javax.inject.Inject
Expand All @@ -43,6 +47,12 @@ class VectorFirebaseMessagingService : FirebaseMessagingService() {
@Inject lateinit var vectorPushHandler: VectorPushHandler
@Inject lateinit var unifiedPushHelper: UnifiedPushHelper

private val scope = CoroutineScope(SupervisorJob())

override fun onDestroy() {
scope.cancel()
super.onDestroy()
}
override fun onNewToken(token: String) {
Timber.tag(loggerTag.value).d("New Firebase token")
fcmHelper.storeFcmToken(token)
Expand All @@ -51,7 +61,9 @@ class VectorFirebaseMessagingService : FirebaseMessagingService() {
activeSessionHolder.hasActiveSession() &&
unifiedPushHelper.isEmbeddedDistributor()
) {
pushersManager.enqueueRegisterPusher(token, getString(R.string.pusher_http_url))
scope.launch {
pushersManager.enqueueRegisterPusher(token, getString(R.string.pusher_http_url))
}
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,14 +22,14 @@ import javax.inject.Inject

interface GetDeviceInfoUseCase {

fun execute(): CryptoDeviceInfo
suspend fun execute(): CryptoDeviceInfo
}

class DefaultGetDeviceInfoUseCase @Inject constructor(
private val activeSessionHolder: ActiveSessionHolder
) : GetDeviceInfoUseCase {

override fun execute(): CryptoDeviceInfo {
override suspend fun execute(): CryptoDeviceInfo {
return activeSessionHolder.getActiveSession().cryptoService().getMyCryptoDevice()
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -49,11 +49,11 @@ class PushersManager @Inject constructor(
)
}

fun enqueueRegisterPusherWithFcmKey(pushKey: String): UUID {
suspend fun enqueueRegisterPusherWithFcmKey(pushKey: String): UUID {
return enqueueRegisterPusher(pushKey, stringProvider.getString(R.string.pusher_http_url))
}

fun enqueueRegisterPusher(
suspend fun enqueueRegisterPusher(
pushKey: String,
gateway: String
): UUID {
Expand All @@ -62,7 +62,7 @@ class PushersManager @Inject constructor(
return currentSession.pushersService().enqueueAddHttpPusher(pusher)
}

private fun createHttpPusher(
private suspend fun createHttpPusher(
pushKey: String,
gateway: String
) = HttpPusher(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,9 @@ class VectorUnifiedPushMessagingReceiver : MessagingReceiver() {
coroutineScope.launch {
unifiedPushHelper.storeCustomOrDefaultGateway(endpoint) {
unifiedPushHelper.getPushGateway()?.let {
pushersManager.enqueueRegisterPusher(endpoint, it)
coroutineScope.launch {
pushersManager.enqueueRegisterPusher(endpoint, it)
}
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -275,7 +275,7 @@ class SelfVerificationController @Inject constructor(
id("notice_div")
}
// Option to verify with another device
if (state.hasAnyOtherSession) {
if (state.hasAnyOtherSession.invoke() == true) {
bottomSheetVerificationActionItem {
id("start")
title(host.stringProvider.getString(R.string.verification_verify_with_another_device))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ data class SelfVerificationViewState(
val transactionId: String? = null,
val currentDeviceCanCrossSign: Boolean = false,
val userWantsToCancel: Boolean = false,
val hasAnyOtherSession: Boolean = false,
val hasAnyOtherSession: Async<Boolean> = Uninitialized,
val quadSContainsSecrets: Boolean = false,
val isVerificationRequired: Boolean = false,
val isThisSessionVerified: Boolean = false,
Expand Down Expand Up @@ -146,21 +146,28 @@ class SelfVerificationViewModel @AssistedInject constructor(
}
}

val hasAnyOtherSession = session.cryptoService()
.getCryptoDeviceInfo(session.myUserId)
.any {
it.deviceId != session.sessionParams.deviceId
}
setState { copy(hasAnyOtherSession = Loading()) }
viewModelScope.launch(Dispatchers.IO) {
Copy link
Member

Choose a reason for hiding this comment

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

I think it's not mandatory to provide a dispatcher here, getCryptoDeviceInfo will switch to this dispatcher using withContext.

val hasAnyOtherSession = session.cryptoService()
.getCryptoDeviceInfo(session.myUserId)
.any {
it.deviceId != session.sessionParams.deviceId
}
setState {
copy(
hasAnyOtherSession = Success(hasAnyOtherSession)
)
}
}

setState {
copy(
currentDeviceCanCrossSign = session.cryptoService().crossSigningService().canCrossSign(),
quadSContainsSecrets = session.sharedSecretStorageService().isRecoverySetup(),
hasAnyOtherSession = hasAnyOtherSession
)
}

viewModelScope.launch {
viewModelScope.launch(Dispatchers.IO) {
val isThisSessionVerified = session.cryptoService().crossSigningService().isCrossSigningVerified()
setState {
copy(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -92,11 +92,6 @@ class UnknownDeviceDetectorSharedViewModel @AssistedInject constructor(
}

init {
val currentSessionTs = session.cryptoService().getCryptoDeviceInfo(session.myUserId)
.firstOrNull { it.deviceId == session.sessionParams.deviceId }
?.firstTimeSeenLocalTs
?: clock.epochMillis()
Timber.v("## Detector - Current Session first time seen $currentSessionTs")

combine(
session.flow().liveUserCryptoDevices(session.myUserId),
Expand All @@ -108,6 +103,12 @@ class UnknownDeviceDetectorSharedViewModel @AssistedInject constructor(

deleteUnusedClientInformation(infoList)

val currentSessionTs = session.cryptoService().getCryptoDeviceInfo(session.myUserId)
.firstOrNull { it.deviceId == session.sessionParams.deviceId }
?.firstTimeSeenLocalTs
?: clock.epochMillis()
Timber.v("## Detector - Current Session first time seen $currentSessionTs")

infoList
.asSequence()
.filter {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
package im.vector.app.features.settings.troubleshoot

import androidx.fragment.app.FragmentActivity
import androidx.lifecycle.lifecycleScope
import androidx.work.WorkInfo
import androidx.work.WorkManager
import im.vector.app.R
Expand Down Expand Up @@ -72,13 +73,15 @@ class TestEndpointAsTokenRegistration @Inject constructor(
}

private fun unregisterThenRegister(testParameters: TestParameters, pushKey: String) {
activeSessionHolder.getSafeActiveSession()?.coroutineScope?.launch {
val scope = activeSessionHolder.getSafeActiveSession()?.coroutineScope ?: return
val io = activeSessionHolder.getActiveSession().coroutineDispatchers.io
scope.launch(io) {
unregisterUnifiedPushUseCase.execute(pushersManager)
registerUnifiedPush(distributor = "", testParameters, pushKey)
}
}

private fun registerUnifiedPush(
private suspend fun registerUnifiedPush(
distributor: String,
testParameters: TestParameters,
pushKey: String,
Expand Down Expand Up @@ -106,7 +109,9 @@ class TestEndpointAsTokenRegistration @Inject constructor(
pushKey: String,
) {
unifiedPushHelper.showSelectDistributorDialog(context) { selection ->
registerUnifiedPush(distributor = selection, testParameters, pushKey)
context.lifecycleScope.launch {
registerUnifiedPush(distributor = selection, testParameters, pushKey)
}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ package im.vector.app.core.device
import im.vector.app.test.fakes.FakeActiveSessionHolder
import im.vector.app.test.fakes.FakeCryptoService
import im.vector.app.test.fakes.FakeSession
import kotlinx.coroutines.runBlocking
import org.amshove.kluent.shouldBeEqualTo
import org.junit.Test

Expand All @@ -32,7 +33,7 @@ class DefaultGetDeviceInfoUseCaseTest {

@Test
fun `when execute, then get crypto device info`() {
val result = getDeviceInfoUseCase.execute()
val result = runBlocking { getDeviceInfoUseCase.execute() }
Copy link
Member

Choose a reason for hiding this comment

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

use runTest instead of runBlocking:

fun `when execute, then get crypto device info`() = runTest {


result shouldBeEqualTo cryptoService.cryptoDeviceInfo
}
Expand Down
Loading