Merge "Time out orphaned / unresponsive restore sessions"

1 files changed, 81 insertions, 18 deletions

diff --git a/services/java/com/android/server/BackupManagerService.java b/services/java/com/android/server/BackupManagerService.java
index 1617a4e..601fa21 100644
--- a/services/java/com/android/server/BackupManagerService.java
+++ b/services/java/com/android/server/BackupManagerService.java
@@ -107,6 +107,7 @@ class BackupManagerService extends IBackupManager.Stub {
     private static final int MSG_RUN_INITIALIZE = 5;
     private static final int MSG_RUN_GET_RESTORE_SETS = 6;
     private static final int MSG_TIMEOUT = 7;
+    private static final int MSG_RESTORE_TIMEOUT = 8;
 
     // Timeout interval for deciding that a bind or clear-data has taken too long
     static final long TIMEOUT_INTERVAL = 10 * 1000;
@@ -396,6 +397,21 @@ class BackupManagerService extends IBackupManager.Stub {
                 }
                 break;
             }
+
+            case MSG_RESTORE_TIMEOUT:
+            {
+                synchronized (BackupManagerService.this) {
+                    if (mActiveRestoreSession != null) {
+                        // Client app left the restore session dangling.  We know that it
+                        // can't be in the middle of an actual restore operation because
+                        // those are executed serially on this same handler thread.  Clean
+                        // up now.
+                        Slog.w(TAG, "Restore session timed out; aborting");
+                        post(mActiveRestoreSession.new EndRestoreRunnable(
+                                BackupManagerService.this, mActiveRestoreSession));
+                    }
+                }
+            }
             }
         }
     }
@@ -1826,6 +1842,11 @@ class BackupManagerService extends IBackupManager.Stub {
                     } catch (RemoteException e) { /* can't happen */ }
                 }
 
+                // Furthermore we need to reset the session timeout clock
+                mBackupHandler.removeMessages(MSG_RESTORE_TIMEOUT);
+                mBackupHandler.sendEmptyMessageDelayed(MSG_RESTORE_TIMEOUT,
+                        TIMEOUT_RESTORE_INTERVAL);
+
                 // done; we can finally release the wakelock
                 mWakelock.release();
             }
@@ -2506,10 +2527,23 @@ class BackupManagerService extends IBackupManager.Stub {
                 return null;
             }
             mActiveRestoreSession = new ActiveRestoreSession(packageName, transport);
+            mBackupHandler.sendEmptyMessageDelayed(MSG_RESTORE_TIMEOUT, TIMEOUT_RESTORE_INTERVAL);
         }
         return mActiveRestoreSession;
     }
 
+    void clearRestoreSession(ActiveRestoreSession currentSession) {
+        synchronized(this) {
+            if (currentSession != mActiveRestoreSession) {
+                Slog.e(TAG, "ending non-current restore session");
+            } else {
+                if (DEBUG) Slog.v(TAG, "Clearing restore session and halting timeout");
+                mActiveRestoreSession = null;
+                mBackupHandler.removeMessages(MSG_RESTORE_TIMEOUT);
+            }
+        }
+    }
+
     // Note that a currently-active backup agent has notified us that it has
     // completed the given outstanding asynchronous backup/restore operation.
     public void opComplete(int token) {
@@ -2528,6 +2562,7 @@ class BackupManagerService extends IBackupManager.Stub {
         private String mPackageName;
         private IBackupTransport mRestoreTransport = null;
         RestoreSet[] mRestoreSets = null;
+        boolean mEnded = false;
 
         ActiveRestoreSession(String packageName, String transport) {
             mPackageName = packageName;
@@ -2542,6 +2577,10 @@ class BackupManagerService extends IBackupManager.Stub {
                 throw new IllegalArgumentException("Observer must not be null");
             }
 
+            if (mEnded) {
+                throw new IllegalStateException("Restore session already ended");
+            }
+
             long oldId = Binder.clearCallingIdentity();
             try {
                 if (mRestoreTransport == null) {
@@ -2569,6 +2608,10 @@ class BackupManagerService extends IBackupManager.Stub {
             if (DEBUG) Slog.d(TAG, "restoreAll token=" + Long.toHexString(token)
                     + " observer=" + observer);
 
+            if (mEnded) {
+                throw new IllegalStateException("Restore session already ended");
+            }
+
             if (mRestoreTransport == null || mRestoreSets == null) {
                 Slog.e(TAG, "Ignoring restoreAll() with no restore set");
                 return -1;
@@ -2600,6 +2643,10 @@ class BackupManagerService extends IBackupManager.Stub {
         public synchronized int restorePackage(String packageName, IRestoreObserver observer) {
             if (DEBUG) Slog.v(TAG, "restorePackage pkg=" + packageName + " obs=" + observer);
 
+            if (mEnded) {
+                throw new IllegalStateException("Restore session already ended");
+            }
+
             if (mPackageName != null) {
                 if (! mPackageName.equals(packageName)) {
                     Slog.e(TAG, "Ignoring attempt to restore pkg=" + packageName
@@ -2656,31 +2703,47 @@ class BackupManagerService extends IBackupManager.Stub {
             return 0;
         }
 
-        public synchronized void endRestoreSession() {
-            if (DEBUG) Slog.d(TAG, "endRestoreSession");
+        // Posted to the handler to tear down a restore session in a cleanly synchronized way
+        class EndRestoreRunnable implements Runnable {
+            BackupManagerService mBackupManager;
+            ActiveRestoreSession mSession;
 
-            synchronized (this) {
-                long oldId = Binder.clearCallingIdentity();
-                try {
-                    if (mRestoreTransport != null) mRestoreTransport.finishRestore();
-                } catch (Exception e) {
-                    Slog.e(TAG, "Error in finishRestore", e);
-                } finally {
-                    mRestoreTransport = null;
-                    Binder.restoreCallingIdentity(oldId);
-                }
+            EndRestoreRunnable(BackupManagerService manager, ActiveRestoreSession session) {
+                mBackupManager = manager;
+                mSession = session;
             }
 
-            synchronized (BackupManagerService.this) {
-                if (BackupManagerService.this.mActiveRestoreSession == this) {
-                    BackupManagerService.this.mActiveRestoreSession = null;
-                } else {
-                    Slog.e(TAG, "ending non-current restore session");
+            public void run() {
+                // clean up the session's bookkeeping
+                synchronized (mSession) {
+                    try {
+                        if (mSession.mRestoreTransport != null) {
+                            mSession.mRestoreTransport.finishRestore();
+                        }
+                    } catch (Exception e) {
+                        Slog.e(TAG, "Error in finishRestore", e);
+                    } finally {
+                        mSession.mRestoreTransport = null;
+                        mSession.mEnded = true;
+                    }
                 }
+
+                // clean up the BackupManagerService side of the bookkeeping
+                // and cancel any pending timeout message
+                mBackupManager.clearRestoreSession(mSession);
             }
         }
-    }
 
+        public synchronized void endRestoreSession() {
+            if (DEBUG) Slog.d(TAG, "endRestoreSession");
+
+            if (mEnded) {
+                throw new IllegalStateException("Restore session already ended");
+            }
+
+            mBackupHandler.post(new EndRestoreRunnable(BackupManagerService.this, this));
+        }
+    }
 
     @Override
     public void dump(FileDescriptor fd, PrintWriter pw, String[] args) {