Skip to content

Commit cf6a51e

Browse files
grotegrote
committed
Launch a foreground service when finalizing backup
to prevent the system from freezing/killing us while finalizing
1 parent d291cb3 commit cf6a51e

File tree

7 files changed

+117
-37
lines changed

7 files changed

+117
-37
lines changed

‎app/src/main/AndroidManifest.xml‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -187,6 +187,12 @@
187187
android:exported="false"
188188
android:label="BackupJobService"
189189
android:permission="android.permission.BIND_JOB_SERVICE" />
190+
<!-- Does app backup finalization as a foreground service -->
191+
<service
192+
android:name=".transport.backup.FinalizeBackupService"
193+
android:exported="false"
194+
android:foregroundServiceType="dataSync"
195+
android:label="FinalizeBackupService" />
190196
<!-- Does app restore as a foreground service -->
191197
<service
192198
android:name=".restore.RestoreService"

‎app/src/main/java/com/stevesoltys/seedvault/transport/backup/FinalizeBackupService.kt‎

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,48 @@
1+
/*
2+
* SPDX-FileCopyrightText: 2024 The Calyx Institute
3+
* SPDX-License-Identifier: Apache-2.0
4+
*/
5+
6+
package com.stevesoltys.seedvault.transport.backup
7+
8+
import android.app.Service
9+
import android.content.Intent
10+
import android.content.pm.ServiceInfo.FOREGROUND_SERVICE_TYPE_MANIFEST
11+
import android.os.IBinder
12+
import android.util.Log
13+
import com.stevesoltys.seedvault.R
14+
import com.stevesoltys.seedvault.ui.notification.BackupNotificationManager
15+
import com.stevesoltys.seedvault.ui.notification.NOTIFICATION_ID_OBSERVER
16+
import org.koin.android.ext.android.inject
17+
18+
class FinalizeBackupService : Service() {
19+
20+
companion object {
21+
private const val TAG = "FinalizeBackupService"
22+
}
23+
24+
private val nm: BackupNotificationManager by inject()
25+
26+
override fun onStartCommand(intent: Intent?, flags: Int, startId: Int): Int {
27+
Log.i(TAG, "onStartCommand $intent $flags $startId")
28+
29+
val str = applicationContext.getString(R.string.notification_finalizing_text)
30+
startForeground(
31+
NOTIFICATION_ID_OBSERVER,
32+
nm.getBackupNotification(str),
33+
FOREGROUND_SERVICE_TYPE_MANIFEST,
34+
)
35+
return START_STICKY_COMPATIBILITY
36+
}
37+
38+
override fun onBind(intent: Intent?): IBinder? {
39+
return null
40+
}
41+
42+
override fun onDestroy() {
43+
Log.i(TAG, "onDestroy")
44+
nm.cancelBackupNotification()
45+
super.onDestroy()
46+
}
47+
48+
}

