fix olm session proliferation

element-hq · Jul 15, 2022 · e8bd7ea · e8bd7ea
1 parent c7b54b8
commit e8bd7ea
Show file tree

Hide file tree

Showing 3 changed files with 30 additions and 14 deletions.
diff --git a/changelog.d/6534.bugfix b/changelog.d/6534.bugfix
@@ -0,0 +1 @@
+Unwedging could cause the SDK to force creating a new olm session every hour
diff --git a/...-sdk-android/src/androidTest/java/org/matrix/android/sdk/internal/crypto/UnwedgingTest.kt b/...-sdk-android/src/androidTest/java/org/matrix/android/sdk/internal/crypto/UnwedgingTest.kt
@@ -60,7 +60,6 @@ import kotlin.coroutines.resume
  */
 @RunWith(AndroidJUnit4::class)
 @FixMethodOrder(MethodSorters.JVM)
-@Ignore
 class UnwedgingTest : InstrumentedTest {
 
     private lateinit var messagesReceivedByBob: List<TimelineEvent>

diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/EventDecryptor.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/EventDecryptor.kt
@@ -18,6 +18,8 @@ package org.matrix.android.sdk.internal.crypto
 
 import kotlinx.coroutines.CoroutineScope
 import kotlinx.coroutines.launch
+import kotlinx.coroutines.sync.Mutex
+import kotlinx.coroutines.sync.withLock
 import kotlinx.coroutines.withContext
 import org.matrix.android.sdk.api.MatrixCallback
 import org.matrix.android.sdk.api.MatrixCoroutineDispatchers
@@ -68,6 +70,7 @@ internal class EventDecryptor @Inject constructor(
             val senderKey: String?
     )
 
+    private val wedgedMutex = Mutex()
     private val wedgedDevices = mutableListOf<WedgedDeviceInfo>()
 
     /**
@@ -151,11 +154,13 @@ internal class EventDecryptor @Inject constructor(
         }
     }
 
-    private fun markOlmSessionForUnwedging(senderId: String, senderKey: String) {
-        val info = WedgedDeviceInfo(senderId, senderKey)
-        if (!wedgedDevices.contains(info)) {
-            Timber.tag(loggerTag.value).d("Marking device from $senderId key:$senderKey as wedged")
-            wedgedDevices.add(info)
+    private suspend fun markOlmSessionForUnwedging(senderId: String, senderKey: String) {
+        wedgedMutex.withLock {
+            val info = WedgedDeviceInfo(senderId, senderKey)
+            if (!wedgedDevices.contains(info)) {
+                Timber.tag(loggerTag.value).d("Marking device from $senderId key:$senderKey as wedged")
+                wedgedDevices.add(info)
+            }
         }
     }
 
@@ -167,15 +172,17 @@ internal class EventDecryptor @Inject constructor(
         Timber.tag(loggerTag.value).v("Unwedging:  ${wedgedDevices.size} are wedged")
         // get the one that should be retried according to rate limit
         val now = clock.epochMillis()
-        val toUnwedge = wedgedDevices.filter {
-            val lastForcedDate = lastNewSessionForcedDates[it] ?: 0
-            if (now - lastForcedDate < DefaultCryptoService.CRYPTO_MIN_FORCE_SESSION_PERIOD_MILLIS) {
-                Timber.tag(loggerTag.value).d("Unwedging, New session for $it already forced with device at $lastForcedDate")
-                return@filter false
+        val toUnwedge = wedgedMutex.withLock {
+            wedgedDevices.filter {
+                val lastForcedDate = lastNewSessionForcedDates[it] ?: 0
+                if (now - lastForcedDate < DefaultCryptoService.CRYPTO_MIN_FORCE_SESSION_PERIOD_MILLIS) {
+                    Timber.tag(loggerTag.value).d("Unwedging, New session for $it already forced with device at $lastForcedDate")
+                    return@filter false
+                }
+                // let's already mark that we tried now
+                lastNewSessionForcedDates[it] = now
+                true
             }
-            // let's already mark that we tried now
-            lastNewSessionForcedDates[it] = now
-            true
         }
 
         if (toUnwedge.isEmpty()) {
@@ -230,6 +237,15 @@ internal class EventDecryptor @Inject constructor(
                                     withContext(coroutineDispatchers.io) {
                                         sendToDeviceTask.executeRetry(sendToDeviceParams, remainingRetry = SEND_TO_DEVICE_RETRY_COUNT)
                                     }
+
+                                    deviceList.values.flatten().forEach { deviceInfo ->
+                                        wedgedMutex.withLock {
+                                            wedgedDevices.removeAll {
+                                                it.senderKey == deviceInfo.identityKey() &&
+                                                        it.userId == deviceInfo.userId
+                                            }
+                                        }
+                                    }
                                 } catch (failure: Throwable) {
                                     deviceList.flatMap { it.value }.joinToString { it.shortDebugString() }.let {
                                         Timber.tag(loggerTag.value).e(failure, "## Failed to unwedge devices: $it}")
Original file line number	Diff line number	Diff line change
		@@ -0,0 +1 @@
		Unwedging could cause the SDK to force creating a new olm session every hour