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

Commit 8d6d7322 authored by Adam Bookatz's avatar Adam Bookatz
Browse files

Stricter adherence to mMaxRunningUsers 

mMaxRunningUsers is a soft limit and can be exceeded in various cases.
Some of those make sense, others less so. Here, we get rid of the less
logical cases.

Changes:
  * When starting a user in the background (without specifying that it
    is for a temporary time only), we now trigger
    stopExcessRunningUsers(). This is new; we used to only do it for
    user-switches, not for background-user-starts. Background starts
    were seldom, but now that we actually use them (e.g. for alarms),
    we want to tighten this. (b/419867128)
  * We now allow user 0's profiles to be stopped during
    stopExcessRunningUsers(). This is new; we used to exempt them, even
    though we didn't exempt other user's profiles.
    (b/310249114#comment8)
  * If user 0's profile has DISALLOW_RUN_IN_BACKGROUND, we now do stop
    it when its parent enters the background, as one would expect.
    (b/324641317#comment3)
  * startProfiles(), which is called when switching to a user, will now
    ignore mMaxRunningUsers, and start all of the user's applicable
    profiles, even if that explicitly exceeds maxRunningUsers.
    (b/419872912) Reasons:
      * The previous code only allowed up to mMaxRunningUsers-1 profiles
        to be *automatically* started, presumably making the assumption
        that no other background users would be kept around (since
        historically, only user 0 could have profiles and there were no
        alwaysVisible users). Those assumptions are no longer valid,
        so it was already possible to exceed mMaxRunningUsers as a
        result. And anyway, one could always explicitly start additional
        profiles beyond mMaxRunningUsers.
      * This is probably a better user experience anyway (in the
        somewhat theoretical case where there are so many running
        profiles). If an OEM allows a user to have lots of profiles, it
        should accept the fact that they can run simultaneously.
      * I strongly suspect that the old code would have been very
        problematic if it actually triggered in real life, since the
        profile's quiet mode wouldn't have been respected and the UI
        probably would have broken as a result. Now we don't have to
        worry about it.
  * We also make sure to always stop excess users after starting the
    profiles. Previously certain code paths did call
    stopExcessRunningUsers() after startProfiles(), but others didn't.
    Now it is embedded in startUser, so it will always be called.
    (b/419872912)

Non-changes (ways mMaxRunningUsers can continue to be exceeded):
  * The current user and its related profiles are never stopped when
    switching/starting users, even if it has more profiles than
    mMaxRunningUsers.
  * If a background user is deemed important (e.g. because it is visible
    or audible), we may abstain from or defer stopping it, even if that
    causes us to exceed mMaxRunningUsers.
  * If we start a background user, and that triggers exceeding the max,
    we make sure we don't stop that same background user, even if it is
    the only available user to stop.

Bug: 419867128
Bug: 310249114
Bug: 324641317
Bug: 419872912
Test: UserControllerTest
Flag: android.multiuser.stop_excess_for_background_starts
Change-Id: I6dd420cf21492b7cab606bd476aa13de1a44406c
parent 0149d5fb

core/java/android/content/pm/multiuser.aconfig

+7 −0
Original line number Diff line number Diff line
@@ -298,6 +298,13 @@ flag { 
    bug: "330351042"

}



flag {

    name: "stop_excess_for_background_starts"

    namespace: "multiuser"

    description: "Be more aggressive when stopping excess users, by triggering this even on background user starts and by allowing user 0's profiles to be stopped."

    bug: "419867128"

}



flag {

    name: "stop_previous_user_apps"

    namespace: "multiuser"

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

+66 −24
Original line number Diff line number Diff line
@@ -254,12 +254,16 @@ class UserController implements Handler.Callback { 
    /**

     * Maximum number of users we allow to be running at a time, including system user.

     *

     * <p>This parameter only affects how many background users will be stopped when switching to a

     * new user. It has no impact on {@link #startUser(int, boolean)} behavior.

     * <p>This parameter only affects how many background users will be stopped when

     * switching to, or starting, a new user. It does not prevent starting a user.

     *

     * <p>Note: Current and system user (and their related profiles) are never stopped when

     * switching users. Due to that, the actual number of running users can exceed mMaxRunningUsers

     // TODO(b/310249114): Strongly consider *not* exempting the SYSTEM user's profile.

     * <p>Note: This is a soft limit; the actual number of running users can exceed mMaxRunningUsers

     * in several situations. For example: <ul>

     * <li>The current user and its related profiles are never stopped when switching/starting

     *     users, even if it has more profiles than mMaxRunningUsers.

     * <li>If a background user is deemed important (e.g. because it is visible or audible), we may

     *     abstain from or defer stopping it, even if that causes us to exceed mMaxRunningUsers.

     * </ul>

     */

    @GuardedBy("mLock")

    private int mMaxRunningUsers;
@@ -423,7 +427,7 @@ class UserController implements Handler.Callback { 
    private final SparseIntArray mCompletedEventTypes = new SparseIntArray();



    /**

     * Sets on {@link #setInitialConfig(boolean, int, boolean)}, which is called by

     * Sets on {@link #setInitialConfig(boolean, int, boolean, int)}, which is called by

     * {@code ActivityManager} when the system is started.

     *

     * <p>It's useful to ignore external operations (i.e., originated outside {@code system_server},
@@ -605,9 +609,17 @@ class UserController implements Handler.Callback { 
    }



    private void stopExcessRunningUsers() {

        stopExcessRunningUsers(UserHandle.USER_NULL);

    }



    private void stopExcessRunningUsers(@CanBeNULL @UserIdInt int exemptedUserId) {

        final ArraySet<Integer> exemptedUsers = new ArraySet<>();

        final ArraySet<Integer> avoidUsers = new ArraySet<>();



        if (exemptedUserId != UserHandle.USER_NULL) {

            exemptedUsers.add(exemptedUserId);

        }



        final List<UserInfo> users = mInjector.getUserManager().getUsers(true);

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

            final int userId = users.get(i).id;
@@ -654,7 +666,8 @@ class UserController implements Handler.Callback { 
            }

            // allowDelayedLocking set here as stopping user is done without any explicit request

            // from outside.

            Slogf.i(TAG, "Too many running users (%d). Attempting to stop user %d",

            Slogf.i(TAG, "%d too many running users (%d total). Attempting to stop user %d.",

                    currentlyRunningLru.size() - maxRunningUsers,

                    currentlyRunningLru.size(), userId);

            if (stopUsersLU(userId,

                    /* stopProfileRegardlessOfParent= */ false, /* allowDelayedLocking= */ true,
@@ -672,12 +685,18 @@ class UserController implements Handler.Callback { 
        int excessUsers = currentlyRunningLru.size() - maxRunningUsers;

        for (int i = 0; excessUsers > 0 && i < candidatesForScheduledStopping.size(); i++) {

            final Integer userId = candidatesForScheduledStopping.get(i);

            Slogf.i(TAG, "Still %d too many running users. Scheduling to later stop user %d",

            Slogf.i(TAG, "%d too many running users. Scheduling to later stop user %d.",

                    excessUsers, userId);

            if (rescheduleStopOfBackgroundUser(userId)) {

                excessUsers--;

            }

        }



        if (excessUsers > 0) {

            Slogf.i(TAG, "%d too many running users. We are left in a state exceeding %d "

                    + "running users, but that's okay because it is a soft limit.",

                    excessUsers, maxRunningUsers);

        }

    }



    /**
@@ -1259,8 +1278,9 @@ class UserController implements Handler.Callback { 
        if (!stopProfileRegardlessOfParent) {

            final int parentId = mUserProfileGroupIds.get(userId, UserInfo.NO_PROFILE_GROUP_ID);

            if (parentId != UserInfo.NO_PROFILE_GROUP_ID && parentId != userId) {

                // TODO(b/310249114): Strongly consider *not* exempting the SYSTEM user's profile.

                if ((UserHandle.USER_SYSTEM == parentId || isCurrentUserLU(parentId))) {

                if ((!android.multiuser.Flags.stopExcessForBackgroundStarts()

                        && UserHandle.USER_SYSTEM == parentId)

                        || isCurrentUserLU(parentId)) {

                    return USER_OP_ERROR_RELATED_USERS_CANNOT_STOP;

                }

            }
@@ -1702,11 +1722,11 @@ class UserController implements Handler.Callback { 
            mLastActiveUsersForDelayedLocking.add(0, userId);

            int totalUnlockedUsers = mStartedUsers.size()

                    + mLastActiveUsersForDelayedLocking.size();

            // TODO: Decouple the delayed locking flows from mMaxRunningUsers. These users aren't

            //  running so this calculation shouldn't be based on this parameter. Also note that

            //  that if these devices ever support background running users (such as profiles), the

            //  implementation is incorrect since starting such users can cause the max to be

            //  exceeded.

            // The delayed locking determination is tangled with mMaxRunningUsers. This is

            // presumably because the primary point of leaving users unlocked via

            // mDelayUserDataLocking is so that we can bulk restart them all later. We don't want to

            // try restarting more of these users than mMaxRunningUsers allows, so we don't leave

            // more than this many users unlocked.

            if (totalUnlockedUsers > mMaxRunningUsers) { // should lock a user

                final int userIdToLock = mLastActiveUsersForDelayedLocking.get(

                        mLastActiveUsersForDelayedLocking.size() - 1);
@@ -1870,6 +1890,19 @@ class UserController implements Handler.Callback { 
                profilesToStart.add(user);

            }

        }

        if (android.multiuser.Flags.stopExcessForBackgroundStarts()) {

            final int profilesToStartSize = profilesToStart.size();

            for (int i = 0; i < profilesToStartSize; ++i) {

                // NOTE: this method is setting the profiles of the current user - which is always

                // assigned to the default display

                startUser(profilesToStart.get(i).id, USER_START_MODE_BACKGROUND_VISIBLE);

            }

            final int maxRunningUsers = getMaxRunningUsers();

            if (profilesToStartSize >= maxRunningUsers) {

                Slogf.w(TAG, "User %d has more profiles to start (%d) than MAX_RUNNING_USERS would"

                        + " allow (%d)", currentUserId, profilesToStartSize, maxRunningUsers);

            }

        } else {

            final int profilesToStartSize = profilesToStart.size();

            int i = 0;

            for (; i < profilesToStartSize && i < (getMaxRunningUsers() - 1); ++i) {
@@ -1881,6 +1914,7 @@ class UserController implements Handler.Callback { 
                Slogf.w(TAG, "More profiles than MAX_RUNNING_USERS");

            }

        }

    }



    private boolean shouldStartWithParent(UserInfo user) {

        final UserProperties properties = getUserProperties(user.id);
@@ -2332,6 +2366,14 @@ class UserController implements Handler.Callback { 
            t.traceBegin("finishUserBoot");

            finishUserBoot(uss);

            t.traceEnd();

            if (android.multiuser.Flags.stopExcessForBackgroundStarts()) {

                // We're willing to let this background start exceed maxRunningUsers if it's

                // just an explicitly temporary thing, but otherwise, trim the excess.

                final boolean shouldStopExcessRunningUsers = autoStopUserInSecs <= 0;

                if (shouldStopExcessRunningUsers) {

                    mHandler.post(() -> stopExcessRunningUsers(userId));

                }

            }

        }



        if (needStart || isSystemUserInHeadlessMode) {

services/tests/servicestests/src/com/android/server/am/UserControllerTest.java

+278 −30

File changed.

Preview size limit exceeded, changes collapsed.