Merge pull request #8485 from vector-im/feature/bca/fix_anrs

Make cryptoDevice calls suspendable
This commit is contained in:
Valere 2023-06-01 16:36:29 +02:00 committed by GitHub
commit 2f1a7b76ad
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
20 changed files with 98 additions and 53 deletions

1
changelog.d/8482.bugfix Normal file
View file

@ -0,0 +1 @@
fix: Make some crypto calls suspendable to avoid reported ANR

View file

@ -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<CryptoDeviceInfo> {
override suspend fun getCryptoDeviceInfo(userId: String): List<CryptoDeviceInfo> {
return cryptoStore.getUserDeviceList(userId).orEmpty()
}
//

View file

@ -73,7 +73,7 @@ interface CryptoService {
suspend fun getUserDevices(userId: String): List<CryptoDeviceInfo>
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<CryptoDeviceInfo>
suspend fun getCryptoDeviceInfo(userId: String): List<CryptoDeviceInfo>
// fun getCryptoDeviceInfoFlow(userId: String): Flow<List<CryptoDeviceInfo>>

View file

@ -27,7 +27,6 @@ import org.matrix.android.sdk.api.session.events.model.toValidDecryptedEvent
import org.matrix.android.sdk.api.session.room.model.message.MessageContent
import org.matrix.android.sdk.api.session.room.model.message.MessagePollContent
import org.matrix.android.sdk.internal.crypto.store.IMXCommonCryptoStore
import timber.log.Timber
import javax.inject.Inject
internal class EventEditValidator @Inject constructor(val cryptoStore: IMXCommonCryptoStore) {
@ -53,7 +52,6 @@ internal class EventEditValidator @Inject constructor(val cryptoStore: IMXCommon
* If the original event was encrypted, the replacement should be too.
*/
fun validateEdit(originalEvent: Event?, replaceEvent: Event): EditValidity {
Timber.v("###REPLACE valide event $originalEvent replaced $replaceEvent")
// we might not know the original event at that time. In this case we can't perform the validation
// Edits should be revalidated when the original event is received
if (originalEvent == null) {

View file

@ -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 {

View file

@ -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
@ -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> {
@ -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)
}
}

View file

@ -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,6 +63,7 @@ class TestTokenRegistration @Inject constructor(
)
quickFix = object : TroubleshootQuickFix(R.string.settings_troubleshoot_test_token_registration_quick_fix) {
override fun doFix() {
context.lifecycleScope.launch(Dispatchers.IO) {
val workId = pushersManager.enqueueRegisterPusherWithFcmKey(fcmToken)
WorkManager.getInstance(context).getWorkInfoByIdLiveData(workId).observe(context, Observer { workInfo ->
if (workInfo != null) {
@ -72,6 +76,7 @@ class TestTokenRegistration @Inject constructor(
})
}
}
}
status = TestStatus.FAILED
} else {

View file

@ -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,9 +72,11 @@ class GoogleFcmHelper @Inject constructor(
.addOnSuccessListener { token ->
storeFcmToken(token)
if (registerPusher) {
scope.launch {
pushersManager.enqueueRegisterPusherWithFcmKey(token)
}
}
}
.addOnFailureListener { e ->
Timber.e(e, "## ensureFcmTokenIsRetrieved() : failed")
}

View file

@ -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,9 +61,11 @@ class VectorFirebaseMessagingService : FirebaseMessagingService() {
activeSessionHolder.hasActiveSession() &&
unifiedPushHelper.isEmbeddedDistributor()
) {
scope.launch {
pushersManager.enqueueRegisterPusher(token, getString(R.string.pusher_http_url))
}
}
}
override fun onMessageReceived(message: RemoteMessage) {
Timber.tag(loggerTag.value).d("New Firebase message")

View file

@ -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()
}
}

View file

@ -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(

View file

@ -76,10 +76,12 @@ class VectorUnifiedPushMessagingReceiver : MessagingReceiver() {
coroutineScope.launch {
unifiedPushHelper.storeCustomOrDefaultGateway(endpoint) {
unifiedPushHelper.getPushGateway()?.let {
coroutineScope.launch {
pushersManager.enqueueRegisterPusher(endpoint, it)
}
}
}
}
} else {
Timber.tag(loggerTag.value).i("onNewEndpoint: skipped")
}

View file

@ -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))

View file

@ -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,
@ -146,21 +146,28 @@ class SelfVerificationViewModel @AssistedInject constructor(
}
}
setState { copy(hasAnyOtherSession = Loading()) }
viewModelScope.launch {
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(

View file

@ -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 {

View file

@ -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 ->
context.lifecycleScope.launch {
registerUnifiedPush(distributor = selection, testParameters, pushKey)
}
}
}
}

View file

@ -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.test.runTest
import org.amshove.kluent.shouldBeEqualTo
import org.junit.Test
@ -31,7 +32,7 @@ class DefaultGetDeviceInfoUseCaseTest {
private val getDeviceInfoUseCase = DefaultGetDeviceInfoUseCase(activeSessionHolder.instance)
@Test
fun `when execute, then get crypto device info`() {
fun `when execute, then get crypto device info`() = runTest {
val result = getDeviceInfoUseCase.execute()
result shouldBeEqualTo cryptoService.cryptoDeviceInfo

View file

@ -29,6 +29,7 @@ import im.vector.app.test.fixtures.CryptoDeviceInfoFixture.aCryptoDeviceInfo
import im.vector.app.test.fixtures.PusherFixture
import im.vector.app.test.fixtures.SessionParamsFixture
import io.mockk.mockk
import kotlinx.coroutines.test.runTest
import org.amshove.kluent.shouldBeEqualTo
import org.junit.Test
import org.matrix.android.sdk.api.session.crypto.model.UnsignedDeviceInfo
@ -56,7 +57,7 @@ class PushersManagerTest {
)
@Test
fun `when enqueueRegisterPusher, then HttpPusher created and enqueued`() {
fun `when enqueueRegisterPusher, then HttpPusher created and enqueued`() = runTest {
val pushKey = "abc"
val gateway = "123"
val pusherAppId = "app-id"

View file

@ -84,5 +84,5 @@ class FakeCryptoService(
}
}
override fun getMyCryptoDevice() = cryptoDeviceInfo
override suspend fun getMyCryptoDevice() = cryptoDeviceInfo
}

View file

@ -17,13 +17,13 @@
package im.vector.app.test.fakes
import im.vector.app.core.device.GetDeviceInfoUseCase
import io.mockk.every
import io.mockk.coEvery
import io.mockk.mockk
import org.matrix.android.sdk.api.session.crypto.model.CryptoDeviceInfo
class FakeGetDeviceInfoUseCase : GetDeviceInfoUseCase by mockk() {
fun givenDeviceInfo(cryptoDeviceInfo: CryptoDeviceInfo) {
every { execute() } returns cryptoDeviceInfo
coEvery { execute() } returns cryptoDeviceInfo
}
}