Merge pull request #2468 from vector-im/feature/bma/fix_cancel

Fix cancellation of event sending not always working

CHANGES.md

@@ -10,6 +10,7 @@ Improvements 🙌:
  - Improve room history visibility setting UX (#1579)
 
 Bugfix 🐛:
+ - Fix cancellation of sending event (#2438)
  - Double bottomsheet effect after verify with passphrase
  - EditText cursor jumps to the start while typing fast (#2469)
 

matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/room/send/DefaultSendService.kt

@@ -210,6 +210,8 @@ internal class DefaultSendService @AssistedInject constructor(
 
     override fun cancelSend(eventId: String) {
         cancelSendTracker.markLocalEchoForCancel(eventId, roomId)
+        // This is maybe the current task, so cancel it too
+        eventSenderProcessor.cancel(eventId, roomId)
         taskExecutor.executorScope.launch {
             localEchoRepository.deleteFailedEcho(roomId, eventId)
         }

matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/room/send/queue/EventSenderProcessor.kt

@@ -16,6 +16,7 @@
 
 package org.matrix.android.sdk.internal.session.room.send.queue
 
+import kotlinx.coroutines.CancellationException
 import kotlinx.coroutines.launch
 import kotlinx.coroutines.runBlocking
 import org.matrix.android.sdk.api.auth.data.SessionParams
@@ -106,17 +107,21 @@ internal class EventSenderProcessor @Inject constructor(
         // non blocking add to queue
         sendingQueue.add(task)
         markAsManaged(task)
-        return object : Cancelable {
-            override fun cancel() {
-                task.cancel()
-            }
-        }
+        return task
+    }
+
+    fun cancel(eventId: String, roomId: String) {
+        (currentTask as? SendEventQueuedTask)
+                ?.takeIf { it -> it.event.eventId == eventId && it.event.roomId == roomId }
+                ?.cancel()
     }
 
     companion object {
         private const val RETRY_WAIT_TIME_MS = 10_000L
     }
 
+    private var currentTask: QueuedTask? = null
+
     private var sendingQueue = LinkedBlockingQueue<QueuedTask>()
 
     private var networkAvailableLock = Object()
@@ -129,6 +134,7 @@ internal class EventSenderProcessor @Inject constructor(
             while (!isInterrupted) {
                 Timber.v("## SendThread wait for task to process")
                 val task = sendingQueue.take()
+                        .also { currentTask = it }
                 Timber.v("## SendThread Found task to process $task")
 
                 if (task.isCancelled()) {
@@ -183,6 +189,10 @@ internal class EventSenderProcessor @Inject constructor(
                                     task.onTaskFailed()
                                     throw InterruptedException()
                                 }
+                                exception is CancellationException                                                         -> {
+                                    Timber.v("## SendThread task has been cancelled")
+                                    break@retryLoop
+                                }
                                 else                                                                                       -> {
                                     Timber.v("## SendThread retryLoop Un-Retryable error, try next task")
                                     // this task is in error, check next one?

matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/room/send/queue/QueueMemento.kt

@@ -17,7 +17,6 @@
 package org.matrix.android.sdk.internal.session.room.send.queue
 
 import android.content.Context
-import org.matrix.android.sdk.api.auth.data.sessionId
 import org.matrix.android.sdk.api.extensions.tryOrNull
 import org.matrix.android.sdk.api.session.crypto.CryptoService
 import org.matrix.android.sdk.api.session.room.send.SendState

matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/room/send/queue/QueuedTask.kt

@@ -16,14 +16,26 @@
 
 package org.matrix.android.sdk.internal.session.room.send.queue
 
-abstract class QueuedTask {
+import org.matrix.android.sdk.api.util.Cancelable
+
+abstract class QueuedTask : Cancelable {
     var retryCount = 0
 
-    abstract suspend fun execute()
+    private var hasBeenCancelled: Boolean = false
+
+    suspend fun execute() {
+        if (!isCancelled()) {
+            doExecute()
+        }
+    }
+
+    abstract suspend fun doExecute()
 
     abstract fun onTaskFailed()
 
-    abstract fun isCancelled() : Boolean
+    open fun isCancelled() = hasBeenCancelled
 
-    abstract fun cancel()
+    final override fun cancel() {
+        hasBeenCancelled = true
+    }
 }

matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/room/send/queue/RedactQueuedTask.kt

@@ -22,20 +22,18 @@ import org.matrix.android.sdk.internal.session.room.send.CancelSendTracker
 import org.matrix.android.sdk.internal.session.room.send.LocalEchoRepository
 
 internal class RedactQueuedTask(
-        val toRedactEventId: String,
+        private val toRedactEventId: String,
         val redactionLocalEchoId: String,
-        val roomId: String,
-        val reason: String?,
-        val redactEventTask: RedactEventTask,
-        val localEchoRepository: LocalEchoRepository,
-        val cancelSendTracker: CancelSendTracker
+        private val roomId: String,
+        private val reason: String?,
+        private val redactEventTask: RedactEventTask,
+        private val localEchoRepository: LocalEchoRepository,
+        private val cancelSendTracker: CancelSendTracker
 ) : QueuedTask() {
 
-    private var _isCancelled: Boolean = false
+    override fun toString() = "[RedactQueuedTask $redactionLocalEchoId]"
 
-    override fun toString() = "[RedactEventRunnableTask $redactionLocalEchoId]"
-
-    override suspend fun execute() {
+    override suspend fun doExecute() {
         redactEventTask.execute(RedactEventTask.Params(redactionLocalEchoId, roomId, toRedactEventId, reason))
     }
 
@@ -44,10 +42,6 @@ internal class RedactQueuedTask(
     }
 
     override fun isCancelled(): Boolean {
-        return _isCancelled || cancelSendTracker.isCancelRequestedFor(redactionLocalEchoId, roomId)
-    }
-
-    override fun cancel() {
-        _isCancelled = true
+        return super.isCancelled() || cancelSendTracker.isCancelRequestedFor(redactionLocalEchoId, roomId)
     }
 }

matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/room/send/queue/SendEventQueuedTask.kt

@@ -33,11 +33,9 @@ internal class SendEventQueuedTask(
         val cancelSendTracker: CancelSendTracker
 ) : QueuedTask() {
 
-    private var _isCancelled: Boolean = false
+    override fun toString() = "[SendEventQueuedTask ${event.eventId}]"
 
-    override fun toString() = "[SendEventRunnableTask ${event.eventId}]"
-
-    override suspend fun execute() {
+    override suspend fun doExecute() {
         sendEventTask.execute(SendEventTask.Params(event, encrypt))
     }
 
@@ -56,10 +54,6 @@ internal class SendEventQueuedTask(
     }
 
     override fun isCancelled(): Boolean {
-        return _isCancelled || cancelSendTracker.isCancelRequestedFor(event.eventId, event.roomId)
-    }
-
-    override fun cancel() {
-        _isCancelled = true
+        return super.isCancelled() || cancelSendTracker.isCancelRequestedFor(event.eventId, event.roomId)
     }
 }