From c28083ab70e0ac8f156a819264efcf930906a0c4 Mon Sep 17 00:00:00 2001 From: Christopher Tate Date: Tue, 14 Dec 2010 16:16:44 -0800 Subject: Don't queue multiple pending backups for the same app Repeated install/replace of an app before it ran its first backup pass would wind up enqueueing multiple backup requests, all of which would run back to back when the time came. This no longer happens. Also, if a backup request is queued for an app that is then uninstalled before that request is honored, we no longer fail in expensive and log- intensive ways; we now fail cleanly, early. Bug 3133212 Change-Id: I745f5b2f966a1c874f34a0107a438ad42fa7f005 --- .../com/android/server/BackupManagerService.java | 65 ++++++++++++++-------- 1 file changed, 43 insertions(+), 22 deletions(-) diff --git a/services/java/com/android/server/BackupManagerService.java b/services/java/com/android/server/BackupManagerService.java index 601fa21483e4..5b4fb1fb538a 100644 --- a/services/java/com/android/server/BackupManagerService.java +++ b/services/java/com/android/server/BackupManagerService.java @@ -149,9 +149,9 @@ class BackupManagerService extends IBackupManager.Stub { return "BackupRequest{app=" + appInfo + " full=" + fullBackup + "}"; } } - // Backups that we haven't started yet. - HashMap mPendingBackups - = new HashMap(); + // Backups that we haven't started yet. Keys are package names. + HashMap mPendingBackups + = new HashMap(); // Pseudoname that we use for the Package Manager metadata "package" static final String PACKAGE_MANAGER_SENTINEL = "@pm@"; @@ -929,42 +929,48 @@ class BackupManagerService extends IBackupManager.Stub { // 'packageName' is null, *all* participating apps will be removed. void removePackageParticipantsLocked(String packageName) { if (DEBUG) Slog.v(TAG, "removePackageParticipantsLocked: " + packageName); - List allApps = null; + List allApps = new ArrayList(); if (packageName != null) { - allApps = new ArrayList(); - try { - int flags = PackageManager.GET_SIGNATURES; - allApps.add(mPackageManager.getPackageInfo(packageName, flags)); - } catch (Exception e) { - // just skip it (???) - } + allApps.add(packageName); } else { // all apps with agents - allApps = allAgentPackages(); + List knownPackages = allAgentPackages(); + for (PackageInfo pkg : knownPackages) { + allApps.add(pkg.packageName); + } } removePackageParticipantsLockedInner(packageName, allApps); } private void removePackageParticipantsLockedInner(String packageName, - List agents) { + List allPackageNames) { if (DEBUG) { Slog.v(TAG, "removePackageParticipantsLockedInner (" + packageName - + ") removing " + agents.size() + " entries"); - for (PackageInfo p : agents) { + + ") removing " + allPackageNames.size() + " entries"); + for (String p : allPackageNames) { Slog.v(TAG, " - " + p); } } - for (PackageInfo pkg : agents) { - if (packageName == null || pkg.packageName.equals(packageName)) { - int uid = pkg.applicationInfo.uid; + for (String pkg : allPackageNames) { + if (packageName == null || pkg.equals(packageName)) { + int uid = -1; + try { + PackageInfo info = mPackageManager.getPackageInfo(packageName, 0); + uid = info.applicationInfo.uid; + } catch (NameNotFoundException e) { + // we don't know this package name, so just skip it for now + continue; + } + HashSet set = mBackupParticipants.get(uid); if (set != null) { // Find the existing entry with the same package name, and remove it. // We can't just remove(app) because the instances are different. for (ApplicationInfo entry: set) { - if (entry.packageName.equals(pkg.packageName)) { + if (entry.packageName.equals(pkg)) { + if (DEBUG) Slog.v(TAG, " removing participant " + pkg); set.remove(entry); - removeEverBackedUp(pkg.packageName); + removeEverBackedUp(pkg); break; } } @@ -1014,7 +1020,11 @@ class BackupManagerService extends IBackupManager.Stub { // brute force but small code size List allApps = allAgentPackages(); - removePackageParticipantsLockedInner(packageName, allApps); + List allAppNames = new ArrayList(); + for (PackageInfo pkg : allApps) { + allAppNames.add(pkg.packageName); + } + removePackageParticipantsLockedInner(packageName, allAppNames); addPackageParticipantsLockedInner(packageName, allApps); } @@ -1381,6 +1391,17 @@ class BackupManagerService extends IBackupManager.Stub { for (BackupRequest request : mQueue) { Slog.d(TAG, "starting agent for backup of " + request); + // Verify that the requested app exists; it might be something that + // requested a backup but was then uninstalled. The request was + // journalled and rather than tamper with the journal it's safer + // to sanity-check here. + try { + mPackageManager.getPackageInfo(request.appInfo.packageName, 0); + } catch (NameNotFoundException e) { + Slog.d(TAG, "Package does not exist; skipping"); + continue; + } + IBackupAgent agent = null; int mode = (request.fullBackup) ? IApplicationThread.BACKUP_MODE_FULL @@ -2068,7 +2089,7 @@ class BackupManagerService extends IBackupManager.Stub { // Add the caller to the set of pending backups. If there is // one already there, then overwrite it, but no harm done. BackupRequest req = new BackupRequest(app, false); - if (mPendingBackups.put(app, req) == null) { + if (mPendingBackups.put(app.packageName, req) == null) { // Journal this request in case of crash. The put() // operation returned null when this package was not already // in the set; we want to avoid touching the disk redundantly. -- cgit v1.2.3-59-g8ed1b