From ba540eb861bdaf85816a1fa7436950e12cb9165e Mon Sep 17 00:00:00 2001 From: ganfra Date: Fri, 15 Apr 2022 11:17:06 +0200 Subject: [PATCH] Continue removing runBlocking + some cleanup --- .../sdk/api/session/crypto/CryptoService.kt | 9 +- .../internal/crypto/DefaultCryptoService.kt | 104 ++++++++---------- .../internal/crypto/tasks/EncryptEventTask.kt | 75 ++++++------- .../sdk/internal/session/room/DefaultRoom.kt | 5 +- .../VerificationBottomSheetViewModel.kt | 51 +++------ 5 files changed, 99 insertions(+), 145 deletions(-) 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 76e691d1be..671128f268 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 @@ -66,7 +66,7 @@ interface CryptoService { suspend fun getUserDevices(userId: String): MutableList - fun getMyDevice(): CryptoDeviceInfo + suspend fun getMyDevice(): CryptoDeviceInfo fun getGlobalBlacklistUnverifiedDevices(): Boolean @@ -104,10 +104,9 @@ interface CryptoService { fun isRoomEncrypted(roomId: String): Boolean - fun encryptEventContent(eventContent: Content, + suspend fun encryptEventContent(eventContent: Content, eventType: String, - roomId: String, - callback: MatrixCallback) + roomId: String): MXEncryptEventContentResult fun discardOutboundSession(roomId: String) @@ -151,7 +150,7 @@ interface CryptoService { * Perform any background tasks that can be done before a message is ready to * send, in order to speed up sending of the message. */ - fun prepareToEncrypt(roomId: String, callback: MatrixCallback) + suspend fun prepareToEncrypt(roomId: String) /** * When LL all room members might not be loaded when setting up encryption. diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/DefaultCryptoService.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/DefaultCryptoService.kt index 6523839cdd..6abcd0f8a5 100755 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/DefaultCryptoService.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/DefaultCryptoService.kt @@ -22,13 +22,13 @@ import androidx.lifecycle.LiveData import androidx.paging.PagedList import kotlinx.coroutines.CancellationException import kotlinx.coroutines.CoroutineScope +import kotlinx.coroutines.NonCancellable import kotlinx.coroutines.async import kotlinx.coroutines.cancelChildren import kotlinx.coroutines.coroutineScope import kotlinx.coroutines.flow.Flow import kotlinx.coroutines.joinAll import kotlinx.coroutines.launch -import kotlinx.coroutines.runBlocking import kotlinx.coroutines.sync.Mutex import kotlinx.coroutines.sync.withLock import kotlinx.coroutines.withContext @@ -76,7 +76,6 @@ import org.matrix.android.sdk.internal.crypto.tasks.SetDeviceNameTask import org.matrix.android.sdk.internal.crypto.verification.RustVerificationService import org.matrix.android.sdk.internal.di.DeviceId import org.matrix.android.sdk.internal.di.UserId -import org.matrix.android.sdk.internal.extensions.foldToCallback import org.matrix.android.sdk.internal.session.SessionScope import org.matrix.android.sdk.internal.session.StreamEventsManager import org.matrix.android.sdk.internal.session.room.membership.LoadRoomMembersTask @@ -219,8 +218,8 @@ internal class DefaultCryptoService @Inject constructor( return if (longFormat) "Rust SDK 0.3" else "0.3" } - override fun getMyDevice(): CryptoDeviceInfo { - return runBlocking { olmMachine.ownDevice() } + override suspend fun getMyDevice(): CryptoDeviceInfo { + return olmMachine.ownDevice() } override suspend fun fetchDevicesList(): List { @@ -344,9 +343,13 @@ internal class DefaultCryptoService @Inject constructor( /** * Close the crypto */ - fun close() = runBlocking(coroutineDispatchers.crypto) { + fun close() { cryptoCoroutineScope.coroutineContext.cancelChildren(CancellationException("Closing crypto module")) - cryptoStore.close() + cryptoCoroutineScope.launch { + withContext(coroutineDispatchers.crypto + NonCancellable) { + cryptoStore.close() + } + } } // Always enabled on Matrix Android SDK2 @@ -513,34 +516,29 @@ internal class DefaultCryptoService @Inject constructor( * @param eventContent the content of the event. * @param eventType the type of the event. * @param roomId the room identifier the event will be sent. - * @param callback the asynchronous callback */ - override fun encryptEventContent(eventContent: Content, - eventType: String, - roomId: String, - callback: MatrixCallback) { + override suspend fun encryptEventContent(eventContent: Content, + eventType: String, + roomId: String): MXEncryptEventContentResult { // moved to crypto scope to have up to date values - cryptoCoroutineScope.launch(coroutineDispatchers.crypto) { + return withContext(coroutineDispatchers.crypto) { val algorithm = getEncryptionAlgorithm(roomId) - if (algorithm != null) { val userIds = getRoomUserIds(roomId) val t0 = System.currentTimeMillis() Timber.tag(loggerTag.value).v("encryptEventContent() starts") - runCatching { - measureTimeMillis { - preshareRoomKey(roomId, userIds) - }.also { - Timber.d("Shared room key in room $roomId took $it ms") - } - val content = encrypt(roomId, eventType, eventContent) - Timber.tag(loggerTag.value).v("## CRYPTO | encryptEventContent() : succeeds after ${System.currentTimeMillis() - t0} ms") - MXEncryptEventContentResult(content, EventType.ENCRYPTED) - }.foldToCallback(callback) + measureTimeMillis { + preshareRoomKey(roomId, userIds) + }.also { + Timber.d("Shared room key in room $roomId took $it ms") + } + val content = encrypt(roomId, eventType, eventContent) + Timber.tag(loggerTag.value).v("## CRYPTO | encryptEventContent() : succeeds after ${System.currentTimeMillis() - t0} ms") + MXEncryptEventContentResult(content, EventType.ENCRYPTED) } else { val reason = String.format(MXCryptoError.UNABLE_TO_ENCRYPT_REASON, MXCryptoError.NO_MORE_ALGORITHM_REASON) Timber.tag(loggerTag.value).e("encryptEventContent() : failed $reason") - callback.onFailure(Failure.CryptoError(MXCryptoError.Base(MXCryptoError.ErrorType.UNABLE_TO_ENCRYPT, reason))) + throw Failure.CryptoError(MXCryptoError.Base(MXCryptoError.ErrorType.UNABLE_TO_ENCRYPT, reason)) } } } @@ -707,26 +705,12 @@ internal class DefaultCryptoService @Inject constructor( } private suspend fun preshareRoomKey(roomId: String, roomMembers: List) { - keyClaimLock.withLock { - val request = this.olmMachine.getMissingSessions(roomMembers) - // This request can only be a keys claim request. - if (request != null) { - when (request) { - is Request.KeysClaim -> { - claimKeys(request) - } - else -> { - } - } - } - } - - val keyShareLock = roomKeyShareLocks.getOrPut(roomId, { Mutex() }) + claimMissingKeys(roomMembers) + val keyShareLock = roomKeyShareLocks.getOrPut(roomId) { Mutex() } var sharedKey = false - keyShareLock.withLock { coroutineScope { - this@DefaultCryptoService.olmMachine.shareRoomKey(roomId, roomMembers).map { + olmMachine.shareRoomKey(roomId, roomMembers).map { when (it) { is Request.ToDevice -> { sharedKey = true @@ -752,6 +736,18 @@ internal class DefaultCryptoService @Inject constructor( } } + private suspend fun claimMissingKeys(roomMembers: List) = keyClaimLock.withLock { + val request = this.olmMachine.getMissingSessions(roomMembers) + // This request can only be a keys claim request. + when (request) { + is Request.KeysClaim -> { + claimKeys(request) + } + else -> { + } + } + } + private suspend fun encrypt(roomId: String, eventType: String, content: Content): Content { return olmMachine.encrypt(roomId, eventType, content) } @@ -810,7 +806,7 @@ internal class DefaultCryptoService @Inject constructor( } } - private suspend fun sendRoomMessage(request: Request.RoomMessage){ + private suspend fun sendRoomMessage(request: Request.RoomMessage) { try { requestSender.sendRoomMessage(request) } catch (throwable: Throwable) { @@ -1083,18 +1079,16 @@ internal class DefaultCryptoService @Inject constructor( cryptoStore.logDbUsageInfo() } - override fun prepareToEncrypt(roomId: String, callback: MatrixCallback) { - cryptoCoroutineScope.launch(coroutineDispatchers.crypto) { + override suspend fun prepareToEncrypt(roomId: String) { + withContext(coroutineDispatchers.crypto) { Timber.tag(loggerTag.value).d("prepareToEncrypt() roomId:$roomId Check room members up to date") // Ensure to load all room members try { loadRoomMembersTask.execute(LoadRoomMembersTask.Params(roomId)) } catch (failure: Throwable) { Timber.tag(loggerTag.value).e("prepareToEncrypt() : Failed to load room members") - callback.onFailure(failure) - return@launch + throw failure } - val userIds = getRoomUserIds(roomId) val algorithm = getEncryptionAlgorithm(roomId) @@ -1102,19 +1096,13 @@ internal class DefaultCryptoService @Inject constructor( if (algorithm == null) { val reason = String.format(MXCryptoError.UNABLE_TO_ENCRYPT_REASON, MXCryptoError.NO_MORE_ALGORITHM_REASON) Timber.tag(loggerTag.value).e("prepareToEncrypt() : $reason") - callback.onFailure(IllegalArgumentException("Missing algorithm")) - return@launch + throw IllegalArgumentException("Missing algorithm") } - - runCatching { + try { preshareRoomKey(roomId, userIds) - }.fold( - { callback.onSuccess(Unit) }, - { - Timber.tag(loggerTag.value).e(it, "prepareToEncrypt() failed.") - callback.onFailure(it) - } - ) + }catch (failure: Throwable){ + Timber.tag(loggerTag.value).e("prepareToEncrypt() : Failed to PreshareRoomKey") + } } } diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/tasks/EncryptEventTask.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/tasks/EncryptEventTask.kt index 627352f568..ac49102a07 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/tasks/EncryptEventTask.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/tasks/EncryptEventTask.kt @@ -22,11 +22,9 @@ import org.matrix.android.sdk.api.session.events.model.toContent import org.matrix.android.sdk.api.session.room.send.SendState import org.matrix.android.sdk.internal.crypto.MXCRYPTO_ALGORITHM_MEGOLM import org.matrix.android.sdk.internal.crypto.MXEventDecryptionResult -import org.matrix.android.sdk.internal.crypto.model.MXEncryptEventContentResult import org.matrix.android.sdk.internal.database.mapper.ContentMapper import org.matrix.android.sdk.internal.session.room.send.LocalEchoRepository import org.matrix.android.sdk.internal.task.Task -import org.matrix.android.sdk.internal.util.awaitCallback import javax.inject.Inject internal interface EncryptEventTask : Task { @@ -56,47 +54,44 @@ internal class DefaultEncryptEventTask @Inject constructor( localMutableContent.remove(it) } -// try { // let it throws - awaitCallback { - cryptoService.encryptEventContent(localMutableContent, localEvent.type, params.roomId, it) - }.let { result -> - val modifiedContent = HashMap(result.eventContent) - params.keepKeys?.forEach { toKeep -> - localEvent.content?.get(toKeep)?.let { - // put it back in the encrypted thing - modifiedContent[toKeep] = it - } - } - val safeResult = result.copy(eventContent = modifiedContent) - // Better handling of local echo, to avoid decrypting transition on remote echo - // Should I only do it for text messages? - val decryptionLocalEcho = if (result.eventContent["algorithm"] == MXCRYPTO_ALGORITHM_MEGOLM) { - MXEventDecryptionResult( - clearEvent = Event( - type = localEvent.type, - content = localEvent.content, - roomId = localEvent.roomId - ).toContent(), - forwardingCurve25519KeyChain = emptyList(), - senderCurve25519Key = result.eventContent["sender_key"] as? String, - claimedEd25519Key = cryptoService.getMyDevice().fingerprint() - ) - } else { - null - } + val result = cryptoService.encryptEventContent(localMutableContent, localEvent.type, params.roomId) - localEchoRepository.updateEcho(localEvent.eventId) { _, localEcho -> - localEcho.type = EventType.ENCRYPTED - localEcho.content = ContentMapper.map(modifiedContent) - decryptionLocalEcho?.also { - localEcho.setDecryptionResult(it) - } + val modifiedContent = HashMap(result.eventContent) + params.keepKeys?.forEach { toKeep -> + localEvent.content?.get(toKeep)?.let { + // put it back in the encrypted thing + modifiedContent[toKeep] = it } - return localEvent.copy( - type = safeResult.eventType, - content = safeResult.eventContent - ) } + val safeResult = result.copy(eventContent = modifiedContent) + // Better handling of local echo, to avoid decrypting transition on remote echo + // Should I only do it for text messages? + val decryptionLocalEcho = if (result.eventContent["algorithm"] == MXCRYPTO_ALGORITHM_MEGOLM) { + MXEventDecryptionResult( + clearEvent = Event( + type = localEvent.type, + content = localEvent.content, + roomId = localEvent.roomId + ).toContent(), + forwardingCurve25519KeyChain = emptyList(), + senderCurve25519Key = result.eventContent["sender_key"] as? String, + claimedEd25519Key = cryptoService.getMyDevice().fingerprint() + ) + } else { + null + } + + localEchoRepository.updateEcho(localEvent.eventId) { _, localEcho -> + localEcho.type = EventType.ENCRYPTED + localEcho.content = ContentMapper.map(modifiedContent) + decryptionLocalEcho?.also { + localEcho.setDecryptionResult(it) + } + } + return localEvent.copy( + type = safeResult.eventType, + content = safeResult.eventContent + ) } } diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/room/DefaultRoom.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/room/DefaultRoom.kt index 2d8c3e9c78..a0c3ac23b3 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/room/DefaultRoom.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/room/DefaultRoom.kt @@ -49,7 +49,6 @@ import org.matrix.android.sdk.internal.session.room.state.SendStateTask import org.matrix.android.sdk.internal.session.room.summary.RoomSummaryDataSource import org.matrix.android.sdk.internal.session.search.SearchTask import org.matrix.android.sdk.internal.session.space.DefaultSpace -import org.matrix.android.sdk.internal.util.awaitCallback import java.security.InvalidParameterException internal class DefaultRoom(override val roomId: String, @@ -117,9 +116,7 @@ internal class DefaultRoom(override val roomId: String, } override suspend fun prepareToEncrypt() { - awaitCallback { - cryptoService.prepareToEncrypt(roomId, it) - } + cryptoService.prepareToEncrypt(roomId) } override suspend fun enableEncryption(algorithm: String, force: Boolean) { diff --git a/vector/src/main/java/im/vector/app/features/crypto/verification/VerificationBottomSheetViewModel.kt b/vector/src/main/java/im/vector/app/features/crypto/verification/VerificationBottomSheetViewModel.kt index f3a272c3e8..5fc752028f 100644 --- a/vector/src/main/java/im/vector/app/features/crypto/verification/VerificationBottomSheetViewModel.kt +++ b/vector/src/main/java/im/vector/app/features/crypto/verification/VerificationBottomSheetViewModel.kt @@ -16,8 +16,6 @@ package im.vector.app.features.crypto.verification import com.airbnb.mvrx.Async -import com.airbnb.mvrx.Fail -import com.airbnb.mvrx.Loading import com.airbnb.mvrx.MavericksState import com.airbnb.mvrx.MavericksViewModelFactory import com.airbnb.mvrx.Success @@ -358,44 +356,21 @@ class VerificationBottomSheetViewModel @AssistedInject constructor( private fun handleRequestVerificationByDM(roomId: String?, otherUserId: String) { viewModelScope.launch { - if (roomId == null) { - val localId = LocalEcho.createLocalEchoId() - setState { - copy( - pendingLocalId = localId, - pendingRequest = Loading() - ) - } - try { - val dmRoomId = session.createDirectRoom(otherUserId) - val pendingRequest = session - .cryptoService() - .verificationService() - .requestKeyVerificationInDMs( - supportedVerificationMethodsProvider.provide(), - otherUserId, - dmRoomId, - localId - ) - setState { - copy( - roomId = dmRoomId, - pendingRequest = Success(pendingRequest) - ) - } - } catch (failure: Throwable) { - setState { - copy(pendingRequest = Fail(failure)) - } - } - } else { - val pendingRequest = session + val localId = LocalEcho.createLocalEchoId() + val dmRoomId = roomId ?: session.createDirectRoom(otherUserId) + setState { copy(pendingLocalId = localId, roomId = dmRoomId) } + suspend { + session .cryptoService() .verificationService() - .requestKeyVerificationInDMs(supportedVerificationMethodsProvider.provide(), otherUserId, roomId) - setState { - copy(pendingRequest = Success(pendingRequest)) - } + .requestKeyVerificationInDMs( + supportedVerificationMethodsProvider.provide(), + otherUserId, + dmRoomId, + localId + ) + }.execute { + copy(pendingRequest = it, roomId = dmRoomId) } } }