From 3f82689d2b59f1437479ccb3fb69e662a98ac2fb Mon Sep 17 00:00:00 2001 From: Ruslan Tkhakokhov Date: Wed, 11 May 2022 00:02:12 +0100 Subject: [PATCH] Make BackupTransport more resilient Currently, if the transport implementation throws an exception, the framework will wait for response for the full timeout duration (10 minutes) before aborting the operation (see attached bug). 1. Add exception handlers to the transport wrapper that runs on the remote side (i.e. transport implementation side) - while exceptions are no longer propagated to system_server with introduction of async transport, we'd still like to know when one occurs to stop waiting for response (instead of relying on timeout). 2. Reduce the 10-minute timeout to 5 minutes - our data shows this is a reasonable value. Bug: 229670840 Test: 1. Manual (test B&R of any app) 2. atest CtsBackupHostTestCases Change-Id: Id97fa470fae65efc0ad587615a6e9edfd1e5f298 --- .../android/app/backup/BackupTransport.java | 218 +++++++++++++----- .../transport/TransportStatusCallback.java | 2 +- 2 files changed, 164 insertions(+), 56 deletions(-) diff --git a/core/java/android/app/backup/BackupTransport.java b/core/java/android/app/backup/BackupTransport.java index 9bb048d7a75f8..f6de72b43de68 100644 --- a/core/java/android/app/backup/BackupTransport.java +++ b/core/java/android/app/backup/BackupTransport.java @@ -665,184 +665,292 @@ public class BackupTransport { @Override public void name(AndroidFuture resultFuture) throws RemoteException { - String result = BackupTransport.this.name(); - resultFuture.complete(result); + try { + String result = BackupTransport.this.name(); + resultFuture.complete(result); + } catch (RuntimeException e) { + resultFuture.cancel(/* mayInterruptIfRunning */ true); + } } @Override public void configurationIntent(AndroidFuture resultFuture) throws RemoteException { - Intent result = BackupTransport.this.configurationIntent(); - resultFuture.complete(result); + try { + Intent result = BackupTransport.this.configurationIntent(); + resultFuture.complete(result); + } catch (RuntimeException e) { + resultFuture.cancel(/* mayInterruptIfRunning */ true); + } } @Override public void currentDestinationString(AndroidFuture resultFuture) throws RemoteException { - String result = BackupTransport.this.currentDestinationString(); - resultFuture.complete(result); + try { + String result = BackupTransport.this.currentDestinationString(); + resultFuture.complete(result); + } catch (RuntimeException e) { + resultFuture.cancel(/* mayInterruptIfRunning */ true); + } } @Override public void dataManagementIntent(AndroidFuture resultFuture) throws RemoteException { - Intent result = BackupTransport.this.dataManagementIntent(); - resultFuture.complete(result); + try { + Intent result = BackupTransport.this.dataManagementIntent(); + resultFuture.complete(result); + } catch (RuntimeException e) { + resultFuture.cancel(/* mayInterruptIfRunning */ true); + } } @Override public void dataManagementIntentLabel(AndroidFuture resultFuture) throws RemoteException { - CharSequence result = BackupTransport.this.dataManagementIntentLabel(); - resultFuture.complete(result); + try { + CharSequence result = BackupTransport.this.dataManagementIntentLabel(); + resultFuture.complete(result); + } catch (RuntimeException e) { + resultFuture.cancel(/* mayInterruptIfRunning */ true); + } } @Override public void transportDirName(AndroidFuture resultFuture) throws RemoteException { - String result = BackupTransport.this.transportDirName(); - resultFuture.complete(result); + try { + String result = BackupTransport.this.transportDirName(); + resultFuture.complete(result); + } catch (RuntimeException e) { + resultFuture.cancel(/* mayInterruptIfRunning */ true); + } } @Override public void requestBackupTime(AndroidFuture resultFuture) throws RemoteException { - long result = BackupTransport.this.requestBackupTime(); - resultFuture.complete(result); + try { + long result = BackupTransport.this.requestBackupTime(); + resultFuture.complete(result); + } catch (RuntimeException e) { + resultFuture.cancel(/* mayInterruptIfRunning */ true); + } } @Override public void initializeDevice(ITransportStatusCallback callback) throws RemoteException { - int result = BackupTransport.this.initializeDevice(); - callback.onOperationCompleteWithStatus(result); + try { + int result = BackupTransport.this.initializeDevice(); + callback.onOperationCompleteWithStatus(result); + } catch (RuntimeException e) { + callback.onOperationCompleteWithStatus(BackupTransport.TRANSPORT_ERROR); + } } @Override public void performBackup(PackageInfo packageInfo, ParcelFileDescriptor inFd, int flags, ITransportStatusCallback callback) throws RemoteException { - int result = BackupTransport.this.performBackup(packageInfo, inFd, flags); - callback.onOperationCompleteWithStatus(result); + try { + int result = BackupTransport.this.performBackup(packageInfo, inFd, flags); + callback.onOperationCompleteWithStatus(result); + } catch (RuntimeException e) { + callback.onOperationCompleteWithStatus(BackupTransport.TRANSPORT_ERROR); + } } @Override public void clearBackupData(PackageInfo packageInfo, ITransportStatusCallback callback) throws RemoteException { - int result = BackupTransport.this.clearBackupData(packageInfo); - callback.onOperationCompleteWithStatus(result); + try { + int result = BackupTransport.this.clearBackupData(packageInfo); + callback.onOperationCompleteWithStatus(result); + } catch (RuntimeException e) { + callback.onOperationCompleteWithStatus(BackupTransport.TRANSPORT_ERROR); + } } @Override public void finishBackup(ITransportStatusCallback callback) throws RemoteException { - int result = BackupTransport.this.finishBackup(); - callback.onOperationCompleteWithStatus(result); + try { + int result = BackupTransport.this.finishBackup(); + callback.onOperationCompleteWithStatus(result); + } catch (RuntimeException e) { + callback.onOperationCompleteWithStatus(BackupTransport.TRANSPORT_ERROR); + } } @Override public void getAvailableRestoreSets(AndroidFuture> resultFuture) throws RemoteException { - RestoreSet[] result = BackupTransport.this.getAvailableRestoreSets(); - resultFuture.complete(Arrays.asList(result)); + try { + RestoreSet[] result = BackupTransport.this.getAvailableRestoreSets(); + resultFuture.complete(Arrays.asList(result)); + } catch (RuntimeException e) { + resultFuture.cancel(/* mayInterruptIfRunning */ true); + } } @Override public void getCurrentRestoreSet(AndroidFuture resultFuture) throws RemoteException { - long result = BackupTransport.this.getCurrentRestoreSet(); - resultFuture.complete(result); + try { + long result = BackupTransport.this.getCurrentRestoreSet(); + resultFuture.complete(result); + } catch (RuntimeException e) { + resultFuture.cancel(/* mayInterruptIfRunning */ true); + } } @Override public void startRestore(long token, PackageInfo[] packages, ITransportStatusCallback callback) throws RemoteException { - int result = BackupTransport.this.startRestore(token, packages); - callback.onOperationCompleteWithStatus(result); + try { + int result = BackupTransport.this.startRestore(token, packages); + callback.onOperationCompleteWithStatus(result); + } catch (RuntimeException e) { + callback.onOperationCompleteWithStatus(BackupTransport.TRANSPORT_ERROR); + } } @Override public void nextRestorePackage(AndroidFuture resultFuture) throws RemoteException { - RestoreDescription result = BackupTransport.this.nextRestorePackage(); - resultFuture.complete(result); + try { + RestoreDescription result = BackupTransport.this.nextRestorePackage(); + resultFuture.complete(result); + } catch (RuntimeException e) { + resultFuture.cancel(/* mayInterruptIfRunning */ true); + } } @Override public void getRestoreData(ParcelFileDescriptor outFd, ITransportStatusCallback callback) throws RemoteException { - int result = BackupTransport.this.getRestoreData(outFd); - callback.onOperationCompleteWithStatus(result); + try { + int result = BackupTransport.this.getRestoreData(outFd); + callback.onOperationCompleteWithStatus(result); + } catch (RuntimeException e) { + callback.onOperationCompleteWithStatus(BackupTransport.TRANSPORT_ERROR); + } } @Override public void finishRestore(ITransportStatusCallback callback) throws RemoteException { - BackupTransport.this.finishRestore(); - callback.onOperationComplete(); + try { + BackupTransport.this.finishRestore(); + callback.onOperationComplete(); + } catch (RuntimeException e) { + callback.onOperationCompleteWithStatus(BackupTransport.TRANSPORT_ERROR); + } } @Override public void requestFullBackupTime(AndroidFuture resultFuture) throws RemoteException { - long result = BackupTransport.this.requestFullBackupTime(); - resultFuture.complete(result); + try { + long result = BackupTransport.this.requestFullBackupTime(); + resultFuture.complete(result); + } catch (RuntimeException e) { + resultFuture.cancel(/* mayInterruptIfRunning */ true); + } } @Override public void performFullBackup(PackageInfo targetPackage, ParcelFileDescriptor socket, int flags, ITransportStatusCallback callback) throws RemoteException { - int result = BackupTransport.this.performFullBackup(targetPackage, socket, flags); - callback.onOperationCompleteWithStatus(result); + try { + int result = BackupTransport.this.performFullBackup(targetPackage, socket, flags); + callback.onOperationCompleteWithStatus(result); + } catch (RuntimeException e) { + callback.onOperationCompleteWithStatus(BackupTransport.TRANSPORT_ERROR); + } } @Override public void checkFullBackupSize(long size, ITransportStatusCallback callback) throws RemoteException { - int result = BackupTransport.this.checkFullBackupSize(size); - callback.onOperationCompleteWithStatus(result); + try { + int result = BackupTransport.this.checkFullBackupSize(size); + callback.onOperationCompleteWithStatus(result); + } catch (RuntimeException e) { + callback.onOperationCompleteWithStatus(BackupTransport.TRANSPORT_ERROR); + } } @Override public void sendBackupData(int numBytes, ITransportStatusCallback callback) throws RemoteException { - int result = BackupTransport.this.sendBackupData(numBytes); - callback.onOperationCompleteWithStatus(result); + try { + int result = BackupTransport.this.sendBackupData(numBytes); + callback.onOperationCompleteWithStatus(result); + } catch (RuntimeException e) { + callback.onOperationCompleteWithStatus(BackupTransport.TRANSPORT_ERROR); + } } @Override public void cancelFullBackup(ITransportStatusCallback callback) throws RemoteException { - BackupTransport.this.cancelFullBackup(); - callback.onOperationComplete(); + try { + BackupTransport.this.cancelFullBackup(); + callback.onOperationComplete(); + } catch (RuntimeException e) { + callback.onOperationCompleteWithStatus(BackupTransport.TRANSPORT_ERROR); + } } @Override public void isAppEligibleForBackup(PackageInfo targetPackage, boolean isFullBackup, AndroidFuture resultFuture) throws RemoteException { - boolean result = BackupTransport.this.isAppEligibleForBackup(targetPackage, - isFullBackup); - resultFuture.complete(result); + try { + boolean result = BackupTransport.this.isAppEligibleForBackup(targetPackage, + isFullBackup); + resultFuture.complete(result); + } catch (RuntimeException e) { + resultFuture.cancel(/* mayInterruptIfRunning */ true); + } } @Override public void getBackupQuota(String packageName, boolean isFullBackup, AndroidFuture resultFuture) throws RemoteException { - long result = BackupTransport.this.getBackupQuota(packageName, isFullBackup); - resultFuture.complete(result); + try { + long result = BackupTransport.this.getBackupQuota(packageName, isFullBackup); + resultFuture.complete(result); + } catch (RuntimeException e) { + resultFuture.cancel(/* mayInterruptIfRunning */ true); + } } @Override public void getTransportFlags(AndroidFuture resultFuture) throws RemoteException { - int result = BackupTransport.this.getTransportFlags(); - resultFuture.complete(result); + try { + int result = BackupTransport.this.getTransportFlags(); + resultFuture.complete(result); + } catch (RuntimeException e) { + resultFuture.cancel(/* mayInterruptIfRunning */ true); + } } @Override public void getNextFullRestoreDataChunk(ParcelFileDescriptor socket, ITransportStatusCallback callback) throws RemoteException { - int result = BackupTransport.this.getNextFullRestoreDataChunk(socket); - callback.onOperationCompleteWithStatus(result); + try { + int result = BackupTransport.this.getNextFullRestoreDataChunk(socket); + callback.onOperationCompleteWithStatus(result); + } catch (RuntimeException e) { + callback.onOperationCompleteWithStatus(BackupTransport.TRANSPORT_ERROR); + } } @Override public void abortFullRestore(ITransportStatusCallback callback) throws RemoteException { - int result = BackupTransport.this.abortFullRestore(); - callback.onOperationCompleteWithStatus(result); + try { + int result = BackupTransport.this.abortFullRestore(); + callback.onOperationCompleteWithStatus(result); + } catch (RuntimeException e) { + callback.onOperationCompleteWithStatus(BackupTransport.TRANSPORT_ERROR); + } } } } diff --git a/services/backup/backuplib/java/com/android/server/backup/transport/TransportStatusCallback.java b/services/backup/backuplib/java/com/android/server/backup/transport/TransportStatusCallback.java index bc5cb0250d564..99526b7ef0d19 100644 --- a/services/backup/backuplib/java/com/android/server/backup/transport/TransportStatusCallback.java +++ b/services/backup/backuplib/java/com/android/server/backup/transport/TransportStatusCallback.java @@ -26,7 +26,7 @@ import com.android.internal.backup.ITransportStatusCallback; public class TransportStatusCallback extends ITransportStatusCallback.Stub { private static final String TAG = "TransportStatusCallback"; - private static final int TIMEOUT_MILLIS = 600 * 1000; // 10 minutes. + private static final int TIMEOUT_MILLIS = 300 * 1000; // 5 minutes. private static final int OPERATION_STATUS_DEFAULT = 0; private final int mOperationTimeout;