‎app/src/main/java/com/stevesoltys/seedvault/ui/notification/BackupNotificationManager.kt‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -138,7 +138,7 @@ internal class BackupNotificationManager(private val context: Context) {
138138
/**
139139
* Call after [onApkBackup] or [onAppsNotBackedUp] were called.
140140
*/
141-
fun onApkBackupDone() {
141+
fun cancelBackupNotification() {
142142
nm.cancel(NOTIFICATION_ID_OBSERVER)
143143
}
144144

‎app/src/main/java/com/stevesoltys/seedvault/ui/notification/NotificationBackupObserver.kt‎

Lines changed: 51 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import android.app.backup.BackupProgress
99
import android.app.backup.BackupTransport.AGENT_ERROR
1010
import android.app.backup.IBackupObserver
1111
import android.content.Context
12+
import android.content.Intent
1213
import android.content.pm.ApplicationInfo.FLAG_SYSTEM
1314
import android.content.pm.PackageManager.NameNotFoundException
1415
import android.os.Looper
@@ -25,6 +26,7 @@ import com.stevesoltys.seedvault.metadata.PackageState.NOT_ALLOWED
2526
import com.stevesoltys.seedvault.repo.AppBackupManager
2627
import com.stevesoltys.seedvault.repo.hexFromProto
2728
import com.stevesoltys.seedvault.settings.SettingsManager
29+
import com.stevesoltys.seedvault.transport.backup.FinalizeBackupService
2830
import com.stevesoltys.seedvault.transport.backup.PackageService
2931
import com.stevesoltys.seedvault.worker.AppBackupPruneWorker
3032
import com.stevesoltys.seedvault.worker.BackupRequester
@@ -115,7 +117,7 @@ internal class NotificationBackupObserver(
115117
// this should only ever happen for system apps, as we use only d2d now
116118
if (target in launchableSystemApps) {
117119
val packageInfo = context.packageManager.getPackageInfo(target, 0)
118-
metadataManager.onPackageBackupError(packageInfo, NOT_ALLOWED)
120+
metadataManager.onPackageBackupError(packageInfo, NOT_ALLOWED)
119121
}
120122
}
121123

@@ -161,32 +163,23 @@ internal class NotificationBackupObserver(
161163
// since the rest of packages from the failed chunk won't get backed up.
162164
// So we either re-include those packages somehow (may fail again in a loop!)
163165
// or we simply fail the entire backup which may cause more failures for users :(
164-
var success = status == 0
165-
val total = try {
166-
packageService.allUserPackages.size
167-
} catch (e: Exception) {
168-
Log.e(TAG, "Error getting number of all user packages: ", e)
169-
requestedPackages
170-
}
171-
val snapshot = runBlocking {
172-
check(!Looper.getMainLooper().isCurrentThread)
173-
Log.d(TAG, "Finalizing backup...")
174-
val snapshot = appBackupManager.afterBackupFinished(success)
175-
success = snapshot != null
176-
snapshot
177-
}
178-
val size = if (snapshot != null) { // TODO for later: count size of APKs separately
179-
val chunkIds = snapshot.appsMap.values.flatMap { it.chunkIdsList }
180-
chunkIds.sumOf {
181-
snapshot.blobsMap[it.hexFromProto()]?.uncompressedLength?.toLong() ?: 0L
166+
val success = status == 0
167+
val i = Intent(context, FinalizeBackupService::class.java)
168+
try {
169+
// Starting a foreground service, because the system may freeze/kill us otherwise
170+
// before we are done here, because the TransportService gets destroyed now
171+
// and the wakelock the system holds for us gets released. See #866 for more info.
172+
context.startService(i)
173+
// finalize backup only when success is true
174+
val error = !success || !finalizeBackup()
175+
if (error) {
176+
runBlocking {
177+
appBackupManager.afterBackupFinished(false)
178+
}
179+
nm.onBackupError()
182180
}
183-
} else 0L
184-
if (success) {
185-
nm.onBackupSuccess(numPackagesToReport, total, size)
186-
// prune old backups
187-
AppBackupPruneWorker.scheduleNow(context)
188-
} else {
189-
nm.onBackupError()
181+
} finally {
182+
context.stopService(i)
190183
}
191184
}
192185
}
@@ -209,6 +202,38 @@ internal class NotificationBackupObserver(
209202
nm.onBackupUpdate(name, numPackages, requestedPackages)
210203
}
211204

205+
/**
206+
* Returns true, if finalizing was successful.
207+
*/
208+
private fun finalizeBackup(): Boolean {
209+
// finalize and save snapshot
210+
val snapshot = runBlocking {
211+
check(!Looper.getMainLooper().isCurrentThread) // off UiThread
212+
Log.d(TAG, "Finalizing backup...")
213+
appBackupManager.afterBackupFinished(true)
214+
}
215+
// abort on error
216+
if (snapshot == null) return false
217+
218+
// get info about backup
219+
val total = try {
220+
packageService.allUserPackages.size
221+
} catch (e: Exception) {
222+
Log.e(TAG, "Error getting number of all user packages: ", e)
223+
requestedPackages
224+
}
225+
val size = snapshot.appsMap.values.flatMap {
226+
it.chunkIdsList // TODO for later: count size of APKs separately
227+
}.sumOf {
228+
snapshot.blobsMap[it.hexFromProto()]?.uncompressedLength?.toLong() ?: 0L
229+
}
230+
// show notification
231+
nm.onBackupSuccess(numPackagesToReport, total, size)
232+
// prune old backups
233+
AppBackupPruneWorker.scheduleNow(context)
234+
return true
235+
}
236+
212237
private fun getAppName(packageId: String): CharSequence = getAppName(context, packageId)
213238

214239
}

‎app/src/main/java/com/stevesoltys/seedvault/worker/ApkBackupManager.kt‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,7 @@ internal class ApkBackupManager(
5353
backUpApks()
5454
}
5555
} finally {
56-
nm.onApkBackupDone()
56+
nm.cancelBackupNotification()
5757
}
5858
}
5959

