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

Commit 242e548e authored by Suprabh Shukla's avatar Suprabh Shukla
Browse files

Tweaking some CPU_TIME capability assignments to bindings 

Capability cpu_time should be propagated across all bindings from the
client.

Removing some unnecessary assignments of the capability that could
happen even when the client did not have this capability. Although
this is practically not expected to happen, this enforces correctness of
state by making failures apparent.

Also making sure we don't skip OomAdjuster updates on provider or
service connections when the cpu_time capabililty needed to be updated
for the host process.

Flag: com.android.server.am.use_cpu_time_capability

Test: atest FrameworksMockingServicesTests:ServiceBindingOomAdjPolicyTest
Test: atest FrameworksMockingServicesTests:MockingOomAdjusterTests

Bug: 370817323
Change-Id: I6915e4aed5aa02d2e2c6addad1f03a1db899b0ca
parent e01df808

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

+18 −2
Original line number Diff line number Diff line
@@ -2840,7 +2840,6 @@ public class OomAdjuster { 
                            return true;

                        }

                    }

                    capability |= PROCESS_CAPABILITY_CPU_TIME;

                }

                // Not doing bind OOM management, so treat

                // this guy more like a started service.
@@ -3089,7 +3088,6 @@ public class OomAdjuster { 
                        return true;

                    }

                }

                capability |= PROCESS_CAPABILITY_CPU_TIME;

            }

        }

        if (cr.hasFlag(Context.BIND_TREAT_LIKE_ACTIVITY)) {
@@ -4243,6 +4241,11 @@ public class OomAdjuster { 
                        != client.getSetCapability()) {

            // The connection might elevate the importance of the service's capabilities.

            needDryRun = true;

        } else if (Flags.useCpuTimeCapability()

                && (client.getSetCapability() & ~app.getSetCapability()

                    & PROCESS_CAPABILITY_CPU_TIME) != 0) {

            // The connection might grant PROCESS_CAPABILITY_CPU_TIME to the service.

            needDryRun = true;

        } else if (Flags.unfreezeBindPolicyFix()

                && cr.hasFlag(Context.BIND_WAIVE_PRIORITY

                            | Context.BIND_ALLOW_OOM_MANAGEMENT)) {
@@ -4290,6 +4293,10 @@ public class OomAdjuster { 
                && client.mOptRecord.shouldNotFreeze()) {

            // Process has shouldNotFreeze and it could have gotten it from the client.

            return true;

        } else if (Flags.useCpuTimeCapability()

                && (client.getSetCapability() & app.getSetCapability()

                    & PROCESS_CAPABILITY_CPU_TIME) != 0) {

            return true;

        }

        return false;

    }
@@ -4309,6 +4316,11 @@ public class OomAdjuster { 
                && client.mOptRecord.shouldNotFreeze()

                && !app.mOptRecord.shouldNotFreeze()) {

            needDryRun = true;

        } else if (Flags.useCpuTimeCapability()

                && (client.getSetCapability() & ~app.getSetCapability()

                    & PROCESS_CAPABILITY_CPU_TIME) != 0) {

            // The connection might grant PROCESS_CAPABILITY_CPU_TIME to the provider.

            needDryRun = true;

        }



        if (needDryRun) {
@@ -4335,6 +4347,10 @@ public class OomAdjuster { 
                && client.mOptRecord.shouldNotFreeze()) {

            // Process has shouldNotFreeze and it could have gotten it from the client.

            return true;

        } else if (Flags.useCpuTimeCapability()

                && (client.getSetCapability() & app.getSetCapability()

                    & PROCESS_CAPABILITY_CPU_TIME) != 0) {

            return true;

        }



        return false;

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

+80 −0
Original line number Diff line number Diff line
@@ -18,6 +18,7 @@ package com.android.server.am; 


import static android.app.ActivityManager.PROCESS_CAPABILITY_ALL;

import static android.app.ActivityManager.PROCESS_CAPABILITY_BFSL;

import static android.app.ActivityManager.PROCESS_CAPABILITY_CPU_TIME;

import static android.app.ActivityManager.PROCESS_STATE_BOUND_FOREGROUND_SERVICE;

import static android.app.ActivityManager.PROCESS_STATE_BOUND_TOP;

import static android.app.ActivityManager.PROCESS_STATE_CACHED_ACTIVITY;
@@ -40,6 +41,7 @@ import static android.app.ActivityManager.PROCESS_STATE_TOP_SLEEPING; 
import static android.app.ActivityManager.PROCESS_STATE_TRANSIENT_BACKGROUND;

import static android.app.ActivityManagerInternal.OOM_ADJ_REASON_ACTIVITY;

import static android.app.ActivityManagerInternal.OOM_ADJ_REASON_NONE;

import static android.app.ActivityManagerInternal.OOM_ADJ_REASON_SHORT_FGS_TIMEOUT;

import static android.content.pm.ServiceInfo.FOREGROUND_SERVICE_TYPE_SHORT_SERVICE;



import static androidx.test.platform.app.InstrumentationRegistry.getInstrumentation;
@@ -283,6 +285,15 @@ public class MockingOomAdjusterTests { 
        }

    }



    private static void assertNoCpuTime(ProcessRecord app) {

        assertEquals(0, app.mState.getSetCapability() & PROCESS_CAPABILITY_CPU_TIME);

    }



    private static void assertCpuTime(ProcessRecord app) {

        assertEquals(PROCESS_CAPABILITY_CPU_TIME,

                app.mState.getSetCapability() & PROCESS_CAPABILITY_CPU_TIME);

    }



    private static void assertBfsl(ProcessRecord app) {

        assertEquals(PROCESS_CAPABILITY_BFSL,

                app.mState.getSetCapability() & PROCESS_CAPABILITY_BFSL);
@@ -661,6 +672,7 @@ public class MockingOomAdjusterTests { 
        // SHORT_SERVICE, timed out already.

        s = ServiceRecord.newEmptyInstanceForTest(mService);

        s.appInfo = new ApplicationInfo();



        mProcessStateController.setStartRequested(s, true);

        s.isForeground = true;

        s.foregroundServiceType = FOREGROUND_SERVICE_TYPE_SHORT_SERVICE;
@@ -685,6 +697,51 @@ public class MockingOomAdjusterTests { 
        }

    }



    @SuppressWarnings("GuardedBy")

    @Test

    @EnableFlags(Flags.FLAG_USE_CPU_TIME_CAPABILITY)

    public void testUpdateOomAdjFreezeState_bindingFromShortFgs() {

        // Setting up a started short FGS within app1.

        final ServiceRecord s = ServiceRecord.newEmptyInstanceForTest(mService);

        s.appInfo = new ApplicationInfo();

        mProcessStateController.setStartRequested(s, true);

        s.isForeground = true;

        s.foregroundServiceType = FOREGROUND_SERVICE_TYPE_SHORT_SERVICE;

        mProcessStateController.setShortFgsInfo(s, SystemClock.uptimeMillis());



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

                MOCKAPP_PROCESSNAME, MOCKAPP_PACKAGENAME, true));

        mProcessStateController.setHostProcess(s, app);

        mProcessStateController.setHasForegroundServices(app.mServices, true,

                FOREGROUND_SERVICE_TYPE_SHORT_SERVICE, /* hasNoneType=*/false);

        mProcessStateController.startService(app.mServices, s);

        app.mState.setLastTopTime(SystemClock.uptimeMillis()

                - mService.mConstants.TOP_TO_FGS_GRACE_DURATION);



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

                MOCKAPP2_PROCESSNAME, MOCKAPP2_PACKAGENAME, false));

        // App1 with short service binds to app2

        bindService(app2, app, null, null, 0, mock(IBinder.class));



        setProcessesToLru(app, app2);

        updateOomAdj(app);



        assertCpuTime(app);

        assertCpuTime(app2);



        // Timeout the short FGS.

        mProcessStateController.setShortFgsInfo(s, SystemClock.uptimeMillis()

                - mService.mConstants.mShortFgsTimeoutDuration

                - mService.mConstants.mShortFgsProcStateExtraWaitDuration);

        mService.mServices.onShortFgsProcstateTimeout(s);

        // mService is a mock, but this verifies that the timeout would trigger an update.

        verify(mService).updateOomAdjLocked(app, OOM_ADJ_REASON_SHORT_FGS_TIMEOUT);

        updateOomAdj(app);



        assertNoCpuTime(app);

        assertNoCpuTime(app2);

    }



    @SuppressWarnings("GuardedBy")

    @Test

    public void testUpdateOomAdj_DoOne_OverlayUi() {
@@ -3142,11 +3199,19 @@ public class MockingOomAdjusterTests { 
        assertEquals(true, app.getUidRecord().isSetAllowListed());

        assertFreezeState(app, false);

        assertFreezeState(app2, false);

        if (Flags.useCpuTimeCapability()) {

            assertCpuTime(app);

            assertCpuTime(app2);

        }



        mProcessStateController.setUidTempAllowlistStateLSP(MOCKAPP_UID, false);

        assertEquals(false, app.getUidRecord().isSetAllowListed());

        assertFreezeState(app, true);

        assertFreezeState(app2, true);

        if (Flags.useCpuTimeCapability()) {

            assertNoCpuTime(app);

            assertNoCpuTime(app2);

        }

    }



    @SuppressWarnings("GuardedBy")
@@ -3171,6 +3236,11 @@ public class MockingOomAdjusterTests { 
        assertFreezeState(app, false);

        assertFreezeState(app2, false);

        assertFreezeState(app3, false);

        if (Flags.useCpuTimeCapability()) {

            assertCpuTime(app);

            assertCpuTime(app2);

            assertCpuTime(app3);

        }



        // Remove app1 from allowlist.

        mProcessStateController.setUidTempAllowlistStateLSP(MOCKAPP_UID, false);
@@ -3179,6 +3249,11 @@ public class MockingOomAdjusterTests { 
        assertFreezeState(app, true);

        assertFreezeState(app2, false);

        assertFreezeState(app3, false);

        if (Flags.useCpuTimeCapability()) {

            assertNoCpuTime(app);

            assertCpuTime(app2);

            assertCpuTime(app3);

        }



        // Now remove app2 from allowlist.

        mProcessStateController.setUidTempAllowlistStateLSP(MOCKAPP2_UID, false);
@@ -3187,6 +3262,11 @@ public class MockingOomAdjusterTests { 
        assertFreezeState(app, true);

        assertFreezeState(app2, true);

        assertFreezeState(app3, true);

        if (Flags.useCpuTimeCapability()) {

            assertNoCpuTime(app);

            assertNoCpuTime(app2);

            assertNoCpuTime(app3);

        }

    }



    @SuppressWarnings("GuardedBy")

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

+53 −0
Original line number Diff line number Diff line
@@ -16,6 +16,7 @@ 


package com.android.server.am;



import static android.app.ActivityManager.PROCESS_CAPABILITY_CPU_TIME;

import static android.app.ActivityManager.PROCESS_CAPABILITY_FOREGROUND_MICROPHONE;

import static android.app.ActivityManager.PROCESS_CAPABILITY_NONE;

import static android.app.ActivityManager.PROCESS_STATE_CACHED_EMPTY;
@@ -64,6 +65,8 @@ import android.content.pm.ServiceInfo; 
import android.os.Handler;

import android.os.HandlerThread;

import android.os.IBinder;

import android.platform.test.annotations.DisableFlags;

import android.platform.test.annotations.EnableFlags;

import android.platform.test.annotations.Presubmit;

import android.platform.test.annotations.RequiresFlagsDisabled;

import android.platform.test.annotations.RequiresFlagsEnabled;
@@ -326,6 +329,7 @@ public final class ServiceBindingOomAdjPolicyTest { 


    @Test

    @RequiresFlagsEnabled(com.android.server.am.Flags.FLAG_UNFREEZE_BIND_POLICY_FIX)

    @DisableFlags(Flags.FLAG_USE_CPU_TIME_CAPABILITY)

    public void testServiceDistinctBindingOomAdjShouldNotFreeze() throws Exception {

        // Enable the flags.

        mSetFlagsRule.enableFlags(Flags.FLAG_SERVICE_BINDING_OOM_ADJ_POLICY);
@@ -418,6 +422,7 @@ public final class ServiceBindingOomAdjPolicyTest { 


    @Test

    @RequiresFlagsEnabled(com.android.server.am.Flags.FLAG_UNFREEZE_BIND_POLICY_FIX)

    @DisableFlags(Flags.FLAG_USE_CPU_TIME_CAPABILITY)

    public void testServiceDistinctBindingOomAdjAllowOomManagement() throws Exception {

        // Enable the flags.

        mSetFlagsRule.enableFlags(Flags.FLAG_SERVICE_BINDING_OOM_ADJ_POLICY);
@@ -497,6 +502,7 @@ public final class ServiceBindingOomAdjPolicyTest { 


    @Test

    @RequiresFlagsEnabled(com.android.server.am.Flags.FLAG_UNFREEZE_BIND_POLICY_FIX)

    @DisableFlags(Flags.FLAG_USE_CPU_TIME_CAPABILITY)

    public void testServiceDistinctBindingOomAdjWaivePriority_propagateUnfreeze() throws Exception {

        // Enable the flags.

        mSetFlagsRule.enableFlags(Flags.FLAG_SERVICE_BINDING_OOM_ADJ_POLICY);
@@ -573,6 +579,50 @@ public final class ServiceBindingOomAdjPolicyTest { 
                atLeastOnce(), atLeastOnce());

    }



    @Test

    @RequiresFlagsEnabled({

            Flags.FLAG_UNFREEZE_BIND_POLICY_FIX,

            Flags.FLAG_SERVICE_BINDING_OOM_ADJ_POLICY

    })

    @EnableFlags(Flags.FLAG_USE_CPU_TIME_CAPABILITY)

    public void testServiceDistinctBindingOomAdj_propagateCpuTimeCapability() throws Exception {

        // Note that PROCESS_CAPABILITY_CPU_TIME is special and should be propagated even when

        // BIND_INCLUDE_CAPABILITIES is not present.

        performTestServiceDistinctBindingOomAdj(TEST_APP1_PID, TEST_APP1_UID,

                PROCESS_STATE_HOME, HOME_APP_ADJ, PROCESS_CAPABILITY_CPU_TIME, TEST_APP1_NAME,

                this::setHomeProcess,

                TEST_APP2_PID, TEST_APP2_UID, PROCESS_STATE_FOREGROUND_SERVICE,

                PERCEPTIBLE_APP_ADJ, PROCESS_CAPABILITY_NONE, TEST_APP2_NAME, TEST_SERVICE2_NAME,

                this::setHasForegroundServices,

                BIND_AUTO_CREATE,

                atLeastOnce(), atLeastOnce());



        // BIND_WAIVE_PRIORITY should not affect propagation of capability CPU_TIME

        performTestServiceDistinctBindingOomAdj(TEST_APP1_PID, TEST_APP1_UID,

                PROCESS_STATE_FOREGROUND_SERVICE, PERCEPTIBLE_APP_ADJ, PROCESS_CAPABILITY_CPU_TIME,

                TEST_APP1_NAME,

                this::setHasForegroundServices,

                TEST_APP2_PID, TEST_APP2_UID, PROCESS_STATE_HOME, HOME_APP_ADJ,

                PROCESS_CAPABILITY_NONE, TEST_APP2_NAME, TEST_SERVICE2_NAME,

                this::setHomeProcess,

                BIND_AUTO_CREATE | BIND_WAIVE_PRIORITY,

                atLeastOnce(), atLeastOnce());



        // If both process have the capability, the bind should not need an update but the unbind

        // is not safe to skip.

        // Note that this check can fail on future changes that are not related to

        // PROCESS_CAPABILITY_CPU_TIME and trigger updates but this is important to ensure

        // efficiency of OomAdjuster.

        performTestServiceDistinctBindingOomAdj(TEST_APP1_PID, TEST_APP1_UID,

                PROCESS_STATE_HOME, HOME_APP_ADJ, PROCESS_CAPABILITY_CPU_TIME, TEST_APP1_NAME,

                this::setHomeProcess,

                TEST_APP2_PID, TEST_APP2_UID, PROCESS_STATE_HOME, HOME_APP_ADJ,

                PROCESS_CAPABILITY_CPU_TIME, TEST_APP2_NAME, TEST_SERVICE2_NAME,

                this::setHomeProcess,

                BIND_AUTO_CREATE,

                never(), atLeastOnce());

    }



    @Test

    @RequiresFlagsDisabled(com.android.server.am.Flags.FLAG_UNFREEZE_BIND_POLICY_FIX)

    public void testServiceDistinctBindingOomAdjWaivePriority() throws Exception {
@@ -624,6 +674,9 @@ public final class ServiceBindingOomAdjPolicyTest { 
        // Enable the flags.

        mSetFlagsRule.enableFlags(Flags.FLAG_SERVICE_BINDING_OOM_ADJ_POLICY);



        // Note that some capabilities like PROCESS_CAPABILITY_CPU_TIME are special and propagated

        // regardless of BIND_INCLUDE_CAPABILITIES. We don't test for them here.



        // Verify that there should be 0 oom adj update

        // because we didn't specify the "BIND_INCLUDE_CAPABILITIES"

        performTestServiceDistinctBindingOomAdj(TEST_APP1_PID, TEST_APP1_UID,