From 7c88e66cbaf143247317bbfee11d4f522fb892ca Mon Sep 17 00:00:00 2001 From: Prabir Pradhan Date: Tue, 15 Feb 2022 06:35:03 -0800 Subject: [PATCH] Support ANRs from windows that are not tracked by WM Previously, ANRs for gesture monitors were reported using the pid of the owner, and ANRs for all windows were reported using its input channel, which was tracked by WM. Now, there can be input windows that are not tracked by WM. We unify the ANR reporting pipeline so that we first try to report an ANR using the window's input channel. If the ANR reporting fails because the input channel was not tracked by WM, we fall back on reporting ANR via the pid of the window owner. Bug: 210978621 Test: manual: Create a gesture monitor that sleeps while processing input events, ensure there is an ANR. Change-Id: If368ccf4a5cc676f64f3ed9de4f765b0abe63284 --- .../server/input/InputManagerService.java | 68 +++--------- .../com/android/server/wm/AnrController.java | 102 +++++++++++++----- .../server/wm/InputManagerCallback.java | 20 ++-- ...droid_server_input_InputManagerService.cpp | 57 ++-------- 4 files changed, 106 insertions(+), 141 deletions(-) diff --git a/services/core/java/com/android/server/input/InputManagerService.java b/services/core/java/com/android/server/input/InputManagerService.java index c15242a454828..140a28f111e62 100644 --- a/services/core/java/com/android/server/input/InputManagerService.java +++ b/services/core/java/com/android/server/input/InputManagerService.java @@ -144,6 +144,7 @@ import java.util.List; import java.util.Locale; import java.util.Map; import java.util.Objects; +import java.util.OptionalInt; /* * Wraps the C++ InputManager and provides its callbacks. @@ -2915,48 +2916,17 @@ public class InputManagerService extends IInputManager.Stub // Native callback @SuppressWarnings("unused") - private void notifyWindowUnresponsive(IBinder token, String reason) { - int gestureMonitorPid = -1; - synchronized (mInputMonitors) { - final GestureMonitorSpyWindow gestureMonitor = mInputMonitors.get(token); - if (gestureMonitor != null) { - gestureMonitorPid = gestureMonitor.mWindowHandle.ownerPid; - } - } - if (gestureMonitorPid != -1) { - mWindowManagerCallbacks.notifyGestureMonitorUnresponsive(gestureMonitorPid, reason); - return; - } - mWindowManagerCallbacks.notifyWindowUnresponsive(token, reason); + private void notifyWindowUnresponsive(IBinder token, int pid, boolean isPidValid, + String reason) { + mWindowManagerCallbacks.notifyWindowUnresponsive(token, + isPidValid ? OptionalInt.of(pid) : OptionalInt.empty(), reason); } // Native callback @SuppressWarnings("unused") - private void notifyMonitorUnresponsive(int pid, String reason) { - mWindowManagerCallbacks.notifyGestureMonitorUnresponsive(pid, reason); - } - - // Native callback - @SuppressWarnings("unused") - private void notifyWindowResponsive(IBinder token) { - int gestureMonitorPid = -1; - synchronized (mInputMonitors) { - final GestureMonitorSpyWindow gestureMonitor = mInputMonitors.get(token); - if (gestureMonitor != null) { - gestureMonitorPid = gestureMonitor.mWindowHandle.ownerPid; - } - } - if (gestureMonitorPid != -1) { - mWindowManagerCallbacks.notifyGestureMonitorResponsive(gestureMonitorPid); - return; - } - mWindowManagerCallbacks.notifyWindowResponsive(token); - } - - // Native callback - @SuppressWarnings("unused") - private void notifyMonitorResponsive(int pid) { - mWindowManagerCallbacks.notifyGestureMonitorResponsive(pid); + private void notifyWindowResponsive(IBinder token, int pid, boolean isPidValid) { + mWindowManagerCallbacks.notifyWindowResponsive(token, + isPidValid ? OptionalInt.of(pid) : OptionalInt.empty()); } // Native callback. @@ -3328,35 +3298,23 @@ public class InputManagerService extends IInputManager.Stub */ void notifyNoFocusedWindowAnr(InputApplicationHandle applicationHandle); - /** - * Notify the window manager about a gesture monitor that is unresponsive. - * - * @param pid the pid of the gesture monitor process - * @param reason the reason why this connection is unresponsive - */ - void notifyGestureMonitorUnresponsive(int pid, @NonNull String reason); - /** * Notify the window manager about a window that is unresponsive. * * @param token the token that can be used to look up the window + * @param pid the pid of the window owner, if known * @param reason the reason why this connection is unresponsive */ - void notifyWindowUnresponsive(@NonNull IBinder token, @NonNull String reason); - - /** - * Notify the window manager about a gesture monitor that has become responsive. - * - * @param pid the pid of the gesture monitor process - */ - void notifyGestureMonitorResponsive(int pid); + void notifyWindowUnresponsive(@NonNull IBinder token, @NonNull OptionalInt pid, + @NonNull String reason); /** * Notify the window manager about a window that has become responsive. * * @param token the token that can be used to look up the window + * @param pid the pid of the window owner, if known */ - void notifyWindowResponsive(@NonNull IBinder token); + void notifyWindowResponsive(@NonNull IBinder token, @NonNull OptionalInt pid); /** * This callback is invoked when an event first arrives to InputDispatcher and before it is diff --git a/services/core/java/com/android/server/wm/AnrController.java b/services/core/java/com/android/server/wm/AnrController.java index 3d54b27c819bc..6befefda8a123 100644 --- a/services/core/java/com/android/server/wm/AnrController.java +++ b/services/core/java/com/android/server/wm/AnrController.java @@ -21,6 +21,7 @@ import static android.view.WindowManager.LayoutParams.TYPE_APPLICATION_OVERLAY; import static com.android.server.wm.ActivityRecord.INVALID_PID; import static com.android.server.wm.WindowManagerDebugConfig.TAG_WM; +import android.annotation.NonNull; import android.os.Build; import android.os.IBinder; import android.os.Process; @@ -35,6 +36,7 @@ import com.android.server.criticalevents.CriticalEventLog; import java.io.File; import java.util.ArrayList; +import java.util.OptionalInt; import java.util.concurrent.CountDownLatch; import java.util.concurrent.TimeUnit; @@ -75,7 +77,33 @@ class AnrController { activity.inputDispatchingTimedOut(reason, INVALID_PID); } - void notifyWindowUnresponsive(IBinder inputToken, String reason) { + + /** + * Notify a window was unresponsive. + * + * @param token - the input token of the window + * @param pid - the pid of the window, if known + * @param reason - the reason for the window being unresponsive + */ + void notifyWindowUnresponsive(@NonNull IBinder token, @NonNull OptionalInt pid, + @NonNull String reason) { + if (notifyWindowUnresponsive(token, reason)) { + return; + } + if (!pid.isPresent()) { + Slog.w(TAG_WM, "Failed to notify that window token=" + token + " was unresponsive."); + return; + } + notifyWindowUnresponsive(pid.getAsInt(), reason); + } + + /** + * Notify a window identified by its input token was unresponsive. + * + * @return true if the window was identified by the given input token and the request was + * handled, false otherwise. + */ + private boolean notifyWindowUnresponsive(@NonNull IBinder inputToken, String reason) { preDumpIfLockTooSlow(); final int pid; final boolean aboveSystem; @@ -83,10 +111,8 @@ class AnrController { synchronized (mService.mGlobalLock) { InputTarget target = mService.getInputTargetFromToken(inputToken); if (target == null) { - Slog.e(TAG_WM, "Unknown token, dropping notifyConnectionUnresponsive request"); - return; + return false; } - WindowState windowState = target.getWindowState(); pid = target.getPid(); // Blame the activity if the input token belongs to the window. If the target is @@ -102,34 +128,63 @@ class AnrController { } else { mService.mAmInternal.inputDispatchingTimedOut(pid, aboveSystem, reason); } + return true; } - void notifyWindowResponsive(IBinder inputToken) { + /** + * Notify a window owned by the provided pid was unresponsive. + */ + private void notifyWindowUnresponsive(int pid, String reason) { + Slog.i(TAG_WM, "ANR in input window owned by pid=" + pid + ". Reason: " + reason); + dumpAnrStateLocked(null /* activity */, null /* windowState */, reason); + + // We cannot determine the z-order of the window, so place the anr dialog as high + // as possible. + mService.mAmInternal.inputDispatchingTimedOut(pid, true /*aboveSystem*/, reason); + } + + /** + * Notify a window was responsive after previously being unresponsive. + * + * @param token - the input token of the window + * @param pid - the pid of the window, if known + */ + void notifyWindowResponsive(@NonNull IBinder token, @NonNull OptionalInt pid) { + if (notifyWindowResponsive(token)) { + return; + } + if (!pid.isPresent()) { + Slog.w(TAG_WM, "Failed to notify that window token=" + token + " was responsive."); + return; + } + notifyWindowResponsive(pid.getAsInt()); + } + + /** + * Notify a window identified by its input token was responsive after previously being + * unresponsive. + * + * @return true if the window was identified by the given input token and the request was + * handled, false otherwise. + */ + private boolean notifyWindowResponsive(@NonNull IBinder inputToken) { final int pid; synchronized (mService.mGlobalLock) { InputTarget target = mService.getInputTargetFromToken(inputToken); if (target == null) { - Slog.e(TAG_WM, "Unknown token, dropping notifyWindowConnectionResponsive request"); - return; + return false; } pid = target.getPid(); } mService.mAmInternal.inputDispatchingResumed(pid); + return true; } - void notifyGestureMonitorUnresponsive(int gestureMonitorPid, String reason) { - preDumpIfLockTooSlow(); - synchronized (mService.mGlobalLock) { - Slog.i(TAG_WM, "ANR in gesture monitor owned by pid:" + gestureMonitorPid - + ". Reason: " + reason); - dumpAnrStateLocked(null /* activity */, null /* windowState */, reason); - } - mService.mAmInternal.inputDispatchingTimedOut(gestureMonitorPid, /* aboveSystem */ true, - reason); - } - - void notifyGestureMonitorResponsive(int gestureMonitorPid) { - mService.mAmInternal.inputDispatchingResumed(gestureMonitorPid); + /** + * Notify a window owned by the provided pid was responsive after previously being unresponsive. + */ + private void notifyWindowResponsive(int pid) { + mService.mAmInternal.inputDispatchingResumed(pid); } /** @@ -228,12 +283,7 @@ class AnrController { mService.mAtmService.saveANRState(reason); } - private boolean isWindowAboveSystem(WindowState windowState) { - if (windowState == null) { - // If the window state is not available we cannot easily determine its z order. Try to - // place the anr dialog as high as possible. - return true; - } + private boolean isWindowAboveSystem(@NonNull WindowState windowState) { int systemAlertLayer = mService.mPolicy.getWindowLayerFromTypeLw( TYPE_APPLICATION_OVERLAY, windowState.mOwnerCanAddInternalSystemWindow); return windowState.mBaseLayer > systemAlertLayer; diff --git a/services/core/java/com/android/server/wm/InputManagerCallback.java b/services/core/java/com/android/server/wm/InputManagerCallback.java index 1f0fdcf0a8d2c..8d1425d17d473 100644 --- a/services/core/java/com/android/server/wm/InputManagerCallback.java +++ b/services/core/java/com/android/server/wm/InputManagerCallback.java @@ -39,6 +39,7 @@ import com.android.internal.util.function.pooled.PooledLambda; import com.android.server.input.InputManagerService; import java.io.PrintWriter; +import java.util.OptionalInt; final class InputManagerCallback implements InputManagerService.WindowManagerCallbacks { private static final String TAG = TAG_WITH_CLASS_NAME ? "InputManagerCallback" : TAG_WM; @@ -98,23 +99,14 @@ final class InputManagerCallback implements InputManagerService.WindowManagerCal } @Override - public void notifyGestureMonitorUnresponsive(int pid, @NonNull String reason) { - mService.mAnrController.notifyGestureMonitorUnresponsive(pid, reason); + public void notifyWindowUnresponsive(@NonNull IBinder token, @NonNull OptionalInt pid, + @NonNull String reason) { + mService.mAnrController.notifyWindowUnresponsive(token, pid, reason); } @Override - public void notifyWindowUnresponsive(@NonNull IBinder token, String reason) { - mService.mAnrController.notifyWindowUnresponsive(token, reason); - } - - @Override - public void notifyGestureMonitorResponsive(int pid) { - mService.mAnrController.notifyGestureMonitorResponsive(pid); - } - - @Override - public void notifyWindowResponsive(@NonNull IBinder token) { - mService.mAnrController.notifyWindowResponsive(token); + public void notifyWindowResponsive(@NonNull IBinder token, @NonNull OptionalInt pid) { + mService.mAnrController.notifyWindowResponsive(token, pid); } /** Notifies that the input device configuration has changed. */ diff --git a/services/core/jni/com_android_server_input_InputManagerService.cpp b/services/core/jni/com_android_server_input_InputManagerService.cpp index 3c122b03d0d17..31b557949e366 100644 --- a/services/core/jni/com_android_server_input_InputManagerService.cpp +++ b/services/core/jni/com_android_server_input_InputManagerService.cpp @@ -96,8 +96,6 @@ static struct { jmethodID notifyNoFocusedWindowAnr; jmethodID notifyWindowUnresponsive; jmethodID notifyWindowResponsive; - jmethodID notifyMonitorUnresponsive; - jmethodID notifyMonitorResponsive; jmethodID notifyFocusChanged; jmethodID notifySensorEvent; jmethodID notifySensorAccuracy; @@ -308,10 +306,9 @@ public: void notifyConfigurationChanged(nsecs_t when) override; // ANR-related callbacks -- start void notifyNoFocusedWindowAnr(const std::shared_ptr& handle) override; - void notifyWindowUnresponsive(const sp& token, const std::string& reason) override; - void notifyWindowResponsive(const sp& token) override; - void notifyMonitorUnresponsive(int32_t pid, const std::string& reason) override; - void notifyMonitorResponsive(int32_t pid) override; + void notifyWindowUnresponsive(const sp& token, std::optional pid, + const std::string& reason) override; + void notifyWindowResponsive(const sp& token, std::optional pid) override; // ANR-related callbacks -- end void notifyInputChannelBroken(const sp& token) override; void notifyFocusChanged(const sp& oldToken, const sp& newToken) override; @@ -838,6 +835,7 @@ void NativeInputManager::notifyNoFocusedWindowAnr( } void NativeInputManager::notifyWindowUnresponsive(const sp& token, + std::optional pid, const std::string& reason) { #if DEBUG_INPUT_DISPATCHER_POLICY ALOGD("notifyWindowUnresponsive"); @@ -851,11 +849,12 @@ void NativeInputManager::notifyWindowUnresponsive(const sp& token, ScopedLocalRef reasonObj(env, env->NewStringUTF(reason.c_str())); env->CallVoidMethod(mServiceObj, gServiceClassInfo.notifyWindowUnresponsive, tokenObj, - reasonObj.get()); + pid.value_or(0), pid.has_value(), reasonObj.get()); checkAndClearExceptionFromCallback(env, "notifyWindowUnresponsive"); } -void NativeInputManager::notifyWindowResponsive(const sp& token) { +void NativeInputManager::notifyWindowResponsive(const sp& token, + std::optional pid) { #if DEBUG_INPUT_DISPATCHER_POLICY ALOGD("notifyWindowResponsive"); #endif @@ -866,39 +865,11 @@ void NativeInputManager::notifyWindowResponsive(const sp& token) { jobject tokenObj = javaObjectForIBinder(env, token); - env->CallVoidMethod(mServiceObj, gServiceClassInfo.notifyWindowResponsive, tokenObj); + env->CallVoidMethod(mServiceObj, gServiceClassInfo.notifyWindowResponsive, tokenObj, + pid.value_or(0), pid.has_value()); checkAndClearExceptionFromCallback(env, "notifyWindowResponsive"); } -void NativeInputManager::notifyMonitorUnresponsive(int32_t pid, const std::string& reason) { -#if DEBUG_INPUT_DISPATCHER_POLICY - ALOGD("notifyMonitorUnresponsive"); -#endif - ATRACE_CALL(); - - JNIEnv* env = jniEnv(); - ScopedLocalFrame localFrame(env); - - ScopedLocalRef reasonObj(env, env->NewStringUTF(reason.c_str())); - - env->CallVoidMethod(mServiceObj, gServiceClassInfo.notifyMonitorUnresponsive, pid, - reasonObj.get()); - checkAndClearExceptionFromCallback(env, "notifyMonitorUnresponsive"); -} - -void NativeInputManager::notifyMonitorResponsive(int32_t pid) { -#if DEBUG_INPUT_DISPATCHER_POLICY - ALOGD("notifyMonitorResponsive"); -#endif - ATRACE_CALL(); - - JNIEnv* env = jniEnv(); - ScopedLocalFrame localFrame(env); - - env->CallVoidMethod(mServiceObj, gServiceClassInfo.notifyMonitorResponsive, pid); - checkAndClearExceptionFromCallback(env, "notifyMonitorResponsive"); -} - void NativeInputManager::notifyInputChannelBroken(const sp& token) { #if DEBUG_INPUT_DISPATCHER_POLICY ALOGD("notifyInputChannelBroken"); @@ -2506,16 +2477,10 @@ int register_android_server_InputManager(JNIEnv* env) { "(Landroid/view/InputApplicationHandle;)V"); GET_METHOD_ID(gServiceClassInfo.notifyWindowUnresponsive, clazz, "notifyWindowUnresponsive", - "(Landroid/os/IBinder;Ljava/lang/String;)V"); - - GET_METHOD_ID(gServiceClassInfo.notifyMonitorUnresponsive, clazz, "notifyMonitorUnresponsive", - "(ILjava/lang/String;)V"); + "(Landroid/os/IBinder;IZLjava/lang/String;)V"); GET_METHOD_ID(gServiceClassInfo.notifyWindowResponsive, clazz, "notifyWindowResponsive", - "(Landroid/os/IBinder;)V"); - - GET_METHOD_ID(gServiceClassInfo.notifyMonitorResponsive, clazz, "notifyMonitorResponsive", - "(I)V"); + "(Landroid/os/IBinder;IZ)V"); GET_METHOD_ID(gServiceClassInfo.filterInputEvent, clazz, "filterInputEvent", "(Landroid/view/InputEvent;I)Z");