Merge "Retry manifest bcast delivery if the receiver dies as part of unfreezing." into udc-dev am: 0bd2ce4eb7

Original change: https://googleplex-android-review.googlesource.com/c/platform/frameworks/base/+/23500523

Change-Id: I66fee62f6e01f610f8039e582ebe7545d16319a3
Signed-off-by: Automerger Merge Worker <android-build-automerger-merge-worker@system.gserviceaccount.com>

services/core/java/com/android/server/am/BroadcastQueueModernImpl.java

@@ -458,7 +458,17 @@ class BroadcastQueueModernImpl extends BroadcastQueue {
             updateWarmProcess(queue);
 
             final boolean processWarm = queue.isProcessWarm();
-            if (!processWarm) {
+            if (processWarm) {
+                mService.mOomAdjuster.unfreezeTemporarily(queue.app,
+                        CachedAppOptimizer.UNFREEZE_REASON_START_RECEIVER);
+                // The process could be killed as part of unfreezing. So, check again if it
+                // is still warm.
+                if (!queue.isProcessWarm()) {
+                    queue = nextQueue;
+                    enqueueUpdateRunningList();
+                    continue;
+                }
+            } else {
                 // We only offer to run one cold-start at a time to preserve
                 // system resources; below we either claim that single slot or
                 // skip to look for another warm process
@@ -530,6 +540,7 @@ class BroadcastQueueModernImpl extends BroadcastQueue {
         mRunningColdStart.reEnqueueActiveBroadcast();
         demoteFromRunningLocked(mRunningColdStart);
         clearRunningColdStart();
+        enqueueUpdateRunningList();
     }
 
     private void checkPendingColdStartValidity() {
@@ -564,6 +575,9 @@ class BroadcastQueueModernImpl extends BroadcastQueue {
     @Override
     public boolean onApplicationAttachedLocked(@NonNull ProcessRecord app)
             throws BroadcastDeliveryFailedException {
+        if (DEBUG_BROADCAST) {
+            logv("Process " + app + " is attached");
+        }
         // Process records can be recycled, so always start by looking up the
         // relevant per-process queue
         final BroadcastProcessQueue queue = getProcessQueue(app);
@@ -613,18 +627,21 @@ class BroadcastQueueModernImpl extends BroadcastQueue {
 
     @Override
     public void onApplicationCleanupLocked(@NonNull ProcessRecord app) {
-        // Process records can be recycled, so always start by looking up the
+        if (DEBUG_BROADCAST) {
-        // relevant per-process queue
+            logv("Process " + app + " is cleaned up");
-        final BroadcastProcessQueue queue = getProcessQueue(app);
-        if (queue != null) {
-            setQueueProcess(queue, null);
         }
 
-        if ((mRunningColdStart != null) && (mRunningColdStart == queue)) {
+        // This cleanup callback could be for an old process and not for the one we are waiting
+        // on, so explicitly check if this for the same ProcessRecord that a queue has.
+        final BroadcastProcessQueue queue = getProcessQueue(app);
+        if ((mRunningColdStart != null) && (mRunningColdStart == queue)
+                && mRunningColdStart.app == app) {
             clearRunningColdStart();
         }
 
-        if (queue != null) {
+        if (queue != null && queue.app == app) {
+            setQueueProcess(queue, null);
+
             // If queue was running a broadcast, fail it
             if (queue.isActive()) {
                 finishReceiverActiveLocked(queue, BroadcastRecord.DELIVERY_FAILURE,
@@ -1073,6 +1090,7 @@ class BroadcastQueueModernImpl extends BroadcastQueue {
                 // If we were trying to deliver a manifest broadcast, throw the error as we need
                 // to try redelivering the broadcast to this receiver.
                 if (receiver instanceof ResolveInfo) {
+                    mLocalHandler.removeMessages(MSG_DELIVERY_TIMEOUT_SOFT, queue);
                     throw new BroadcastDeliveryFailedException(e);
                 }
                 finishReceiverActiveLocked(queue, BroadcastRecord.DELIVERY_FAILURE,

services/tests/mockingservicestests/src/com/android/server/am/BroadcastQueueTest.java

@@ -261,6 +261,7 @@ public class BroadcastQueueTest {
                     // Create a different process that will be linked to the
                     // returned process via a predecessor/successor relationship
                     mActiveProcesses.remove(res);
+                    res.setKilled(true);
                     deliverRes = makeActiveProcessRecord(ai, processName,
                           ProcessBehavior.NORMAL, UnaryOperator.identity());
                     deliverRes.mPredecessor = res;
@@ -1316,6 +1317,53 @@ public class BroadcastQueueTest {
         verifyScheduleReceiver(times(1), receiverOrangeApp, timezone);
     }
 
+    /**
+     * Verify that a broadcast sent to a frozen app, which gets killed as part of unfreezing
+     * process due to pending sync binder transactions, is delivered as expected.
+     */
+    @Test
+    public void testDeliveryToFrozenApp_killedWhileUnfreeze() throws Exception {
+        final ProcessRecord callerApp = makeActiveProcessRecord(PACKAGE_RED);
+        final ProcessRecord receiverBlueApp = makeActiveProcessRecord(PACKAGE_BLUE);
+
+        // Mark the app as killed while unfreezing it, which can happen either when we directly
+        // try to unfreeze it or when it is done as part of OomAdjust computation.
+        doAnswer(invocation -> {
+            final ProcessRecord app = invocation.getArgument(0);
+            if (app == receiverBlueApp) {
+                app.setKilled(true);
+                mActiveProcesses.remove(app);
+            }
+            return null;
+        }).when(mAms.mOomAdjuster).unfreezeTemporarily(eq(receiverBlueApp), anyInt());
+        doAnswer(invocation -> {
+            final ProcessRecord app = invocation.getArgument(0);
+            if (app == receiverBlueApp) {
+                app.setKilled(true);
+                mActiveProcesses.remove(app);
+            }
+            return null;
+        }).when(mAms).enqueueOomAdjTargetLocked(eq(receiverBlueApp));
+
+        final Intent airplane = new Intent(Intent.ACTION_AIRPLANE_MODE_CHANGED);
+        enqueueBroadcast(makeBroadcastRecord(airplane, callerApp, List.of(
+                makeManifestReceiver(PACKAGE_BLUE, CLASS_BLUE))));
+
+        waitForIdle();
+        final ProcessRecord restartedReceiverBlueApp = mAms.getProcessRecordLocked(PACKAGE_BLUE,
+                getUidForPackage(PACKAGE_BLUE));
+        assertNotEquals(receiverBlueApp, restartedReceiverBlueApp);
+        // Legacy queue will always try delivering the broadcast even if the process
+        // has been killed.
+        if (mImpl == Impl.MODERN) {
+            verifyScheduleReceiver(never(), receiverBlueApp, airplane);
+        } else {
+            verifyScheduleReceiver(times(1), receiverBlueApp, airplane);
+        }
+        // Verify that the new process receives the broadcast.
+        verifyScheduleReceiver(times(1), restartedReceiverBlueApp, airplane);
+    }
+
     @Test
     public void testCold_Success() throws Exception {
         doCold(ProcessStartBehavior.SUCCESS);