From 2e40d115ca4332d88424d1b591fdd8d5f78d1831 Mon Sep 17 00:00:00 2001 From: Christopher Tate Date: Tue, 15 Jul 2014 12:37:38 -0700 Subject: Add BackupAgent.onRestoreFinished() callback The agent's onRestoreFinished() method is called after all available data has been delivered to the app, whether via the key/value restore API or the full-data file-at-a-time API. This gives the app a stable opportunity to do any postprocessing that might be appropriate. Also fixes a lingering bug in the framework's handling of backup agent lifetimes. In cases where an existing agent instances was being rebound, the framework was forgetting to notify the dependent that the agent was available. This was causing timeouts and restore failure. Bug 16241004 Change-Id: I3f52b299312d30d38b0cba63a2cfaeb934991ef2 --- api/current.txt | 1 + core/java/android/app/ActivityThread.java | 55 +++++----- core/java/android/app/IBackupAgent.aidl | 15 +++ core/java/android/app/backup/BackupAgent.java | 37 ++++++- .../server/backup/BackupManagerService.java | 112 +++++++++++++++++---- 5 files changed, 171 insertions(+), 49 deletions(-) diff --git a/api/current.txt b/api/current.txt index fac1d8c..f799671 100644 --- a/api/current.txt +++ b/api/current.txt @@ -5454,6 +5454,7 @@ package android.app.backup { method public void onFullBackup(android.app.backup.FullBackupDataOutput) throws java.io.IOException; method public abstract void onRestore(android.app.backup.BackupDataInput, int, android.os.ParcelFileDescriptor) throws java.io.IOException; method public void onRestoreFile(android.os.ParcelFileDescriptor, long, java.io.File, int, long, long) throws java.io.IOException; + method public void onRestoreFinished(); field public static final int TYPE_DIRECTORY = 2; // 0x2 field public static final int TYPE_FILE = 1; // 0x1 } diff --git a/core/java/android/app/ActivityThread.java b/core/java/android/app/ActivityThread.java index 15f3a75..48954f4 100644 --- a/core/java/android/app/ActivityThread.java +++ b/core/java/android/app/ActivityThread.java @@ -2610,15 +2610,7 @@ public final class ActivityThread { return; } - if (mBackupAgents.get(packageName) != null) { - Slog.d(TAG, "BackupAgent " + " for " + packageName - + " already exists"); - return; - } - - BackupAgent agent = null; String classname = data.appInfo.backupAgentName; - // full backup operation but no app-supplied agent? use the default implementation if (classname == null && (data.backupMode == IApplicationThread.BACKUP_MODE_FULL || data.backupMode == IApplicationThread.BACKUP_MODE_RESTORE_FULL)) { @@ -2627,29 +2619,38 @@ public final class ActivityThread { try { IBinder binder = null; - try { - if (DEBUG_BACKUP) Slog.v(TAG, "Initializing agent class " + classname); + BackupAgent agent = mBackupAgents.get(packageName); + if (agent != null) { + // reusing the existing instance + if (DEBUG_BACKUP) { + Slog.v(TAG, "Reusing existing agent instance"); + } + binder = agent.onBind(); + } else { + try { + if (DEBUG_BACKUP) Slog.v(TAG, "Initializing agent class " + classname); - java.lang.ClassLoader cl = packageInfo.getClassLoader(); - agent = (BackupAgent) cl.loadClass(classname).newInstance(); + java.lang.ClassLoader cl = packageInfo.getClassLoader(); + agent = (BackupAgent) cl.loadClass(classname).newInstance(); - // set up the agent's context - ContextImpl context = ContextImpl.createAppContext(this, packageInfo); - context.setOuterContext(agent); - agent.attach(context); + // set up the agent's context + ContextImpl context = ContextImpl.createAppContext(this, packageInfo); + context.setOuterContext(agent); + agent.attach(context); - agent.onCreate(); - binder = agent.onBind(); - mBackupAgents.put(packageName, agent); - } catch (Exception e) { - // If this is during restore, fail silently; otherwise go - // ahead and let the user see the crash. - Slog.e(TAG, "Agent threw during creation: " + e); - if (data.backupMode != IApplicationThread.BACKUP_MODE_RESTORE - && data.backupMode != IApplicationThread.BACKUP_MODE_RESTORE_FULL) { - throw e; + agent.onCreate(); + binder = agent.onBind(); + mBackupAgents.put(packageName, agent); + } catch (Exception e) { + // If this is during restore, fail silently; otherwise go + // ahead and let the user see the crash. + Slog.e(TAG, "Agent threw during creation: " + e); + if (data.backupMode != IApplicationThread.BACKUP_MODE_RESTORE + && data.backupMode != IApplicationThread.BACKUP_MODE_RESTORE_FULL) { + throw e; + } + // falling through with 'binder' still null } - // falling through with 'binder' still null } // tell the OS that we're live now diff --git a/core/java/android/app/IBackupAgent.aidl b/core/java/android/app/IBackupAgent.aidl index 7036aea..451af99 100644 --- a/core/java/android/app/IBackupAgent.aidl +++ b/core/java/android/app/IBackupAgent.aidl @@ -125,6 +125,21 @@ oneway interface IBackupAgent { int token, IBackupManager callbackBinder); /** + * Provide the app with a canonical "all data has been delivered" end-of-restore + * callback so that it can do any postprocessing of the restored data that might + * be appropriate. This is issued after both key/value and full data restore + * operations have completed. + * + * @param token Opaque token identifying this transaction. This must + * be echoed back to the backup service binder once the agent is + * finished restoring the application based on the restore data + * contents. + * @param callbackBinder Binder on which to indicate operation completion, + * passed here as a convenience to the agent. + */ + void doRestoreFinished(int token, IBackupManager callbackBinder); + + /** * Out of band: instruct the agent to crash within the client process. This is used * when the backup infrastructure detects a semantic error post-hoc and needs to * pass the problem back to the app. diff --git a/core/java/android/app/backup/BackupAgent.java b/core/java/android/app/backup/BackupAgent.java index e2a86e8..87d785a 100644 --- a/core/java/android/app/backup/BackupAgent.java +++ b/core/java/android/app/backup/BackupAgent.java @@ -209,7 +209,7 @@ public abstract class BackupAgent extends ContextWrapper { * output stream. */ public abstract void onBackup(ParcelFileDescriptor oldState, BackupDataOutput data, - ParcelFileDescriptor newState) throws IOException; + ParcelFileDescriptor newState) throws IOException; /** * The application is being restored from backup and should replace any @@ -243,8 +243,7 @@ public abstract class BackupAgent extends ContextWrapper { * When a full-backup dataset is being restored, this will be null. */ public abstract void onRestore(BackupDataInput data, int appVersionCode, - ParcelFileDescriptor newState) - throws IOException; + ParcelFileDescriptor newState) throws IOException; /** * The application is having its entire file system contents backed up. {@code data} @@ -575,6 +574,20 @@ public abstract class BackupAgent extends ContextWrapper { FullBackup.restoreFile(data, size, type, mode, mtime, null); } + /** + * The application's restore operation has completed. This method is called after + * all available data has been delivered to the application for restore (via either + * the {@link #onRestore(BackupDataInput, int, ParcelFileDescriptor) onRestore()} or + * {@link #onRestoreFile(ParcelFileDescriptor, long, File, int, long, long) onRestoreFile()} + * callbacks). This provides the app with a stable end-of-restore opportunity to + * perform any appropriate post-processing on the data that was just delivered. + * + * @see #onRestore(BackupDataInput, int, ParcelFileDescriptor) + * @see #onRestoreFile(ParcelFileDescriptor, long, File, int, long, long) + */ + public void onRestoreFinished() { + } + // ----- Core implementation ----- /** @hide */ @@ -723,6 +736,24 @@ public abstract class BackupAgent extends ContextWrapper { } @Override + public void doRestoreFinished(int token, IBackupManager callbackBinder) { + long ident = Binder.clearCallingIdentity(); + try { + BackupAgent.this.onRestoreFinished(); + } finally { + // Ensure that any side-effect SharedPreferences writes have landed + waitForSharedPrefs(); + + Binder.restoreCallingIdentity(ident); + try { + callbackBinder.opComplete(token); + } catch (RemoteException e) { + // we'll time out anyway, so we're safe + } + } + } + + @Override public void fail(String message) { getHandler().post(new FailRunnable(message)); } diff --git a/services/backup/java/com/android/server/backup/BackupManagerService.java b/services/backup/java/com/android/server/backup/BackupManagerService.java index e8e2813..f54f798 100644 --- a/services/backup/java/com/android/server/backup/BackupManagerService.java +++ b/services/backup/java/com/android/server/backup/BackupManagerService.java @@ -233,6 +233,7 @@ public class BackupManagerService extends IBackupManager.Stub { static final long TIMEOUT_FULL_BACKUP_INTERVAL = 5 * 60 * 1000; static final long TIMEOUT_SHARED_BACKUP_INTERVAL = 30 * 60 * 1000; static final long TIMEOUT_RESTORE_INTERVAL = 60 * 1000; + static final long TIMEOUT_RESTORE_FINISHED_INTERVAL = 30 * 1000; // User confirmation timeout for a full backup/restore operation. It's this long in // order to give them time to enter the backup password. @@ -6309,6 +6310,7 @@ if (MORE_DEBUG) Slog.v(TAG, " + got " + nRead + "; now wanting " + (size - soF RUNNING_QUEUE, RESTORE_KEYVALUE, RESTORE_FULL, + RESTORE_FINISHED, FINAL } @@ -6343,6 +6345,9 @@ if (MORE_DEBUG) Slog.v(TAG, " + got " + nRead + "; now wanting " + (size - soF // Our bookkeeping about the ancestral dataset private PackageManagerBackupAgent mPmAgent; + // Currently-bound backup agent for restore + restoreFinished purposes + private IBackupAgent mAgent; + // What sort of restore we're doing now private RestoreDescription mRestoreDescription; @@ -6441,6 +6446,13 @@ if (MORE_DEBUG) Slog.v(TAG, " + got " + nRead + "; now wanting " + (size - soF // this one is always valid too } } + + if (MORE_DEBUG) { + Slog.v(TAG, "Restore; accept set size is " + mAcceptSet.size()); + for (PackageInfo info : mAcceptSet) { + Slog.v(TAG, " " + info.packageName); + } + } } private String[] packagesToNames(List apps) { @@ -6473,6 +6485,10 @@ if (MORE_DEBUG) Slog.v(TAG, " + got " + nRead + "; now wanting " + (size - soF restoreFull(); break; + case RESTORE_FINISHED: + restoreFinished(); + break; + case FINAL: if (!mFinished) finalizeRestore(); else { @@ -6529,7 +6545,7 @@ if (MORE_DEBUG) Slog.v(TAG, " + got " + nRead + "; now wanting " + (size - soF private void startRestore() { sendStartRestore(mAcceptSet.size()); - UnifiedRestoreState nextState = UnifiedRestoreState.RUNNING_QUEUE; + UnifiedRestoreState nextState = UnifiedRestoreState.RESTORE_FINISHED; try { // If we don't yet have PM metadata for this token, synthesize an // entry for the PM pseudopackage and make it the first to be @@ -6544,6 +6560,7 @@ if (MORE_DEBUG) Slog.v(TAG, " + got " + nRead + "; now wanting " + (size - soF mPmAgent = new PackageManagerBackupAgent(metadataFile); } catch (IOException e) { // Nope, we need to get it via restore + if (MORE_DEBUG) Slog.v(TAG, "Need to restore @pm@"); PackageInfo pmPackage = new PackageInfo(); pmPackage.packageName = PACKAGE_MANAGER_SENTINEL; mAcceptSet.add(0, pmPackage); @@ -6581,8 +6598,11 @@ if (MORE_DEBUG) Slog.v(TAG, " + got " + nRead + "; now wanting " + (size - soF mCurrentPackage = new PackageInfo(); mCurrentPackage.packageName = PACKAGE_MANAGER_SENTINEL; mPmAgent = new PackageManagerBackupAgent(mPackageManager, null); - initiateOneRestore(mCurrentPackage, 0, - IBackupAgent.Stub.asInterface(mPmAgent.onBind())); + mAgent = IBackupAgent.Stub.asInterface(mPmAgent.onBind()); + if (MORE_DEBUG) { + Slog.v(TAG, "initiating restore for PMBA"); + } + initiateOneRestore(mCurrentPackage, 0); // The PM agent called operationComplete() already, because our invocation // of it is process-local and therefore synchronous. That means that a // RUNNING_QUEUE message is already enqueued. Only if we're unable to @@ -6621,6 +6641,11 @@ if (MORE_DEBUG) Slog.v(TAG, " + got " + nRead + "; now wanting " + (size - soF nextState = UnifiedRestoreState.FINAL; return; } + } else { + // We have the PMBA already, so we can proceed directly to + // the RUNNING_QUEUE state ourselves. + if (MORE_DEBUG) Slog.v(TAG, "PMBA from cache; proceeding to run queue"); + nextState = UnifiedRestoreState.RUNNING_QUEUE; } } catch (RemoteException e) { // If we lost the transport at any time, halt @@ -6762,10 +6787,10 @@ if (MORE_DEBUG) Slog.v(TAG, " + got " + nRead + "; now wanting " + (size - soF } // Good to go! Set up and bind the agent... - IBackupAgent agent = bindToAgentSynchronous( + mAgent = bindToAgentSynchronous( mCurrentPackage.applicationInfo, IApplicationThread.BACKUP_MODE_INCREMENTAL); - if (agent == null) { + if (mAgent == null) { Slog.w(TAG, "Can't find backup agent for " + packageName); EventLog.writeEvent(EventLogTags.RESTORE_AGENT_FAILURE, packageName, "Restore agent missing"); @@ -6775,7 +6800,7 @@ if (MORE_DEBUG) Slog.v(TAG, " + got " + nRead + "; now wanting " + (size - soF // And then finally start the restore on this agent try { - initiateOneRestore(mCurrentPackage, metaInfo.versionCode, agent); + initiateOneRestore(mCurrentPackage, metaInfo.versionCode); ++mCount; } catch (Exception e) { Slog.e(TAG, "Error when attempting restore: " + e.toString()); @@ -6785,7 +6810,7 @@ if (MORE_DEBUG) Slog.v(TAG, " + got " + nRead + "; now wanting " + (size - soF } // Guts of a key/value restore operation - void initiateOneRestore(PackageInfo app, int appVersionCode, IBackupAgent agent) { + void initiateOneRestore(PackageInfo app, int appVersionCode) { final String packageName = app.packageName; if (DEBUG) Slog.d(TAG, "initiateOneRestore packageName=" + packageName); @@ -6881,7 +6906,7 @@ if (MORE_DEBUG) Slog.v(TAG, " + got " + nRead + "; now wanting " + (size - soF // the operationComplete() callback will schedule the next step, // so we do not do that here. prepareOperationTimeout(token, TIMEOUT_RESTORE_INTERVAL, this); - agent.doRestore(mBackupData, appVersionCode, mNewState, + mAgent.doRestore(mBackupData, appVersionCode, mNewState, token, mBackupManagerBinder); } catch (Exception e) { Slog.e(TAG, "Unable to call app for restore: " + packageName, e); @@ -6926,6 +6951,20 @@ if (MORE_DEBUG) Slog.v(TAG, " + got " + nRead + "; now wanting " + (size - soF } } + // state RESTORE_FINISHED : provide the "no more data" signpost callback at the end + private void restoreFinished() { + try { + final int token = generateToken(); + prepareOperationTimeout(token, TIMEOUT_RESTORE_FINISHED_INTERVAL, this); + mAgent.doRestoreFinished(token, mBackupManagerBinder); + // If we get this far, the callback or timeout will schedule the + // next restore state, so we're done + } catch (Exception e) { + Slog.e(TAG, "Unable to finalize restore of " + mCurrentPackage.packageName); + executeNextState(UnifiedRestoreState.FINAL); + } + } + class StreamFeederThread extends RestoreEngine implements Runnable { final String TAG = "StreamFeederThread"; FullRestoreEngine mEngine; @@ -7202,23 +7241,58 @@ if (MORE_DEBUG) Slog.v(TAG, " + got " + nRead + "; now wanting " + (size - soF @Override public void operationComplete() { - int size = (int) mBackupDataName.length(); - EventLog.writeEvent(EventLogTags.RESTORE_PACKAGE, mCurrentPackage.packageName, size); + if (MORE_DEBUG) { + Slog.i(TAG, "operationComplete() during restore: target=" + + mCurrentPackage.packageName + + " state=" + mState); + } - // Just go back to running the restore queue - keyValueAgentCleanup(); + final UnifiedRestoreState nextState; + switch (mState) { + case RESTORE_KEYVALUE: + case RESTORE_FULL: { + // Okay, we've just heard back from the agent that it's done with + // the restore itself. We now have to send the same agent its + // doRestoreFinished() callback, so roll into that state. + nextState = UnifiedRestoreState.RESTORE_FINISHED; + break; + } + + case RESTORE_FINISHED: { + // Okay, we're done with this package. Tidy up and go on to the next + // app in the queue. + int size = (int) mBackupDataName.length(); + EventLog.writeEvent(EventLogTags.RESTORE_PACKAGE, + mCurrentPackage.packageName, size); + + // Just go back to running the restore queue + keyValueAgentCleanup(); + + // If there was widget state associated with this app, get the OS to + // incorporate it into current bookeeping and then pass that along to + // the app as part of the restore-time work. + if (mWidgetData != null) { + restoreWidgetData(mCurrentPackage.packageName, mWidgetData); + } - // If there was widget state associated with this app, get the OS to - // incorporate it into current bookeeping and then pass that along to - // the app as part of the restore operation. - if (mWidgetData != null) { - restoreWidgetData(mCurrentPackage.packageName, mWidgetData); + nextState = UnifiedRestoreState.RUNNING_QUEUE; + break; + } + + default: { + // Some kind of horrible semantic error; we're in an unexpected state. + // Back off hard and wind up. + Slog.e(TAG, "Unexpected restore callback into state " + mState); + keyValueAgentErrorCleanup(); + nextState = UnifiedRestoreState.FINAL; + break; + } } - executeNextState(UnifiedRestoreState.RUNNING_QUEUE); + executeNextState(nextState); } - // A call to agent.doRestore() has timed out + // A call to agent.doRestore() or agent.doRestoreFinished() has timed out @Override public void handleTimeout() { Slog.e(TAG, "Timeout restoring application " + mCurrentPackage.packageName); -- cgit v1.1