From 268cbb83cd0ee4f70e9c9b2f058c7b77e7289392 Mon Sep 17 00:00:00 2001 From: valere Date: Tue, 30 May 2023 23:47:50 +0200 Subject: [PATCH] Make cryptoDevice calls suspendable --- changelog.d/8482.bugfix | 1 + .../internal/crypto/DefaultCryptoService.kt | 4 ++-- .../sdk/api/session/crypto/CryptoService.kt | 4 ++-- .../session/room/read/DefaultReadService.kt | 7 ++++-- .../sdk/internal/crypto/RustCryptoService.kt | 4 ++-- .../troubleshoot/TestTokenRegistration.kt | 23 +++++++++++-------- .../im/vector/app/push/fcm/GoogleFcmHelper.kt | 12 +++++++++- .../fcm/VectorFirebaseMessagingService.kt | 14 ++++++++++- .../app/core/device/GetDeviceInfoUseCase.kt | 4 ++-- .../vector/app/core/pushers/PushersManager.kt | 6 ++--- .../VectorUnifiedPushMessagingReceiver.kt | 4 +++- .../self/SelfVerificationController.kt | 2 +- .../self/SelfVerificationViewModel.kt | 23 ++++++++++++------- .../UnknownDeviceDetectorSharedViewModel.kt | 11 +++++---- .../TestEndpointAsTokenRegistration.kt | 11 ++++++--- 15 files changed, 88 insertions(+), 42 deletions(-) create mode 100644 changelog.d/8482.bugfix diff --git a/changelog.d/8482.bugfix b/changelog.d/8482.bugfix new file mode 100644 index 0000000000..3e4184b3bf --- /dev/null +++ b/changelog.d/8482.bugfix @@ -0,0 +1 @@ +fix: Make some crypto calls suspendable to avoid reported ANR diff --git a/matrix-sdk-android/src/kotlinCrypto/java/org/matrix/android/sdk/internal/crypto/DefaultCryptoService.kt b/matrix-sdk-android/src/kotlinCrypto/java/org/matrix/android/sdk/internal/crypto/DefaultCryptoService.kt index f948063c77..b25c04aa9b 100755 --- a/matrix-sdk-android/src/kotlinCrypto/java/org/matrix/android/sdk/internal/crypto/DefaultCryptoService.kt +++ b/matrix-sdk-android/src/kotlinCrypto/java/org/matrix/android/sdk/internal/crypto/DefaultCryptoService.kt @@ -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 } @@ -536,7 +536,7 @@ internal class DefaultCryptoService @Inject constructor( // .executeBy(taskExecutor) // } - override fun getCryptoDeviceInfo(userId: String): List { + override suspend fun getCryptoDeviceInfo(userId: String): List { return cryptoStore.getUserDeviceList(userId).orEmpty() } // diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/session/crypto/CryptoService.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/session/crypto/CryptoService.kt index 043d0a29f5..31d11f6730 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/session/crypto/CryptoService.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/session/crypto/CryptoService.kt @@ -73,7 +73,7 @@ interface CryptoService { suspend fun getUserDevices(userId: String): List - fun getMyCryptoDevice(): CryptoDeviceInfo + suspend fun getMyCryptoDevice(): CryptoDeviceInfo fun getGlobalBlacklistUnverifiedDevices(): Boolean @@ -130,7 +130,7 @@ interface CryptoService { suspend fun getCryptoDeviceInfo(userId: String, deviceId: String?): CryptoDeviceInfo? - fun getCryptoDeviceInfo(userId: String): List + suspend fun getCryptoDeviceInfo(userId: String): List // fun getCryptoDeviceInfoFlow(userId: String): Flow> diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/room/read/DefaultReadService.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/room/read/DefaultReadService.kt index 36ec5e8dac..73b7ae05fe 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/room/read/DefaultReadService.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/room/read/DefaultReadService.kt @@ -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 @@ -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 @@ -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 { diff --git a/matrix-sdk-android/src/rustCrypto/java/org/matrix/android/sdk/internal/crypto/RustCryptoService.kt b/matrix-sdk-android/src/rustCrypto/java/org/matrix/android/sdk/internal/crypto/RustCryptoService.kt index 1fa6c2c259..df435d024b 100755 --- a/matrix-sdk-android/src/rustCrypto/java/org/matrix/android/sdk/internal/crypto/RustCryptoService.kt +++ b/matrix-sdk-android/src/rustCrypto/java/org/matrix/android/sdk/internal/crypto/RustCryptoService.kt @@ -344,8 +344,8 @@ internal class RustCryptoService @Inject constructor( return olmMachine.getCryptoDeviceInfo(userId, deviceId) } - override fun getCryptoDeviceInfo(userId: String): List { - return runBlocking { + override suspend fun getCryptoDeviceInfo(userId: String): List { + return withContext(coroutineDispatchers.io) { olmMachine.getCryptoDeviceInfo(userId) } } diff --git a/vector-app/src/gplay/java/im/vector/app/gplay/features/settings/troubleshoot/TestTokenRegistration.kt b/vector-app/src/gplay/java/im/vector/app/gplay/features/settings/troubleshoot/TestTokenRegistration.kt index cafc2d65e6..313073da4d 100644 --- a/vector-app/src/gplay/java/im/vector/app/gplay/features/settings/troubleshoot/TestTokenRegistration.kt +++ b/vector-app/src/gplay/java/im/vector/app/gplay/features/settings/troubleshoot/TestTokenRegistration.kt @@ -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 @@ -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 @@ -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) + } } - } - }) + }) + } } } diff --git a/vector-app/src/gplay/java/im/vector/app/push/fcm/GoogleFcmHelper.kt b/vector-app/src/gplay/java/im/vector/app/push/fcm/GoogleFcmHelper.kt index 53e65f88b4..676cd0258f 100755 --- a/vector-app/src/gplay/java/im/vector/app/push/fcm/GoogleFcmHelper.kt +++ b/vector-app/src/gplay/java/im/vector/app/push/fcm/GoogleFcmHelper.kt @@ -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 @@ -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" } @@ -64,7 +72,9 @@ class GoogleFcmHelper @Inject constructor( .addOnSuccessListener { token -> storeFcmToken(token) if (registerPusher) { - pushersManager.enqueueRegisterPusherWithFcmKey(token) + scope.launch { + pushersManager.enqueueRegisterPusherWithFcmKey(token) + } } } .addOnFailureListener { e -> diff --git a/vector-app/src/gplay/java/im/vector/app/push/fcm/VectorFirebaseMessagingService.kt b/vector-app/src/gplay/java/im/vector/app/push/fcm/VectorFirebaseMessagingService.kt index 7fd55bd165..6ab9b90a84 100644 --- a/vector-app/src/gplay/java/im/vector/app/push/fcm/VectorFirebaseMessagingService.kt +++ b/vector-app/src/gplay/java/im/vector/app/push/fcm/VectorFirebaseMessagingService.kt @@ -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 @@ -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) @@ -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)) + } } } diff --git a/vector/src/main/java/im/vector/app/core/device/GetDeviceInfoUseCase.kt b/vector/src/main/java/im/vector/app/core/device/GetDeviceInfoUseCase.kt index 4a66988b46..a97fa32a61 100644 --- a/vector/src/main/java/im/vector/app/core/device/GetDeviceInfoUseCase.kt +++ b/vector/src/main/java/im/vector/app/core/device/GetDeviceInfoUseCase.kt @@ -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() } } diff --git a/vector/src/main/java/im/vector/app/core/pushers/PushersManager.kt b/vector/src/main/java/im/vector/app/core/pushers/PushersManager.kt index fb78feaef1..402471ecef 100644 --- a/vector/src/main/java/im/vector/app/core/pushers/PushersManager.kt +++ b/vector/src/main/java/im/vector/app/core/pushers/PushersManager.kt @@ -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 { @@ -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( diff --git a/vector/src/main/java/im/vector/app/core/pushers/VectorUnifiedPushMessagingReceiver.kt b/vector/src/main/java/im/vector/app/core/pushers/VectorUnifiedPushMessagingReceiver.kt index 838cbd5935..f0e5cd2c2c 100644 --- a/vector/src/main/java/im/vector/app/core/pushers/VectorUnifiedPushMessagingReceiver.kt +++ b/vector/src/main/java/im/vector/app/core/pushers/VectorUnifiedPushMessagingReceiver.kt @@ -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) + } } } } diff --git a/vector/src/main/java/im/vector/app/features/crypto/verification/self/SelfVerificationController.kt b/vector/src/main/java/im/vector/app/features/crypto/verification/self/SelfVerificationController.kt index 4097cf957b..9871d21601 100644 --- a/vector/src/main/java/im/vector/app/features/crypto/verification/self/SelfVerificationController.kt +++ b/vector/src/main/java/im/vector/app/features/crypto/verification/self/SelfVerificationController.kt @@ -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)) diff --git a/vector/src/main/java/im/vector/app/features/crypto/verification/self/SelfVerificationViewModel.kt b/vector/src/main/java/im/vector/app/features/crypto/verification/self/SelfVerificationViewModel.kt index e92616f7df..ded10467a9 100644 --- a/vector/src/main/java/im/vector/app/features/crypto/verification/self/SelfVerificationViewModel.kt +++ b/vector/src/main/java/im/vector/app/features/crypto/verification/self/SelfVerificationViewModel.kt @@ -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 = Uninitialized, val quadSContainsSecrets: Boolean = false, val isVerificationRequired: Boolean = false, val isThisSessionVerified: Boolean = false, @@ -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) { + 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( diff --git a/vector/src/main/java/im/vector/app/features/home/UnknownDeviceDetectorSharedViewModel.kt b/vector/src/main/java/im/vector/app/features/home/UnknownDeviceDetectorSharedViewModel.kt index 28016f109f..6ba5976eb8 100644 --- a/vector/src/main/java/im/vector/app/features/home/UnknownDeviceDetectorSharedViewModel.kt +++ b/vector/src/main/java/im/vector/app/features/home/UnknownDeviceDetectorSharedViewModel.kt @@ -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), @@ -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 { diff --git a/vector/src/main/java/im/vector/app/features/settings/troubleshoot/TestEndpointAsTokenRegistration.kt b/vector/src/main/java/im/vector/app/features/settings/troubleshoot/TestEndpointAsTokenRegistration.kt index b355b55903..6489fe537d 100644 --- a/vector/src/main/java/im/vector/app/features/settings/troubleshoot/TestEndpointAsTokenRegistration.kt +++ b/vector/src/main/java/im/vector/app/features/settings/troubleshoot/TestEndpointAsTokenRegistration.kt @@ -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 @@ -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, @@ -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) + } } } }