From 91da7fece9d5ddff5bb942966821c58f80e82e8c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jorge=20Mart=C3=ADn?= Date: Thu, 27 Feb 2025 16:00:08 +0100 Subject: [PATCH] Restore manual `Client` cleanup on session logout This is done to fix an issue where the Nodes that contain these dependencies are leaked and the `Client` and other SDK-related components can keep working in background for a while, until the caches holding the Nodes are flushed or the app is restarted. --- .../matrix/impl/RustClientSessionDelegate.kt | 16 ++++++++++++---- .../libraries/matrix/impl/RustMatrixClient.kt | 7 +++++++ .../impl/auth/RustMatrixAuthenticationService.kt | 12 ++++++++++++ .../impl/encryption/RustEncryptionService.kt | 8 ++++++-- .../impl/notification/RustNotificationService.kt | 4 ++++ 5 files changed, 41 insertions(+), 6 deletions(-) diff --git a/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/RustClientSessionDelegate.kt b/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/RustClientSessionDelegate.kt index cb6d5aa24ee..401798b4b1d 100644 --- a/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/RustClientSessionDelegate.kt +++ b/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/RustClientSessionDelegate.kt @@ -19,6 +19,7 @@ import org.matrix.rustcomponents.sdk.ClientDelegate import org.matrix.rustcomponents.sdk.ClientSessionDelegate import org.matrix.rustcomponents.sdk.Session import timber.log.Timber +import java.lang.ref.WeakReference import java.util.concurrent.atomic.AtomicBoolean /** @@ -43,13 +44,20 @@ class RustClientSessionDelegate( private val updateTokensDispatcher = coroutineDispatchers.io.limitedParallelism(1) // This Client needs to be set up as soon as possible so `didReceiveAuthError` can work properly. - private var client: RustMatrixClient? = null + private var client: WeakReference = WeakReference(null) /** * Sets the [ClientDelegate] for the [RustMatrixClient], and keeps a reference to the client so it can be used later. */ fun bindClient(client: RustMatrixClient) { - this.client = client + this.client = WeakReference(client) + } + + /** + * Clears the current client reference. + */ + fun clearCurrentClient() { + this.client.clear() } override fun saveSessionInKeychain(session: Session) { @@ -81,7 +89,7 @@ class RustClientSessionDelegate( clientLog.v("didReceiveAuthError -> do the cleanup") // TODO handle isSoftLogout parameter. appCoroutineScope.launch(updateTokensDispatcher) { - val currentClient = client + val currentClient = client.get() if (currentClient == null) { clientLog.w("didReceiveAuthError -> no client, exiting") isLoggingOut.set(false) @@ -101,7 +109,7 @@ class RustClientSessionDelegate( } else { clientLog.d("No session data found.") } - client?.logout(userInitiated = false, ignoreSdkError = true) + currentClient.logout(userInitiated = false, ignoreSdkError = true) }.invokeOnCompletion { if (it != null) { clientLog.e(it, "Failed to remove session data.") diff --git a/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/RustMatrixClient.kt b/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/RustMatrixClient.kt index 9ba2655c768..d65403c0361 100644 --- a/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/RustMatrixClient.kt +++ b/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/RustMatrixClient.kt @@ -482,6 +482,13 @@ class RustMatrixClient( clientDelegateTaskHandle?.cancelAndDestroy() notificationSettingsService.destroy() verificationService.destroy() + + sessionDelegate.clearCurrentClient() + innerRoomListService.destroy() + notificationService.destroy() + notificationProcessSetup.destroy() + encryptionService.destroy() + innerClient.destroy() } override suspend fun getCacheSize(): Long { diff --git a/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/auth/RustMatrixAuthenticationService.kt b/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/auth/RustMatrixAuthenticationService.kt index 0a4b1364926..725627104c9 100644 --- a/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/auth/RustMatrixAuthenticationService.kt +++ b/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/auth/RustMatrixAuthenticationService.kt @@ -156,6 +156,10 @@ class RustMatrixAuthenticationService @Inject constructor( ) newMatrixClientObserver?.invoke(rustMatrixClientFactory.create(client)) sessionStore.storeData(sessionData) + + // Clean up the strong reference held here since it's no longer necessary + currentClient = null + SessionId(sessionData.userId) }.mapFailure { failure -> failure.mapAuthenticationException() @@ -229,6 +233,10 @@ class RustMatrixAuthenticationService @Inject constructor( pendingOidcAuthorizationData = null newMatrixClientObserver?.invoke(rustMatrixClientFactory.create(client)) sessionStore.storeData(sessionData) + + // Clean up the strong reference held here since it's no longer necessary + currentClient = null + SessionId(sessionData.userId) }.mapFailure { failure -> failure.mapAuthenticationException() @@ -264,6 +272,10 @@ class RustMatrixAuthenticationService @Inject constructor( ) newMatrixClientObserver?.invoke(rustMatrixClientFactory.create(client)) sessionStore.storeData(sessionData) + + // Clean up the strong reference held here since it's no longer necessary + currentClient = null + SessionId(sessionData.userId) }.mapFailure { when (it) { diff --git a/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/encryption/RustEncryptionService.kt b/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/encryption/RustEncryptionService.kt index 2730d93c878..7864c8c94fa 100644 --- a/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/encryption/RustEncryptionService.kt +++ b/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/encryption/RustEncryptionService.kt @@ -186,11 +186,11 @@ internal class RustEncryptionService( } override suspend fun deviceCurve25519(): String? { - return service.curve25519Key() + return runCatching { service.curve25519Key() }.getOrNull() } override suspend fun deviceEd25519(): String? { - return service.ed25519Key() + return runCatching { service.ed25519Key() }.getOrNull() } override suspend fun startIdentityReset(): Result { @@ -219,4 +219,8 @@ internal class RustEncryptionService( // requestFromHomeserverIfNeeded = true, ) ?: error("User identity not found") } + + fun destroy() { + service.destroy() + } } diff --git a/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/notification/RustNotificationService.kt b/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/notification/RustNotificationService.kt index 2e817900bdd..7b8e2f46d84 100644 --- a/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/notification/RustNotificationService.kt +++ b/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/notification/RustNotificationService.kt @@ -35,4 +35,8 @@ class RustNotificationService( } } } + + fun destroy() { + notificationClient.destroy() + } }