Fix potential thread-safety issue when changing backends

seedvault-app · Oct 11, 2024 · 166c272 · 166c272
1 parent 5caa0ed
commit 166c272
Show file tree

Hide file tree

Showing 5 changed files with 36 additions and 12 deletions.
diff --git a/app/src/main/java/com/stevesoltys/seedvault/backend/BackendManager.kt b/app/src/main/java/com/stevesoltys/seedvault/backend/BackendManager.kt
@@ -25,7 +25,10 @@ class BackendManager(
     backendFactory: BackendFactory,
 ) {
 
+    @Volatile
     private var mBackend: Backend?
+
+    @Volatile
     private var mBackendProperties: BackendProperties<*>?
 
     val backend: Backend
@@ -81,6 +84,8 @@ class BackendManager(
      * IMPORTANT: Do no call this while current plugins are being used,
      *            e.g. while backup/restore operation is still running.
      */
+    @WorkerThread
+    @Synchronized
     fun <T> changePlugins(
         backend: Backend,
         storageProperties: BackendProperties<T>,

diff --git a/app/src/main/java/com/stevesoltys/seedvault/backend/saf/SafHandler.kt b/app/src/main/java/com/stevesoltys/seedvault/backend/saf/SafHandler.kt
@@ -90,6 +90,7 @@ internal class SafHandler(
         return false
     }
 
+    @WorkerThread
     fun setPlugin(safProperties: SafProperties) {
         backendManager.changePlugins(
             backend = backendFactory.createSafBackend(safProperties),

diff --git a/app/src/main/java/com/stevesoltys/seedvault/backend/webdav/WebDavHandler.kt b/app/src/main/java/com/stevesoltys/seedvault/backend/webdav/WebDavHandler.kt
@@ -87,6 +87,7 @@ internal class WebDavHandler(
         settingsManager.saveWebDavConfig(properties.config)
     }
 
+    @WorkerThread
     fun setPlugin(properties: WebDavProperties, backend: Backend) {
         backendManager.changePlugins(
             backend = backend,

diff --git a/app/src/main/java/com/stevesoltys/seedvault/repo/BlobCache.kt b/app/src/main/java/com/stevesoltys/seedvault/repo/BlobCache.kt
@@ -7,6 +7,7 @@ package com.stevesoltys.seedvault.repo
 
 import android.content.Context
 import android.content.Context.MODE_APPEND
+import androidx.annotation.WorkerThread
 import com.stevesoltys.seedvault.MemoryLogger
 import com.stevesoltys.seedvault.proto.Snapshot
 import com.stevesoltys.seedvault.proto.Snapshot.Blob
@@ -90,6 +91,7 @@ class BlobCache(
      * * changing to a different backup to prevent usage of blobs that don't exist there
      * * uploading a new snapshot to prevent the persistent cache from growing indefinitely
      */
+    @WorkerThread
     fun clearLocalCache() {
         log.info { "Clearing local cache..." }
         context.deleteFile(CACHE_FILE_NAME)

diff --git a/app/src/main/java/com/stevesoltys/seedvault/ui/storage/BackupStorageViewModel.kt b/app/src/main/java/com/stevesoltys/seedvault/ui/storage/BackupStorageViewModel.kt
@@ -10,6 +10,7 @@ import android.app.backup.IBackupManager
 import android.app.job.JobInfo
 import android.os.UserHandle
 import android.util.Log
+import androidx.annotation.UiThread
 import androidx.lifecycle.viewModelScope
 import androidx.work.ExistingPeriodicWorkPolicy.CANCEL_AND_REENQUEUE
 import com.stevesoltys.seedvault.R
@@ -23,6 +24,7 @@ import com.stevesoltys.seedvault.worker.AppBackupWorker
 import com.stevesoltys.seedvault.worker.BackupRequester.Companion.requestFilesAndAppBackup
 import kotlinx.coroutines.Dispatchers
 import kotlinx.coroutines.launch
+import kotlinx.coroutines.withContext
 import org.calyxos.backup.storage.api.StorageBackup
 import org.calyxos.backup.storage.backup.BackupJobService
 import org.calyxos.seedvault.core.backends.Backend
@@ -46,25 +48,38 @@ internal class BackupStorageViewModel(
 
     override val isRestoreOperation = false
 
+    @UiThread
     override fun onSafUriSet(safProperties: SafProperties) {
         safHandler.save(safProperties)
-        safHandler.setPlugin(safProperties)
-        if (safProperties.isUsb) {
-            // disable storage backup if new storage is on USB
-            cancelBackupWorkers()
-        } else {
-            // enable it, just in case the previous storage was on USB,
-            // also to update the network requirement of the new storage
-            scheduleBackupWorkers()
+        viewModelScope.launch {
+            withContext(Dispatchers.IO) {
+                safHandler.setPlugin(safProperties)
+            }
+            withContext(Dispatchers.Main) { // UiThread
+                if (safProperties.isUsb) {
+                    // disable storage backup if new storage is on USB
+                    cancelBackupWorkers()
+                } else {
+                    // enable it, just in case the previous storage was on USB,
+                    // also to update the network requirement of the new storage
+                    scheduleBackupWorkers()
+                }
+                onStorageLocationSet(safProperties.isUsb)
+            }
         }
-        onStorageLocationSet(safProperties.isUsb)
     }
 
     override fun onWebDavConfigSet(properties: WebDavProperties, backend: Backend) {
         webdavHandler.save(properties)
-        webdavHandler.setPlugin(properties, backend)
-        scheduleBackupWorkers()
-        onStorageLocationSet(isUsb = false)
+        viewModelScope.launch {
+            withContext(Dispatchers.IO) {
+                webdavHandler.setPlugin(properties, backend)
+            }
+            withContext(Dispatchers.Main) {
+                scheduleBackupWorkers()
+                onStorageLocationSet(isUsb = false)
+            }
+        }
     }
 
     private fun onStorageLocationSet(isUsb: Boolean) {