From 623ad185ca439b44b843a380229a3a1e4f2c140e Mon Sep 17 00:00:00 2001 From: Ruslan Tkhakokhov Date: Sun, 4 Oct 2020 23:45:46 +0100 Subject: [PATCH] Increase restore timeout for system agents This change is motivated by b/161248425 where the restore times out before all call logs can be restored by CallLogBackupAgent. The CL increases restore time limit for agents running in the system process. See the linked bug below for rationale as to why the change is limited to system agents only. Changes in the CL: * Split timeouts for restore session and agent restore into 2 separate values in BackupAgentTimeoutParameters, so that the latter can be changed independently. * Pass application UID to #getRestoreSessionTimeoutMillis() so that we adjust the return value based on whether the agent is part of the system. Bug: 170076589 Test: 1. atest BackupAgentTimeoutParametersTest 2. Populate 7000 call log entries; run backup; clear call history; run restore and verify it fails without the change and succeeds with the change. Change-Id: Id32e34973be380f7206cb9000ca95e6b76bbf8c0 --- .../backup/BackupAgentTimeoutParameters.java | 43 ++++++++++++++++++- .../backup/UserBackupManagerService.java | 2 +- .../server/backup/internal/BackupHandler.java | 2 +- .../backup/restore/FullRestoreEngine.java | 3 +- .../restore/PerformUnifiedRestoreTask.java | 9 +++- .../BackupAgentTimeoutParametersTest.java | 34 +++++++++++++-- 6 files changed, 83 insertions(+), 10 deletions(-) diff --git a/services/backup/java/com/android/server/backup/BackupAgentTimeoutParameters.java b/services/backup/java/com/android/server/backup/BackupAgentTimeoutParameters.java index 0e99b34e9dd18..bcec9c6655274 100644 --- a/services/backup/java/com/android/server/backup/BackupAgentTimeoutParameters.java +++ b/services/backup/java/com/android/server/backup/BackupAgentTimeoutParameters.java @@ -19,6 +19,7 @@ package com.android.server.backup; import android.content.ContentResolver; import android.os.Handler; +import android.os.UserHandle; import android.provider.Settings; import android.util.KeyValueListParser; import android.util.KeyValueSettingObserver; @@ -52,10 +53,18 @@ public class BackupAgentTimeoutParameters extends KeyValueSettingObserver { public static final String SETTING_RESTORE_AGENT_TIMEOUT_MILLIS = "restore_agent_timeout_millis"; + @VisibleForTesting + public static final String SETTING_RESTORE_SYSTEM_AGENT_TIMEOUT_MILLIS = + "restore_system_agent_timeout_millis"; + @VisibleForTesting public static final String SETTING_RESTORE_AGENT_FINISHED_TIMEOUT_MILLIS = "restore_agent_finished_timeout_millis"; + @VisibleForTesting + public static final String SETTING_RESTORE_SESSION_TIMEOUT_MILLIS = + "restore_session_timeout_millis"; + @VisibleForTesting public static final String SETTING_QUOTA_EXCEEDED_TIMEOUT_MILLIS = "quota_exceeded_timeout_millis"; @@ -71,9 +80,14 @@ public class BackupAgentTimeoutParameters extends KeyValueSettingObserver { @VisibleForTesting public static final long DEFAULT_RESTORE_AGENT_TIMEOUT_MILLIS = 60 * 1000; + @VisibleForTesting public static final long DEFAULT_RESTORE_SYSTEM_AGENT_TIMEOUT_MILLIS = + 180 * 1000; + @VisibleForTesting public static final long DEFAULT_RESTORE_AGENT_FINISHED_TIMEOUT_MILLIS = 30 * 1000; + @VisibleForTesting public static final long DEFAULT_RESTORE_SESSION_TIMEOUT_MILLIS = 60 * 1000; + @VisibleForTesting public static final long DEFAULT_QUOTA_EXCEEDED_TIMEOUT_MILLIS = 3 * 1000; @@ -89,6 +103,12 @@ public class BackupAgentTimeoutParameters extends KeyValueSettingObserver { @GuardedBy("mLock") private long mRestoreAgentTimeoutMillis; + @GuardedBy("mLock") + private long mRestoreSystemAgentTimeoutMillis; + + @GuardedBy("mLock") + private long mRestoreSessionTimeoutMillis; + @GuardedBy("mLock") private long mRestoreAgentFinishedTimeoutMillis; @@ -123,10 +143,18 @@ public class BackupAgentTimeoutParameters extends KeyValueSettingObserver { parser.getLong( SETTING_RESTORE_AGENT_TIMEOUT_MILLIS, DEFAULT_RESTORE_AGENT_TIMEOUT_MILLIS); + mRestoreSystemAgentTimeoutMillis = + parser.getLong( + SETTING_RESTORE_SYSTEM_AGENT_TIMEOUT_MILLIS, + DEFAULT_RESTORE_SYSTEM_AGENT_TIMEOUT_MILLIS); mRestoreAgentFinishedTimeoutMillis = parser.getLong( SETTING_RESTORE_AGENT_FINISHED_TIMEOUT_MILLIS, DEFAULT_RESTORE_AGENT_FINISHED_TIMEOUT_MILLIS); + mRestoreSessionTimeoutMillis = + parser.getLong( + SETTING_RESTORE_SESSION_TIMEOUT_MILLIS, + DEFAULT_RESTORE_SESSION_TIMEOUT_MILLIS); mQuotaExceededTimeoutMillis = parser.getLong( SETTING_QUOTA_EXCEEDED_TIMEOUT_MILLIS, @@ -152,9 +180,20 @@ public class BackupAgentTimeoutParameters extends KeyValueSettingObserver { } } - public long getRestoreAgentTimeoutMillis() { + /** + * @param applicationUid UID of the application for which to get restore timeout + * @return restore timeout in milliseconds + */ + public long getRestoreAgentTimeoutMillis(int applicationUid) { synchronized (mLock) { - return mRestoreAgentTimeoutMillis; + return UserHandle.isCore(applicationUid) ? mRestoreSystemAgentTimeoutMillis : + mRestoreAgentTimeoutMillis; + } + } + + public long getRestoreSessionTimeoutMillis() { + synchronized (mLock) { + return mRestoreSessionTimeoutMillis; } } diff --git a/services/backup/java/com/android/server/backup/UserBackupManagerService.java b/services/backup/java/com/android/server/backup/UserBackupManagerService.java index e68c07ed73f75..725365703da7c 100644 --- a/services/backup/java/com/android/server/backup/UserBackupManagerService.java +++ b/services/backup/java/com/android/server/backup/UserBackupManagerService.java @@ -4088,7 +4088,7 @@ public class UserBackupManagerService { mActiveRestoreSession = new ActiveRestoreSession(this, packageName, transport, getEligibilityRulesForOperation(operationType)); mBackupHandler.sendEmptyMessageDelayed(MSG_RESTORE_SESSION_TIMEOUT, - mAgentTimeoutParameters.getRestoreAgentTimeoutMillis()); + mAgentTimeoutParameters.getRestoreSessionTimeoutMillis()); } return mActiveRestoreSession; } diff --git a/services/backup/java/com/android/server/backup/internal/BackupHandler.java b/services/backup/java/com/android/server/backup/internal/BackupHandler.java index 100dbae9f01d0..1cb7c11e9499f 100644 --- a/services/backup/java/com/android/server/backup/internal/BackupHandler.java +++ b/services/backup/java/com/android/server/backup/internal/BackupHandler.java @@ -390,7 +390,7 @@ public class BackupHandler extends Handler { // Done: reset the session timeout clock removeMessages(MSG_RESTORE_SESSION_TIMEOUT); sendEmptyMessageDelayed(MSG_RESTORE_SESSION_TIMEOUT, - mAgentTimeoutParameters.getRestoreAgentTimeoutMillis()); + mAgentTimeoutParameters.getRestoreSessionTimeoutMillis()); params.listener.onFinished(callerLogString); } diff --git a/services/backup/java/com/android/server/backup/restore/FullRestoreEngine.java b/services/backup/java/com/android/server/backup/restore/FullRestoreEngine.java index 16077cb6082f8..5718bdfc49713 100644 --- a/services/backup/java/com/android/server/backup/restore/FullRestoreEngine.java +++ b/services/backup/java/com/android/server/backup/restore/FullRestoreEngine.java @@ -403,7 +403,8 @@ public class FullRestoreEngine extends RestoreEngine { final boolean isSharedStorage = pkg.equals(SHARED_BACKUP_AGENT_PACKAGE); final long timeout = isSharedStorage ? mAgentTimeoutParameters.getSharedBackupAgentTimeoutMillis() : - mAgentTimeoutParameters.getRestoreAgentTimeoutMillis(); + mAgentTimeoutParameters.getRestoreAgentTimeoutMillis( + mTargetApp.uid); try { mBackupManagerService.prepareOperationTimeout(token, timeout, diff --git a/services/backup/java/com/android/server/backup/restore/PerformUnifiedRestoreTask.java b/services/backup/java/com/android/server/backup/restore/PerformUnifiedRestoreTask.java index abf11bd542a12..261ebe69a15ea 100644 --- a/services/backup/java/com/android/server/backup/restore/PerformUnifiedRestoreTask.java +++ b/services/backup/java/com/android/server/backup/restore/PerformUnifiedRestoreTask.java @@ -46,6 +46,7 @@ import android.content.pm.PackageManagerInternal; import android.os.Bundle; import android.os.Message; import android.os.ParcelFileDescriptor; +import android.os.Process; import android.os.RemoteException; import android.os.SystemClock; import android.os.UserHandle; @@ -433,6 +434,8 @@ public class PerformUnifiedRestoreTask implements BackupRestoreTask { // Pull the Package Manager metadata from the restore set first mCurrentPackage = new PackageInfo(); mCurrentPackage.packageName = PACKAGE_MANAGER_SENTINEL; + mCurrentPackage.applicationInfo = new ApplicationInfo(); + mCurrentPackage.applicationInfo.uid = Process.SYSTEM_UID; mPmAgent = backupManagerService.makeMetadataAgent(null); mAgent = IBackupAgent.Stub.asInterface(mPmAgent.onBind()); if (MORE_DEBUG) { @@ -760,7 +763,8 @@ public class PerformUnifiedRestoreTask implements BackupRestoreTask { // Kick off the restore, checking for hung agents. The timeout or // the operationComplete() callback will schedule the next step, // so we do not do that here. - long restoreAgentTimeoutMillis = mAgentTimeoutParameters.getRestoreAgentTimeoutMillis(); + long restoreAgentTimeoutMillis = mAgentTimeoutParameters.getRestoreAgentTimeoutMillis( + app.applicationInfo.uid); backupManagerService.prepareOperationTimeout( mEphemeralOpToken, restoreAgentTimeoutMillis, this, OP_TYPE_RESTORE_WAIT); startedAgentRestore = true; @@ -1122,7 +1126,8 @@ public class PerformUnifiedRestoreTask implements BackupRestoreTask { } else { // We were invoked via an active restore session, not by the Package // Manager, so start up the session timeout again. - long restoreAgentTimeoutMillis = mAgentTimeoutParameters.getRestoreAgentTimeoutMillis(); + long restoreAgentTimeoutMillis = + mAgentTimeoutParameters.getRestoreSessionTimeoutMillis(); backupManagerService.getBackupHandler().sendEmptyMessageDelayed( MSG_RESTORE_SESSION_TIMEOUT, restoreAgentTimeoutMillis); diff --git a/services/robotests/backup/src/com/android/server/backup/BackupAgentTimeoutParametersTest.java b/services/robotests/backup/src/com/android/server/backup/BackupAgentTimeoutParametersTest.java index 5b226f36d565f..f58e295b49579 100644 --- a/services/robotests/backup/src/com/android/server/backup/BackupAgentTimeoutParametersTest.java +++ b/services/robotests/backup/src/com/android/server/backup/BackupAgentTimeoutParametersTest.java @@ -23,6 +23,7 @@ import static org.junit.Assert.assertEquals; import android.content.ContentResolver; import android.content.Context; import android.os.Handler; +import android.os.Process; import android.platform.test.annotations.Presubmit; import android.provider.Settings; @@ -64,7 +65,7 @@ public class BackupAgentTimeoutParametersTest { long kvBackupAgentTimeoutMillis = mParameters.getKvBackupAgentTimeoutMillis(); long fullBackupAgentTimeoutMillis = mParameters.getFullBackupAgentTimeoutMillis(); long sharedBackupAgentTimeoutMillis = mParameters.getSharedBackupAgentTimeoutMillis(); - long restoreAgentTimeoutMillis = mParameters.getRestoreAgentTimeoutMillis(); + long restoreSessionTimeoutMillis = mParameters.getRestoreSessionTimeoutMillis(); long restoreAgentFinishedTimeoutMillis = mParameters.getRestoreAgentFinishedTimeoutMillis(); assertEquals( @@ -77,13 +78,40 @@ public class BackupAgentTimeoutParametersTest { BackupAgentTimeoutParameters.DEFAULT_SHARED_BACKUP_AGENT_TIMEOUT_MILLIS, sharedBackupAgentTimeoutMillis); assertEquals( - BackupAgentTimeoutParameters.DEFAULT_RESTORE_AGENT_TIMEOUT_MILLIS, - restoreAgentTimeoutMillis); + BackupAgentTimeoutParameters.DEFAULT_RESTORE_SESSION_TIMEOUT_MILLIS, + restoreSessionTimeoutMillis); assertEquals( BackupAgentTimeoutParameters.DEFAULT_RESTORE_AGENT_FINISHED_TIMEOUT_MILLIS, restoreAgentFinishedTimeoutMillis); } + @Test + public void + testGetRestoreAgentTimeout_afterConstructorWithStartForSystemAgent_returnsDefaultValue() { + mParameters.start(); + + // Numbers before FIRST_APPLICATION_UID are reserved as UIDs for system components. + long restoreTimeout = + mParameters.getRestoreAgentTimeoutMillis(Process.FIRST_APPLICATION_UID - 1); + + assertThat(restoreTimeout) + .isEqualTo( + BackupAgentTimeoutParameters.DEFAULT_RESTORE_SYSTEM_AGENT_TIMEOUT_MILLIS); + } + + @Test + public void + testGetRestoreAgentTimeout_afterConstructorWithStartForAppAgent_returnsDefaultValue() { + mParameters.start(); + + // Numbers starting from FIRST_APPLICATION_UID are reserved for app UIDs. + long restoreTimeout = + mParameters.getRestoreAgentTimeoutMillis(Process.FIRST_APPLICATION_UID); + + assertThat(restoreTimeout) + .isEqualTo(BackupAgentTimeoutParameters.DEFAULT_RESTORE_AGENT_TIMEOUT_MILLIS); + } + @Test public void testGetQuotaExceededTimeoutMillis_returnsDefaultValue() { mParameters.start();