From 16545ce60e9d5a7e22962ba88e69873cd16ef3fb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jorge=20Mart=C3=ADn?= Date: Fri, 28 Jun 2024 11:28:00 +0200 Subject: [PATCH] Fix session verification incorrectly displaying as 'not verified' when the user opened the app with no network connection. It turns out `encryptionService.verificationState()` runs a network request that will cause a deadlock when it fails. Also fixed another deadlock that caused the screen to remain blank sometimes after logging in, because DataStore got stuck when checking the `skipVerification` state for some reason I don't fully understand. --- .../ftue/impl/state/DefaultFtueService.kt | 10 +++- .../RustSessionVerificationService.kt | 47 +++++++++++++++---- 2 files changed, 45 insertions(+), 12 deletions(-) diff --git a/features/ftue/impl/src/main/kotlin/io/element/android/features/ftue/impl/state/DefaultFtueService.kt b/features/ftue/impl/src/main/kotlin/io/element/android/features/ftue/impl/state/DefaultFtueService.kt index cad221f643..049bba4dbb 100644 --- a/features/ftue/impl/src/main/kotlin/io/element/android/features/ftue/impl/state/DefaultFtueService.kt +++ b/features/ftue/impl/src/main/kotlin/io/element/android/features/ftue/impl/state/DefaultFtueService.kt @@ -119,8 +119,14 @@ class DefaultFtueService @Inject constructor( emit(SessionVerifiedStatus.NotVerified) } .first() - val skipVerification = suspend { sessionPreferencesStore.isSessionVerificationSkipped().first() } - return readyVerifiedSessionStatus == SessionVerifiedStatus.NotVerified && !skipVerification() + // For some obscure reason we need to call this *before* we check the `readyVerifiedSessionStatus`, otherwise there's a deadlock + // It seems like a DataStore bug + val skipVerification = canSkipVerification() + return readyVerifiedSessionStatus == SessionVerifiedStatus.NotVerified && !skipVerification + } + + private suspend fun canSkipVerification(): Boolean { + return sessionPreferencesStore.isSessionVerificationSkipped().first() } private suspend fun needsAnalyticsOptIn(): Boolean { diff --git a/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/verification/RustSessionVerificationService.kt b/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/verification/RustSessionVerificationService.kt index 5b6d960d09..95ae68de44 100644 --- a/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/verification/RustSessionVerificationService.kt +++ b/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/verification/RustSessionVerificationService.kt @@ -61,7 +61,7 @@ class RustSessionVerificationService( private val verificationStateListenerTaskHandle = encryptionService.verificationStateListener(object : VerificationStateListener { override fun onUpdate(status: VerificationState) { Timber.d("New verification state: $status") - updateVerificationStatus(status) + sessionCoroutineScope.launch { updateVerificationStatus() } } }) @@ -70,7 +70,7 @@ class RustSessionVerificationService( override fun onUpdate(status: RecoveryState) { Timber.d("New recovery state: $status") // We could check the `RecoveryState`, but it's easier to just use the verification state directly - updateVerificationStatus(encryptionService.verificationState()) + sessionCoroutineScope.launch { updateVerificationStatus() } } }) @@ -92,15 +92,16 @@ class RustSessionVerificationService( init { // Update initial state in case sliding sync isn't ready - updateVerificationStatus(encryptionService.verificationState()) + sessionCoroutineScope.launch { updateVerificationStatus() } isReady.onEach { isReady -> if (isReady) { Timber.d("Starting verification service") // Immediate status update - updateVerificationStatus(encryptionService.verificationState()) + updateVerificationStatus() } else { Timber.d("Stopping verification service") + updateVerificationStatus() } } .launchIn(sessionCoroutineScope) @@ -161,8 +162,9 @@ class RustSessionVerificationService( } } .onSuccess { - updateVerificationStatus(VerificationState.VERIFIED) + // Order here is important, first set the flow state as finished, then update the verification status _verificationFlowState.value = VerificationFlowState.Finished + updateVerificationStatus() } .onFailure { Timber.e(it, "Verification finished, but the Rust SDK still reports the session as unverified.") @@ -200,11 +202,36 @@ class RustSessionVerificationService( } } - private fun updateVerificationStatus(verificationState: VerificationState) { - _sessionVerifiedStatus.value = when (verificationState) { - VerificationState.UNKNOWN -> SessionVerifiedStatus.Unknown - VerificationState.VERIFIED -> SessionVerifiedStatus.Verified - VerificationState.UNVERIFIED -> SessionVerifiedStatus.NotVerified + private suspend fun updateVerificationStatus() { + if (verificationFlowState.value == VerificationFlowState.Finished) { + // Calling `encryptionService.verificationState()` performs a network call and it will deadlock if there is no network + // So we need to check that *only* if we know there is network connection, which is the case when the verification flow just finished + Timber.d("Updating verification status: flow just finished") + runCatching { + encryptionService.waitForE2eeInitializationTasks() + }.onSuccess { + _sessionVerifiedStatus.value = when (encryptionService.verificationState()) { + VerificationState.UNKNOWN -> SessionVerifiedStatus.Unknown + VerificationState.VERIFIED -> SessionVerifiedStatus.Verified + VerificationState.UNVERIFIED -> SessionVerifiedStatus.NotVerified + } + Timber.d("New verification status: ${_sessionVerifiedStatus.value}") + } + } else { + // Otherwise, just check the current verification status from the session verification controller instead + Timber.d("Updating verification status: flow is pending or was finished some time ago") + runCatching { + if (!this@RustSessionVerificationService::verificationController.isInitialized) { + verificationController = client.getSessionVerificationController() + verificationController.setDelegate(this@RustSessionVerificationService) + } + _sessionVerifiedStatus.value = if (verificationController.isVerified()) { + SessionVerifiedStatus.Verified + } else { + SessionVerifiedStatus.NotVerified + } + Timber.d("New verification status: ${_sessionVerifiedStatus.value}") + } } } }