Merge "Offload teardown logic after BackupAgent crash to a worker thread" into tm-qpr-dev

services/backup/java/com/android/server/backup/UserBackupManagerService.java

@@ -3722,21 +3722,34 @@ public class UserBackupManagerService {
             Slog.w(TAG, "agentDisconnected: the backup agent for " + packageName
                     + " died: cancel current operations");
 
-            // handleCancel() causes the PerformFullTransportBackupTask to go on to
+            // Offload operation cancellation off the main thread as the cancellation callbacks
-            // tearDownAgentAndKill: that will unbindBackupAgent in the Activity Manager, so
+            // might call out to BackupTransport. Other operations started on the same package
-            // that the package being backed up doesn't get stuck in restricted mode until the
+            // before the cancellation callback has executed will also be cancelled by the callback.
-            // backup time-out elapses.
+            Runnable cancellationRunnable = () -> {
-            for (int token : mOperationStorage.operationTokensForPackage(packageName)) {
+                // handleCancel() causes the PerformFullTransportBackupTask to go on to
-                if (MORE_DEBUG) {
+                // tearDownAgentAndKill: that will unbindBackupAgent in the Activity Manager, so
-                    Slog.d(TAG, "agentDisconnected: will handleCancel(all) for token:"
+                // that the package being backed up doesn't get stuck in restricted mode until the
-                            + Integer.toHexString(token));
+                // backup time-out elapses.
+                for (int token : mOperationStorage.operationTokensForPackage(packageName)) {
+                    if (MORE_DEBUG) {
+                        Slog.d(TAG, "agentDisconnected: will handleCancel(all) for token:"
+                                + Integer.toHexString(token));
+                    }
+                    handleCancel(token, true /* cancelAll */);
                 }
-                handleCancel(token, true /* cancelAll */);
+            };
-            }
+            getThreadForAsyncOperation(/* operationName */ "agent-disconnected",
+                    cancellationRunnable).start();
+
             mAgentConnectLock.notifyAll();
         }
     }
 
+    @VisibleForTesting
+    Thread getThreadForAsyncOperation(String operationName, Runnable operation) {
+        return new Thread(operation, operationName);
+    }
+
     /**
      * An application being installed will need a restore pass, then the {@link PackageManager} will
      * need to be told when the restore is finished.

services/tests/servicestests/src/com/android/server/backup/UserBackupManagerServiceTest.java

@@ -18,6 +18,7 @@ package com.android.server.backup;
 
 import static com.google.common.truth.Truth.assertThat;
 
+import static org.junit.Assert.fail;
 import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.ArgumentMatchers.anyBoolean;
 import static org.mockito.ArgumentMatchers.anyInt;
@@ -61,6 +62,7 @@ import java.util.function.IntConsumer;
 public class UserBackupManagerServiceTest {
     private static final String TEST_PACKAGE = "package1";
     private static final String[] TEST_PACKAGES = new String[] { TEST_PACKAGE };
+    private static final int WORKER_THREAD_TIMEOUT_MILLISECONDS = 1;
 
     @Mock Context mContext;
     @Mock IBackupManagerMonitor mBackupManagerMonitor;
@@ -179,6 +181,7 @@ public class UserBackupManagerServiceTest {
 
         mService.agentDisconnected("com.android.foo");
 
+        mService.waitForAsyncOperation();
         verify(mOperationStorage).cancelOperation(eq(123), eq(true), any(IntConsumer.class));
         verify(mOperationStorage).cancelOperation(eq(456), eq(true), any());
         verify(mOperationStorage).cancelOperation(eq(789), eq(true), any());
@@ -207,6 +210,8 @@ public class UserBackupManagerServiceTest {
         boolean isEnabledStatePersisted = false;
         boolean shouldUseNewBackupEligibilityRules = false;
 
+        private volatile Thread mWorkerThread = null;
+
         TestBackupService(Context context, PackageManager packageManager,
                 LifecycleOperationStorage operationStorage) {
             super(context, packageManager, operationStorage);
@@ -229,5 +234,23 @@ public class UserBackupManagerServiceTest {
         boolean shouldUseNewBackupEligibilityRules() {
             return shouldUseNewBackupEligibilityRules;
         }
+
+        @Override
+        Thread getThreadForAsyncOperation(String operationName, Runnable operation) {
+            mWorkerThread = super.getThreadForAsyncOperation(operationName, operation);
+            return mWorkerThread;
+        }
+
+        private void waitForAsyncOperation() {
+            if (mWorkerThread == null) {
+                return;
+            }
+
+            try {
+                mWorkerThread.join(/* millis */ WORKER_THREAD_TIMEOUT_MILLISECONDS);
+            } catch (InterruptedException e) {
+                fail("Failed waiting for worker thread to complete: " + e.getMessage());
+            }
+        }
     }
 }