diff options
5 files changed, 287 insertions, 91 deletions
diff --git a/services/core/java/com/android/server/am/AnrTimer.java b/services/core/java/com/android/server/am/AnrTimer.java index 378a38602211..9ba49ce35dad 100644 --- a/services/core/java/com/android/server/am/AnrTimer.java +++ b/services/core/java/com/android/server/am/AnrTimer.java @@ -108,6 +108,14 @@ class AnrTimer<V> { private static final boolean ENABLE_TRACING = false; /** + * Return true if the feature is enabled. By default, the value is take from the Flags class + * but it can be changed for local testing. + */ + private static boolean anrTimerServiceEnabled() { + return Flags.anrTimerServiceEnabled(); + } + + /** * The status of an ANR timer. TIMER_INVALID status is returned when an error is detected. */ private static final int TIMER_INVALID = 0; @@ -327,18 +335,33 @@ class AnrTimer<V> { */ @VisibleForTesting static class Injector { + private final Handler mReferenceHandler; + + Injector(@NonNull Handler handler) { + mReferenceHandler = handler; + } + /** - * Return a handler for the given Callback. + * Return a handler for the given Callback, based on the reference handler. The handler + * might be mocked, in which case it does not have a valid Looper. In this case, use the + * main Looper. */ + @NonNull Handler getHandler(@NonNull Handler.Callback callback) { - return null; + Looper looper = mReferenceHandler.getLooper(); + if (looper == null) looper = Looper.getMainLooper(); + return new Handler(looper, callback); }; - /** - * Return a CpuTracker. - */ + /** Return a CpuTracker. */ + @NonNull CpuTracker getTracker() { - return null; + return new CpuTracker(); + } + + /** Return true if the feature is enabled. */ + boolean getFeatureEnabled() { + return anrTimerServiceEnabled(); } } @@ -375,12 +398,6 @@ class AnrTimer<V> { /** The interface to fetch process statistics that might extend an ANR timeout. */ private final CpuTracker mCpu; - /** Create a HandlerTimerService based on the input handler. */ - HandlerTimerService(@NonNull Handler handler) { - mHandler = new Handler(handler.getLooper(), this::expires); - mCpu = new CpuTracker(); - } - /** Create a HandlerTimerService that directly uses the supplied handler and tracker. */ @VisibleForTesting HandlerTimerService(@NonNull Injector injector) { @@ -491,38 +508,56 @@ class AnrTimer<V> { private final boolean mLenientCancel = true; /** + * The top-level switch for the feature enabled or disabled. + */ + private final FeatureSwitch mFeature; + + /** * The common constructor. A null injector results in a normal, production timer. */ @VisibleForTesting AnrTimer(@NonNull Handler handler, int what, @NonNull String label, boolean extend, - @Nullable Injector injector) { + @NonNull Injector injector) { mHandler = handler; mWhat = what; mLabel = label; mExtend = extend; - if (injector == null) { - mTimerService = new HandlerTimerService(handler); + boolean enabled = injector.getFeatureEnabled(); + if (!enabled) { + mFeature = new FeatureDisabled(); + mTimerService = null; } else { + mFeature = new FeatureEnabled(); mTimerService = new HandlerTimerService(injector); + + synchronized (sAnrTimerList) { + sAnrTimerList.add(new WeakReference(this)); + } } - synchronized (sAnrTimerList) { - sAnrTimerList.add(new WeakReference(this)); - } - Log.i(TAG, formatSimple("created %s label: \"%s\"", mTimerService.toString(), label)); + Log.i(TAG, formatSimple("created %s label: \"%s\"", mTimerService, label)); } /** * Create one timer instance for production. The client can ask for extensible timeouts. */ AnrTimer(@NonNull Handler handler, int what, @NonNull String label, boolean extend) { - this(handler, what, label, extend, null); + this(handler, what, label, extend, new Injector(handler)); } /** * Create one timer instance for production. There are no extensible timeouts. */ AnrTimer(@NonNull Handler handler, int what, @NonNull String label) { - this(handler, what, label, false, null); + this(handler, what, label, false); + } + + /** + * Return true if the service is enabled on this instance. Clients should use this method to + * decide if the feature is enabled, and not read the flags directly. This method should be + * deleted if and when the feature is enabled permanently. + */ + boolean serviceEnabled() { + return mFeature.enabled(); } /** @@ -613,93 +648,186 @@ class AnrTimer<V> { Log.i(TAG, msg + " " + timer + " " + Objects.toString(timer.arg)); } - /** - * Start a timer. + /** + * The FeatureSwitch class provides a quick switch between feature-enabled behavior and + * feature-disabled behavior. */ - boolean start(@NonNull V arg, int pid, int uid, long timeoutMs) { - final Timer timer = Timer.obtain(pid, uid, arg, timeoutMs, this); - synchronized (mLock) { - Timer old = mTimerMap.get(arg); - if (old != null) { - // There is an existing timer. This is a protocol error in the client. Record - // the error and then clean up by canceling running timers and discarding expired - // timers. - restartedLocked(old.status, arg); - if (old.status == TIMER_EXPIRED) { - discard(arg); + private abstract class FeatureSwitch { + abstract boolean start(@NonNull V arg, int pid, int uid, long timeoutMs); + abstract boolean cancel(@NonNull V arg); + abstract boolean accept(@NonNull V arg); + abstract boolean discard(@NonNull V arg); + abstract boolean enabled(); + } + + /** + * The FeatureDisabled class bypasses almost all AnrTimer logic. It is used when the AnrTimer + * service is disabled via Flags.anrTimerServiceEnabled. + */ + private class FeatureDisabled extends FeatureSwitch { + /** Start a timer by sending a message to the client's handler. */ + boolean start(@NonNull V arg, int pid, int uid, long timeoutMs) { + final Message msg = mHandler.obtainMessage(mWhat, arg); + mHandler.sendMessageDelayed(msg, timeoutMs); + return true; + } + + /** Cancel a timer by removing the message from the client's handler. */ + boolean cancel(@NonNull V arg) { + mHandler.removeMessages(mWhat, arg); + return true; + } + + /** accept() is a no-op when the feature is disabled. */ + boolean accept(@NonNull V arg) { + return true; + } + + /** discard() is a no-op when the feature is disabled. */ + boolean discard(@NonNull V arg) { + return true; + } + + /** The feature is not enabled. */ + boolean enabled() { + return false; + } + } + + /** + * The FeatureEnabled class enables the AnrTimer logic. It is used when the AnrTimer service + * is enabled via Flags.anrTimerServiceEnabled. + */ + private class FeatureEnabled extends FeatureSwitch { + + /** + * Start a timer. + */ + boolean start(@NonNull V arg, int pid, int uid, long timeoutMs) { + final Timer timer = Timer.obtain(pid, uid, arg, timeoutMs, AnrTimer.this); + synchronized (mLock) { + Timer old = mTimerMap.get(arg); + if (old != null) { + // There is an existing timer. This is a protocol error in the client. + // Record the error and then clean up by canceling running timers and + // discarding expired timers. + restartedLocked(old.status, arg); + if (old.status == TIMER_EXPIRED) { + discard(arg); + } else { + cancel(arg); + } + } + if (mTimerService.start(timer)) { + timer.status = TIMER_RUNNING; + mTimerMap.put(arg, timer); + mTotalStarted++; + mMaxStarted = Math.max(mMaxStarted, mTimerMap.size()); + if (DEBUG) report(timer, "start"); + return true; } else { - cancel(arg); + Log.e(TAG, "AnrTimer.start failed"); + return false; } } - if (mTimerService.start(timer)) { - timer.status = TIMER_RUNNING; - mTimerMap.put(arg, timer); - mTotalStarted++; - mMaxStarted = Math.max(mMaxStarted, mTimerMap.size()); - if (DEBUG) report(timer, "start"); + } + + /** + * Cancel a timer. Return false if the timer was not found. + */ + boolean cancel(@NonNull V arg) { + synchronized (mLock) { + Timer timer = removeLocked(arg); + if (timer == null) { + if (!mLenientCancel) notFoundLocked("cancel", arg); + return false; + } + mTimerService.cancel(timer); + // There may be an expiration message in flight. Cancel it. + mHandler.removeMessages(mWhat, arg); + if (DEBUG) report(timer, "cancel"); + timer.release(); return true; - } else { - Log.e(TAG, "AnrTimer.start failed"); - return false; } } + + /** + * Accept a timer in the framework-level handler. The timeout has been accepted and the + * timeout handler is executing. Return false if the timer was not found. + */ + boolean accept(@NonNull V arg) { + synchronized (mLock) { + Timer timer = removeLocked(arg); + if (timer == null) { + notFoundLocked("accept", arg); + return false; + } + mTimerService.accept(timer); + traceEnd(timer); + if (DEBUG) report(timer, "accept"); + timer.release(); + return true; + } + } + + /** + * Discard a timer in the framework-level handler. For whatever reason, the timer is no + * longer interesting. No statistics are collected. Return false if the time was not + * found. + */ + boolean discard(@NonNull V arg) { + synchronized (mLock) { + Timer timer = removeLocked(arg); + if (timer == null) { + notFoundLocked("discard", arg); + return false; + } + mTimerService.discard(timer); + traceEnd(timer); + if (DEBUG) report(timer, "discard"); + timer.release(); + return true; + } + } + + /** The feature is enabled. */ + boolean enabled() { + return true; + } } /** - * Cancel a timer. Return false if the timer was not found. + * Start a timer associated with arg. If a timer already exists with the same arg, then that + * timer is canceled and a new timer is created. This returns false if the timer cannot be + * created. + */ + boolean start(@NonNull V arg, int pid, int uid, long timeoutMs) { + return mFeature.start(arg, pid, uid, timeoutMs); + } + + /** + * Cancel a running timer and remove it from any list. This returns true if the timer was + * found and false otherwise. It is not an error to cancel a non-existent timer. It is also + * not an error to cancel an expired timer. */ boolean cancel(@NonNull V arg) { - synchronized (mLock) { - Timer timer = removeLocked(arg); - if (timer == null) { - if (!mLenientCancel) notFoundLocked("cancel", arg); - return false; - } - mTimerService.cancel(timer); - // There may be an expiration message in flight. Cancel it. - mHandler.removeMessages(mWhat, arg); - if (DEBUG) report(timer, "cancel"); - timer.release(); - return true; - } + return mFeature.cancel(arg); } /** - * Accept a timer in the framework-level handler. The timeout has been accepted and the - * timeout handler is executing. Return false if the timer was not found. + * Accept an expired timer. This returns false if the timer was not found or if the timer was + * not expired. */ boolean accept(@NonNull V arg) { - synchronized (mLock) { - Timer timer = removeLocked(arg); - if (timer == null) { - notFoundLocked("accept", arg); - return false; - } - mTimerService.accept(timer); - traceEnd(timer); - if (DEBUG) report(timer, "accept"); - timer.release(); - return true; - } + return mFeature.accept(arg); } /** - * Discard a timer in the framework-level handler. For whatever reason, the timer is no - * longer interesting. No statistics are collected. Return false if the time was not found. + * Discard an expired timer. This returns false if the timer was not found or if the timer was + * not expired. */ boolean discard(@NonNull V arg) { - synchronized (mLock) { - Timer timer = removeLocked(arg); - if (timer == null) { - notFoundLocked("discard", arg); - return false; - } - mTimerService.discard(timer); - traceEnd(timer); - if (DEBUG) report(timer, "discard"); - timer.release(); - return true; - } + return mFeature.discard(arg); } /** diff --git a/services/core/java/com/android/server/am/BroadcastProcessQueue.java b/services/core/java/com/android/server/am/BroadcastProcessQueue.java index 5d31d1545b8d..e07c2bcaaaed 100644 --- a/services/core/java/com/android/server/am/BroadcastProcessQueue.java +++ b/services/core/java/com/android/server/am/BroadcastProcessQueue.java @@ -106,6 +106,14 @@ class BroadcastProcessQueue { private boolean mTimeoutScheduled; /** + * Snapshotted value of {@link ProcessRecord#getCpuDelayTime()}, typically + * used when deciding if we should extend the soft ANR timeout. + * + * Required when Flags.anrTimerServiceEnabled is false. + */ + long lastCpuDelayTime; + + /** * Snapshotted value of {@link ProcessStateRecord#getCurProcState()} before * dispatching the current broadcast to the receiver in this process. */ diff --git a/services/core/java/com/android/server/am/BroadcastQueueModernImpl.java b/services/core/java/com/android/server/am/BroadcastQueueModernImpl.java index eb219a8819c6..a42890707368 100644 --- a/services/core/java/com/android/server/am/BroadcastQueueModernImpl.java +++ b/services/core/java/com/android/server/am/BroadcastQueueModernImpl.java @@ -258,6 +258,9 @@ class BroadcastQueueModernImpl extends BroadcastQueue { private static final int MSG_PROCESS_FREEZABLE_CHANGED = 6; private static final int MSG_UID_STATE_CHANGED = 7; + // Required when Flags.anrTimerServiceEnabled is false. + private static final int MSG_DELIVERY_TIMEOUT_SOFT = 8; + private void enqueueUpdateRunningList() { mLocalHandler.removeMessages(MSG_UPDATE_RUNNING_LIST); mLocalHandler.sendEmptyMessage(MSG_UPDATE_RUNNING_LIST); @@ -271,6 +274,13 @@ class BroadcastQueueModernImpl extends BroadcastQueue { updateRunningList(); return true; } + // Required when Flags.anrTimerServiceEnabled is false. + case MSG_DELIVERY_TIMEOUT_SOFT: { + synchronized (mService) { + deliveryTimeoutSoftLocked((BroadcastProcessQueue) msg.obj, msg.arg1); + return true; + } + } case MSG_DELIVERY_TIMEOUT: { deliveryTimeout((BroadcastProcessQueue) msg.obj); return true; @@ -1030,7 +1040,7 @@ class BroadcastQueueModernImpl extends BroadcastQueue { queue.setTimeoutScheduled(true); final int softTimeoutMillis = (int) (r.isForeground() ? mFgConstants.TIMEOUT : mBgConstants.TIMEOUT); - mAnrTimer.start(queue, softTimeoutMillis); + startDeliveryTimeoutLocked(queue, softTimeoutMillis); } else { queue.setTimeoutScheduled(false); } @@ -1110,7 +1120,7 @@ class BroadcastQueueModernImpl extends BroadcastQueue { // If we were trying to deliver a manifest broadcast, throw the error as we need // to try redelivering the broadcast to this receiver. if (receiver instanceof ResolveInfo) { - mAnrTimer.cancel(queue); + cancelDeliveryTimeoutLocked(queue); throw new BroadcastDeliveryFailedException(e); } finishReceiverActiveLocked(queue, BroadcastRecord.DELIVERY_FAILURE, @@ -1159,6 +1169,41 @@ class BroadcastQueueModernImpl extends BroadcastQueue { r.resultTo = null; } + // Required when Flags.anrTimerServiceEnabled is false. + private void startDeliveryTimeoutLocked(@NonNull BroadcastProcessQueue queue, + int softTimeoutMillis) { + if (mAnrTimer.serviceEnabled()) { + mAnrTimer.start(queue, softTimeoutMillis); + } else { + queue.lastCpuDelayTime = queue.app.getCpuDelayTime(); + mLocalHandler.sendMessageDelayed(Message.obtain(mLocalHandler, + MSG_DELIVERY_TIMEOUT_SOFT, softTimeoutMillis, 0, queue), softTimeoutMillis); + } + } + + // Required when Flags.anrTimerServiceEnabled is false. + private void cancelDeliveryTimeoutLocked(@NonNull BroadcastProcessQueue queue) { + mAnrTimer.cancel(queue); + if (!mAnrTimer.serviceEnabled()) { + mLocalHandler.removeMessages(MSG_DELIVERY_TIMEOUT_SOFT, queue); + } + } + + // Required when Flags.anrTimerServiceEnabled is false. + private void deliveryTimeoutSoftLocked(@NonNull BroadcastProcessQueue queue, + int softTimeoutMillis) { + if (queue.app != null) { + // Instead of immediately triggering an ANR, extend the timeout by + // the amount of time the process was runnable-but-waiting; we're + // only willing to do this once before triggering an hard ANR + final long cpuDelayTime = queue.app.getCpuDelayTime() - queue.lastCpuDelayTime; + final long hardTimeoutMillis = MathUtils.constrain(cpuDelayTime, 0, softTimeoutMillis); + mAnrTimer.start(queue, hardTimeoutMillis); + } else { + deliveryTimeoutLocked(queue); + } + } + private void deliveryTimeout(@NonNull BroadcastProcessQueue queue) { synchronized (mService) { deliveryTimeoutLocked(queue); @@ -1292,7 +1337,7 @@ class BroadcastQueueModernImpl extends BroadcastQueue { mAnrTimer.discard(queue); } } else if (queue.timeoutScheduled()) { - mAnrTimer.cancel(queue); + cancelDeliveryTimeoutLocked(queue); } // Given that a receiver just finished, check if the "waitingFor" conditions are met. diff --git a/services/core/java/com/android/server/am/flags.aconfig b/services/core/java/com/android/server/am/flags.aconfig index b03cc6295b8d..26d99d843c7e 100644 --- a/services/core/java/com/android/server/am/flags.aconfig +++ b/services/core/java/com/android/server/am/flags.aconfig @@ -6,4 +6,12 @@ flag { description: "Utilize new OomAdjuster implementation" bug: "298055811" is_fixed_read_only: true -}
\ No newline at end of file +} + +flag { + name: "anr_timer_service_enabled" + namespace: "system_performance" + is_fixed_read_only: true + description: "Feature flag for the ANR timer service" + bug: "282428924" +} diff --git a/services/tests/servicestests/src/com/android/server/am/AnrTimerTest.java b/services/tests/servicestests/src/com/android/server/am/AnrTimerTest.java index 9fdbdda38c75..70527ce2ad32 100644 --- a/services/tests/servicestests/src/com/android/server/am/AnrTimerTest.java +++ b/services/tests/servicestests/src/com/android/server/am/AnrTimerTest.java @@ -228,6 +228,7 @@ public class AnrTimerTest { TestHandler mTestHandler; TestInjector(int skip, boolean immediate) { + super(mHandler); mTracker = new TestTracker(skip); mImmediate = immediate; } @@ -249,9 +250,16 @@ public class AnrTimerTest { return mTestHandler; } + @Override AnrTimer.CpuTracker getTracker() { return mTracker; } + + /** For test purposes, always enable the feature. */ + @Override + boolean getFeatureEnabled() { + return true; + } } // Tests @@ -261,7 +269,6 @@ public class AnrTimerTest { // 4. Start a couple of timers. Verify max active timers. Discard one and verify the active // count drops by 1. Accept one and verify the active count drops by 1. - @Test public void testSimpleTimeout() throws Exception { // Create an immediate TestHandler. |