From 3e4b28028b32569fde64b2eb1803631363052add Mon Sep 17 00:00:00 2001 From: Jeff Sharkey Date: Wed, 9 Nov 2022 10:53:02 -0700 Subject: [PATCH] BroadcastQueue: fix subtle soft/hard timeout bug. deliveryTimeoutSoftLocked() had been using mConstants.TIMEOUT for clamping purposes, but that always ended up being the foreground timeout, even for background broadcasts. Fix this bug by passing through the actual ANR timeout value used, and apply that for clamping purposes, making sure we're always willing to up to double the original timeout. Bug: 258204427 Test: atest FrameworksMockingServicesTests:BroadcastRecordTest Test: atest FrameworksMockingServicesTests:BroadcastQueueTest Test: atest FrameworksMockingServicesTests:BroadcastQueueModernImplTest Change-Id: I057f14f9c1055f99b5fd98e991a3d55cefcbd3bb --- .../server/am/BroadcastQueueModernImpl.java | 19 ++++++++++++++----- 1 file changed, 14 insertions(+), 5 deletions(-) diff --git a/services/core/java/com/android/server/am/BroadcastQueueModernImpl.java b/services/core/java/com/android/server/am/BroadcastQueueModernImpl.java index c3839a99ba529..bc831d463da93 100644 --- a/services/core/java/com/android/server/am/BroadcastQueueModernImpl.java +++ b/services/core/java/com/android/server/am/BroadcastQueueModernImpl.java @@ -37,6 +37,7 @@ import static com.android.server.am.BroadcastRecord.isDeliveryStateTerminal; import static com.android.server.am.OomAdjuster.OOM_ADJ_REASON_FINISH_RECEIVER; import static com.android.server.am.OomAdjuster.OOM_ADJ_REASON_START_RECEIVER; +import android.annotation.DurationMillisLong; import android.annotation.NonNull; import android.annotation.Nullable; import android.annotation.UptimeMillisLong; @@ -239,7 +240,11 @@ class BroadcastQueueModernImpl extends BroadcastQueue { } case MSG_DELIVERY_TIMEOUT_SOFT: { synchronized (mService) { - deliveryTimeoutSoftLocked((BroadcastProcessQueue) msg.obj); + final SomeArgs args = (SomeArgs) msg.obj; + final BroadcastProcessQueue queue = (BroadcastProcessQueue) args.arg1; + final long originalTimeout = args.argl1; + args.recycle(); + deliveryTimeoutSoftLocked(queue, originalTimeout); } return true; } @@ -746,8 +751,11 @@ class BroadcastQueueModernImpl extends BroadcastQueue { queue.lastCpuDelayTime = queue.app.getCpuDelayTime(); final long timeout = r.isForeground() ? mFgConstants.TIMEOUT : mBgConstants.TIMEOUT; + final SomeArgs args = SomeArgs.obtain(); + args.arg1 = queue; + args.argl1 = timeout; mLocalHandler.sendMessageDelayed( - Message.obtain(mLocalHandler, MSG_DELIVERY_TIMEOUT_SOFT, queue), timeout); + Message.obtain(mLocalHandler, MSG_DELIVERY_TIMEOUT_SOFT, args), timeout); } if (r.allowBackgroundActivityStarts) { @@ -834,15 +842,16 @@ class BroadcastQueueModernImpl extends BroadcastQueue { r.resultTo = null; } - private void deliveryTimeoutSoftLocked(@NonNull BroadcastProcessQueue queue) { + private void deliveryTimeoutSoftLocked(@NonNull BroadcastProcessQueue queue, + @DurationMillisLong long originalTimeout) { 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 timeout = MathUtils.constrain(cpuDelayTime, 0, mConstants.TIMEOUT); + final long hardTimeout = MathUtils.constrain(cpuDelayTime, 0, originalTimeout); mLocalHandler.sendMessageDelayed( - Message.obtain(mLocalHandler, MSG_DELIVERY_TIMEOUT_HARD, queue), timeout); + Message.obtain(mLocalHandler, MSG_DELIVERY_TIMEOUT_HARD, queue), hardTimeout); } else { deliveryTimeoutHardLocked(queue); }