Fix activity leaks in backup, restore dialogs and properly handle db transactions

app/src/main/java/eu/kanade/tachiyomi/data/backup/BackupCreateService.kt

@@ -14,6 +14,7 @@ import eu.kanade.tachiyomi.data.backup.models.Backup.MANGAS
 import eu.kanade.tachiyomi.data.backup.models.Backup.VERSION
 import eu.kanade.tachiyomi.data.database.models.Manga
 import eu.kanade.tachiyomi.ui.setting.SettingsBackupFragment
+import eu.kanade.tachiyomi.util.AndroidComponentUtil
 import eu.kanade.tachiyomi.util.sendLocalBroadcast
 import timber.log.Timber
 import eu.kanade.tachiyomi.BuildConfig.APPLICATION_ID as ID
@@ -61,6 +62,10 @@ class BackupCreateService : IntentService(NAME) {
             }
             context.startService(intent)
         }
+
+        fun isRunning(context: Context): Boolean {
+            return AndroidComponentUtil.isServiceRunning(context, BackupCreateService::class.java)
+        }
     }
 
     private val backupManager by lazy { BackupManager(this) }

app/src/main/java/eu/kanade/tachiyomi/data/backup/BackupRestoreService.kt

@@ -35,6 +35,8 @@ import uy.kohesive.injekt.injectLazy
 import java.io.File
 import java.text.SimpleDateFormat
 import java.util.*
+import java.util.concurrent.ExecutorService
+import java.util.concurrent.Executors
 import eu.kanade.tachiyomi.BuildConfig.APPLICATION_ID as ID
 
 /**
@@ -119,6 +121,8 @@ class BackupRestoreService : Service() {
      */
     private val db: DatabaseHelper by injectLazy()
 
+    lateinit var executor: ExecutorService
+
     /**
      * Method called when the service is created. It injects dependencies and acquire the wake lock.
      */
