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

Commit 64f88fa1 authored by Jing Ji's avatar Jing Ji
Browse files

Handle the potential recursions within updateOomAdj 

Some code paths within the updateOomAdj* would potentially call into
the updateOomAdj* again and result in undefined behavior. This CL
adds the handling for it - in case of recursion is encountered,
the oomAdj update request will be enqueued, and at the end of the
current pass of updateOomAdj*, we'll do another updateOomAdj* to
handle those requests.

Bug: 167773591
Bug: 162476856
Test: atest CtsAppTestCases
Test: atest MockingOomAdjusterTests
Change-Id: I4788ba12f06744204c68382b399216f232059138
parent e3ff87dd

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

+121 −8
Original line number Diff line number Diff line
@@ -72,6 +72,7 @@ import static com.android.server.am.AppProfiler.TAG_PSS; 
import static com.android.server.am.ProcessList.TAG_PROCESS_OBSERVERS;

import static com.android.server.wm.ActivityTaskManagerDebugConfig.DEBUG_SWITCH;



import android.annotation.Nullable;

import android.app.ActivityManager;

import android.app.ActivityThread;

import android.app.ApplicationExitInfo;
@@ -229,6 +230,22 @@ public final class OomAdjuster { 
    private final ArrayDeque<ProcessRecord> mTmpQueue;

    private final ArraySet<ProcessRecord> mPendingProcessSet = new ArraySet<>();



    /**

     * Flag to mark if there is an ongoing oomAdjUpdate: potentially the oomAdjUpdate

     * could be called recursively because of the indirect calls during the update;

     * however the oomAdjUpdate itself doesn't support recursion - in this case we'd

     * have to queue up the new targets found during the update, and perform another

     * round of oomAdjUpdate at the end of last update.

     */

    @GuardedBy("mService")

    private boolean mOomAdjUpdateOngoing = false;



    /**

     * Flag to mark if there is a pending full oomAdjUpdate.

     */

    @GuardedBy("mService")

    private boolean mPendingFullOomAdjUpdate = false;



    private final PlatformCompatCache mPlatformCompatCache;



    private static class PlatformCompatCache {
@@ -439,6 +456,23 @@ public final class OomAdjuster { 
        if (oomAdjAll && mConstants.OOMADJ_UPDATE_QUICK) {

            return updateOomAdjLSP(app, oomAdjReason);

        }

        if (checkAndEnqueueOomAdjTargetLocked(app)) {

            // Simply return true as there is an oomAdjUpdate ongoing

            return true;

        }

        try {

            mOomAdjUpdateOngoing = true;

            return performUpdateOomAdjLSP(app, oomAdjAll, oomAdjReason);

        } finally {

            // Kick off the handling of any pending targets enqueued during the above update

            mOomAdjUpdateOngoing = false;

            updateOomAdjPendingTargetsLocked(oomAdjReason);

        }

    }



    @GuardedBy({"mService", "mProcLock"})

    private boolean performUpdateOomAdjLSP(ProcessRecord app, boolean oomAdjAll,

            String oomAdjReason) {

        final ProcessRecord topApp = mService.getTopApp();

        final ProcessStateRecord state = app.mState;

        final boolean wasCached = state.isCached();
@@ -453,20 +487,21 @@ public final class OomAdjuster { 
                ? state.getCurRawAdj() : ProcessList.UNKNOWN_ADJ;

        // Check if this process is in the pending list too, remove from pending list if so.

        mPendingProcessSet.remove(app);

        boolean success = updateOomAdjLSP(app, cachedAdj, topApp, false,

        boolean success = performUpdateOomAdjLSP(app, cachedAdj, topApp, false,

                SystemClock.uptimeMillis());

        if (oomAdjAll

                && (wasCached != state.isCached()

                    || state.getCurRawAdj() == ProcessList.UNKNOWN_ADJ)) {

            // Changed to/from cached state, so apps after it in the LRU

            // list may also be changed.

            updateOomAdjLSP(oomAdjReason);

            performUpdateOomAdjLSP(oomAdjReason);

        }



        return success;

    }



    @GuardedBy({"mService", "mProcLock"})

    private boolean updateOomAdjLSP(ProcessRecord app, int cachedAdj,

    private boolean performUpdateOomAdjLSP(ProcessRecord app, int cachedAdj,

            ProcessRecord TOP_APP, boolean doingAll, long now) {

        if (app.getThread() == null) {

            return false;
@@ -519,6 +554,22 @@ public final class OomAdjuster { 


    @GuardedBy({"mService", "mProcLock"})

    private void updateOomAdjLSP(String oomAdjReason) {

        if (checkAndEnqueueOomAdjTargetLocked(null)) {

            // Simply return as there is an oomAdjUpdate ongoing

            return;

        }

        try {

            mOomAdjUpdateOngoing = true;

            performUpdateOomAdjLSP(oomAdjReason);

        } finally {

            // Kick off the handling of any pending targets enqueued during the above update

            mOomAdjUpdateOngoing = false;

            updateOomAdjPendingTargetsLocked(oomAdjReason);

        }

    }



    @GuardedBy({"mService", "mProcLock"})

    private void performUpdateOomAdjLSP(String oomAdjReason) {

        final ProcessRecord topApp = mService.getTopApp();

        // Clear any pending ones because we are doing a full update now.

        mPendingProcessSet.clear();
@@ -548,6 +599,23 @@ public final class OomAdjuster { 
            return true;

        }



        if (checkAndEnqueueOomAdjTargetLocked(app)) {

            // Simply return true as there is an oomAdjUpdate ongoing

            return true;

        }



        try {

            mOomAdjUpdateOngoing = true;

            return performUpdateOomAdjLSP(app, oomAdjReason);

        } finally {

            // Kick off the handling of any pending targets enqueued during the above update

            mOomAdjUpdateOngoing = false;

            updateOomAdjPendingTargetsLocked(oomAdjReason);

        }

    }



    @GuardedBy({"mService", "mProcLock"})

    private boolean performUpdateOomAdjLSP(ProcessRecord app, String oomAdjReason) {

        final ProcessRecord topApp = mService.getTopApp();



        Trace.traceBegin(Trace.TRACE_TAG_ACTIVITY_MANAGER, oomAdjReason);
@@ -567,7 +635,7 @@ public final class OomAdjuster { 
        state.resetCachedInfo();

        // Check if this process is in the pending list too, remove from pending list if so.

        mPendingProcessSet.remove(app);

        boolean success = updateOomAdjLSP(app, cachedAdj, topApp, false,

        boolean success = performUpdateOomAdjLSP(app, cachedAdj, topApp, false,

                SystemClock.uptimeMillis());

        if (!success || (wasCached == state.isCached() && oldAdj != ProcessList.INVALID_ADJ

                && wasBackground == ActivityManager.isProcStateBackground(
@@ -695,15 +763,60 @@ public final class OomAdjuster { 
        }

    }



    /**

     * Check if there is an ongoing oomAdjUpdate, enqueue the given process record

     * to {@link #mPendingProcessSet} if there is one.

     *

     * @param app The target app to get an oomAdjUpdate, or a full oomAdjUpdate if it's null.

     * @return {@code true} if there is an ongoing oomAdjUpdate.

     */

    @GuardedBy("mService")

    private boolean checkAndEnqueueOomAdjTargetLocked(@Nullable ProcessRecord app) {

        if (!mOomAdjUpdateOngoing) {

            return false;

        }

        if (app != null) {

            mPendingProcessSet.add(app);

        } else {

            mPendingFullOomAdjUpdate = true;

        }

        return true;

    }



    /**

     * Kick off an oom adj update pass for the pending targets which are enqueued via

     * {@link #enqueueOomAdjTargetLocked}.

     */

    @GuardedBy("mService")

    void updateOomAdjPendingTargetsLocked(String oomAdjReason) {

        // First check if there is pending full update

        if (mPendingFullOomAdjUpdate) {

            mPendingFullOomAdjUpdate = false;

            mPendingProcessSet.clear();

            updateOomAdjLocked(oomAdjReason);

            return;

        }

        if (mPendingProcessSet.isEmpty()) {

            return;

        }



        if (mOomAdjUpdateOngoing) {

            // There's another oomAdjUpdate ongoing, return from here now;

            // that ongoing update would call us again at the end of it.

            return;

        }

        try {

            mOomAdjUpdateOngoing = true;

            performUpdateOomAdjPendingTargetsLocked(oomAdjReason);

        } finally {

            // Kick off the handling of any pending targets enqueued during the above update

            mOomAdjUpdateOngoing = false;

            updateOomAdjPendingTargetsLocked(oomAdjReason);

        }

    }



    @GuardedBy("mService")

    private void performUpdateOomAdjPendingTargetsLocked(String oomAdjReason) {

        final ProcessRecord topApp = mService.getTopApp();



        Trace.traceBegin(Trace.TRACE_TAG_ACTIVITY_MANAGER, oomAdjReason);
@@ -1846,8 +1959,8 @@ public final class OomAdjuster { 
                        computeOomAdjLSP(client, cachedAdj, topApp, doingAll, now,

                                cycleReEval, true);

                    } else {

                        cstate.setCurRawAdj(cstate.getSetAdj());

                        cstate.setCurRawProcState(cstate.getSetProcState());

                        cstate.setCurRawAdj(cstate.getCurAdj());

                        cstate.setCurRawProcState(cstate.getCurProcState());

                    }



                    int clientAdj = cstate.getCurRawAdj();
@@ -2130,8 +2243,8 @@ public final class OomAdjuster { 
                if (computeClients) {

                    computeOomAdjLSP(client, cachedAdj, topApp, doingAll, now, cycleReEval, true);

                } else {

                    cstate.setCurRawAdj(cstate.getSetAdj());

                    cstate.setCurRawProcState(cstate.getSetProcState());

                    cstate.setCurRawAdj(cstate.getCurAdj());

                    cstate.setCurRawProcState(cstate.getCurProcState());

                }



                if (shouldSkipDueToCycle(state, cstate, procState, adj, cycleReEval)) {

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

+155 −7
Original line number Diff line number Diff line
@@ -64,6 +64,7 @@ import static org.junit.Assert.assertEquals; 
import static org.junit.Assert.assertTrue;

import static org.mockito.AdditionalAnswers.answer;

import static org.mockito.Mockito.any;

import static org.mockito.Mockito.anyBoolean;

import static org.mockito.Mockito.anyInt;

import static org.mockito.Mockito.anyLong;

import static org.mockito.Mockito.doAnswer;
@@ -73,6 +74,8 @@ import static org.mockito.Mockito.doReturn; 
import static org.mockito.Mockito.mock;

import static org.mockito.Mockito.spy;



import android.app.ActivityManager;

import android.app.AppOpsManager;

import android.app.IApplicationThread;

import android.app.IServiceConnection;

import android.content.ComponentName;
@@ -178,11 +181,20 @@ public class MockingOomAdjusterTests { 
        setFieldValue(ActivityManagerService.class, sService, "mAppProfiler", profiler);

        setFieldValue(ActivityManagerService.class, sService, "mProcLock",

                new ActivityManagerProcLock());

        setFieldValue(ActivityManagerService.class, sService, "mServices",

                spy(new ActiveServices(sService)));

        setFieldValue(ActivityManagerService.class, sService, "mInternal",

                mock(ActivityManagerService.LocalService.class));

        setFieldValue(ActivityManagerService.class, sService, "mBatteryStatsService",

                mock(BatteryStatsService.class));

        doReturn(mock(AppOpsManager.class)).when(sService).getAppOpsManager();

        doCallRealMethod().when(sService).enqueueOomAdjTargetLocked(any(ProcessRecord.class));

        doCallRealMethod().when(sService).updateOomAdjPendingTargetsLocked(any(String.class));

        setFieldValue(AppProfiler.class, profiler, "mProfilerLock", new Object());

        doReturn(new ActivityManagerService.ProcessChangeItem()).when(pr)

                .enqueueProcessChangeItemLocked(anyInt(), anyInt());

        sService.mOomAdjuster = new OomAdjuster(sService, sService.mProcessList,

                mock(ActiveUids.class));

                new ActiveUids(sService, false));

        sService.mOomAdjuster.mAdjSeq = 10000;

        sService.mWakefulness = new AtomicInteger(PowerManagerInternal.WAKEFULNESS_AWAKE);

    }
@@ -1313,6 +1325,115 @@ public class MockingOomAdjusterTests { 
                SCHED_GROUP_DEFAULT);

    }



    @SuppressWarnings("GuardedBy")

    @Test

    public void testUpdateOomAdj_UidIdle_StopService() {

        final ProcessRecord app1 = spy(makeDefaultProcessRecord(MOCKAPP_PID, MOCKAPP_UID,

                MOCKAPP_PROCESSNAME, MOCKAPP_PACKAGENAME, false));

        final ProcessRecord app2 = spy(makeDefaultProcessRecord(MOCKAPP2_PID, MOCKAPP2_UID,

                MOCKAPP2_PROCESSNAME, MOCKAPP2_PACKAGENAME, false));

        final ProcessRecord client1 = spy(makeDefaultProcessRecord(MOCKAPP3_PID, MOCKAPP3_UID,

                MOCKAPP3_PROCESSNAME, MOCKAPP3_PACKAGENAME, false));

        final ProcessRecord client2 = spy(makeDefaultProcessRecord(MOCKAPP4_PID, MOCKAPP3_UID,

                MOCKAPP4_PROCESSNAME, MOCKAPP3_PACKAGENAME, false));

        final ProcessRecord app3 = spy(makeDefaultProcessRecord(MOCKAPP5_PID, MOCKAPP5_UID,

                MOCKAPP5_PROCESSNAME, MOCKAPP5_PACKAGENAME, false));

        final UidRecord app1UidRecord = new UidRecord(MOCKAPP_UID, sService);

        final UidRecord app2UidRecord = new UidRecord(MOCKAPP2_UID, sService);

        final UidRecord app3UidRecord = new UidRecord(MOCKAPP5_UID, sService);

        final UidRecord clientUidRecord = new UidRecord(MOCKAPP3_UID, sService);

        app1.setUidRecord(app1UidRecord);

        app2.setUidRecord(app2UidRecord);

        app3.setUidRecord(app3UidRecord);

        client1.setUidRecord(clientUidRecord);

        client2.setUidRecord(clientUidRecord);



        client1.mServices.setHasForegroundServices(true, 0);

        client2.mState.setForcingToImportant(new Object());

        ArrayList<ProcessRecord> lru = sService.mProcessList.getLruProcessesLOSP();

        lru.clear();

        lru.add(app1);

        lru.add(app2);

        lru.add(app3);

        lru.add(client1);

        lru.add(client2);

        sService.mWakefulness.set(PowerManagerInternal.WAKEFULNESS_AWAKE);



        final ComponentName cn1 = ComponentName.unflattenFromString(

                MOCKAPP_PACKAGENAME + "/.TestService");

        final ServiceRecord s1 = bindService(app1, client1, null, 0, mock(IBinder.class));

        setFieldValue(ServiceRecord.class, s1, "name", cn1);

        s1.startRequested = true;



        final ComponentName cn2 = ComponentName.unflattenFromString(

                MOCKAPP2_PACKAGENAME + "/.TestService");

        final ServiceRecord s2 = bindService(app2, client2, null, 0, mock(IBinder.class));

        setFieldValue(ServiceRecord.class, s2, "name", cn2);

        s2.startRequested = true;



        final ComponentName cn3 = ComponentName.unflattenFromString(

                MOCKAPP5_PACKAGENAME + "/.TestService");

        final ServiceRecord s3 = bindService(app3, client1, null, 0, mock(IBinder.class));

        setFieldValue(ServiceRecord.class, s3, "name", cn3);

        s3.startRequested = true;



        final ComponentName cn4 = ComponentName.unflattenFromString(

                MOCKAPP3_PACKAGENAME + "/.TestService");

        final ServiceRecord c2s = makeServiceRecord(client2);

        setFieldValue(ServiceRecord.class, c2s, "name", cn4);

        c2s.startRequested = true;



        try {

            sService.mOomAdjuster.mActiveUids.put(MOCKAPP_UID, app1UidRecord);

            sService.mOomAdjuster.mActiveUids.put(MOCKAPP2_UID, app2UidRecord);

            sService.mOomAdjuster.mActiveUids.put(MOCKAPP5_UID, app3UidRecord);

            sService.mOomAdjuster.mActiveUids.put(MOCKAPP3_UID, clientUidRecord);



            setServiceMap(s1, MOCKAPP_UID, cn1);

            setServiceMap(s2, MOCKAPP2_UID, cn2);

            setServiceMap(s3, MOCKAPP5_UID, cn3);

            setServiceMap(c2s, MOCKAPP3_UID, cn4);

            app2UidRecord.setIdle(false);

            sService.mOomAdjuster.updateOomAdjLocked(OomAdjuster.OOM_ADJ_REASON_NONE);



            assertProcStates(app1, PROCESS_STATE_FOREGROUND_SERVICE, PERCEPTIBLE_APP_ADJ,

                    SCHED_GROUP_DEFAULT);

            assertProcStates(app3, PROCESS_STATE_FOREGROUND_SERVICE, PERCEPTIBLE_APP_ADJ,

                    SCHED_GROUP_DEFAULT);

            assertProcStates(client1, PROCESS_STATE_FOREGROUND_SERVICE, PERCEPTIBLE_APP_ADJ,

                    SCHED_GROUP_DEFAULT);

            assertEquals(PROCESS_STATE_TRANSIENT_BACKGROUND, app2.mState.getSetProcState());

            assertEquals(PROCESS_STATE_TRANSIENT_BACKGROUND, client2.mState.getSetProcState());



            client1.mServices.setHasForegroundServices(false, 0);

            client2.mState.setForcingToImportant(null);

            app1UidRecord.reset();

            app2UidRecord.reset();

            app3UidRecord.reset();

            clientUidRecord.reset();

            app1UidRecord.setIdle(true);

            app2UidRecord.setIdle(true);

            app3UidRecord.setIdle(true);

            clientUidRecord.setIdle(true);

            doReturn(ActivityManager.APP_START_MODE_DELAYED).when(sService)

                    .getAppStartModeLOSP(anyInt(), any(String.class), anyInt(),

                    anyInt(), anyBoolean(), anyBoolean(), anyBoolean());

            doNothing().when(sService.mServices)

                    .scheduleServiceTimeoutLocked(any(ProcessRecord.class));

            sService.mOomAdjuster.updateOomAdjLocked(client1, OomAdjuster.OOM_ADJ_REASON_NONE);



            assertEquals(PROCESS_STATE_CACHED_EMPTY, client1.mState.getSetProcState());

            assertEquals(PROCESS_STATE_SERVICE, app1.mState.getSetProcState());

            assertEquals(PROCESS_STATE_SERVICE, client2.mState.getSetProcState());

        } finally {

            doCallRealMethod().when(sService)

                    .getAppStartModeLOSP(anyInt(), any(String.class), anyInt(),

                    anyInt(), anyBoolean(), anyBoolean(), anyBoolean());

            sService.mServices.mServiceMap.clear();

            sService.mOomAdjuster.mActiveUids.clear();

        }

    }



    @SuppressWarnings("GuardedBy")

    @Test

    public void testUpdateOomAdj_DoAll_Unbound() {
@@ -1815,15 +1936,42 @@ public class MockingOomAdjusterTests { 
        return app;

    }



    private ServiceRecord bindService(ProcessRecord service, ProcessRecord client,

            ServiceRecord record, int bindFlags, IBinder binder) {

        if (record == null) {

            record = mock(ServiceRecord.class);

            record.app = service;

    private ServiceRecord makeServiceRecord(ProcessRecord app) {

        final ServiceRecord record = mock(ServiceRecord.class);

        record.app = app;

        setFieldValue(ServiceRecord.class, record, "connections",

                new ArrayMap<IBinder, ArrayList<ConnectionRecord>>());

            service.mServices.startService(record);

        doCallRealMethod().when(record).getConnections();

        setFieldValue(ServiceRecord.class, record, "packageName", app.info.packageName);

        app.mServices.startService(record);

        record.appInfo = app.info;

        setFieldValue(ServiceRecord.class, record, "bindings", new ArrayMap<>());

        setFieldValue(ServiceRecord.class, record, "pendingStarts", new ArrayList<>());

        return record;

    }



    private void setServiceMap(ServiceRecord s, int uid, ComponentName cn) {

        ActiveServices.ServiceMap serviceMap = sService.mServices.mServiceMap.get(

                UserHandle.getUserId(uid));

        if (serviceMap == null) {

            serviceMap = mock(ActiveServices.ServiceMap.class);

            setFieldValue(ActiveServices.ServiceMap.class, serviceMap, "mServicesByInstanceName",

                    new ArrayMap<>());

            setFieldValue(ActiveServices.ServiceMap.class, serviceMap, "mActiveForegroundApps",

                    new ArrayMap<>());

            setFieldValue(ActiveServices.ServiceMap.class, serviceMap, "mServicesByIntent",

                    new ArrayMap<>());

            setFieldValue(ActiveServices.ServiceMap.class, serviceMap, "mDelayedStartList",

                    new ArrayList<>());

            sService.mServices.mServiceMap.put(UserHandle.getUserId(uid), serviceMap);

        }

        serviceMap.mServicesByInstanceName.put(cn, s);

    }



    private ServiceRecord bindService(ProcessRecord service, ProcessRecord client,

            ServiceRecord record, int bindFlags, IBinder binder) {

        if (record == null) {

            record = makeServiceRecord(service);

        }

        AppBindRecord binding = new AppBindRecord(record, null, client);

        ConnectionRecord cr = spy(new ConnectionRecord(binding,