Merge "Check if the pending cold start is valid periodically." into udc-dev

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();
         }

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

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);