‎app/src/main/res/values/strings.xml‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -163,6 +163,7 @@
163163
<string name="notification_success_channel_title">Success notification</string>
164164
<string name="notification_title">Backup running</string>
165165
<string name="notification_init_text">Preparing existing backup data for reuse…</string>
166+
<string name="notification_finalizing_text">Finalizing backup…</string>
166167
<string name="notification_apk_text">Backing up APK of %s</string>
167168
<string name="notification_apk_not_backed_up">Saving list of apps we can not back up.</string>
168169
<string name="notification_backup_already_running">Backup already in progress</string>

‎app/src/test/java/com/stevesoltys/seedvault/worker/ApkBackupManagerTest.kt‎

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -81,7 +81,7 @@ internal class ApkBackupManagerTest : TransportTest() {
8181
every { metadataManager.onPackageDoesNotGetBackedUp(packageInfo, NOT_ALLOWED) } just Runs
8282

8383
every { settingsManager.backupApks() } returns false
84-
every { nm.onApkBackupDone() } just Runs
84+
every { nm.cancelBackupNotification() } just Runs
8585

8686
apkBackupManager.backup()
8787

@@ -107,7 +107,7 @@ internal class ApkBackupManagerTest : TransportTest() {
107107
every { metadataManager.onPackageDoesNotGetBackedUp(packageInfo, NOT_ALLOWED) } just Runs
108108

109109
every { settingsManager.backupApks() } returns false
110-
every { nm.onApkBackupDone() } just Runs
110+
every { nm.cancelBackupNotification() } just Runs
111111

112112
apkBackupManager.backup()
113113

@@ -141,7 +141,7 @@ internal class ApkBackupManagerTest : TransportTest() {
141141
every { metadataManager.onPackageDoesNotGetBackedUp(packageInfo, WAS_STOPPED) } just Runs
142142

143143
every { settingsManager.backupApks() } returns false
144-
every { nm.onApkBackupDone() } just Runs
144+
every { nm.cancelBackupNotification() } just Runs
145145

146146
apkBackupManager.backup()
147147

@@ -167,7 +167,7 @@ internal class ApkBackupManagerTest : TransportTest() {
167167
every { packageMetadata.state } returns NOT_ALLOWED
168168

169169
every { settingsManager.backupApks() } returns false
170-
every { nm.onApkBackupDone() } just Runs
170+
every { nm.cancelBackupNotification() } just Runs
171171

172172
apkBackupManager.backup()
173173

@@ -186,7 +186,7 @@ internal class ApkBackupManagerTest : TransportTest() {
186186
expectUploadIcons()
187187

188188
every { settingsManager.backupApks() } returns false
189-
every { nm.onApkBackupDone() } just Runs
189+
every { nm.cancelBackupNotification() } just Runs
190190

191191
apkBackupManager.backup()
192192

@@ -227,7 +227,7 @@ internal class ApkBackupManagerTest : TransportTest() {
227227
// was backed up, get new packageMetadata
228228
coEvery { apkBackup.backupApkIfNecessary(notAllowedPackages[1], snapshot) } just Runs
229229

230-
every { nm.onApkBackupDone() } just Runs
230+
every { nm.cancelBackupNotification() } just Runs
231231

232232
apkBackupManager.backup()
233233

@@ -248,7 +248,7 @@ internal class ApkBackupManagerTest : TransportTest() {
248248
every { settingsManager.backupApks() } returns true
249249
every { packageService.allUserPackages } returns packages
250250
every { backendManager.canDoBackupNow() } returns false
251-
every { nm.onApkBackupDone() } just Runs
251+
every { nm.cancelBackupNotification() } just Runs
252252

253253
assertThrows<IllegalStateException> { apkBackupManager.backup() }
254254
Unit
@@ -265,7 +265,7 @@ internal class ApkBackupManagerTest : TransportTest() {
265265
every { settingsManager.backupApks() } returns true
266266
every { packageService.allUserPackages } returns packages
267267
every { backendManager.canDoBackupNow() } returns true andThen false
268-
every { nm.onApkBackupDone() } just Runs
268+
every { nm.cancelBackupNotification() } just Runs
269269

270270
assertThrows<IllegalStateException> { apkBackupManager.backup() }
271271
Unit
@@ -290,7 +290,7 @@ internal class ApkBackupManagerTest : TransportTest() {
290290

291291
every { settingsManager.backupApks() } returns false
292292

293-
every { nm.onApkBackupDone() } just Runs
293+
every { nm.cancelBackupNotification() } just Runs
294294

295295
apkBackupManager.backup()
296296

0 commit comments

Comments
 (0)