From f6cc05634ff3852be95b20a35c783bb1c0e59660 Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Fri, 27 Nov 2020 16:38:46 +0100 Subject: [PATCH 1/2] Send task: small rework and cleanup --- .../room/send/queue/EventSenderProcessor.kt | 6 +---- .../session/room/send/queue/QueueMemento.kt | 1 - .../session/room/send/queue/QueuedTask.kt | 12 +++++++--- .../room/send/queue/RedactQueuedTask.kt | 22 +++++++------------ .../room/send/queue/SendEventQueuedTask.kt | 10 ++------- 5 files changed, 20 insertions(+), 31 deletions(-) diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/room/send/queue/EventSenderProcessor.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/room/send/queue/EventSenderProcessor.kt index 62e225c624..b8f6e52674 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/room/send/queue/EventSenderProcessor.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/room/send/queue/EventSenderProcessor.kt @@ -106,11 +106,7 @@ 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 } companion object { diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/room/send/queue/QueueMemento.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/room/send/queue/QueueMemento.kt index e69c65ec4c..dfbac347d9 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/room/send/queue/QueueMemento.kt +++ b/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 diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/room/send/queue/QueuedTask.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/room/send/queue/QueuedTask.kt index bccbc97ff4..e5c1cf7435 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/room/send/queue/QueuedTask.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/room/send/queue/QueuedTask.kt @@ -16,14 +16,20 @@ 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 + private var hasBeenCancelled: Boolean = false + abstract suspend fun execute() abstract fun onTaskFailed() - abstract fun isCancelled() : Boolean + open fun isCancelled() = hasBeenCancelled - abstract fun cancel() + final override fun cancel() { + hasBeenCancelled = true + } } diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/room/send/queue/RedactQueuedTask.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/room/send/queue/RedactQueuedTask.kt index a3c19a1f7c..e2fb978cd0 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/room/send/queue/RedactQueuedTask.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/room/send/queue/RedactQueuedTask.kt @@ -22,18 +22,16 @@ 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() = "[RedactEventRunnableTask $redactionLocalEchoId]" + override fun toString() = "[RedactQueuedTask $redactionLocalEchoId]" override suspend fun execute() { 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) } } diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/room/send/queue/SendEventQueuedTask.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/room/send/queue/SendEventQueuedTask.kt index 21a4145a9d..f934aad67b 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/room/send/queue/SendEventQueuedTask.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/room/send/queue/SendEventQueuedTask.kt @@ -33,9 +33,7 @@ internal class SendEventQueuedTask( val cancelSendTracker: CancelSendTracker ) : QueuedTask() { - private var _isCancelled: Boolean = false - - override fun toString() = "[SendEventRunnableTask ${event.eventId}]" + override fun toString() = "[SendEventQueuedTask ${event.eventId}]" override suspend fun execute() { 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) } } From cd983de058557ae0bd9902138d80f863dfa78323 Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Mon, 30 Nov 2020 10:08:31 +0100 Subject: [PATCH 2/2] Fix cancellation of sending event (#2438) --- CHANGES.md | 2 +- .../session/room/send/DefaultSendService.kt | 2 ++ .../room/send/queue/EventSenderProcessor.kt | 14 ++++++++++++++ .../internal/session/room/send/queue/QueuedTask.kt | 8 +++++++- .../session/room/send/queue/RedactQueuedTask.kt | 2 +- .../session/room/send/queue/SendEventQueuedTask.kt | 2 +- 6 files changed, 26 insertions(+), 4 deletions(-) diff --git a/CHANGES.md b/CHANGES.md index e48281081b..7f626da497 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -8,7 +8,7 @@ Improvements 🙌: - Bugfix 🐛: - - + - Fix cancellation of sending event (#2438) Translations 🗣: - diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/room/send/DefaultSendService.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/room/send/DefaultSendService.kt index b13ce15da6..5a71ff7b76 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/room/send/DefaultSendService.kt +++ b/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) } diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/room/send/queue/EventSenderProcessor.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/room/send/queue/EventSenderProcessor.kt index b8f6e52674..5014d94558 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/room/send/queue/EventSenderProcessor.kt +++ b/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 @@ -109,10 +110,18 @@ internal class EventSenderProcessor @Inject constructor( 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() private var networkAvailableLock = Object() @@ -125,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()) { @@ -179,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? diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/room/send/queue/QueuedTask.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/room/send/queue/QueuedTask.kt index e5c1cf7435..9a7fcd8d91 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/room/send/queue/QueuedTask.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/room/send/queue/QueuedTask.kt @@ -23,7 +23,13 @@ abstract class QueuedTask : Cancelable { private var hasBeenCancelled: Boolean = false - abstract suspend fun execute() + suspend fun execute() { + if (!isCancelled()) { + doExecute() + } + } + + abstract suspend fun doExecute() abstract fun onTaskFailed() diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/room/send/queue/RedactQueuedTask.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/room/send/queue/RedactQueuedTask.kt index e2fb978cd0..8e7ba2f155 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/room/send/queue/RedactQueuedTask.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/room/send/queue/RedactQueuedTask.kt @@ -33,7 +33,7 @@ internal class RedactQueuedTask( override fun toString() = "[RedactQueuedTask $redactionLocalEchoId]" - override suspend fun execute() { + override suspend fun doExecute() { redactEventTask.execute(RedactEventTask.Params(redactionLocalEchoId, roomId, toRedactEventId, reason)) } diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/room/send/queue/SendEventQueuedTask.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/room/send/queue/SendEventQueuedTask.kt index f934aad67b..ea097082c7 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/room/send/queue/SendEventQueuedTask.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/room/send/queue/SendEventQueuedTask.kt @@ -35,7 +35,7 @@ internal class SendEventQueuedTask( override fun toString() = "[SendEventQueuedTask ${event.eventId}]" - override suspend fun execute() { + override suspend fun doExecute() { sendEventTask.execute(SendEventTask.Params(event, encrypt)) }