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

Commit eb628f05 authored by Sudheer Shanka's avatar Sudheer Shanka
Browse files

Check if the pending cold start is valid periodically. 

There are some cases where AMS doesn't notify BroadcastQueue
of process getting killed. Ideally, we should make sure that
BroadcastQueue always get a callback when a process dies but
this should be sufficient to make sure BroadcastQueue doesn't
get stuck.

Bug: 281597599
Test: atest ./services/tests/mockingservicestests/src/com/android/server/am/BroadcastQueueTest.java
Test: atest ./services/tests/mockingservicestests/src/com/android/server/am/BroadcastQueueModernImplTest.java
Change-Id: Ie68388f6122b8ba60a67316a7477686264f7d652
parent 429962dd

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

+14 −0
Original line number Diff line number Diff line
@@ -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

+45 −1
Original line number Diff line number Diff line
@@ -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

+54 −0
Original line number Diff line number Diff line
@@ -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);