From bb89d23c33c5595012bdf044855b7baba96e1851 Mon Sep 17 00:00:00 2001 From: Hui Yu Date: Thu, 17 Mar 2022 13:17:08 -0700 Subject: [PATCH] Increase startForeground() timeout to 30 seconds. Right now there is 10 seconds grace period from Context.startForegroudService() to Service.startForeground() call, afterwards we crash the app and thow ForegroundServiceDidNotStartInTimeException, at the same time generate ANR report. Make the startForegroundService() to startForeground() grace period configurable, increase the default value from 10 seconds to 30 seconds. This value can be changed by command "adb shell device_config put activity_manager service_start_foreground_timeout_ms 30000" After the 30 seconds service_start_foreground_timeout_ms and ForegroundServiceDidNotStartInTimeException, delay the ANR generation for another 10 seconds configurable by service_start_foreground_anr_delay_ms. This value can be changed by "adb shell device_config put activity_manager service_start_foreground_anr_delay_ms 10000". At the end of service_start_foreground_anr_delay_ms, if the app's process is still alive, the ANR report will be taken. Otherwise, there is no ANR report. Bug: 219514357 Test: adb shell device_config put activity_manager service_start_foreground_timeout_ms, adb shell device_config put activity_manager service_start_foreground_anr_delay_ms, Manual test, in test app, startForegroundService() but not call startForeground(), 30 seconds later, observe the ForegroundServiceDidNotStartInTimeException, 10 seconds later, try to take the ANR report. Change-Id: I5f30d0996bff23f8a302cedef01eaa31e6ff29ce --- .../android/app/ActivityManagerInternal.java | 5 ++ .../com/android/server/am/ActiveServices.java | 27 ++++++---- .../server/am/ActivityManagerConstants.java | 51 ++++++++++++++++++- .../server/am/ActivityManagerService.java | 12 +++++ .../server/am/BaseAppStateTracker.java | 3 +- 5 files changed, 84 insertions(+), 14 deletions(-) diff --git a/core/java/android/app/ActivityManagerInternal.java b/core/java/android/app/ActivityManagerInternal.java index 9fe8e792681ce..9c8639d4b7b89 100644 --- a/core/java/android/app/ActivityManagerInternal.java +++ b/core/java/android/app/ActivityManagerInternal.java @@ -683,6 +683,11 @@ public abstract class ActivityManagerInternal { */ public abstract @TempAllowListType int getPushMessagingOverQuotaBehavior(); + /** + * Return the startForeground() grace period after calling startForegroundService(). + */ + public abstract int getServiceStartForegroundTimeout(); + /** * Returns the capability of the given uid */ diff --git a/services/core/java/com/android/server/am/ActiveServices.java b/services/core/java/com/android/server/am/ActiveServices.java index 48e3264b2c111..91bd775b07c48 100644 --- a/services/core/java/com/android/server/am/ActiveServices.java +++ b/services/core/java/com/android/server/am/ActiveServices.java @@ -203,10 +203,6 @@ public final class ActiveServices { // How long we wait for a service to finish executing. static final int SERVICE_BACKGROUND_TIMEOUT = SERVICE_TIMEOUT * 10; - // How long the startForegroundService() grace period is to get around to - // calling startForeground() before we ANR + stop it. - static final int SERVICE_START_FOREGROUND_TIMEOUT = 10 * 1000 * Build.HW_TIMEOUT_MULTIPLIER; - // Foreground service types that always get immediate notification display, // expressed in the same bitmask format that ServiceRecord.foregroundServiceType // uses. @@ -4223,7 +4219,7 @@ public final class ActiveServices { + " for fg-service launch"); } mAm.tempAllowlistUidLocked(r.appInfo.uid, - SERVICE_START_FOREGROUND_TIMEOUT, REASON_SERVICE_LAUNCH, + mAm.mConstants.mServiceStartForegroundTimeoutMs, REASON_SERVICE_LAUNCH, "fg-service-launch", TEMPORARY_ALLOW_LIST_TYPE_FOREGROUND_SERVICE_ALLOWED, r.mRecentCallingUid); @@ -5696,12 +5692,23 @@ public final class ActiveServices { } if (app != null) { - mAm.mAnrHelper.appNotResponding(app, - "Context.startForegroundService() did not then call Service.startForeground(): " - + r); + final String annotation = "Context.startForegroundService() did not then call " + + "Service.startForeground(): " + r; + Message msg = mAm.mHandler.obtainMessage( + ActivityManagerService.SERVICE_FOREGROUND_TIMEOUT_ANR_MSG); + SomeArgs args = SomeArgs.obtain(); + args.arg1 = app; + args.arg2 = annotation; + msg.obj = args; + mAm.mHandler.sendMessageDelayed(msg, + mAm.mConstants.mServiceStartForegroundAnrDelayMs); } } + void serviceForegroundTimeoutANR(ProcessRecord app, String annotation) { + mAm.mAnrHelper.appNotResponding(app, annotation); + } + public void updateServiceApplicationInfoLocked(ApplicationInfo applicationInfo) { final int userId = UserHandle.getUserId(applicationInfo.uid); ServiceMap serviceMap = mServiceMap.get(userId); @@ -5722,7 +5729,7 @@ public final class ActiveServices { ComponentName service) { mAm.crashApplicationWithTypeWithExtras( app.uid, app.getPid(), app.info.packageName, app.userId, - "Context.startForegroundService() did not then call Service.startForeground(): " + "Context.startForegroundService() did not then call " + "Service.startForeground(): " + serviceRecord, false /*force*/, ForegroundServiceDidNotStartInTimeException.TYPE_ID, ForegroundServiceDidNotStartInTimeException.createExtrasForService(service)); @@ -5747,7 +5754,7 @@ public final class ActiveServices { ActivityManagerService.SERVICE_FOREGROUND_TIMEOUT_MSG); msg.obj = r; r.fgWaiting = true; - mAm.mHandler.sendMessageDelayed(msg, SERVICE_START_FOREGROUND_TIMEOUT); + mAm.mHandler.sendMessageDelayed(msg, mAm.mConstants.mServiceStartForegroundTimeoutMs); } final class ServiceDumper { diff --git a/services/core/java/com/android/server/am/ActivityManagerConstants.java b/services/core/java/com/android/server/am/ActivityManagerConstants.java index c42666247e989..67eae0ec3a625 100644 --- a/services/core/java/com/android/server/am/ActivityManagerConstants.java +++ b/services/core/java/com/android/server/am/ActivityManagerConstants.java @@ -216,8 +216,12 @@ final class ActivityManagerConstants extends ContentObserver { private static final String DEFAULT_COMPONENT_ALIAS_OVERRIDES = ""; private static final int DEFAULT_DEFER_BOOT_COMPLETED_BROADCAST = - DEFER_BOOT_COMPLETED_BROADCAST_BACKGROUND_RESTRICTED_ONLY - | DEFER_BOOT_COMPLETED_BROADCAST_TARGET_T_ONLY; + DEFER_BOOT_COMPLETED_BROADCAST_BACKGROUND_RESTRICTED_ONLY + | DEFER_BOOT_COMPLETED_BROADCAST_TARGET_T_ONLY; + + private static final int DEFAULT_SERVICE_START_FOREGROUND_TIMEOUT_MS = 30 * 1000; + + private static final int DEFAULT_SERVICE_START_FOREGROUND_ANR_DELAY_MS = 10 * 1000; // Flag stored in the DeviceConfig API. /** @@ -311,6 +315,12 @@ final class ActivityManagerConstants extends ContentObserver { private static final String KEY_DEFER_BOOT_COMPLETED_BROADCAST = "defer_boot_completed_broadcast"; + private static final String KEY_SERVICE_START_FOREGROUND_TIMEOUT_MS = + "service_start_foreground_timeout_ms"; + + private static final String KEY_SERVICE_START_FOREGROUND_ANR_DELAY_MS = + "service_start_foreground_anr_delay_ms"; + // Maximum number of cached processes we will allow. public int MAX_CACHED_PROCESSES = DEFAULT_MAX_CACHED_PROCESSES; @@ -630,6 +640,19 @@ final class ActivityManagerConstants extends ContentObserver { volatile @BroadcastConstants.DeferBootCompletedBroadcastType int mDeferBootCompletedBroadcast = DEFAULT_DEFER_BOOT_COMPLETED_BROADCAST; + /** + * How long the Context.startForegroundService() grace period is to get around to + * calling Service.startForeground() before we generate ANR. + */ + volatile int mServiceStartForegroundTimeoutMs = DEFAULT_SERVICE_START_FOREGROUND_TIMEOUT_MS; + + /** + * How long from Service.startForeground() timed-out to when we generate ANR of the user app. + * This delay is after the timeout {@link #mServiceStartForegroundTimeoutMs}. + */ + volatile int mServiceStartForegroundAnrDelayMs = + DEFAULT_SERVICE_START_FOREGROUND_ANR_DELAY_MS; + /** * Defines component aliases. Format * ComponentName ":" ComponentName ( "," ComponentName ":" ComponentName )* @@ -913,6 +936,12 @@ final class ActivityManagerConstants extends ContentObserver { case KEY_DEFER_BOOT_COMPLETED_BROADCAST: updateDeferBootCompletedBroadcast(); break; + case KEY_SERVICE_START_FOREGROUND_TIMEOUT_MS: + updateServiceStartForegroundTimeoutMs(); + break; + case KEY_SERVICE_START_FOREGROUND_ANR_DELAY_MS: + updateServiceStartForegroundAnrDealyMs(); + break; case KEY_NO_KILL_CACHED_PROCESSES_UNTIL_BOOT_COMPLETED: updateNoKillCachedProcessesUntilBootCompleted(); break; @@ -1389,6 +1418,20 @@ final class ActivityManagerConstants extends ContentObserver { DEFAULT_MAX_EMPTY_TIME_MILLIS); } + private void updateServiceStartForegroundTimeoutMs() { + mServiceStartForegroundTimeoutMs = DeviceConfig.getInt( + DeviceConfig.NAMESPACE_ACTIVITY_MANAGER, + KEY_SERVICE_START_FOREGROUND_TIMEOUT_MS, + DEFAULT_SERVICE_START_FOREGROUND_TIMEOUT_MS); + } + + private void updateServiceStartForegroundAnrDealyMs() { + mServiceStartForegroundAnrDelayMs = DeviceConfig.getInt( + DeviceConfig.NAMESPACE_ACTIVITY_MANAGER, + KEY_SERVICE_START_FOREGROUND_ANR_DELAY_MS, + DEFAULT_SERVICE_START_FOREGROUND_ANR_DELAY_MS); + } + private long[] parseLongArray(@NonNull String key, @NonNull long[] def) { final String val = DeviceConfig.getString(DeviceConfig.NAMESPACE_ACTIVITY_MANAGER, key, null); @@ -1669,6 +1712,10 @@ final class ActivityManagerConstants extends ContentObserver { pw.print("="); pw.println(mNoKillCachedProcessesPostBootCompletedDurationMillis); pw.print(" "); pw.print(KEY_MAX_EMPTY_TIME_MILLIS); pw.print("="); pw.println(mMaxEmptyTimeMillis); + pw.print(" "); pw.print(KEY_SERVICE_START_FOREGROUND_TIMEOUT_MS); + pw.print("="); pw.println(mServiceStartForegroundTimeoutMs); + pw.print(" "); pw.print(KEY_SERVICE_START_FOREGROUND_ANR_DELAY_MS); + pw.print("="); pw.println(mServiceStartForegroundAnrDelayMs); pw.println(); if (mOverrideMaxCachedProcesses >= 0) { diff --git a/services/core/java/com/android/server/am/ActivityManagerService.java b/services/core/java/com/android/server/am/ActivityManagerService.java index 0735648c1069a..cd40bd4afd946 100644 --- a/services/core/java/com/android/server/am/ActivityManagerService.java +++ b/services/core/java/com/android/server/am/ActivityManagerService.java @@ -1534,6 +1534,7 @@ public class ActivityManagerService extends IActivityManager.Stub static final int IDLE_UIDS_MSG = 58; static final int HANDLE_TRUST_STORAGE_UPDATE_MSG = 63; static final int SERVICE_FOREGROUND_TIMEOUT_MSG = 66; + static final int SERVICE_FOREGROUND_TIMEOUT_ANR_MSG = 67; static final int PUSH_TEMP_ALLOWLIST_UI_MSG = 68; static final int SERVICE_FOREGROUND_CRASH_MSG = 69; static final int DISPATCH_OOM_ADJ_OBSERVER_MSG = 70; @@ -1722,6 +1723,12 @@ public class ActivityManagerService extends IActivityManager.Stub case SERVICE_FOREGROUND_TIMEOUT_MSG: { mServices.serviceForegroundTimeout((ServiceRecord) msg.obj); } break; + case SERVICE_FOREGROUND_TIMEOUT_ANR_MSG: { + SomeArgs args = (SomeArgs) msg.obj; + mServices.serviceForegroundTimeoutANR((ProcessRecord) args.arg1, + (String) args.arg2); + args.recycle(); + } break; case SERVICE_FOREGROUND_CRASH_MSG: { SomeArgs args = (SomeArgs) msg.obj; mServices.serviceForegroundCrash( @@ -17129,6 +17136,11 @@ public class ActivityManagerService extends IActivityManager.Stub } } + @Override + public int getServiceStartForegroundTimeout() { + return mConstants.mServiceStartForegroundTimeoutMs; + } + @Override public int getUidCapability(int uid) { synchronized (ActivityManagerService.this) { diff --git a/services/core/java/com/android/server/am/BaseAppStateTracker.java b/services/core/java/com/android/server/am/BaseAppStateTracker.java index 0fada53d622e4..d177ca3c000f1 100644 --- a/services/core/java/com/android/server/am/BaseAppStateTracker.java +++ b/services/core/java/com/android/server/am/BaseAppStateTracker.java @@ -16,7 +16,6 @@ package com.android.server.am; -import static com.android.server.am.ActiveServices.SERVICE_START_FOREGROUND_TIMEOUT; import static com.android.server.am.ActivityManagerDebugConfig.TAG_AM; import static com.android.server.am.ActivityManagerDebugConfig.TAG_WITH_CLASS_NAME; @@ -348,7 +347,7 @@ public abstract class BaseAppStateTracker { } long getServiceStartForegroundTimeout() { - return SERVICE_START_FOREGROUND_TIMEOUT; + return mActivityManagerInternal.getServiceStartForegroundTimeout(); } RoleManager getRoleManager() {