From d91ed2985d2a0b7c5b03ef0834d4c6ad0cf9c4be Mon Sep 17 00:00:00 2001 From: ganfra Date: Fri, 8 Jan 2021 12:40:20 +0100 Subject: [PATCH] Sync: fix initial sync is not retried correctly when there is some network error. [#2632] --- CHANGES.md | 2 +- .../internal/session/sync/job/SyncService.kt | 24 ++++--- .../app/core/services/VectorSyncService.kt | 71 ++++++++----------- 3 files changed, 44 insertions(+), 53 deletions(-) diff --git a/CHANGES.md b/CHANGES.md index bff1a7480e..8a69aea6fd 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -15,7 +15,7 @@ Bugfix 🐛: - Room Topic not displayed correctly after visiting a link (#2551) - Hiding membership events works the exact opposite (#2603) - Tapping drawer having more than 1 room in notifications gives "malformed link" error (#2605) - + - Initial sync is not retried correctly when there is some network error. (#2632) Translations 🗣: - diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/sync/job/SyncService.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/sync/job/SyncService.kt index 6d100a71f9..9d854229df 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/sync/job/SyncService.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/sync/job/SyncService.kt @@ -36,6 +36,7 @@ import org.matrix.android.sdk.internal.task.TaskExecutor import org.matrix.android.sdk.internal.util.BackgroundDetectionObserver import org.matrix.android.sdk.internal.util.MatrixCoroutineDispatchers import timber.log.Timber +import java.net.SocketTimeoutException import java.util.concurrent.atomic.AtomicBoolean /** @@ -68,14 +69,12 @@ abstract class SyncService : Service() { override fun onStartCommand(intent: Intent?, flags: Int, startId: Int): Int { Timber.i("## Sync: onStartCommand [$this] $intent with action: ${intent?.action}") - - // We should start we have to ensure we fulfill contract to show notification - // for foreground service (as per design for this service) - // TODO can we check if it's really in foreground - onStart(isInitialSync) when (intent?.action) { ACTION_STOP -> { Timber.i("## Sync: stop command received") + // We should start we have to ensure we fulfill contract to show notification + // for foreground service (as per design for this service) + onStart(isInitialSync) // If it was periodic we ensure that it will not reschedule itself preventReschedule = true // we don't want to cancel initial syncs, let it finish @@ -85,11 +84,12 @@ abstract class SyncService : Service() { } else -> { val isInit = initialize(intent) + onStart(isInitialSync) if (isInit) { periodic = intent?.getBooleanExtra(EXTRA_PERIODIC, false) ?: false val onNetworkBack = intent?.getBooleanExtra(EXTRA_NETWORK_BACK_RESTART, false) ?: false Timber.d("## Sync: command received, periodic: $periodic networkBack: $onNetworkBack") - if (onNetworkBack && !backgroundDetectionObserver.isInBackground) { + if (!isInitialSync && onNetworkBack && !backgroundDetectionObserver.isInBackground) { // the restart after network occurs while the app is in foreground // so just stop. It will be restarted when entering background preventReschedule = true @@ -165,10 +165,16 @@ abstract class SyncService : Service() { preventReschedule = true } if (throwable is Failure.NetworkConnection) { - // Network is off, no need to reschedule endless alarms :/ + // Timeout is not critical, so retry as soon as possible. + val retryDelay = if (isInitialSync || throwable.cause is SocketTimeoutException) { + 0 + } else { + syncDelaySeconds + } + // Network might be off, no need to reschedule endless alarms :/ preventReschedule = true - // Instead start a work to restart background sync when network is back - onNetworkError(sessionId ?: "", isInitialSync, syncTimeoutSeconds, syncDelaySeconds) + // Instead start a work to restart background sync when network is on + onNetworkError(sessionId ?: "", isInitialSync, syncTimeoutSeconds, retryDelay) } // JobCancellation could be caught here when onDestroy cancels the coroutine context if (isRunning.get()) stopMe() diff --git a/vector/src/main/java/im/vector/app/core/services/VectorSyncService.kt b/vector/src/main/java/im/vector/app/core/services/VectorSyncService.kt index bf78d5b7fb..0950bdf121 100644 --- a/vector/src/main/java/im/vector/app/core/services/VectorSyncService.kt +++ b/vector/src/main/java/im/vector/app/core/services/VectorSyncService.kt @@ -21,7 +21,6 @@ import android.app.PendingIntent import android.content.Context import android.content.Intent import android.os.Build -import androidx.core.content.ContextCompat.getSystemService import androidx.core.content.getSystemService import androidx.work.Constraints import androidx.work.Data @@ -49,22 +48,19 @@ class VectorSyncService : SyncService() { } } - fun newPeriodicIntent(context: Context, sessionId: String, timeoutSeconds: Int, delayInSeconds: Int): Intent { + fun newPeriodicIntent( + context: Context, + sessionId: String, + timeoutSeconds: Int, + delayInSeconds: Int, + networkBack: Boolean = false + ): Intent { return Intent(context, VectorSyncService::class.java).also { it.putExtra(EXTRA_SESSION_ID, sessionId) it.putExtra(EXTRA_TIMEOUT_SECONDS, timeoutSeconds) it.putExtra(EXTRA_PERIODIC, true) it.putExtra(EXTRA_DELAY_SECONDS, delayInSeconds) - } - } - - fun newPeriodicNetworkBackIntent(context: Context, sessionId: String, timeoutSeconds: Int, delayInSeconds: Int): Intent { - return Intent(context, VectorSyncService::class.java).also { - it.putExtra(EXTRA_SESSION_ID, sessionId) - it.putExtra(EXTRA_TIMEOUT_SECONDS, timeoutSeconds) - it.putExtra(EXTRA_PERIODIC, true) - it.putExtra(EXTRA_DELAY_SECONDS, delayInSeconds) - it.putExtra(EXTRA_NETWORK_BACK_RESTART, true) + it.putExtra(EXTRA_NETWORK_BACK_RESTART, networkBack) } } @@ -93,12 +89,12 @@ class VectorSyncService : SyncService() { } override fun onRescheduleAsked(sessionId: String, isInitialSync: Boolean, timeout: Int, delay: Int) { - reschedule(sessionId, timeout, delay) + rescheduleSyncService(sessionId, timeout, delay) } override fun onNetworkError(sessionId: String, isInitialSync: Boolean, timeout: Int, delay: Int) { Timber.d("## Sync: A network error occured during sync") - val uploadWorkRequest: WorkRequest = + val rescheduleSyncWorkRequest: WorkRequest = OneTimeWorkRequestBuilder() .setInputData(Data.Builder() .putString("sessionId", sessionId) @@ -115,7 +111,7 @@ class VectorSyncService : SyncService() { Timber.d("## Sync: Schedule a work to restart service when network will be on") WorkManager .getInstance(applicationContext) - .enqueue(uploadWorkRequest) + .enqueue(rescheduleSyncWorkRequest) } override fun onDestroy() { @@ -128,42 +124,31 @@ class VectorSyncService : SyncService() { notificationManager.cancel(NotificationUtils.NOTIFICATION_ID_FOREGROUND_SERVICE) } - private fun reschedule(sessionId: String, timeout: Int, delay: Int) { - val pendingIntent = if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.O) { - PendingIntent.getForegroundService(this, 0, newPeriodicIntent(this, sessionId, timeout, delay), 0) - } else { - PendingIntent.getService(this, 0, newPeriodicIntent(this, sessionId, timeout, delay), 0) - } - val firstMillis = System.currentTimeMillis() + delay * 1000L - val alarmMgr = getSystemService()!! - if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.M) { - alarmMgr.setAndAllowWhileIdle(AlarmManager.RTC_WAKEUP, firstMillis, pendingIntent) - } else { - alarmMgr.set(AlarmManager.RTC_WAKEUP, firstMillis, pendingIntent) - } - } - class RestartWhenNetworkOn(appContext: Context, workerParams: WorkerParameters) : Worker(appContext, workerParams) { override fun doWork(): Result { val sessionId = inputData.getString("sessionId") ?: return Result.failure() val timeout = inputData.getInt("timeout", 6) val delay = inputData.getInt("delay", 60) - - val pendingIntent = if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.O) { - PendingIntent.getForegroundService(applicationContext, 0, newPeriodicNetworkBackIntent(applicationContext, sessionId, timeout, delay), 0) - } else { - PendingIntent.getService(applicationContext, 0, newPeriodicNetworkBackIntent(applicationContext, sessionId, timeout, delay), 0) - } - val firstMillis = System.currentTimeMillis() + delay * 1000L - val alarmMgr = getSystemService(applicationContext, AlarmManager::class.java)!! - if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.M) { - alarmMgr.setAndAllowWhileIdle(AlarmManager.RTC_WAKEUP, firstMillis, pendingIntent) - } else { - alarmMgr.set(AlarmManager.RTC_WAKEUP, firstMillis, pendingIntent) - } + applicationContext.rescheduleSyncService(sessionId, timeout, delay, true) // Indicate whether the work finished successfully with the Result return Result.success() } } } + +private fun Context.rescheduleSyncService(sessionId: String, timeout: Int, delay: Int, networkBack: Boolean = false) { + val periodicIntent = VectorSyncService.newPeriodicIntent(this, sessionId, timeout, delay, networkBack) + val pendingIntent = if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.O) { + PendingIntent.getForegroundService(this, 0, periodicIntent, 0) + } else { + PendingIntent.getService(this, 0, periodicIntent, 0) + } + val firstMillis = System.currentTimeMillis() + delay * 1000L + val alarmMgr = getSystemService()!! + if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.M) { + alarmMgr.setAndAllowWhileIdle(AlarmManager.RTC_WAKEUP, firstMillis, pendingIntent) + } else { + alarmMgr.set(AlarmManager.RTC_WAKEUP, firstMillis, pendingIntent) + } +}