diff --git a/services/core/java/com/android/server/am/BroadcastConstants.java b/services/core/java/com/android/server/am/BroadcastConstants.java index 030d596a16769..8c1fd516028ea 100644 --- a/services/core/java/com/android/server/am/BroadcastConstants.java +++ b/services/core/java/com/android/server/am/BroadcastConstants.java @@ -292,6 +292,15 @@ public class BroadcastConstants { private static final String KEY_CORE_DEFER_UNTIL_ACTIVE = "bcast_core_defer_until_active"; private static final boolean DEFAULT_CORE_DEFER_UNTIL_ACTIVE = true; + /** + * For {@link BroadcastQueueModernImpl}: How frequently we should check for the pending + * cold start validity. + */ + public long PENDING_COLD_START_CHECK_INTERVAL_MILLIS = 30 * 1000; + private static final String KEY_PENDING_COLD_START_CHECK_INTERVAL_MILLIS = + "pending_cold_start_check_interval_millis"; + private static final long DEFAULT_PENDING_COLD_START_CHECK_INTERVAL_MILLIS = 30_000; + // Settings override tracking for this instance private String mSettingsKey; private SettingsObserver mSettingsObserver; @@ -441,6 +450,9 @@ public class BroadcastConstants { DEFAULT_MAX_HISTORY_SUMMARY_SIZE); CORE_DEFER_UNTIL_ACTIVE = getDeviceConfigBoolean(KEY_CORE_DEFER_UNTIL_ACTIVE, DEFAULT_CORE_DEFER_UNTIL_ACTIVE); + PENDING_COLD_START_CHECK_INTERVAL_MILLIS = getDeviceConfigLong( + KEY_PENDING_COLD_START_CHECK_INTERVAL_MILLIS, + DEFAULT_PENDING_COLD_START_CHECK_INTERVAL_MILLIS); } // TODO: migrate BroadcastRecord to accept a BroadcastConstants @@ -499,6 +511,8 @@ public class BroadcastConstants { MAX_CONSECUTIVE_NORMAL_DISPATCHES).println(); pw.print(KEY_CORE_DEFER_UNTIL_ACTIVE, CORE_DEFER_UNTIL_ACTIVE).println(); + pw.print(KEY_PENDING_COLD_START_CHECK_INTERVAL_MILLIS, + PENDING_COLD_START_CHECK_INTERVAL_MILLIS).println(); pw.decreaseIndent(); pw.println(); } diff --git a/services/core/java/com/android/server/am/BroadcastQueueModernImpl.java b/services/core/java/com/android/server/am/BroadcastQueueModernImpl.java index d6e692cfdc009..f180f02f77426 100644 --- a/services/core/java/com/android/server/am/BroadcastQueueModernImpl.java +++ b/services/core/java/com/android/server/am/BroadcastQueueModernImpl.java @@ -248,6 +248,7 @@ class BroadcastQueueModernImpl extends BroadcastQueue { private static final int MSG_DELIVERY_TIMEOUT_HARD = 3; private static final int MSG_BG_ACTIVITY_START_TIMEOUT = 4; private static final int MSG_CHECK_HEALTH = 5; + private static final int MSG_CHECK_PENDING_COLD_START_VALIDITY = 6; private void enqueueUpdateRunningList() { mLocalHandler.removeMessages(MSG_UPDATE_RUNNING_LIST); @@ -284,6 +285,10 @@ class BroadcastQueueModernImpl extends BroadcastQueue { checkHealth(); return true; } + case MSG_CHECK_PENDING_COLD_START_VALIDITY: { + checkPendingColdStartValidity(); + return true; + } } return false; }; @@ -450,10 +455,14 @@ class BroadcastQueueModernImpl extends BroadcastQueue { // skip to look for another warm process if (mRunningColdStart == null) { mRunningColdStart = queue; - } else { + } else if (isPendingColdStartValid()) { // Move to considering next runnable queue queue = nextQueue; continue; + } else { + // Pending cold start is not valid, so clear it and move on. + clearInvalidPendingColdStart(); + mRunningColdStart = queue; } } @@ -486,11 +495,46 @@ class BroadcastQueueModernImpl extends BroadcastQueue { mService.updateOomAdjPendingTargetsLocked(OOM_ADJ_REASON_START_RECEIVER); } + checkPendingColdStartValidity(); checkAndRemoveWaitingFor(); traceEnd(cookie); } + private boolean isPendingColdStartValid() { + if (mRunningColdStart.app.getPid() > 0) { + // If the process has already started, check if it wasn't killed. + return !mRunningColdStart.app.isKilled(); + } else { + // Otherwise, check if the process start is still pending. + return mRunningColdStart.app.isPendingStart(); + } + } + + private void clearInvalidPendingColdStart() { + logw("Clearing invalid pending cold start: " + mRunningColdStart); + onApplicationCleanupLocked(mRunningColdStart.app); + } + + private void checkPendingColdStartValidity() { + // There are a few cases where a starting process gets killed but AMS doesn't report + // this event. So, once we start waiting for a pending cold start, periodically check + // if the pending start is still valid and if not, clear it so that the queue doesn't + // keep waiting for the process start forever. + synchronized (mService) { + // If there is no pending cold start, then nothing to do. + if (mRunningColdStart == null) { + return; + } + if (isPendingColdStartValid()) { + mLocalHandler.sendEmptyMessageDelayed(MSG_CHECK_PENDING_COLD_START_VALIDITY, + mConstants.PENDING_COLD_START_CHECK_INTERVAL_MILLIS); + } else { + clearInvalidPendingColdStart(); + } + } + } + @Override public boolean onApplicationAttachedLocked(@NonNull ProcessRecord app) { // Process records can be recycled, so always start by looking up the diff --git a/services/tests/mockingservicestests/src/com/android/server/am/BroadcastQueueTest.java b/services/tests/mockingservicestests/src/com/android/server/am/BroadcastQueueTest.java index ad5f0d7233cac..6365764923620 100644 --- a/services/tests/mockingservicestests/src/com/android/server/am/BroadcastQueueTest.java +++ b/services/tests/mockingservicestests/src/com/android/server/am/BroadcastQueueTest.java @@ -269,7 +269,9 @@ public class BroadcastQueueTest { deliverRes = res; break; } + res.setPendingStart(true); mHandlerThread.getThreadHandler().post(() -> { + res.setPendingStart(false); synchronized (mAms) { switch (behavior) { case SUCCESS: @@ -281,6 +283,10 @@ public class BroadcastQueueTest { mActiveProcesses.remove(deliverRes); mQueue.onApplicationTimeoutLocked(deliverRes); break; + case KILLED_WITHOUT_NOTIFY: + mActiveProcesses.remove(res); + res.setKilled(true); + break; default: throw new UnsupportedOperationException(); } @@ -310,6 +316,7 @@ public class BroadcastQueueTest { mConstants = new BroadcastConstants(Settings.Global.BROADCAST_FG_CONSTANTS); mConstants.TIMEOUT = 100; mConstants.ALLOW_BG_ACTIVITY_START_TIMEOUT = 0; + mConstants.PENDING_COLD_START_CHECK_INTERVAL_MILLIS = 500; mSkipPolicy = spy(new BroadcastSkipPolicy(mAms)); doReturn(null).when(mSkipPolicy).shouldSkipMessage(any(), any()); @@ -381,6 +388,8 @@ public class BroadcastQueueTest { FAIL_TIMEOUT_PREDECESSOR, /** Process fails by immediately returning null */ FAIL_NULL, + /** Process is killed without reporting to BroadcastQueue */ + KILLED_WITHOUT_NOTIFY, } private enum ProcessBehavior { @@ -522,6 +531,11 @@ public class BroadcastQueueTest { return info; } + static BroadcastFilter withPriority(BroadcastFilter filter, int priority) { + filter.setPriority(priority); + return filter; + } + static ResolveInfo makeManifestReceiver(String packageName, String name) { return makeManifestReceiver(packageName, name, UserHandle.USER_SYSTEM); } @@ -1261,6 +1275,46 @@ public class BroadcastQueueTest { new ComponentName(PACKAGE_GREEN, CLASS_GREEN)); } + /** + * Verify that when BroadcastQueue doesn't get notified when a process gets killed, it + * doesn't get stuck. + */ + @Test + public void testKillWithoutNotify() throws Exception { + final ProcessRecord callerApp = makeActiveProcessRecord(PACKAGE_RED); + final ProcessRecord receiverBlueApp = makeActiveProcessRecord(PACKAGE_BLUE); + + mNextProcessStartBehavior.set(ProcessStartBehavior.KILLED_WITHOUT_NOTIFY); + + final Intent airplane = new Intent(Intent.ACTION_AIRPLANE_MODE_CHANGED); + enqueueBroadcast(makeBroadcastRecord(airplane, callerApp, List.of( + withPriority(makeManifestReceiver(PACKAGE_GREEN, CLASS_GREEN), 10), + withPriority(makeRegisteredReceiver(receiverBlueApp), 5), + withPriority(makeManifestReceiver(PACKAGE_YELLOW, CLASS_YELLOW), 0)))); + + final Intent timezone = new Intent(Intent.ACTION_TIMEZONE_CHANGED); + enqueueBroadcast(makeBroadcastRecord(timezone, callerApp, + List.of(makeManifestReceiver(PACKAGE_ORANGE, CLASS_ORANGE)))); + + waitForIdle(); + final ProcessRecord receiverGreenApp = mAms.getProcessRecordLocked(PACKAGE_GREEN, + getUidForPackage(PACKAGE_GREEN)); + final ProcessRecord receiverYellowApp = mAms.getProcessRecordLocked(PACKAGE_YELLOW, + getUidForPackage(PACKAGE_YELLOW)); + final ProcessRecord receiverOrangeApp = mAms.getProcessRecordLocked(PACKAGE_ORANGE, + getUidForPackage(PACKAGE_ORANGE)); + + if (mImpl == Impl.MODERN) { + // Modern queue does not retry sending a broadcast once any broadcast delivery fails. + assertNull(receiverGreenApp); + } else { + verifyScheduleReceiver(times(1), receiverGreenApp, airplane); + } + verifyScheduleRegisteredReceiver(times(1), receiverBlueApp, airplane); + verifyScheduleReceiver(times(1), receiverYellowApp, airplane); + verifyScheduleReceiver(times(1), receiverOrangeApp, timezone); + } + @Test public void testCold_Success() throws Exception { doCold(ProcessStartBehavior.SUCCESS);