Try mitigating unexpected logouts (#2251)

* Try mitigating unexpected logouts.

Try making getting/storing session data use a Mutex for synchronization.

Also added some more logs so we can understand exactly where it's failing.

changelog.d/+try-mitigating-unexpected-logouts.misc

@@ -0,0 +1,3 @@
+Try mitigating unexpected logouts by making getting/storing session data use a Mutex for synchronization.
+
+Also added some more logs so we can understand exactly where it's failing.

libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/RustMatrixClient.kt

@@ -139,6 +139,8 @@ class RustMatrixClient(
                 // TODO handle isSoftLogout parameter.
                 appCoroutineScope.launch {
                     val existingData = sessionStore.getSession(client.userId())
+                    val anonymizedToken = existingData?.accessToken?.takeLast(4)
+                    Timber.d("Removing session data with token: '...$anonymizedToken'.")
                     if (existingData != null) {
                         // Set isTokenValid to false
                         val newData = client.session().toSessionData(
@@ -146,8 +148,15 @@ class RustMatrixClient(
                             loginType = existingData.loginType,
                         )
                         sessionStore.updateData(newData)
+                        Timber.d("Removed session data with token: '...$anonymizedToken'.")
+                    } else {
+                        Timber.d("No session data found.")
                     }
                     doLogout(doRequest = false, removeSession = false, ignoreSdkError = false)
+                }.invokeOnCompletion {
+                    if (it != null) {
+                        Timber.e(it, "Failed to remove session data.")
+                    }
                 }
             } else {
                 Timber.v("didReceiveAuthError -> already cleaning up")
@@ -158,11 +167,18 @@ class RustMatrixClient(
             Timber.w("didRefreshTokens()")
             appCoroutineScope.launch {
                 val existingData = sessionStore.getSession(client.userId()) ?: return@launch
+                val anonymizedToken = client.session().accessToken.takeLast(4)
+                Timber.d("Saving new session data with token: '...$anonymizedToken'. Was token valid: ${existingData.isTokenValid}")
                 val newData = client.session().toSessionData(
-                    isTokenValid = existingData.isTokenValid,
+                    isTokenValid = true,
                     loginType = existingData.loginType,
                 )
                 sessionStore.updateData(newData)
+                Timber.d("Saved new session data with token: '...$anonymizedToken'.")
+            }.invokeOnCompletion {
+                if (it != null) {
+                    Timber.e(it, "Failed to save new session data.")
+                }
             }
         }
     }

libraries/session-storage/impl/src/main/kotlin/io/element/android/libraries/sessionstorage/impl/DatabaseSessionStore.kt

@@ -28,6 +28,8 @@ import io.element.android.libraries.sessionstorage.api.SessionData
 import io.element.android.libraries.sessionstorage.api.SessionStore
 import kotlinx.coroutines.flow.Flow
 import kotlinx.coroutines.flow.map
+import kotlinx.coroutines.sync.Mutex
+import kotlinx.coroutines.sync.withLock
 import timber.log.Timber
 import javax.inject.Inject
 
@@ -37,6 +39,8 @@ class DatabaseSessionStore @Inject constructor(
     private val database: SessionDatabase,
     private val dispatchers: CoroutineDispatchers,
 ) : SessionStore {
+    private val sessionDataMutex = Mutex()
+
     override fun isLoggedIn(): Flow<LoggedInState> {
         return database.sessionDataQueries.selectFirst()
             .asFlow()
@@ -53,11 +57,11 @@ class DatabaseSessionStore @Inject constructor(
             }
     }
 
-    override suspend fun storeData(sessionData: SessionData) {
+    override suspend fun storeData(sessionData: SessionData) = sessionDataMutex.withLock {
         database.sessionDataQueries.insertSessionData(sessionData.toDbModel())
     }
 
-    override suspend fun updateData(sessionData: SessionData) {
+    override suspend fun updateData(sessionData: SessionData) = sessionDataMutex.withLock {
         val result = database.sessionDataQueries.selectByUserId(sessionData.userId)
             .executeAsOneOrNull()
             ?.toApiModel()
@@ -66,8 +70,7 @@ class DatabaseSessionStore @Inject constructor(
             Timber.e("User ${sessionData.userId} not found in session database")
             return
         }
-
+            // Copy new data from SDK, but keep login timestamp
-        // Copy new data from SDK, but keep login timestamp
         database.sessionDataQueries.updateSession(
             sessionData.copy(
                 loginTimestamp = result.loginTimestamp,
@@ -76,21 +79,27 @@ class DatabaseSessionStore @Inject constructor(
     }
 
     override suspend fun getLatestSession(): SessionData? {
-        return database.sessionDataQueries.selectFirst()
+        return sessionDataMutex.withLock {
-            .executeAsOneOrNull()
+            database.sessionDataQueries.selectFirst()
-            ?.toApiModel()
+                .executeAsOneOrNull()
+                ?.toApiModel()
+        }
     }
 
     override suspend fun getSession(sessionId: String): SessionData? {
-        return database.sessionDataQueries.selectByUserId(sessionId)
+        return sessionDataMutex.withLock {
-            .executeAsOneOrNull()
+            database.sessionDataQueries.selectByUserId(sessionId)
-            ?.toApiModel()
+                .executeAsOneOrNull()
+                ?.toApiModel()
+        }
     }
 
     override suspend fun getAllSessions(): List<SessionData> {
-        return database.sessionDataQueries.selectAll()
+        return sessionDataMutex.withLock {
-            .executeAsList()
+            database.sessionDataQueries.selectAll()
-            .map { it.toApiModel() }
+                .executeAsList()
+                .map { it.toApiModel() }
+        }
     }
 
     override fun sessionsFlow(): Flow<List<SessionData>> {
@@ -102,6 +111,8 @@ class DatabaseSessionStore @Inject constructor(
     }
 
     override suspend fun removeSession(sessionId: String) {
-        database.sessionDataQueries.removeSession(sessionId)
+        sessionDataMutex.withLock {
+            database.sessionDataQueries.removeSession(sessionId)
+        }
     }
 }