Move workers to foreground service context a bit more safely (#10202)

The system will crash the app if the worker that calls setForeground() finished before the service runner be able to call Service.startForeground(). This edge case is not handled by WorkManager and there is no way to check if the required calls are done. So here we suspend the worker by an arbitrary duration assuming the transition to foreground service is done by then.
2023-12-02 23:46:59 +07:00 · 2023-12-02 23:46:59 +07:00 · 24e1b4034e
commit 24e1b4034e
parent dfa5c229b3
7 changed files with 41 additions and 41 deletions
--- a/app/src/main/java/eu/kanade/tachiyomi/data/backup/BackupCreateJob.kt
+++ b/app/src/main/java/eu/kanade/tachiyomi/data/backup/BackupCreateJob.kt
@ -17,6 +17,7 @@ import com.hippo.unifile.UniFile
 import eu.kanade.tachiyomi.data.notification.Notifications
 import eu.kanade.tachiyomi.util.system.cancelNotification
 import eu.kanade.tachiyomi.util.system.isRunning
+import eu.kanade.tachiyomi.util.system.setForegroundSafely
 import eu.kanade.tachiyomi.util.system.workManager
 import logcat.LogPriority
 import tachiyomi.core.util.system.logcat
@ -39,19 +40,14 @@ class BackupCreateJob(private val context: Context, workerParams: WorkerParamete

        if (isAutoBackup && BackupRestoreJob.isRunning(context)) return Result.retry()

-        val backupPreferences = Injekt.get<BackupPreferences>()
-
        val uri = inputData.getString(LOCATION_URI_KEY)?.toUri()
            ?: getAutomaticBackupLocation()
            ?: return Result.failure()

-        val flags = inputData.getInt(BACKUP_FLAGS_KEY, BackupCreateFlags.AutomaticDefaults)
+        setForegroundSafely()

-        try {
-            setForeground(getForegroundInfo())
-        } catch (e: IllegalStateException) {
-            logcat(LogPriority.ERROR, e) { "Not allowed to run on foreground service" }
-        }
+        val flags = inputData.getInt(BACKUP_FLAGS_KEY, BackupCreateFlags.AutomaticDefaults)
+        val backupPreferences = Injekt.get<BackupPreferences>()

        return try {
            val location = BackupCreator(context).createBackup(uri, flags, isAutoBackup)
--- a/app/src/main/java/eu/kanade/tachiyomi/data/backup/BackupRestoreJob.kt
+++ b/app/src/main/java/eu/kanade/tachiyomi/data/backup/BackupRestoreJob.kt
@ -12,6 +12,7 @@ import androidx.work.workDataOf
 import eu.kanade.tachiyomi.data.notification.Notifications
 import eu.kanade.tachiyomi.util.system.cancelNotification
 import eu.kanade.tachiyomi.util.system.isRunning
+import eu.kanade.tachiyomi.util.system.setForegroundSafely
 import eu.kanade.tachiyomi.util.system.workManager
 import kotlinx.coroutines.CancellationException
 import logcat.LogPriority
@ -29,11 +30,7 @@ class BackupRestoreJob(private val context: Context, workerParams: WorkerParamet
            ?: return Result.failure()
        val sync = inputData.getBoolean(SYNC_KEY, false)

-        try {
-            setForeground(getForegroundInfo())
-        } catch (e: IllegalStateException) {
-            logcat(LogPriority.ERROR, e) { "Not allowed to run on foreground service" }
-        }
+        setForegroundSafely()

        return try {
            val restorer = BackupRestorer(context, notifier)
--- a/app/src/main/java/eu/kanade/tachiyomi/data/download/DownloadJob.kt
+++ b/app/src/main/java/eu/kanade/tachiyomi/data/download/DownloadJob.kt
@ -16,11 +16,10 @@ import eu.kanade.tachiyomi.data.notification.Notifications
 import eu.kanade.tachiyomi.util.system.isConnectedToWifi
 import eu.kanade.tachiyomi.util.system.isOnline
 import eu.kanade.tachiyomi.util.system.notificationBuilder
+import eu.kanade.tachiyomi.util.system.setForegroundSafely
 import kotlinx.coroutines.delay
 import kotlinx.coroutines.flow.Flow
 import kotlinx.coroutines.flow.map
-import logcat.LogPriority
-import tachiyomi.core.util.system.logcat
 import tachiyomi.domain.download.service.DownloadPreferences
 import uy.kohesive.injekt.Injekt
 import uy.kohesive.injekt.api.get
@ -51,21 +50,18 @@ class DownloadJob(context: Context, workerParams: WorkerParameters) : CoroutineW
    }

    override suspend fun doWork(): Result {
-        try {
-            setForeground(getForegroundInfo())
-        } catch (e: IllegalStateException) {
-            logcat(LogPriority.ERROR, e) { "Not allowed to set foreground job" }
+        var active = checkConnectivity() && downloadManager.downloaderStart()
+
+        if (!active) {
+            return Result.failure()
        }

-        var networkCheck = checkConnectivity()
-        var active = networkCheck
-        downloadManager.downloaderStart()
+        setForegroundSafely()

        // Keep the worker running when needed
        while (active) {
            delay(100)
-            networkCheck = checkConnectivity()
-            active = !isStopped && networkCheck && downloadManager.isRunning
+            active = !isStopped && downloadManager.isRunning && checkConnectivity()
        }

        return Result.success()
--- a/app/src/main/java/eu/kanade/tachiyomi/data/library/LibraryUpdateJob.kt
+++ b/app/src/main/java/eu/kanade/tachiyomi/data/library/LibraryUpdateJob.kt
@ -30,6 +30,7 @@ import eu.kanade.tachiyomi.util.storage.getUriCompat
 import eu.kanade.tachiyomi.util.system.createFileInCacheDir
 import eu.kanade.tachiyomi.util.system.isConnectedToWifi
 import eu.kanade.tachiyomi.util.system.isRunning
+import eu.kanade.tachiyomi.util.system.setForegroundSafely
 import eu.kanade.tachiyomi.util.system.workManager
 import kotlinx.coroutines.CancellationException
 import kotlinx.coroutines.async
@ -108,11 +109,7 @@ class LibraryUpdateJob(private val context: Context, workerParams: WorkerParamet
            }
        }

-        try {
-            setForeground(getForegroundInfo())
-        } catch (e: IllegalStateException) {
-            logcat(LogPriority.ERROR, e) { "Not allowed to set foreground job" }
-        }
+        setForegroundSafely()

        libraryPreferences.lastUpdatedTimestamp().set(Date().time)

--- a/app/src/main/java/eu/kanade/tachiyomi/data/library/MetadataUpdateJob.kt
+++ b/app/src/main/java/eu/kanade/tachiyomi/data/library/MetadataUpdateJob.kt
@ -18,6 +18,7 @@ import eu.kanade.tachiyomi.data.notification.Notifications
 import eu.kanade.tachiyomi.source.UnmeteredSource
 import eu.kanade.tachiyomi.util.prepUpdateCover
 import eu.kanade.tachiyomi.util.system.isRunning
+import eu.kanade.tachiyomi.util.system.setForegroundSafely
 import eu.kanade.tachiyomi.util.system.workManager
 import kotlinx.coroutines.CancellationException
 import kotlinx.coroutines.async
@ -53,11 +54,7 @@ class MetadataUpdateJob(private val context: Context, workerParams: WorkerParame
    private var mangaToUpdate: List<LibraryManga> = mutableListOf()

    override suspend fun doWork(): Result {
-        try {
-            setForeground(getForegroundInfo())
-        } catch (e: IllegalStateException) {
-            logcat(LogPriority.ERROR, e) { "Not allowed to set foreground job" }
-        }
+        setForegroundSafely()

        addMangaToQueue()

--- a/app/src/main/java/eu/kanade/tachiyomi/data/updater/AppUpdateDownloadJob.kt
+++ b/app/src/main/java/eu/kanade/tachiyomi/data/updater/AppUpdateDownloadJob.kt
@ -17,13 +17,12 @@ import eu.kanade.tachiyomi.network.await
 import eu.kanade.tachiyomi.network.newCachelessCallWithProgress
 import eu.kanade.tachiyomi.util.storage.getUriCompat
 import eu.kanade.tachiyomi.util.storage.saveTo
+import eu.kanade.tachiyomi.util.system.setForegroundSafely
 import eu.kanade.tachiyomi.util.system.workManager
-import logcat.LogPriority
 import okhttp3.internal.http2.ErrorCode
 import okhttp3.internal.http2.StreamResetException
 import tachiyomi.core.i18n.stringResource
 import tachiyomi.core.util.lang.withIOContext
-import tachiyomi.core.util.system.logcat
 import tachiyomi.i18n.MR
 import uy.kohesive.injekt.injectLazy
 import java.io.File
@ -43,11 +42,7 @@ class AppUpdateDownloadJob(private val context: Context, workerParams: WorkerPar
            return Result.failure()
        }

-        try {
-            setForeground(getForegroundInfo())
-        } catch (e: IllegalStateException) {
-            logcat(LogPriority.ERROR, e) { "Not allowed to run on foreground service" }
-        }
+        setForegroundSafely()

        withIOContext {
            downloadApk(title, url)
--- a/app/src/main/java/eu/kanade/tachiyomi/util/system/WorkManagerExtensions.kt
+++ b/app/src/main/java/eu/kanade/tachiyomi/util/system/WorkManagerExtensions.kt
@ -1,8 +1,12 @@
 package eu.kanade.tachiyomi.util.system

 import android.content.Context
+import androidx.work.CoroutineWorker
 import androidx.work.WorkInfo
 import androidx.work.WorkManager
+import kotlinx.coroutines.delay
+import logcat.LogPriority
+import tachiyomi.core.util.system.logcat

 val Context.workManager: WorkManager
    get() = WorkManager.getInstance(this)
@ -11,3 +15,21 @@ fun WorkManager.isRunning(tag: String): Boolean {
    val list = this.getWorkInfosByTag(tag).get()
    return list.any { it.state == WorkInfo.State.RUNNING }
 }
+
+/**
+ * Makes this worker run in the context of a foreground service.
+ *
+ * Note that this function is a no-op if the process is subject to foreground
+ * service restrictions.
+ *
+ * Moving to foreground service context requires the worker to run a bit longer,
+ * allowing Service.startForeground() to be called and avoiding system crash.
+ */
+suspend fun CoroutineWorker.setForegroundSafely() {
+    try {
+        setForeground(getForegroundInfo())
+        delay(500)
+    } catch (e: IllegalStateException) {
+        logcat(LogPriority.ERROR, e) { "Not allowed to set foreground job" }
+    }
+}