From 5564cddbd70a3745e516c39f5299de3886fdf936 Mon Sep 17 00:00:00 2001 From: Jeff Sharkey Date: Fri, 28 Oct 2022 14:47:06 -0600 Subject: [PATCH 1/2] BroadcastQueue: fix subtle OOM adjustment bug. updateOomAdjPendingTargetsLocked() only takes effect for processes that are already running as warm; it doesn't work on a process that is still being cold started. This change fixes the bug by making an explicit update call once a pending cold start has become a warm process. Bug: 255985948 Test: atest CtsAppTestCases:ActivityManagerProcessStateTest#testBackgroundCheckBroadcastService Change-Id: I7e42984c7b7fd6e496fc9245304f1eff8ad71075 --- .../server/am/BroadcastQueueModernImpl.java | 17 ++++++++++++----- 1 file changed, 12 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 9e9eb71db4e52..822294859f3a3 100644 --- a/services/core/java/com/android/server/am/BroadcastQueueModernImpl.java +++ b/services/core/java/com/android/server/am/BroadcastQueueModernImpl.java @@ -412,21 +412,24 @@ class BroadcastQueueModernImpl extends BroadcastQueue { queue.runningTraceTrackName = TAG + ".mRunning[" + queueIndex + "]"; queue.runningOomAdjusted = queue.isPendingManifest(); + // If already warm, we can make OOM adjust request immediately; + // otherwise we need to wait until process becomes warm + if (processWarm) { + notifyStartedRunning(queue); + updateOomAdj |= queue.runningOomAdjusted; + } + // If we're already warm, schedule next pending broadcast now; // otherwise we'll wait for the cold start to circle back around queue.makeActiveNextPending(); if (processWarm) { queue.traceProcessRunningBegin(); - notifyStartedRunning(queue); scheduleReceiverWarmLocked(queue); } else { queue.traceProcessStartingBegin(); scheduleReceiverColdLocked(queue); } - // Only kick off an OOM adjustment pass if needed - updateOomAdj |= queue.runningOomAdjusted; - // Move to considering next runnable queue queue = nextQueue; } @@ -464,9 +467,13 @@ class BroadcastQueueModernImpl extends BroadcastQueue { // now; dispatch its next broadcast and clear the slot mRunningColdStart = null; + // Now that we're running warm, we can finally request that OOM + // adjust we've been waiting for + notifyStartedRunning(queue); + mService.updateOomAdjPendingTargetsLocked(OOM_ADJ_REASON_START_RECEIVER); + queue.traceProcessEnd(); queue.traceProcessRunningBegin(); - notifyStartedRunning(queue); scheduleReceiverWarmLocked(queue); // We might be willing to kick off another cold start From 3c265597bd167d10af324eb4bce42ef95096cc00 Mon Sep 17 00:00:00 2001 From: Jeff Sharkey Date: Fri, 28 Oct 2022 19:37:09 -0600 Subject: [PATCH 2/2] BroadcastQueue: fix subtle resultTo bugs. Even when no remote IApplicationThread is provided, we should still clear out the IIntentReceiver reference so that it can be GC'ed in the remote process. Additionally, fix related bug where startActivityIntentSender() has an IApplicationThread instance but doesn't pass it along. Add log message to aid future investigations where IApplicationThread might have been omitted by the caller. Bug: 255833934 Test: atest --rerun-until-failure 100 NexusLauncherTests:WellbeingTests Change-Id: I84a40c7f44eb761943e3c2aa0d3b78ffc2cbe901 --- .../java/com/android/server/am/BroadcastQueueModernImpl.java | 4 ++-- .../core/java/com/android/server/am/PendingIntentRecord.java | 5 +++++ .../com/android/server/wm/ActivityTaskManagerService.java | 2 +- 3 files changed, 8 insertions(+), 3 deletions(-) diff --git a/services/core/java/com/android/server/am/BroadcastQueueModernImpl.java b/services/core/java/com/android/server/am/BroadcastQueueModernImpl.java index 822294859f3a3..57506199f0e1e 100644 --- a/services/core/java/com/android/server/am/BroadcastQueueModernImpl.java +++ b/services/core/java/com/android/server/am/BroadcastQueueModernImpl.java @@ -816,9 +816,9 @@ class BroadcastQueueModernImpl extends BroadcastQueue { * ordered broadcast; assumes the sender is still a warm process. */ private void scheduleResultTo(@NonNull BroadcastRecord r) { - if ((r.resultToApp == null) || (r.resultTo == null)) return; + if (r.resultTo == null) return; final ProcessRecord app = r.resultToApp; - final IApplicationThread thread = app.getOnewayThread(); + final IApplicationThread thread = (app != null) ? app.getOnewayThread() : null; if (thread != null) { mService.mOomAdjuster.mCachedAppOptimizer.unfreezeTemporarily( app, OOM_ADJ_REASON_FINISH_RECEIVER); diff --git a/services/core/java/com/android/server/am/PendingIntentRecord.java b/services/core/java/com/android/server/am/PendingIntentRecord.java index 740efbc658ba9..14a169737b382 100644 --- a/services/core/java/com/android/server/am/PendingIntentRecord.java +++ b/services/core/java/com/android/server/am/PendingIntentRecord.java @@ -490,6 +490,11 @@ public final class PendingIntentRecord extends IIntentSender.Stub { final IApplicationThread finishedReceiverThread = caller; boolean sendFinish = finishedReceiver != null; + if ((finishedReceiver != null) && (finishedReceiverThread == null)) { + Slog.w(TAG, "Sending of " + intent + " from " + Binder.getCallingUid() + + " requested resultTo without an IApplicationThread!", new Throwable()); + } + int userId = key.userId; if (userId == UserHandle.USER_CURRENT) { userId = controller.mUserController.getCurrentOrTargetUserId(); diff --git a/services/core/java/com/android/server/wm/ActivityTaskManagerService.java b/services/core/java/com/android/server/wm/ActivityTaskManagerService.java index ecc43f7b938b1..b153a85a4048f 100644 --- a/services/core/java/com/android/server/wm/ActivityTaskManagerService.java +++ b/services/core/java/com/android/server/wm/ActivityTaskManagerService.java @@ -1316,7 +1316,7 @@ public class ActivityTaskManagerService extends IActivityTaskManager.Stub { mAppSwitchesState = APP_SWITCH_ALLOW; } } - return pir.sendInner(0, fillInIntent, resolvedType, allowlistToken, null, null, + return pir.sendInner(caller, 0, fillInIntent, resolvedType, allowlistToken, null, null, resultTo, resultWho, requestCode, flagsMask, flagsValues, bOptions); }