From d0c42dc67839c774aba20821c9d45e66d2b3e61a Mon Sep 17 00:00:00 2001
From: Ruslan Tkhakokhov <rthakohov@google.com>
Date: Wed, 23 Mar 2022 14:58:28 +0000
Subject: [PATCH] Handle CancellationException in BackupTransportClient

Logic added in ag/16968183 can produce a CancellationException which is
not currently caught, resulting in system server crashes (see attached
bug). Catch the exception and update tests accordingly.

Bug: 224777563
Test: atest BackupTransportClientTest
Change-Id: Ibee991dbc9673c7b6b4d0051f09b4e8862ad6af4
---
 .../server/backup/transport/BackupTransportClient.java | 10 ++++++++--
 .../backup/transport/BackupTransportClientTest.java    |  7 ++-----
 2 files changed, 10 insertions(+), 7 deletions(-)

diff --git a/services/backup/backuplib/java/com/android/server/backup/transport/BackupTransportClient.java b/services/backup/backuplib/java/com/android/server/backup/transport/BackupTransportClient.java
index 7e3ede15aa04d..d75d6484bec31 100644
--- a/services/backup/backuplib/java/com/android/server/backup/transport/BackupTransportClient.java
+++ b/services/backup/backuplib/java/com/android/server/backup/transport/BackupTransportClient.java
@@ -34,6 +34,7 @@ import java.util.HashSet;
 import java.util.List;
 import java.util.Queue;
 import java.util.Set;
+import java.util.concurrent.CancellationException;
 import java.util.concurrent.ExecutionException;
 import java.util.concurrent.TimeUnit;
 import java.util.concurrent.TimeoutException;
@@ -374,7 +375,8 @@ public class BackupTransportClient {
     private <T> T getFutureResult(AndroidFuture<T> future) {
         try {
             return future.get(600, TimeUnit.SECONDS);
-        } catch (InterruptedException | ExecutionException | TimeoutException e) {
+        } catch (InterruptedException | ExecutionException | TimeoutException
+                | CancellationException e) {
             Slog.w(TAG, "Failed to get result from transport:", e);
             return null;
         } finally {
@@ -403,7 +405,11 @@ public class BackupTransportClient {
         void cancelActiveFutures() {
             synchronized (mActiveFuturesLock) {
                 for (AndroidFuture<?> future : mActiveFutures) {
-                    future.cancel(true);
+                    try {
+                        future.cancel(true);
+                    } catch (CancellationException ignored) {
+                        // This is expected, so ignore the exception.
+                    }
                 }
                 mActiveFutures.clear();
             }
diff --git a/services/tests/servicestests/src/com/android/server/backup/transport/BackupTransportClientTest.java b/services/tests/servicestests/src/com/android/server/backup/transport/BackupTransportClientTest.java
index b154d6f6db0c6..1171518130cc0 100644
--- a/services/tests/servicestests/src/com/android/server/backup/transport/BackupTransportClientTest.java
+++ b/services/tests/servicestests/src/com/android/server/backup/transport/BackupTransportClientTest.java
@@ -110,10 +110,7 @@ public class BackupTransportClientTest {
 
         Thread thread = new Thread(() -> {
             try {
-                /*String name =*/ client.transportDirName();
-                fail("transportDirName should be cancelled");
-            } catch (CancellationException ex) {
-                // This is expected.
+                assertThat(client.transportDirName()).isNull();
             } catch (Exception ex) {
                 fail("unexpected Exception: " + ex.getClass().getCanonicalName());
             }
@@ -189,7 +186,7 @@ public class BackupTransportClientTest {
     }
 
     @Test
-    public void testFinishBackup_canceledBeforeCompletion_throwsException() throws Exception {
+    public void testFinishBackup_canceledBeforeCompletion_returnsError() throws Exception {
         TestCallbacksFakeTransportBinder binder = new TestCallbacksFakeTransportBinder();
         BackupTransportClient client = new BackupTransportClient(binder);