Don't let bmgr leave a restore session hanging on error

Specifically, don't wait for the RestoreObserver to be informed that the restore
has completed unless performRestore() ran.  We were winding up in a case where
bmgr was hanging forever waiting on a nonexistent restore process instead of
calling endRestoreSession().

Also improve the documentation, explicitly calling out the need to call
endRestoreSession() even if previous operations on the session were
unsuccessful.

3 files changed, 17 insertions, 8 deletions

diff --git a/cmds/bmgr/src/com/android/commands/bmgr/Bmgr.java b/cmds/bmgr/src/com/android/commands/bmgr/Bmgr.java
index 6b6d3c9..8c15d0b 100644
--- a/cmds/bmgr/src/com/android/commands/bmgr/Bmgr.java
+++ b/cmds/bmgr/src/com/android/commands/bmgr/Bmgr.java
@@ -315,8 +315,7 @@ public final class Bmgr {
                 for (RestoreSet s : sets) {
                     if (s.token == token) {
                         System.out.println("Scheduling restore: " + s.name);
-                        mRestore.performRestore(token, observer);
-                        didRestore = true;
+                        didRestore = (mRestore.performRestore(token, observer) == 0);
                         break;
                     }
                 }
@@ -330,12 +329,15 @@ public final class Bmgr {
                 }
             }
 
-            // now wait for it to be done
-            synchronized (observer) {
-                while (!observer.done) {
-                    try {
-                        observer.wait();
-                    } catch (InterruptedException ex) {
+            // if we kicked off a restore successfully, we have to wait for it
+            // to complete before we can shut down the restore session safely
+            if (didRestore) {
+                synchronized (observer) {
+                    while (!observer.done) {
+                        try {
+                            observer.wait();
+                        } catch (InterruptedException ex) {
+                        }
                     }
                 }
             }
diff --git a/core/java/android/backup/IRestoreSession.aidl b/core/java/android/backup/IRestoreSession.aidl
index 2a1fbc1..fd40d98 100644
--- a/core/java/android/backup/IRestoreSession.aidl
+++ b/core/java/android/backup/IRestoreSession.aidl
@@ -40,6 +40,8 @@ interface IRestoreSession {
      * Restore the given set onto the device, replacing the current data of any app
      * contained in the restore set with the data previously backed up.
      *
+     * @return Zero on success; nonzero on error.  The observer will only receive
+     *   progress callbacks if this method returned zero.
      * @param token The token from {@link getAvailableRestoreSets()} corresponding to
      *   the restore set that should be used.
      * @param observer If non-null, this binder points to an object that will receive
@@ -50,6 +52,9 @@ interface IRestoreSession {
     /**
      * End this restore session.  After this method is called, the IRestoreSession binder
      * is no longer valid.
+     *
+     * <p><b>Note:</b> The caller <i>must</i> invoke this method to end the restore session,
+     *   even if {@link getAvailableRestoreSets} or {@link performRestore} failed.
      */
     void endRestoreSession();
 }
diff --git a/services/java/com/android/server/BackupManagerService.java b/services/java/com/android/server/BackupManagerService.java
index 1e5302e..36dca7d 100644
--- a/services/java/com/android/server/BackupManagerService.java
+++ b/services/java/com/android/server/BackupManagerService.java
@@ -1725,6 +1725,8 @@ class BackupManagerService extends IBackupManager.Stub {
                     return 0;
                 }
             }
+
+            Log.w(TAG, "Restore token " + Long.toHexString(token) + " not found");
             return -1;
         }