@@ -127,6 +131,7 @@ class BackupRestoreService : Service() {
         wakeLock = (getSystemService(Context.POWER_SERVICE) as PowerManager).newWakeLock(
                 PowerManager.PARTIAL_WAKE_LOCK, "BackupRestoreService:WakeLock")
         wakeLock.acquire()
+        executor = Executors.newSingleThreadExecutor()
     }
 
     /**
@@ -135,6 +140,7 @@ class BackupRestoreService : Service() {
      */
     override fun onDestroy() {
         subscription?.unsubscribe()
+        executor.shutdown() // must be called after unsubscribe
         if (wakeLock.isHeld) {
             wakeLock.release()
         }
@@ -159,53 +165,19 @@ class BackupRestoreService : Service() {
     override fun onStartCommand(intent: Intent?, flags: Int, startId: Int): Int {
         if (intent == null) return Service.START_NOT_STICKY
 
+        val file = UniFile.fromUri(this, Uri.parse(intent.getStringExtra(EXTRA_URI)))
+
         // Unsubscribe from any previous subscription if needed.
         subscription?.unsubscribe()
 
-        val startTime = System.currentTimeMillis()
-        subscription = Observable.defer {
-            // Get URI
-            val uri = Uri.parse(intent.getStringExtra(EXTRA_URI))
-            // Get file from Uri
-            val file = UniFile.fromUri(this, uri)
-
-            // Clear errors
-            errors.clear()
+        subscription = Observable.using(
+                { db.lowLevel().beginTransaction() },
+                { getRestoreObservable(file).doOnNext{ db.lowLevel().setTransactionSuccessful() } },
+                { executor.execute { db.lowLevel().endTransaction() } })
+                .doAfterTerminate { stopSelf(startId) }
+                .subscribeOn(Schedulers.from(executor))
+                .subscribe()
 
-            // Reset progress
-            restoreProgress = 0
-
-            db.lowLevel().beginTransaction()
-            getRestoreObservable(file)
-        }
-        .subscribeOn(Schedulers.io())
-        .subscribe({
-        }, { error ->
-            db.lowLevel().endTransaction()
-            Timber.e(error)
-            writeErrorLog()
-            val errorIntent = Intent(SettingsBackupFragment.INTENT_FILTER).apply {
-                putExtra(SettingsBackupFragment.ACTION, SettingsBackupFragment.ACTION_ERROR_RESTORE_DIALOG)
-                putExtra(SettingsBackupFragment.EXTRA_ERROR_MESSAGE, error.message)
-            }
-            sendLocalBroadcast(errorIntent)
-            stopSelf(startId)
-        }, {
-            db.lowLevel().setTransactionSuccessful()
-            db.lowLevel().endTransaction()
-            val endTime = System.currentTimeMillis()
-            val time = endTime - startTime
-            val file = writeErrorLog()
-            val completeIntent = Intent(SettingsBackupFragment.INTENT_FILTER).apply {
-                putExtra(SettingsBackupFragment.EXTRA_TIME, time)
-                putExtra(SettingsBackupFragment.EXTRA_ERRORS, errors.size)
-                putExtra(SettingsBackupFragment.EXTRA_ERROR_FILE_PATH, file.parent)
-                putExtra(SettingsBackupFragment.EXTRA_ERROR_FILE, file.name)
-                putExtra(SettingsBackupFragment.ACTION, SettingsBackupFragment.ACTION_RESTORE_COMPLETED_DIALOG)
-            }
-            sendLocalBroadcast(completeIntent)
-            stopSelf(startId)
-        })
         return Service.START_NOT_STICKY
     }
 
@@ -215,7 +187,9 @@ class BackupRestoreService : Service() {
      * @param file restore file
      * @return [Observable<Manga>]
      */
-    private fun getRestoreObservable(file: UniFile): Observable<Manga> {
+    private fun getRestoreObservable(file: UniFile): Observable<List<Manga>> {
+        val startTime = System.currentTimeMillis()
+
         val reader = JsonReader(file.openInputStream().bufferedReader())
         val json = JsonParser().parse(reader).asJsonObject
 
@@ -228,6 +202,8 @@ class BackupRestoreService : Service() {
         val mangasJson = json.get(MANGAS).asJsonArray
 
         restoreAmount = mangasJson.size() + 1 // +1 for categories
+        restoreProgress = 0
+        errors.clear()
 
         // Restore categories
         json.get(CATEGORIES)?.let {
@@ -256,6 +232,31 @@ class BackupRestoreService : Service() {
                         Observable.just(manga)
                     }
                 }
+                .toList()
+                .doOnNext {
+                    val endTime = System.currentTimeMillis()
+                    val time = endTime - startTime
+                    val logFile = writeErrorLog()
+                    val completeIntent = Intent(SettingsBackupFragment.INTENT_FILTER).apply {
+                        putExtra(SettingsBackupFragment.EXTRA_TIME, time)
+                        putExtra(SettingsBackupFragment.EXTRA_ERRORS, errors.size)
+                        putExtra(SettingsBackupFragment.EXTRA_ERROR_FILE_PATH, logFile.parent)
+                        putExtra(SettingsBackupFragment.EXTRA_ERROR_FILE, logFile.name)
+                        putExtra(SettingsBackupFragment.ACTION, SettingsBackupFragment.ACTION_RESTORE_COMPLETED_DIALOG)
+                    }
+                    sendLocalBroadcast(completeIntent)
+
+                }
+                .doOnError { error ->
+                    Timber.e(error)
+                    writeErrorLog()
+                    val errorIntent = Intent(SettingsBackupFragment.INTENT_FILTER).apply {
+                        putExtra(SettingsBackupFragment.ACTION, SettingsBackupFragment.ACTION_ERROR_RESTORE_DIALOG)
+                        putExtra(SettingsBackupFragment.EXTRA_ERROR_MESSAGE, error.message)
+                    }
+                    sendLocalBroadcast(errorIntent)
+                }
+                .onErrorReturn { emptyList() }
     }
 
     /**

app/src/main/java/eu/kanade/tachiyomi/ui/setting/SettingsBackupFragment.kt

@@ -1,6 +1,7 @@
 package eu.kanade.tachiyomi.ui.setting
 
 import android.app.Activity
+import android.app.Dialog
 import android.content.BroadcastReceiver
 import android.content.Context
 import android.content.Intent
@@ -25,6 +26,7 @@ import eu.kanade.tachiyomi.util.*
 import eu.kanade.tachiyomi.widget.CustomLayoutPickerActivity
 import eu.kanade.tachiyomi.widget.preference.IntListPreference
 import net.xpece.android.support.preference.Preference
+import rx.subscriptions.Subscriptions
 import uy.kohesive.injekt.injectLazy
 import java.io.File
 import java.util.concurrent.TimeUnit
@@ -149,7 +151,7 @@ class SettingsBackupFragment : SettingsFragment() {
                                 sendIntent.putExtra(Intent.EXTRA_STREAM, file.uri)
                                 startActivity(Intent.createChooser(sendIntent, ""))
                             }
-                            .show()
+                            .safeShow()
 
                 }
                 ACTION_SET_PROGRESS_DIALOG -> {
@@ -194,7 +196,7 @@ class SettingsBackupFragment : SettingsFragment() {
                                     }
                                     materialDialog.dismiss()
                                 }
-                                .show()
+                                .safeShow()
                     }
                 }
                 ACTION_ERROR_BACKUP_DIALOG -> {
@@ -210,19 +212,28 @@ class SettingsBackupFragment : SettingsFragment() {
 
     }
 
-    override fun onPause() {
-        context.unregisterLocalReceiver(receiver)
-        super.onPause()
-    }
-
     override fun onStart() {
         super.onStart()
         context.registerLocalReceiver(receiver, IntentFilter(INTENT_FILTER))
     }
 
+    override fun onPause() {
+        context.unregisterLocalReceiver(receiver)
+        super.onPause()
+    }
+
     override fun onViewCreated(view: View, savedState: Bundle?) {
         super.onViewCreated(view, savedState)
 
+        if (savedState != null) {
+            if (BackupRestoreService.isRunning(context)) {
+                restoreDialog.safeShow()
+            }
+            else if (BackupCreateService.isRunning(context)) {
+                backupDialog.safeShow()
+            }
+        }
+
         (activity as BaseActivity).requestPermissionsOnMarshmallow()
 
         // Set onClickListeners
@@ -268,7 +279,7 @@ class SettingsBackupFragment : SettingsFragment() {
                     .itemsDisabledIndices(0)
                     .positiveText(getString(R.string.action_create))
                     .negativeText(android.R.string.cancel)
-                    .show()
+                    .safeShow()
             true
         }
 
@@ -359,7 +370,7 @@ class SettingsBackupFragment : SettingsFragment() {
                     val dir = data.data.path
                     val file = File(dir, Backup.getDefaultFilename())
 
-                    backupDialog.show()
+                    backupDialog.safeShow()
                     BackupCreateService.makeBackup(context, file.toURI().toString(), backup_flags)
                 } else {
                     val uri = data.data
@@ -369,45 +380,43 @@ class SettingsBackupFragment : SettingsFragment() {
                     context.contentResolver.takePersistableUriPermission(uri, flags)
                     val file = UniFile.fromUri(context, uri)
 
-                    backupDialog.show()
+                    backupDialog.safeShow()
                     BackupCreateService.makeBackup(context, file.uri.toString(), backup_flags)
                 }
             }
             BACKUP_RESTORE -> if (data != null && resultCode == Activity.RESULT_OK) {
-                if (Build.VERSION.SDK_INT < Build.VERSION_CODES.KITKAT) {
-                    val uri = Uri.fromFile(File(data.data.path))
-
-                    MaterialDialog.Builder(context)
-                            .title(getString(R.string.pref_restore_backup))
-                            .content(getString(R.string.backup_restore_content))
-                            .positiveText(getString(R.string.action_restore))
-                            .onPositive { materialDialog, _ ->
-                                materialDialog.dismiss()
-                                restoreDialog.show()
-                                BackupRestoreService.start(context, uri.toString())
-                            }
-                            .show()
+                val uri = if (Build.VERSION.SDK_INT < Build.VERSION_CODES.KITKAT) {
+                    Uri.fromFile(File(data.data.path))
                 } else {
                     val uri = data.data
                     val flags = Intent.FLAG_GRANT_READ_URI_PERMISSION or
                             Intent.FLAG_GRANT_WRITE_URI_PERMISSION
 
                     context.contentResolver.takePersistableUriPermission(uri, flags)
-                    val file = UniFile.fromUri(context, uri)
-
-                    MaterialDialog.Builder(context)
-                            .title(getString(R.string.pref_restore_backup))
-                            .content(getString(R.string.backup_restore_content))
-                            .positiveText(getString(R.string.action_restore))
-                            .onPositive { materialDialog, _ ->
-                                materialDialog.dismiss()
-                                restoreDialog.show()
-                                BackupRestoreService.start(context, file.uri.toString())
-                            }
-                            .show()
+                    UniFile.fromUri(context, uri).uri
                 }
+
+                MaterialDialog.Builder(context)
+                        .title(getString(R.string.pref_restore_backup))
+                        .content(getString(R.string.backup_restore_content))
+                        .positiveText(getString(R.string.action_restore))
+                        .onPositive { _, _ ->
+                            restoreDialog.safeShow()
+                            BackupRestoreService.start(context, uri.toString())
+                        }
+                        .safeShow()
             }
         }
     }
 
+    fun MaterialDialog.Builder.safeShow(): Dialog {
+        return build().safeShow()
+    }
+
+    fun Dialog.safeShow(): Dialog {
+        subscriptions += Subscriptions.create { dismiss() }
+        show()
+        return this
+    }
+
 }