From eaf6a9923a7559fffbabf66e44322d4efc04eac5 Mon Sep 17 00:00:00 2001
From: Benoit Marty <benoit.marty@gmail.com>
Date: Thu, 8 Aug 2019 16:03:11 +0200
Subject: [PATCH] Cancel sync request on pause and timeout to 0 after pause
 (#404)

---
 CHANGES.md                                    |  1 +
 .../internal/session/sync/job/SyncThread.kt   | 50 ++++++++++++-------
 2 files changed, 33 insertions(+), 18 deletions(-)

diff --git a/CHANGES.md b/CHANGES.md
index f1eca45d..08da482e 100644
--- a/CHANGES.md
+++ b/CHANGES.md
@@ -12,6 +12,7 @@ Improvements:
  - Enable proper cancellation of suspending functions (including db transaction)
  - Enhances network connectivity checks in SDK
  - Add "View Edit History" item in the message bottom sheet (#401)
+ - Cancel sync request on pause and timeout to 0 after pause (#404)
 
 Other changes:
  - Show sync progress also in room detail screen (#403)
diff --git a/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/session/sync/job/SyncThread.kt b/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/session/sync/job/SyncThread.kt
index 0b9365dc..f6ff11c1 100644
--- a/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/session/sync/job/SyncThread.kt
+++ b/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/session/sync/job/SyncThread.kt
@@ -30,6 +30,7 @@ import im.vector.matrix.android.internal.task.TaskExecutor
 import im.vector.matrix.android.internal.task.TaskThread
 import im.vector.matrix.android.internal.task.configureWith
 import im.vector.matrix.android.internal.util.BackgroundDetectionObserver
+import kotlinx.coroutines.CancellationException
 import timber.log.Timber
 import java.net.SocketTimeoutException
 import java.util.concurrent.CountDownLatch
@@ -70,6 +71,8 @@ internal class SyncThread @Inject constructor(private val syncTask: SyncTask,
         if (state is SyncState.RUNNING) {
             Timber.v("Pause sync...")
             updateStateTo(SyncState.PAUSED)
+            cancelableTask?.cancel()
+            lock.notify()
         }
     }
 
@@ -90,18 +93,25 @@ internal class SyncThread @Inject constructor(private val syncTask: SyncTask,
         backgroundDetectionObserver.register(this)
 
         while (state != SyncState.KILLING) {
+            Timber.v("Entering loop, state: $state")
+
             if (!networkConnectivityChecker.isConnected() || state == SyncState.PAUSED) {
-                Timber.v("Sync is Paused. Waiting...")
+                Timber.v("No network or sync is Paused. Waiting...")
                 synchronized(lock) {
                     lock.wait()
                 }
+                Timber.v("...unlocked")
             } else {
                 if (state !is SyncState.RUNNING) {
                     updateStateTo(SyncState.RUNNING(afterPause = true))
                 }
-                Timber.v("[$this] Execute sync request with timeout $DEFAULT_LONG_POOL_TIMEOUT")
+
+                // No timeout after a pause
+                val timeout = state.let { if (it is SyncState.RUNNING && it.afterPause) 0 else DEFAULT_LONG_POOL_TIMEOUT }
+
+                Timber.v("Execute sync request with timeout $timeout")
                 val latch = CountDownLatch(1)
-                val params = SyncTask.Params(DEFAULT_LONG_POOL_TIMEOUT)
+                val params = SyncTask.Params(timeout)
 
                 cancelableTask = syncTask.configureWith(params) {
                     this.callbackThread = TaskThread.SYNC
@@ -109,29 +119,31 @@ internal class SyncThread @Inject constructor(private val syncTask: SyncTask,
                     this.callback = object : MatrixCallback<Unit> {
 
                         override fun onSuccess(data: Unit) {
+                            Timber.v("onSuccess")
                             latch.countDown()
                         }
 
                         override fun onFailure(failure: Throwable) {
-                            if (failure is Failure.NetworkConnection
-                                    && failure.cause is SocketTimeoutException) {
+                            if (failure is Failure.NetworkConnection && failure.cause is SocketTimeoutException) {
                                 // Timeout are not critical
                                 Timber.v("Timeout")
-                            } else {
-                                Timber.e(failure)
-                            }
-
-                            if (failure !is Failure.NetworkConnection
-                                    || failure.cause is JsonEncodingException) {
-                                // Wait 10s before retrying
-                                sleep(RETRY_WAIT_TIME_MS)
-                            }
-
-                            if (failure is Failure.ServerError
+                            } else if (failure is Failure.Unknown && failure.throwable is CancellationException) {
+                                Timber.v("Cancelled")
+                            } else if (failure is Failure.ServerError
                                     && (failure.error.code == MatrixError.UNKNOWN_TOKEN || failure.error.code == MatrixError.MISSING_TOKEN)) {
                                 // No token or invalid token, stop the thread
+                                Timber.w(failure)
                                 updateStateTo(SyncState.KILLING)
+                            } else {
+                                Timber.e(failure)
+
+                                if (failure !is Failure.NetworkConnection || failure.cause is JsonEncodingException) {
+                                    // Wait 10s before retrying
+                                    Timber.v("Wait 10s")
+                                    sleep(RETRY_WAIT_TIME_MS)
+                                }
                             }
+
                             latch.countDown()
                         }
                     }
@@ -139,8 +151,10 @@ internal class SyncThread @Inject constructor(private val syncTask: SyncTask,
                         .executeBy(taskExecutor)
 
                 latch.await()
-                if (state is SyncState.RUNNING) {
-                    updateStateTo(SyncState.RUNNING(afterPause = false))
+                state.let {
+                    if (it is SyncState.RUNNING && it.afterPause) {
+                        updateStateTo(SyncState.RUNNING(afterPause = false))
+                    }
                 }
 
                 Timber.v("...Continue")