From d5965cb506bde84612109bf26c3fcc6712ca91e5 Mon Sep 17 00:00:00 2001 From: Sascha Prueter Date: Tue, 19 Nov 2013 06:51:21 +0000 Subject: Trying to unbreak build... Revert "Harden against transiently unavailable backup transports" This reverts commit 8f98252afea3fd0e68693635ec21b6004a52fa69. Change-Id: I3aabb80f1a5932d530bce6b82d4b30c6cd1cdd5a --- cmds/bmgr/src/com/android/commands/bmgr/Bmgr.java | 19 +- core/java/android/app/backup/IBackupManager.aidl | 4 +- .../com/android/server/BackupManagerService.java | 217 ++++++--------------- 3 files changed, 70 insertions(+), 170 deletions(-) diff --git a/cmds/bmgr/src/com/android/commands/bmgr/Bmgr.java b/cmds/bmgr/src/com/android/commands/bmgr/Bmgr.java index db3d8bb..2666b41 100644 --- a/cmds/bmgr/src/com/android/commands/bmgr/Bmgr.java +++ b/cmds/bmgr/src/com/android/commands/bmgr/Bmgr.java @@ -187,12 +187,6 @@ public final class Bmgr { } private void doWipe() { - String transport = nextArg(); - if (transport == null) { - showUsage(); - return; - } - String pkg = nextArg(); if (pkg == null) { showUsage(); @@ -200,8 +194,8 @@ public final class Bmgr { } try { - mBmgr.clearBackupData(transport, pkg); - System.out.println("Wiped backup data for " + pkg + " on " + transport); + mBmgr.clearBackupData(pkg); + System.out.println("Wiped backup data for " + pkg); } catch (RemoteException e) { System.err.println(e.toString()); System.err.println(BMGR_NOT_RUNNING_ERR); @@ -452,7 +446,7 @@ public final class Bmgr { System.err.println(" bmgr restore TOKEN PACKAGE..."); System.err.println(" bmgr restore PACKAGE"); System.err.println(" bmgr run"); - System.err.println(" bmgr wipe TRANSPORT PACKAGE"); + System.err.println(" bmgr wipe PACKAGE"); System.err.println(""); System.err.println("The 'backup' command schedules a backup pass for the named package."); System.err.println("Note that the backup pass will effectively be a no-op if the package"); @@ -468,8 +462,8 @@ public final class Bmgr { System.err.println(""); System.err.println("The 'list transports' command reports the names of the backup transports"); System.err.println("currently available on the device. These names can be passed as arguments"); - System.err.println("to the 'transport' and 'wipe' commands. The currently selected transport"); - System.err.println("is indicated with a '*' character."); + System.err.println("to the 'transport' command. The currently selected transport is indicated"); + System.err.println("with a '*' character."); System.err.println(""); System.err.println("The 'list sets' command reports the token and name of each restore set"); System.err.println("available to the device via the current transport."); @@ -497,8 +491,7 @@ public final class Bmgr { System.err.println("data changes."); System.err.println(""); System.err.println("The 'wipe' command causes all backed-up data for the given package to be"); - System.err.println("erased from the given transport's storage. The next backup operation"); + System.err.println("erased from the current transport's storage. The next backup operation"); System.err.println("that the given application performs will rewrite its entire data set."); - System.err.println("Transport names to use here are those reported by 'list transports'."); } } diff --git a/core/java/android/app/backup/IBackupManager.aidl b/core/java/android/app/backup/IBackupManager.aidl index 12ee3b6..bb4f5f1 100644 --- a/core/java/android/app/backup/IBackupManager.aidl +++ b/core/java/android/app/backup/IBackupManager.aidl @@ -43,14 +43,14 @@ interface IBackupManager { void dataChanged(String packageName); /** - * Erase all backed-up data for the given package from the given storage + * Erase all backed-up data for the given package from the storage * destination. * * Any application can invoke this method for its own package, but * only callers who hold the android.permission.BACKUP permission * may invoke it for arbitrary packages. */ - void clearBackupData(String transportName, String packageName); + void clearBackupData(String packageName); /** * Notifies the Backup Manager Service that an agent has become available. This diff --git a/services/java/com/android/server/BackupManagerService.java b/services/java/com/android/server/BackupManagerService.java index 744ea77..baef607 100644 --- a/services/java/com/android/server/BackupManagerService.java +++ b/services/java/com/android/server/BackupManagerService.java @@ -161,9 +161,6 @@ class BackupManagerService extends IBackupManager.Stub { // the first backup pass. private static final long FIRST_BACKUP_INTERVAL = 12 * AlarmManager.INTERVAL_HOUR; - // Retry interval for clear/init when the transport is unavailable - private static final long TRANSPORT_RETRY_INTERVAL = 1 * AlarmManager.INTERVAL_HOUR; - private static final String RUN_BACKUP_ACTION = "android.app.backup.intent.RUN"; private static final String RUN_INITIALIZE_ACTION = "android.app.backup.intent.INIT"; private static final String RUN_CLEAR_ACTION = "android.app.backup.intent.CLEAR"; @@ -177,8 +174,6 @@ class BackupManagerService extends IBackupManager.Stub { private static final int MSG_RESTORE_TIMEOUT = 8; private static final int MSG_FULL_CONFIRMATION_TIMEOUT = 9; private static final int MSG_RUN_FULL_RESTORE = 10; - private static final int MSG_RETRY_INIT = 11; - private static final int MSG_RETRY_CLEAR = 12; // backup task state machine tick static final int MSG_BACKUP_RESTORE_STEP = 20; @@ -311,7 +306,6 @@ class BackupManagerService extends IBackupManager.Stub { class RestoreParams { public IBackupTransport transport; - public String dirName; public IRestoreObserver observer; public long token; public PackageInfo pkgInfo; @@ -319,10 +313,9 @@ class BackupManagerService extends IBackupManager.Stub { public boolean needFullBackup; public String[] filterSet; - RestoreParams(IBackupTransport _transport, String _dirName, IRestoreObserver _obs, + RestoreParams(IBackupTransport _transport, IRestoreObserver _obs, long _token, PackageInfo _pkg, int _pmToken, boolean _needFullBackup) { transport = _transport; - dirName = _dirName; observer = _obs; token = _token; pkgInfo = _pkg; @@ -331,10 +324,9 @@ class BackupManagerService extends IBackupManager.Stub { filterSet = null; } - RestoreParams(IBackupTransport _transport, String _dirName, IRestoreObserver _obs, - long _token, boolean _needFullBackup) { + RestoreParams(IBackupTransport _transport, IRestoreObserver _obs, long _token, + boolean _needFullBackup) { transport = _transport; - dirName = _dirName; observer = _obs; token = _token; pkgInfo = null; @@ -343,10 +335,9 @@ class BackupManagerService extends IBackupManager.Stub { filterSet = null; } - RestoreParams(IBackupTransport _transport, String _dirName, IRestoreObserver _obs, - long _token, String[] _filterSet, boolean _needFullBackup) { + RestoreParams(IBackupTransport _transport, IRestoreObserver _obs, long _token, + String[] _filterSet, boolean _needFullBackup) { transport = _transport; - dirName = _dirName; observer = _obs; token = _token; pkgInfo = null; @@ -366,16 +357,6 @@ class BackupManagerService extends IBackupManager.Stub { } } - class ClearRetryParams { - public String transportName; - public String packageName; - - ClearRetryParams(String transport, String pkg) { - transportName = transport; - packageName = pkg; - } - } - class FullParams { public ParcelFileDescriptor fd; public final AtomicBoolean latch; @@ -535,28 +516,13 @@ class BackupManagerService extends IBackupManager.Stub { // When it completes successfully, that old journal file will be // deleted. If we crash prior to that, the old journal is parsed // at next boot and the journaled requests fulfilled. - boolean staged = true; if (queue.size() > 0) { // Spin up a backup state sequence and set it running - try { - String dirName = transport.transportDirName(); - PerformBackupTask pbt = new PerformBackupTask(transport, dirName, - queue, oldJournal); - Message pbtMessage = obtainMessage(MSG_BACKUP_RESTORE_STEP, pbt); - sendMessage(pbtMessage); - } catch (RemoteException e) { - // unable to ask the transport its dir name -- transient failure, since - // the above check succeeded. Try again next time. - Slog.e(TAG, "Transport became unavailable attempting backup"); - staged = false; - } + PerformBackupTask pbt = new PerformBackupTask(transport, queue, oldJournal); + Message pbtMessage = obtainMessage(MSG_BACKUP_RESTORE_STEP, pbt); + sendMessage(pbtMessage); } else { Slog.v(TAG, "Backup requested but nothing pending"); - staged = false; - } - - if (!staged) { - // if we didn't actually hand off the wakelock, rewind until next time synchronized (mQueueLock) { mBackupRunning = false; } @@ -606,7 +572,7 @@ class BackupManagerService extends IBackupManager.Stub { RestoreParams params = (RestoreParams)msg.obj; Slog.d(TAG, "MSG_RUN_RESTORE observer=" + params.observer); PerformRestoreTask task = new PerformRestoreTask( - params.transport, params.dirName, params.observer, + params.transport, params.observer, params.token, params.pkgInfo, params.pmToken, params.needFullBackup, params.filterSet); Message restoreMsg = obtainMessage(MSG_BACKUP_RESTORE_STEP, task); @@ -633,14 +599,6 @@ class BackupManagerService extends IBackupManager.Stub { break; } - case MSG_RETRY_CLEAR: - { - // reenqueues if the transport remains unavailable - ClearRetryParams params = (ClearRetryParams)msg.obj; - clearBackupData(params.transportName, params.packageName); - break; - } - case MSG_RUN_INITIALIZE: { HashSet queue; @@ -655,16 +613,6 @@ class BackupManagerService extends IBackupManager.Stub { break; } - case MSG_RETRY_INIT: - { - synchronized (mQueueLock) { - recordInitPendingLocked(msg.arg1 != 0, (String)msg.obj); - mAlarmManager.set(AlarmManager.RTC_WAKEUP, System.currentTimeMillis(), - mRunInitIntent); - } - break; - } - case MSG_RUN_GET_RESTORE_SETS: { // Like other async operations, this is entered with the wakelock held @@ -1302,47 +1250,29 @@ class BackupManagerService extends IBackupManager.Stub { void recordInitPendingLocked(boolean isPending, String transportName) { if (DEBUG) Slog.i(TAG, "recordInitPendingLocked: " + isPending + " on transport " + transportName); - mBackupHandler.removeMessages(MSG_RETRY_INIT); - try { IBackupTransport transport = getTransport(transportName); - if (transport != null) { - String transportDirName = transport.transportDirName(); - File stateDir = new File(mBaseStateDir, transportDirName); - File initPendingFile = new File(stateDir, INIT_SENTINEL_FILE_NAME); - - if (isPending) { - // We need an init before we can proceed with sending backup data. - // Record that with an entry in our set of pending inits, as well as - // journaling it via creation of a sentinel file. - mPendingInits.add(transportName); - try { - (new FileOutputStream(initPendingFile)).close(); - } catch (IOException ioe) { - // Something is badly wrong with our permissions; just try to move on - } - } else { - // No more initialization needed; wipe the journal and reset our state. - initPendingFile.delete(); - mPendingInits.remove(transportName); + String transportDirName = transport.transportDirName(); + File stateDir = new File(mBaseStateDir, transportDirName); + File initPendingFile = new File(stateDir, INIT_SENTINEL_FILE_NAME); + + if (isPending) { + // We need an init before we can proceed with sending backup data. + // Record that with an entry in our set of pending inits, as well as + // journaling it via creation of a sentinel file. + mPendingInits.add(transportName); + try { + (new FileOutputStream(initPendingFile)).close(); + } catch (IOException ioe) { + // Something is badly wrong with our permissions; just try to move on } - return; // done; don't fall through to the error case + } else { + // No more initialization needed; wipe the journal and reset our state. + initPendingFile.delete(); + mPendingInits.remove(transportName); } } catch (RemoteException e) { - // transport threw when asked its name; fall through to the lookup-failed case - } - - // The named transport doesn't exist or threw. This operation is - // important, so we record the need for a an init and post a message - // to retry the init later. - if (isPending) { - mPendingInits.add(transportName); - mBackupHandler.sendMessageDelayed( - mBackupHandler.obtainMessage(MSG_RETRY_INIT, - (isPending ? 1 : 0), - 0, - transportName), - TRANSPORT_RETRY_INTERVAL); + // can't happen; the transport is local } } @@ -1418,10 +1348,7 @@ class BackupManagerService extends IBackupManager.Stub { } } } catch (RemoteException e) { - // the transport threw when asked its file naming prefs; declare it invalid - Slog.e(TAG, "Unable to register transport as " + name); - mTransportNames.remove(component); - mTransports.remove(name); + // can't happen, the transport is local } } @@ -1741,7 +1668,7 @@ class BackupManagerService extends IBackupManager.Stub { agent = mConnectedAgent; } } catch (RemoteException e) { - // can't happen - ActivityManager is local + // can't happen } } return agent; @@ -1917,13 +1844,17 @@ class BackupManagerService extends IBackupManager.Stub { int mStatus; boolean mFinished; - public PerformBackupTask(IBackupTransport transport, String dirName, - ArrayList queue, File journal) { + public PerformBackupTask(IBackupTransport transport, ArrayList queue, + File journal) { mTransport = transport; mOriginalQueue = queue; mJournal = journal; - mStateDir = new File(mBaseStateDir, dirName); + try { + mStateDir = new File(mBaseStateDir, transport.transportDirName()); + } catch (RemoteException e) { + // can't happen; the transport is local + } mCurrentState = BackupState.INITIAL; mFinished = false; @@ -2169,12 +2100,8 @@ class BackupManagerService extends IBackupManager.Stub { addBackupTrace("success; recording token"); try { mCurrentToken = mTransport.getCurrentRestoreSet(); - writeRestoreTokens(); - } catch (RemoteException e) { - // nothing for it at this point, unfortunately, but this will be - // recorded the next time we fully succeed. - addBackupTrace("transport threw returning token"); - } + } catch (RemoteException e) {} // can't happen + writeRestoreTokens(); } // Set up the next backup pass - at this point we can set mBackupRunning @@ -2395,7 +2322,7 @@ class BackupManagerService extends IBackupManager.Stub { addBackupTrace("unbinding " + mCurrentPackage.packageName); try { // unbind even on timeout, just in case mActivityManager.unbindBackupAgent(mCurrentPackage.applicationInfo); - } catch (RemoteException e) { /* can't happen; activity manager is local */ } + } catch (RemoteException e) {} } } @@ -4410,7 +4337,7 @@ class BackupManagerService extends IBackupManager.Stub { } } - PerformRestoreTask(IBackupTransport transport, String dirName, IRestoreObserver observer, + PerformRestoreTask(IBackupTransport transport, IRestoreObserver observer, long restoreSetToken, PackageInfo targetPackage, int pmToken, boolean needFullBackup, String[] filterSet) { mCurrentState = RestoreState.INITIAL; @@ -4433,7 +4360,11 @@ class BackupManagerService extends IBackupManager.Stub { mFilterSet = null; } - mStateDir = new File(mBaseStateDir, dirName); + try { + mStateDir = new File(mBaseStateDir, transport.transportDirName()); + } catch (RemoteException e) { + // can't happen; the transport is local + } } // Execute one tick of whatever state machine the task implements @@ -5159,8 +5090,8 @@ class BackupManagerService extends IBackupManager.Stub { } // Clear the given package's backup data from the current transport - public void clearBackupData(String transportName, String packageName) { - if (DEBUG) Slog.v(TAG, "clearBackupData() of " + packageName + " on " + transportName); + public void clearBackupData(String packageName) { + if (DEBUG) Slog.v(TAG, "clearBackupData() of " + packageName); PackageInfo info; try { info = mPackageManager.getPackageInfo(packageName, PackageManager.GET_SIGNATURES); @@ -5191,22 +5122,13 @@ class BackupManagerService extends IBackupManager.Stub { // Is the given app an available participant? if (apps.contains(packageName)) { - // found it; fire off the clear request if (DEBUG) Slog.v(TAG, "Found the app - running clear process"); - mBackupHandler.removeMessages(MSG_RETRY_CLEAR); + // found it; fire off the clear request synchronized (mQueueLock) { - final IBackupTransport transport = getTransport(transportName); - if (transport == null) { - // transport is currently unavailable -- make sure to retry - Message msg = mBackupHandler.obtainMessage(MSG_RETRY_CLEAR, - new ClearRetryParams(transportName, packageName)); - mBackupHandler.sendMessageDelayed(msg, TRANSPORT_RETRY_INTERVAL); - return; - } long oldId = Binder.clearCallingIdentity(); mWakelock.acquire(); Message msg = mBackupHandler.obtainMessage(MSG_RUN_CLEAR, - new ClearParams(transport, info)); + new ClearParams(getTransport(mCurrentTransport), info)); mBackupHandler.sendMessage(msg); Binder.restoreCallingIdentity(oldId); } @@ -5704,36 +5626,21 @@ class BackupManagerService extends IBackupManager.Stub { + " restoreSet=" + Long.toHexString(restoreSet)); if (mAutoRestore && mProvisioned && restoreSet != 0) { - // Do we have a transport to fetch data for us? - IBackupTransport transport = getTransport(mCurrentTransport); - if (transport == null) { - if (DEBUG) Slog.w(TAG, "No transport for install-time restore"); - return; - } - - try { - // okay, we're going to attempt a restore of this package from this restore set. - // The eventual message back into the Package Manager to run the post-install - // steps for 'token' will be issued from the restore handling code. - - // This can throw and so *must* happen before the wakelock is acquired - String dirName = transport.transportDirName(); + // okay, we're going to attempt a restore of this package from this restore set. + // The eventual message back into the Package Manager to run the post-install + // steps for 'token' will be issued from the restore handling code. - // We can use a synthetic PackageInfo here because: - // 1. We know it's valid, since the Package Manager supplied the name - // 2. Only the packageName field will be used by the restore code - PackageInfo pkg = new PackageInfo(); - pkg.packageName = packageName; + // We can use a synthetic PackageInfo here because: + // 1. We know it's valid, since the Package Manager supplied the name + // 2. Only the packageName field will be used by the restore code + PackageInfo pkg = new PackageInfo(); + pkg.packageName = packageName; - mWakelock.acquire(); - Message msg = mBackupHandler.obtainMessage(MSG_RUN_RESTORE); - msg.obj = new RestoreParams(transport, dirName, null, - restoreSet, pkg, token, true); - mBackupHandler.sendMessage(msg); - } catch (RemoteException e) { - // Binding to the transport broke; back off and proceed with the installation. - Slog.e(TAG, "Unable to contact transport for install-time restore"); - } + mWakelock.acquire(); + Message msg = mBackupHandler.obtainMessage(MSG_RUN_RESTORE); + msg.obj = new RestoreParams(getTransport(mCurrentTransport), null, + restoreSet, pkg, token, true); + mBackupHandler.sendMessage(msg); } else { // Auto-restore disabled or no way to attempt a restore; just tell the Package // Manager to proceed with the post-install handling for this package. -- cgit v1.1