Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit af59fa3b authored by Jeff Sharkey's avatar Jeff Sharkey
Browse files

BroadcastQueue: Handle successor ProcessRecords. 

In rare cases, a request to startProcessLocked() returns a different
ProcessRecord from what is eventually attached.  This change fixes
the modern stack to not be confused in these situations, by always
looking up the relevant BroadcastProcessQueue, instead of trying to
do a raw ProcessRecord equality check.

Add health check to verify that cold starts aren't abandoned.  Add
debugging details for each "blocked" broadcast.

Bug: 253617038
Test: atest FrameworksMockingServicesTests:BroadcastRecordTest
Test: atest FrameworksMockingServicesTests:BroadcastQueueTest
Test: atest FrameworksMockingServicesTests:BroadcastQueueModernImplTest
Change-Id: Iab95f80ee554589f66eefe351370d37e44210295
parent a5156323

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

+21 −16
Original line number Diff line number Diff line
@@ -113,6 +113,14 @@ class BroadcastProcessQueue { 
     */

    private int mActiveIndex;



    /**

     * When defined, the receiver actively being dispatched into this process

     * was considered "blocked" until at least the given count of other

     * receivers have reached a terminal state; typically used for ordered

     * broadcasts and priority traunches.

     */

    private int mActiveBlockedUntilTerminalCount;



    /**

     * Count of {@link #mActive} broadcasts that have been dispatched since this

     * queue was last idle.
@@ -304,6 +312,7 @@ class BroadcastProcessQueue { 
        final SomeArgs next = mPending.removeFirst();

        mActive = (BroadcastRecord) next.arg1;

        mActiveIndex = next.argi1;

        mActiveBlockedUntilTerminalCount = next.argi2;

        mActiveCountSinceIdle++;

        mActiveViaColdStart = false;

        next.recycle();
@@ -316,6 +325,7 @@ class BroadcastProcessQueue { 
    public void makeActiveIdle() {

        mActive = null;

        mActiveIndex = 0;

        mActiveBlockedUntilTerminalCount = -1;

        mActiveCountSinceIdle = 0;

        mActiveViaColdStart = false;

        invalidateRunnableAt();
@@ -664,27 +674,14 @@ class BroadcastProcessQueue { 
        }

        pw.print(" because ");

        pw.print(reasonToString(mRunnableAtReason));

        if (mRunnableAtReason == REASON_BLOCKED) {

            final SomeArgs next = mPending.peekFirst();

            if (next != null) {

                final BroadcastRecord r = (BroadcastRecord) next.arg1;

                final int blockedUntilTerminalCount = next.argi2;

                pw.print(" waiting for ");

                pw.print(blockedUntilTerminalCount);

                pw.print(" at ");

                pw.print(r.terminalCount);

                pw.print(" of ");

                pw.print(r.receivers.size());

            }

        }

        pw.println();

        pw.increaseIndent();

        if (mActive != null) {

            dumpRecord(now, pw, mActive, mActiveIndex);

            dumpRecord(now, pw, mActive, mActiveIndex, mActiveBlockedUntilTerminalCount);

        }

        for (SomeArgs args : mPending) {

            final BroadcastRecord r = (BroadcastRecord) args.arg1;

            dumpRecord(now, pw, r, args.argi1);

            dumpRecord(now, pw, r, args.argi1, args.argi2);

        }

        pw.decreaseIndent();

        pw.println();
@@ -692,7 +689,7 @@ class BroadcastProcessQueue { 


    @NeverCompile

    private void dumpRecord(@UptimeMillisLong long now, @NonNull IndentingPrintWriter pw,

            @NonNull BroadcastRecord record, int recordIndex) {

            @NonNull BroadcastRecord record, int recordIndex, int blockedUntilTerminalCount) {

        TimeUtils.formatDuration(record.enqueueTime, now, pw);

        pw.print(' ');

        pw.println(record.toShortString());
@@ -714,5 +711,13 @@ class BroadcastProcessQueue { 
            pw.print(info.activityInfo.name);

        }

        pw.println();

        if (blockedUntilTerminalCount != -1) {

            pw.print("    blocked until ");

            pw.print(blockedUntilTerminalCount);

            pw.print(", currently at ");

            pw.print(record.terminalCount);

            pw.print(" of ");

            pw.println(record.receivers.size());

        }

    }

}

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

+28 −16
Original line number Diff line number Diff line
@@ -396,17 +396,12 @@ class BroadcastQueueModernImpl extends BroadcastQueue { 
            // Emit all trace events for this process into a consistent track

            queue.traceTrackName = TAG + ".mRunning[" + queueIndex + "]";



            // If we're already warm, boost OOM adjust now; if cold we'll boost

            // it after the app has been started

            if (processWarm) {

                notifyStartedRunning(queue);

            }



            // 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();
@@ -441,15 +436,22 @@ class BroadcastQueueModernImpl extends BroadcastQueue { 


    @Override

    public boolean onApplicationAttachedLocked(@NonNull ProcessRecord app) {

        // Process records can be recycled, so always start by looking up the

        // relevant per-process queue

        final BroadcastProcessQueue queue = getProcessQueue(app);

        if (queue != null) {

            queue.app = app;

        }



        boolean didSomething = false;

        if ((mRunningColdStart != null) && (mRunningColdStart.app == app)) {

        if ((mRunningColdStart != null) && (mRunningColdStart == queue)) {

            // We've been waiting for this app to cold start, and it's ready

            // now; dispatch its next broadcast and clear the slot

            final BroadcastProcessQueue queue = mRunningColdStart;

            mRunningColdStart = null;



            queue.traceProcessEnd();

            queue.traceProcessRunningBegin();

            notifyStartedRunning(queue);

            scheduleReceiverWarmLocked(queue);



            // We might be willing to kick off another cold start
@@ -471,19 +473,25 @@ class BroadcastQueueModernImpl extends BroadcastQueue { 


    @Override

    public void onApplicationCleanupLocked(@NonNull ProcessRecord app) {

        if ((mRunningColdStart != null) && (mRunningColdStart.app == app)) {

        // Process records can be recycled, so always start by looking up the

        // relevant per-process queue

        final BroadcastProcessQueue queue = getProcessQueue(app);

        if (queue != null) {

            queue.app = null;

        }



        if ((mRunningColdStart != null) && (mRunningColdStart == queue)) {

            // We've been waiting for this app to cold start, and it had

            // trouble; clear the slot and fail delivery below

            mRunningColdStart = null;



            queue.traceProcessEnd();



            // We might be willing to kick off another cold start

            enqueueUpdateRunningList();

        }



        final BroadcastProcessQueue queue = getProcessQueue(app);

        if (queue != null) {

            queue.app = null;



            // If queue was running a broadcast, fail it

            if (queue.isActive()) {

                finishReceiverLocked(queue, BroadcastRecord.DELIVERY_FAILURE);
@@ -567,7 +575,7 @@ class BroadcastQueueModernImpl extends BroadcastQueue { 
                }

            } else {

                // Otherwise we don't need to block at all

                blockedUntilTerminalCount = 0;

                blockedUntilTerminalCount = -1;

            }



            queue.enqueueOrReplaceBroadcast(r, i, blockedUntilTerminalCount);
@@ -619,9 +627,7 @@ class BroadcastQueueModernImpl extends BroadcastQueue { 
        if (DEBUG_BROADCAST) logv("Scheduling " + r + " to cold " + queue);

        queue.app = mService.startProcessLocked(queue.processName, info, true, intentFlags,

                hostingRecord, zygotePolicyFlags, allowWhileBooting, false);

        if (queue.app != null) {

            notifyStartedRunning(queue);

        } else {

        if (queue.app == null) {

            mRunningColdStart = null;

            finishReceiverLocked(queue, BroadcastRecord.DELIVERY_FAILURE);

            return;
@@ -1159,6 +1165,12 @@ class BroadcastQueueModernImpl extends BroadcastQueue { 
                }

            }



            // Verify that pending cold start hasn't been orphaned

            if (mRunningColdStart != null) {

                checkState(getRunningIndexOf(mRunningColdStart) >= 0,

                        "isOrphaned " + mRunningColdStart);

            }



            // Verify health of all known process queues

            for (int i = 0; i < mProcessQueues.size(); i++) {

                BroadcastProcessQueue leaf = mProcessQueues.valueAt(i);

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

+102 −13
Original line number Diff line number Diff line
@@ -113,6 +113,7 @@ import java.util.Map; 
import java.util.Objects;

import java.util.Set;

import java.util.concurrent.atomic.AtomicInteger;

import java.util.concurrent.atomic.AtomicReference;

import java.util.function.UnaryOperator;



/**
@@ -154,10 +155,11 @@ public class BroadcastQueueTest { 
    private BroadcastQueue mQueue;



    /**

     * When enabled {@link ActivityManagerService#startProcessLocked} will fail

     * by returning {@code null}; otherwise it will spawn a new mock process.

     * Desired behavior of the next

     * {@link ActivityManagerService#startProcessLocked} call.

     */

    private boolean mFailStartProcess;

    private AtomicReference<ProcessStartBehavior> mNextProcessStartBehavior = new AtomicReference<>(

            ProcessStartBehavior.SUCCESS);



    /**

     * Map from PID to registered registered runtime receivers.
@@ -216,16 +218,46 @@ public class BroadcastQueueTest { 
        doAnswer((invocation) -> {

            Log.v(TAG, "Intercepting startProcessLocked() for "

                    + Arrays.toString(invocation.getArguments()));

            if (mFailStartProcess) {

            final ProcessStartBehavior behavior = mNextProcessStartBehavior

                    .getAndSet(ProcessStartBehavior.SUCCESS);

            if (behavior == ProcessStartBehavior.FAIL_NULL) {

                return null;

            }

            final String processName = invocation.getArgument(0);

            final ApplicationInfo ai = invocation.getArgument(1);

            final ProcessRecord res = makeActiveProcessRecord(ai, processName,

                    ProcessBehavior.NORMAL, UnaryOperator.identity());

            final ProcessRecord deliverRes;

            switch (behavior) {

                case SUCCESS_PREDECESSOR:

                case FAIL_TIMEOUT_PREDECESSOR:

                    // Create a different process that will be linked to the

                    // returned process via a predecessor/successor relationship

                    mActiveProcesses.remove(res);

                    deliverRes = makeActiveProcessRecord(ai, processName,

                          ProcessBehavior.NORMAL, UnaryOperator.identity());

                    deliverRes.mPredecessor = res;

                    res.mSuccessor = deliverRes;

                    break;

                default:

                    deliverRes = res;

                    break;

            }

            mHandlerThread.getThreadHandler().post(() -> {

                synchronized (mAms) {

                    mQueue.onApplicationAttachedLocked(res);

                    switch (behavior) {

                        case SUCCESS:

                        case SUCCESS_PREDECESSOR:

                            mQueue.onApplicationAttachedLocked(deliverRes);

                            break;

                        case FAIL_TIMEOUT:

                        case FAIL_TIMEOUT_PREDECESSOR:

                            mActiveProcesses.remove(deliverRes);

                            mQueue.onApplicationTimeoutLocked(deliverRes);

                            break;

                        default:

                            throw new UnsupportedOperationException();

                    }

                }

            });

            return res;
@@ -281,9 +313,10 @@ public class BroadcastQueueTest { 


        // Verify that all processes have finished handling broadcasts

        for (ProcessRecord app : mActiveProcesses) {

            assertTrue(app.toShortString(), app.mReceivers.numberOfCurReceivers() == 0);

            assertTrue(app.toShortString(), mQueue.getPreferredSchedulingGroupLocked(app)

                    == ProcessList.SCHED_GROUP_UNDEFINED);

            assertEquals(app.toShortString(), 0,

                    app.mReceivers.numberOfCurReceivers());

            assertEquals(app.toShortString(), ProcessList.SCHED_GROUP_UNDEFINED,

                    mQueue.getPreferredSchedulingGroupLocked(app));

        }

    }
@@ -325,6 +358,19 @@ public class BroadcastQueueTest { 
        }

    }



    private enum ProcessStartBehavior {

        /** Process starts successfully */

        SUCCESS,

        /** Process starts successfully via predecessor */

        SUCCESS_PREDECESSOR,

        /** Process fails by reporting timeout */

        FAIL_TIMEOUT,

        /** Process fails by reporting timeout via predecessor */

        FAIL_TIMEOUT_PREDECESSOR,

        /** Process fails by immediately returning null */

        FAIL_NULL,

    }



    private enum ProcessBehavior {

        /** Process broadcasts normally */

        NORMAL,
@@ -956,18 +1002,16 @@ public class BroadcastQueueTest { 
        final ProcessRecord callerApp = makeActiveProcessRecord(PACKAGE_RED);



        // Send broadcast while process starts are failing

        mFailStartProcess = true;

        mNextProcessStartBehavior.set(ProcessStartBehavior.FAIL_NULL);

        final Intent airplane = new Intent(Intent.ACTION_AIRPLANE_MODE_CHANGED);

        enqueueBroadcast(makeBroadcastRecord(airplane, callerApp,

                List.of(makeManifestReceiver(PACKAGE_GREEN, CLASS_GREEN),

                        makeManifestReceiver(PACKAGE_YELLOW, CLASS_YELLOW))));

                List.of(makeManifestReceiver(PACKAGE_GREEN, CLASS_GREEN))));



        // Confirm that queue goes idle, with no processes

        waitForIdle();

        assertEquals(1, mActiveProcesses.size());



        // Send more broadcasts with working process starts

        mFailStartProcess = false;

        final Intent timezone = new Intent(Intent.ACTION_TIMEZONE_CHANGED);

        enqueueBroadcast(makeBroadcastRecord(timezone, callerApp,

                List.of(makeManifestReceiver(PACKAGE_GREEN, CLASS_GREEN),
@@ -981,7 +1025,6 @@ public class BroadcastQueueTest { 
        final ProcessRecord receiverYellowApp = mAms.getProcessRecordLocked(PACKAGE_YELLOW,

                getUidForPackage(PACKAGE_YELLOW));

        verifyScheduleReceiver(never(), receiverGreenApp, airplane);

        verifyScheduleReceiver(never(), receiverYellowApp, airplane);

        verifyScheduleReceiver(times(1), receiverGreenApp, timezone);

        verifyScheduleReceiver(times(1), receiverYellowApp, timezone);

    }
@@ -1071,6 +1114,52 @@ public class BroadcastQueueTest { 
                new ComponentName(PACKAGE_GREEN, CLASS_GREEN));

    }



    @Test

    public void testCold_Success() throws Exception {

        doCold(ProcessStartBehavior.SUCCESS);

    }



    @Test

    public void testCold_Success_Predecessor() throws Exception {

        doCold(ProcessStartBehavior.SUCCESS_PREDECESSOR);

    }



    @Test

    public void testCold_Fail_Null() throws Exception {

        doCold(ProcessStartBehavior.FAIL_NULL);

    }



    @Test

    public void testCold_Fail_Timeout() throws Exception {

        doCold(ProcessStartBehavior.FAIL_TIMEOUT);

    }



    @Test

    public void testCold_Fail_Timeout_Predecessor() throws Exception {

        doCold(ProcessStartBehavior.FAIL_TIMEOUT_PREDECESSOR);

    }



    private void doCold(ProcessStartBehavior behavior) throws Exception {

        final ProcessRecord callerApp = makeActiveProcessRecord(PACKAGE_RED);



        mNextProcessStartBehavior.set(behavior);

        final Intent airplane = new Intent(Intent.ACTION_AIRPLANE_MODE_CHANGED);

        enqueueBroadcast(makeBroadcastRecord(airplane, callerApp,

                List.of(makeManifestReceiver(PACKAGE_GREEN, CLASS_GREEN))));

        waitForIdle();



        // Regardless of success/failure of above, we should always be able to

        // recover and begin sending future broadcasts

        final Intent timezone = new Intent(Intent.ACTION_TIMEZONE_CHANGED);

        enqueueBroadcast(makeBroadcastRecord(timezone, callerApp,

                List.of(makeManifestReceiver(PACKAGE_GREEN, CLASS_GREEN))));

        waitForIdle();



        final ProcessRecord receiverApp = mAms.getProcessRecordLocked(PACKAGE_GREEN,

                getUidForPackage(PACKAGE_GREEN));

        verifyScheduleReceiver(receiverApp, timezone);

    }



    /**

     * Verify that we skip broadcasts to an app being backed up.

     */