From 7b234eef4a7508801c9957b8f7b7b61182b23eab Mon Sep 17 00:00:00 2001 From: Jeff Sharkey Date: Thu, 25 Aug 2022 10:42:05 -0600 Subject: [PATCH 1/3] [3/?] Reduce BroadcastQueue interface complexity. Instead of requesting a specific concrete action of the broadcast stack, pivot to communicating application lifecycle changes, which the broadcast stack can then respond to internally. Slight rename to skipCurrentOrPendingReceiverLocked() to reflect that it will skip either the current or pending receiver. This renamed method is then called via onApplicationTimeoutLocked(), which previously tried applying pending-vs-current nuance where it appears to have not been needed. Bug: 243656033 Test: atest CtsContentTestCases:BroadcastReceiverTest Change-Id: I451df11e499465940658e15d730491f44add9b62 --- .../server/am/ActivityManagerService.java | 68 ++++--------------- .../com/android/server/am/BroadcastQueue.java | 31 +++++++-- .../android/server/am/BroadcastQueueImpl.java | 35 +++++++--- .../server/am/ProcessErrorStateRecord.java | 4 +- 4 files changed, 64 insertions(+), 74 deletions(-) diff --git a/services/core/java/com/android/server/am/ActivityManagerService.java b/services/core/java/com/android/server/am/ActivityManagerService.java index a0c2ad57241bd..c18117aee2d9a 100644 --- a/services/core/java/com/android/server/am/ActivityManagerService.java +++ b/services/core/java/com/android/server/am/ActivityManagerService.java @@ -4600,6 +4600,10 @@ public class ActivityManagerService extends IActivityManager.Stub mCpHelper.cleanupAppInLaunchingProvidersLocked(app, true); // Take care of any services that are waiting for the process. mServices.processStartTimedOutLocked(app); + // Take care of any broadcasts waiting for the process. + for (BroadcastQueue queue : mBroadcastQueues) { + queue.onApplicationTimeoutLocked(app); + } if (!isKillTimeout) { mBatteryStatsService.noteProcessFinish(app.processName, app.info.uid); app.killLocked("start timeout", @@ -4631,16 +4635,6 @@ public class ActivityManagerService extends IActivityManager.Stub } }); } - if (!isKillTimeout) { - if (isPendingBroadcastProcessLocked(pid)) { - Slog.w(TAG, "Unattached app died before broadcast acknowledged, skipping"); - skipPendingBroadcastLocked(pid); - } - } else { - if (isPendingBroadcastProcessLocked(app)) { - skipCurrentReceiverLocked(app); - } - } } else { Slog.w(TAG, "Spurious process start timeout - pid not known for " + app); } @@ -4965,10 +4959,12 @@ public class ActivityManagerService extends IActivityManager.Stub } // Check if a next-broadcast receiver is in this process... - if (!badApp && isPendingBroadcastProcessLocked(pid)) { + if (!badApp) { try { - didSomething |= sendPendingBroadcastsLocked(app); - checkTime(startTime, "attachApplicationLocked: after sendPendingBroadcastsLocked"); + for (BroadcastQueue queue : mBroadcastQueues) { + didSomething |= queue.onApplicationAttachedLocked(app); + } + checkTime(startTime, "attachApplicationLocked: after dispatching broadcasts"); } catch (Exception e) { // If the app died trying to launch the receiver we declare it 'bad' Slog.wtf(TAG, "Exception thrown dispatching broadcasts in " + app, e); @@ -8355,12 +8351,6 @@ public class ActivityManagerService extends IActivityManager.Stub } } - void skipCurrentReceiverLocked(ProcessRecord app) { - for (BroadcastQueue queue : mBroadcastQueues) { - queue.skipCurrentReceiverLocked(app); - } - } - /** * Used by {@link com.android.internal.os.RuntimeInit} to report when an application crashes. * The application process will exit immediately after this call returns. @@ -12373,7 +12363,9 @@ public class ActivityManagerService extends IActivityManager.Stub mOomAdjuster.mCachedAppOptimizer.onCleanupApplicationRecordLocked(app); } mAppProfiler.onCleanupApplicationRecordLocked(app); - skipCurrentReceiverLocked(app); + for (BroadcastQueue queue : mBroadcastQueues) { + queue.onApplicationCleanupLocked(app); + } updateProcessForegroundLocked(app, false, 0, false); mServices.killServicesLocked(app, allowRestart); mPhantomProcessList.onAppDied(pid); @@ -13080,42 +13072,6 @@ public class ActivityManagerService extends IActivityManager.Stub } } - boolean isPendingBroadcastProcessLocked(int pid) { - for (BroadcastQueue queue : mBroadcastQueues) { - BroadcastRecord r = queue.getPendingBroadcastLocked(); - if (r != null && r.curApp.getPid() == pid) { - return true; - } - } - return false; - } - - boolean isPendingBroadcastProcessLocked(ProcessRecord app) { - for (BroadcastQueue queue : mBroadcastQueues) { - BroadcastRecord r = queue.getPendingBroadcastLocked(); - if (r != null && r.curApp == app) { - return true; - } - } - return false; - } - - void skipPendingBroadcastLocked(int pid) { - Slog.w(TAG, "Unattached app died before broadcast acknowledged, skipping"); - for (BroadcastQueue queue : mBroadcastQueues) { - queue.skipPendingBroadcastLocked(pid); - } - } - - // The app just attached; send any pending broadcasts that it should receive - boolean sendPendingBroadcastsLocked(ProcessRecord app) { - boolean didSomething = false; - for (BroadcastQueue queue : mBroadcastQueues) { - didSomething |= queue.sendPendingBroadcastsLocked(app); - } - return didSomething; - } - void updateUidReadyForBootCompletedBroadcastLocked(int uid) { for (BroadcastQueue queue : mBroadcastQueues) { queue.updateUidReadyForBootCompletedBroadcastLocked(uid); diff --git a/services/core/java/com/android/server/am/BroadcastQueue.java b/services/core/java/com/android/server/am/BroadcastQueue.java index d0946bec730a1..bd21ab67340d4 100644 --- a/services/core/java/com/android/server/am/BroadcastQueue.java +++ b/services/core/java/com/android/server/am/BroadcastQueue.java @@ -77,12 +77,6 @@ public abstract class BroadcastQueue { public abstract void updateUidReadyForBootCompletedBroadcastLocked(int uid); - public abstract boolean sendPendingBroadcastsLocked(ProcessRecord app); - - public abstract void skipPendingBroadcastLocked(int pid); - - public abstract void skipCurrentReceiverLocked(ProcessRecord app); - public abstract BroadcastRecord getMatchingOrderedReceiver(IBinder receiver); /** @@ -99,6 +93,31 @@ public abstract class BroadcastQueue { public abstract void processNextBroadcastLocked(boolean fromMsg, boolean skipOomAdj); + /** + * Signal from OS internals that the given process has just been actively + * attached, and is ready to begin receiving broadcasts. + */ + public abstract boolean onApplicationAttachedLocked(ProcessRecord app); + + /** + * Signal from OS internals that the given process has timed out during + * an attempted start and attachment. + */ + public abstract boolean onApplicationTimeoutLocked(ProcessRecord app); + + /** + * Signal from OS internals that the given process, which had already been + * previously attached, has now encountered a problem such as crashing or + * not responding. + */ + public abstract boolean onApplicationProblemLocked(ProcessRecord app); + + /** + * Signal from OS internals that the given process has been killed, and is + * no longer actively running. + */ + public abstract boolean onApplicationCleanupLocked(ProcessRecord app); + /** * Signal from OS internals that the given package (or some subset of that * package) has been disabled or uninstalled, and that any pending diff --git a/services/core/java/com/android/server/am/BroadcastQueueImpl.java b/services/core/java/com/android/server/am/BroadcastQueueImpl.java index 4bffe351a3790..08c5188a666c2 100644 --- a/services/core/java/com/android/server/am/BroadcastQueueImpl.java +++ b/services/core/java/com/android/server/am/BroadcastQueueImpl.java @@ -423,6 +423,26 @@ public class BroadcastQueueImpl extends BroadcastQueue { scheduleBroadcastsLocked(); } + public boolean onApplicationAttachedLocked(ProcessRecord app) { + if (mPendingBroadcast != null && mPendingBroadcast.curApp == app) { + return sendPendingBroadcastsLocked(app); + } else { + return false; + } + } + + public boolean onApplicationTimeoutLocked(ProcessRecord app) { + return skipCurrentOrPendingReceiverLocked(app); + } + + public boolean onApplicationProblemLocked(ProcessRecord app) { + return skipCurrentOrPendingReceiverLocked(app); + } + + public boolean onApplicationCleanupLocked(ProcessRecord app) { + return skipCurrentOrPendingReceiverLocked(app); + } + public boolean sendPendingBroadcastsLocked(ProcessRecord app) { boolean didSomething = false; final BroadcastRecord br = mPendingBroadcast; @@ -452,18 +472,8 @@ public class BroadcastQueueImpl extends BroadcastQueue { return didSomething; } - public void skipPendingBroadcastLocked(int pid) { - final BroadcastRecord br = mPendingBroadcast; - if (br != null && br.curApp.getPid() == pid) { - br.state = BroadcastRecord.IDLE; - br.nextReceiver = mPendingBroadcastRecvIndex; - mPendingBroadcast = null; - scheduleBroadcastsLocked(); - } - } - // Skip the current receiver, if any, that is in flight to the given process - public void skipCurrentReceiverLocked(ProcessRecord app) { + public boolean skipCurrentOrPendingReceiverLocked(ProcessRecord app) { BroadcastRecord r = null; final BroadcastRecord curActive = mDispatcher.getActiveBroadcastLocked(); if (curActive != null && curActive.curApp == app) { @@ -481,6 +491,9 @@ public class BroadcastQueueImpl extends BroadcastQueue { if (r != null) { skipReceiverLocked(r); + return true; + } else { + return false; } } diff --git a/services/core/java/com/android/server/am/ProcessErrorStateRecord.java b/services/core/java/com/android/server/am/ProcessErrorStateRecord.java index 3a8a077c4e8e4..ceea01b3ccc73 100644 --- a/services/core/java/com/android/server/am/ProcessErrorStateRecord.java +++ b/services/core/java/com/android/server/am/ProcessErrorStateRecord.java @@ -605,7 +605,9 @@ class ProcessErrorStateRecord { mService.mContext, mApp.info.packageName, mApp.info.flags); } } - mService.skipCurrentReceiverLocked(mApp); + for (BroadcastQueue queue : mService.mBroadcastQueues) { + queue.onApplicationProblemLocked(mApp); + } } @GuardedBy("mService") From 014a5be31bc2777d927935d597d58a42693b0c8a Mon Sep 17 00:00:00 2001 From: Jeff Sharkey Date: Thu, 25 Aug 2022 10:56:57 -0600 Subject: [PATCH 2/3] [4/?] Reduce BroadcastQueue interface complexity. Hide dispatch of processNextBroadcastLocked() as an implementation detail inside the broadcast stack. This is a no-op change. Bug: 243656033 Test: atest CtsContentTestCases:BroadcastReceiverTest Change-Id: I7b5fa50ee5dcd47ca0f5dcda4a59c14d4238634b --- .../com/android/server/am/ActivityManagerService.java | 5 ----- .../core/java/com/android/server/am/BroadcastQueue.java | 2 -- .../java/com/android/server/am/BroadcastQueueImpl.java | 8 ++++++-- 3 files changed, 6 insertions(+), 9 deletions(-) diff --git a/services/core/java/com/android/server/am/ActivityManagerService.java b/services/core/java/com/android/server/am/ActivityManagerService.java index c18117aee2d9a..8eccbf5018a02 100644 --- a/services/core/java/com/android/server/am/ActivityManagerService.java +++ b/services/core/java/com/android/server/am/ActivityManagerService.java @@ -13349,8 +13349,6 @@ public class ActivityManagerService extends IActivityManager.Stub r.resultAbort, false); if (doNext) { doTrim = true; - r.queue.processNextBroadcastLocked(/* frommsg */ false, - /* skipOomAdj */ true); } } @@ -14546,9 +14544,6 @@ public class ActivityManagerService extends IActivityManager.Stub doNext = r.queue.finishReceiverLocked(r, resultCode, resultData, resultExtras, resultAbort, true); } - if (doNext) { - r.queue.processNextBroadcastLocked(/*fromMsg=*/ false, /*skipOomAdj=*/ true); - } // updateOomAdjLocked() will be done here trimApplicationsLocked(false, OomAdjuster.OOM_ADJ_REASON_FINISH_RECEIVER); } diff --git a/services/core/java/com/android/server/am/BroadcastQueue.java b/services/core/java/com/android/server/am/BroadcastQueue.java index bd21ab67340d4..64d62f81ab1bb 100644 --- a/services/core/java/com/android/server/am/BroadcastQueue.java +++ b/services/core/java/com/android/server/am/BroadcastQueue.java @@ -91,8 +91,6 @@ public abstract class BroadcastQueue { public abstract void backgroundServicesFinishedLocked(int userId); - public abstract void processNextBroadcastLocked(boolean fromMsg, boolean skipOomAdj); - /** * Signal from OS internals that the given process has just been actively * attached, and is ready to begin receiving broadcasts. diff --git a/services/core/java/com/android/server/am/BroadcastQueueImpl.java b/services/core/java/com/android/server/am/BroadcastQueueImpl.java index 08c5188a666c2..de0a151780e5c 100644 --- a/services/core/java/com/android/server/am/BroadcastQueueImpl.java +++ b/services/core/java/com/android/server/am/BroadcastQueueImpl.java @@ -677,8 +677,12 @@ public class BroadcastQueueImpl extends BroadcastQueue { // We will process the next receiver right now if this is finishing // an app receiver (which is always asynchronous) or after we have // come back from calling a receiver. - return state == BroadcastRecord.APP_RECEIVE - || state == BroadcastRecord.CALL_DONE_RECEIVE; + final boolean doNext = (state == BroadcastRecord.APP_RECEIVE) + || (state == BroadcastRecord.CALL_DONE_RECEIVE); + if (doNext) { + processNextBroadcastLocked(/* fromMsg= */ false, /* skipOomAdj= */ true); + } + return doNext; } public void backgroundServicesFinishedLocked(int userId) { From 8341514bf67c383269017eb8f9275722067d2b6a Mon Sep 17 00:00:00 2001 From: Jeff Sharkey Date: Thu, 25 Aug 2022 10:58:48 -0600 Subject: [PATCH 3/3] [5/?] Reduce BroadcastQueue interface complexity. Hide dispatch of updateUidReadyForBootCompletedBroadcastLocked() as an implementation detail inside the broadcast stack. This is a no-op change. Bug: 243656033 Test: atest CtsContentTestCases:BroadcastReceiverTest Change-Id: I7c02707787b8f24b2492fce0cb735ae230dfc615 --- .../com/android/server/am/ActivityManagerService.java | 10 ---------- .../java/com/android/server/am/BroadcastQueue.java | 2 -- .../java/com/android/server/am/BroadcastQueueImpl.java | 2 ++ 3 files changed, 2 insertions(+), 12 deletions(-) diff --git a/services/core/java/com/android/server/am/ActivityManagerService.java b/services/core/java/com/android/server/am/ActivityManagerService.java index 8eccbf5018a02..be0335e93d407 100644 --- a/services/core/java/com/android/server/am/ActivityManagerService.java +++ b/services/core/java/com/android/server/am/ActivityManagerService.java @@ -4954,10 +4954,6 @@ public class ActivityManagerService extends IActivityManager.Stub } } - if (!badApp) { - updateUidReadyForBootCompletedBroadcastLocked(app.uid); - } - // Check if a next-broadcast receiver is in this process... if (!badApp) { try { @@ -13072,12 +13068,6 @@ public class ActivityManagerService extends IActivityManager.Stub } } - void updateUidReadyForBootCompletedBroadcastLocked(int uid) { - for (BroadcastQueue queue : mBroadcastQueues) { - queue.updateUidReadyForBootCompletedBroadcastLocked(uid); - } - } - /** * @deprecated Use {@link #registerReceiverWithFeature} */ diff --git a/services/core/java/com/android/server/am/BroadcastQueue.java b/services/core/java/com/android/server/am/BroadcastQueue.java index 64d62f81ab1bb..9be22c0e41c01 100644 --- a/services/core/java/com/android/server/am/BroadcastQueue.java +++ b/services/core/java/com/android/server/am/BroadcastQueue.java @@ -75,8 +75,6 @@ public abstract class BroadcastQueue { */ public abstract void enqueueBroadcastLocked(BroadcastRecord r); - public abstract void updateUidReadyForBootCompletedBroadcastLocked(int uid); - public abstract BroadcastRecord getMatchingOrderedReceiver(IBinder receiver); /** diff --git a/services/core/java/com/android/server/am/BroadcastQueueImpl.java b/services/core/java/com/android/server/am/BroadcastQueueImpl.java index de0a151780e5c..2b3b21182a6c9 100644 --- a/services/core/java/com/android/server/am/BroadcastQueueImpl.java +++ b/services/core/java/com/android/server/am/BroadcastQueueImpl.java @@ -424,6 +424,8 @@ public class BroadcastQueueImpl extends BroadcastQueue { } public boolean onApplicationAttachedLocked(ProcessRecord app) { + updateUidReadyForBootCompletedBroadcastLocked(app.uid); + if (mPendingBroadcast != null && mPendingBroadcast.curApp == app) { return sendPendingBroadcastsLocked(app); } else {