From 465e3b7115ac8974cae42cdf7b4e3896504ca9bc Mon Sep 17 00:00:00 2001 From: Zim Date: Tue, 23 Feb 2021 12:16:50 +0000 Subject: [PATCH] Extend AnrController to improve ANR dialog controls I5431d22bd8faa75e3deab1960a2f721784f98d42 introduced a basic ANR controller interface to allow services within the system_server register to delay the ANR dialog during an ANR. Some limitations of that interface were: 1. No way to cancel the ANR dialog if the normal function of the app resumed during the delay 2. The AnrController#getAnrDelay method was serving a dual purpose of returning the delay and showing an interim UI 3. The interim UI might live within an updatable mainline module, hence forcing the system_server to block (asynchronously) on the 'getter' for the binder call. This cl addresses the problems above with the following new methods on AnrController: onAnrDelayStarted and onAnrDelayCompleted. These methods provide a clearer interface for the following reasons: 1. onAnrDelayCompleted will return a boolean value to indicate whether the ANR dialog should still be shown after the delay 2. onAnrDelayStarted is a more explicit API that can be called asynchronously without blocking to notify external components to show an interim UI 3. The existing getAnrDelay can be handled completely within the system_server without blocking on an external binder call See design (2a) in go/transcoding-anrs for more details Test: Manual Bug: 170486601 Change-Id: Idb9190a0e6014ce64bf1412c26f6ae03f97e922d --- core/java/android/app/AnrController.java | 24 +++++++++++++- .../android/server/StorageManagerService.java | 32 ++++++++++++++----- .../java/com/android/server/am/AppErrors.java | 22 ++++++++++++- .../server/am/ErrorDialogController.java | 21 ++++++++++++ .../server/am/ProcessErrorStateRecord.java | 16 +++++++--- .../server/wm/ActivityTaskManagerService.java | 23 +++++++++---- 6 files changed, 118 insertions(+), 20 deletions(-) diff --git a/core/java/android/app/AnrController.java b/core/java/android/app/AnrController.java index cfc9d27157200..a0d4b3a6a753d 100644 --- a/core/java/android/app/AnrController.java +++ b/core/java/android/app/AnrController.java @@ -23,7 +23,29 @@ package android.app; public interface AnrController { /** * Returns the delay in milliseconds for an ANR dialog that is about to be shown for - * {@code packageName}. + * {@code packageName} with {@code uid}. + * + * Implementations should only return a positive value if they actually expect the + * {@code packageName} to be delayed due to them. + + * If there are multiple controllers registered, the controller with the max delay will + * be selected and will receive an {@link #onAnrDelayStarted} callback at the start of the + * delay and an {@link #onAnrDelayCompleted} at the end of the delay. */ long getAnrDelayMillis(String packageName, int uid); + + /** + * Notifies the controller at the start of the ANR dialog delay for {@code packageName} with + * {@code uid}. The controller can decide to show a progress UI after this notification. + */ + void onAnrDelayStarted(String packageName, int uid); + + /** + * Notifies the controller at the end of the ANR dialog delay for {@code packageName} with + * {@code uid}. + * + * @return whether the ANR dialog should be shown or cancelled. {@code true} if the + * ANR dialog should be shown, {@code false} if it should be cancelled. + */ + boolean onAnrDelayCompleted(String packageName, int uid); } diff --git a/services/core/java/com/android/server/StorageManagerService.java b/services/core/java/com/android/server/StorageManagerService.java index 2f9819997257a..6be7f05f6cc64 100644 --- a/services/core/java/com/android/server/StorageManagerService.java +++ b/services/core/java/com/android/server/StorageManagerService.java @@ -51,6 +51,7 @@ import android.annotation.Nullable; import android.annotation.UserIdInt; import android.app.ActivityManager; import android.app.ActivityManagerInternal; +import android.app.AnrController; import android.app.AppOpsManager; import android.app.IActivityManager; import android.app.KeyguardManager; @@ -938,14 +939,29 @@ class StorageManagerService extends IStorageManager.Stub if (transcodeEnabled) { LocalServices.getService(ActivityManagerInternal.class) - .registerAnrController((packageName, uid) -> { - try { - return mStorageSessionController.getAnrDelayMillis(packageName, uid); - } catch (ExternalStorageServiceException e) { - Log.e(TAG, "Failed to get ANR delay for " + packageName, e); - return 0; - } - }); + .registerAnrController(new ExternalStorageServiceAnrController()); + } + } + + // TODO(b/170486601): Check transcoding status based on events pushed from the MediaProvider + private class ExternalStorageServiceAnrController implements AnrController { + @Override + public long getAnrDelayMillis(String packageName, int uid) { + int delay = SystemProperties.getInt("sys.fuse.transcode_anr_delay", 0); + Log.d(TAG, "getAnrDelayMillis: " + packageName + ". Delaying for " + delay + "ms"); + return delay; + } + + @Override + public void onAnrDelayStarted(String packageName, int uid) { + Log.d(TAG, "onAnrDelayStarted: " + packageName); + } + + @Override + public boolean onAnrDelayCompleted(String packageName, int uid) { + boolean show = SystemProperties.getBoolean("sys.fuse.transcode_anr_dialog_show", true); + Log.d(TAG, "onAnrDelayCompleted: " + packageName + ". Show: " + show); + return show; } } diff --git a/services/core/java/com/android/server/am/AppErrors.java b/services/core/java/com/android/server/am/AppErrors.java index e5a5cff409b3e..3602f44cd7853 100644 --- a/services/core/java/com/android/server/am/AppErrors.java +++ b/services/core/java/com/android/server/am/AppErrors.java @@ -29,6 +29,7 @@ import static com.android.server.wm.ActivityTaskManagerService.RELAUNCH_REASON_N import android.app.ActivityManager; import android.app.ActivityOptions; +import android.app.AnrController; import android.app.ApplicationErrorReport; import android.app.ApplicationExitInfo; import android.content.ActivityNotFoundException; @@ -1058,7 +1059,26 @@ class AppErrors { Settings.Secure.ANR_SHOW_BACKGROUND, 0, mService.mUserController.getCurrentUserId()) != 0; if (mService.mAtmInternal.canShowErrorDialogs() || showBackground) { - errState.getDialogController().showAnrDialogs(data); + AnrController anrController = errState.getDialogController().getAnrController(); + if (anrController == null) { + errState.getDialogController().showAnrDialogs(data); + } else { + String packageName = proc.info.packageName; + int uid = proc.info.uid; + boolean showDialog = anrController.onAnrDelayCompleted(packageName, uid); + + if (showDialog) { + Slog.d(TAG, "ANR delay completed. Showing ANR dialog for package: " + + packageName); + errState.getDialogController().showAnrDialogs(data); + } else { + Slog.d(TAG, "ANR delay completed. Cancelling ANR dialog for package: " + + packageName); + errState.setNotResponding(false); + errState.setNotRespondingReport(null); + errState.getDialogController().clearAnrDialogs(); + } + } } else { MetricsLogger.action(mContext, MetricsProto.MetricsEvent.ACTION_APP_ANR, AppNotRespondingDialog.CANT_SHOW); diff --git a/services/core/java/com/android/server/am/ErrorDialogController.java b/services/core/java/com/android/server/am/ErrorDialogController.java index ef135d55e43cd..f23d309e424a9 100644 --- a/services/core/java/com/android/server/am/ErrorDialogController.java +++ b/services/core/java/com/android/server/am/ErrorDialogController.java @@ -16,6 +16,8 @@ package com.android.server.am; +import android.annotation.Nullable; +import android.app.AnrController; import android.app.Dialog; import android.content.Context; @@ -57,6 +59,13 @@ final class ErrorDialogController { @GuardedBy("mProcLock") private AppWaitingForDebuggerDialog mWaitDialog; + /** + * ANR dialog controller + */ + @GuardedBy("mProcLock") + @Nullable + private AnrController mAnrController; + @GuardedBy("mProcLock") boolean hasCrashDialogs() { return mCrashDialogs != null; @@ -118,6 +127,7 @@ final class ErrorDialogController { } forAllDialogs(mAnrDialogs, Dialog::dismiss); mAnrDialogs = null; + mAnrController = null; } @GuardedBy("mProcLock") @@ -220,6 +230,17 @@ final class ErrorDialogController { }); } + @GuardedBy("mProcLock") + @Nullable + AnrController getAnrController() { + return mAnrController; + } + + @GuardedBy("mProcLock") + void setAnrController(AnrController controller) { + mAnrController = controller; + } + /** * Helper function to collect contexts from crashed app located displays. * diff --git a/services/core/java/com/android/server/am/ProcessErrorStateRecord.java b/services/core/java/com/android/server/am/ProcessErrorStateRecord.java index 165312352990a..3258f8af0da21 100644 --- a/services/core/java/com/android/server/am/ProcessErrorStateRecord.java +++ b/services/core/java/com/android/server/am/ProcessErrorStateRecord.java @@ -22,6 +22,7 @@ import static com.android.server.am.ActivityManagerService.MY_PID; import static com.android.server.am.ProcessRecord.TAG; import android.app.ActivityManager; +import android.app.AnrController; import android.app.ApplicationErrorReport; import android.app.ApplicationExitInfo; import android.content.ComponentName; @@ -418,10 +419,16 @@ class ProcessErrorStateRecord { // Retrieve max ANR delay from AnrControllers without the mService lock since the // controllers might in turn call into apps - long anrDialogDelayMs = mService.mActivityTaskManager.getMaxAnrDelayMillis(aInfo); - if (aInfo != null && aInfo.packageName != null && anrDialogDelayMs > 0) { - Slog.i(TAG, "Delaying ANR dialog for " + aInfo.packageName + " for " + anrDialogDelayMs - + "ms"); + AnrController anrController = mService.mActivityTaskManager.getAnrController(aInfo); + long anrDialogDelayMs = 0; + if (anrController != null) { + String packageName = aInfo.packageName; + int uid = aInfo.uid; + anrDialogDelayMs = anrController.getAnrDelayMillis(packageName, uid); + // Might execute an async binder call to a system app to show an interim + // ANR progress UI + anrController.onAnrDelayStarted(packageName, uid); + Slog.i(TAG, "ANR delay of " + anrDialogDelayMs + "ms started for " + packageName); } synchronized (mService) { @@ -440,6 +447,7 @@ class ProcessErrorStateRecord { // Set the app's notResponding state, and look up the errorReportReceiver makeAppNotRespondingLSP(activityShortComponentName, annotation != null ? "ANR " + annotation : "ANR", info.toString()); + mDialogController.setAnrController(anrController); } // Notify package manager service to possibly update package state diff --git a/services/core/java/com/android/server/wm/ActivityTaskManagerService.java b/services/core/java/com/android/server/wm/ActivityTaskManagerService.java index 75a7660c0ac1a..0f29fdcd966c5 100644 --- a/services/core/java/com/android/server/wm/ActivityTaskManagerService.java +++ b/services/core/java/com/android/server/wm/ActivityTaskManagerService.java @@ -2095,10 +2095,14 @@ public class ActivityTaskManagerService extends IActivityTaskManager.Stub { } } - /** @return the max ANR delay from all registered {@link AnrController} instances */ - public long getMaxAnrDelayMillis(ApplicationInfo info) { + /** + * @return the controller with the max ANR delay from all registered + * {@link AnrController} instances + */ + @Nullable + public AnrController getAnrController(ApplicationInfo info) { if (info == null || info.packageName == null) { - return 0; + return null; } final ArrayList controllers; @@ -2107,12 +2111,19 @@ public class ActivityTaskManagerService extends IActivityTaskManager.Stub { } final String packageName = info.packageName; + final int uid = info.uid; long maxDelayMs = 0; + AnrController controllerWithMaxDelay = null; + for (AnrController controller : controllers) { - maxDelayMs = Math.max(maxDelayMs, controller.getAnrDelayMillis(packageName, info.uid)); + long delayMs = controller.getAnrDelayMillis(packageName, uid); + if (delayMs > 0 && delayMs > maxDelayMs) { + controllerWithMaxDelay = controller; + maxDelayMs = delayMs; + } } - maxDelayMs = Math.max(maxDelayMs, 0); - return maxDelayMs; + + return controllerWithMaxDelay; } @Override