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

Commit 7a907dbe authored by Felipe Leme's avatar Felipe Leme
Browse files

Refactored user switch workflow to decouple UserInfo from TargetUser. 

UserController calls SystemServiceManager on some stages of the user
switch lifecycle, and SystemServiceManager calls the respective
callbacks in the SystemService objects it manage.

Prior to Android 11, these callbacks would take a @UserIdInt userid,
but now they take a TargetUser (which in turns contains a Userinfo).
So, currently, UserController calls SystemServiceManager passing the
userid, and SystemServiceManager gets the UserInfo from
UserManagerService. That works fine most of the time, but there might
be some race conditions when a user is removed that can crash the
system when such user is stopped (because UserManagerService doesn't
have its UserInfo anymore).

This CL fixes this issue by refactoring the user switch workflow so:

- TargetUser doesn't hold a reference to the UserInfo anymore, but
  offer the methods needed by SystemService implementations.
- SystemServiceManager keeps track of the TargetUser objects used in
  the callbacks.

Test: manual verification

Fixes: 159831354

Change-Id: If68ab1b5e247c7389c8cac16432d8b8809507e86
parent 0beeb34e

services/autofill/java/com/android/server/autofill/AutofillManagerService.java

+1 −1
Original line number Diff line number Diff line
@@ -360,7 +360,7 @@ public final class AutofillManagerService 


    @Override // from SystemService

    public boolean isUserSupported(TargetUser user) {

        return user.getUserInfo().isFull() || user.getUserInfo().isManagedProfile();

        return user.isFull() || user.isManagedProfile();

    }



    @Override // from SystemService

services/contentcapture/java/com/android/server/contentcapture/ContentCaptureManagerService.java

+1 −1
Original line number Diff line number Diff line
@@ -219,7 +219,7 @@ public final class ContentCaptureManagerService extends 


    @Override // from SystemService

    public boolean isUserSupported(TargetUser user) {

        return user.getUserInfo().isFull() || user.getUserInfo().isManagedProfile();

        return user.isFull() || user.isManagedProfile();

    }



    @Override // from SystemService

services/core/java/android/os/UserManagerInternal.java

+1 −2
Original line number Diff line number Diff line
@@ -262,8 +262,7 @@ public abstract class UserManagerInternal { 
    public abstract boolean hasUserRestriction(String restriction, int userId);



    /**

     * Gets an {@link UserInfo} for the given {@code userId}, or {@code null} if not

     * found.

     * Gets a {@link UserInfo} for the given {@code userId}, or {@code null} if not found.

     */

    public abstract @Nullable UserInfo getUserInfo(@UserIdInt int userId);

services/core/java/com/android/server/SystemService.java

+59 −14
Original line number Diff line number Diff line
@@ -23,6 +23,7 @@ import android.annotation.NonNull; 
import android.annotation.Nullable;

import android.annotation.SystemApi;

import android.annotation.SystemApi.Client;

import android.annotation.UserIdInt;

import android.app.ActivityThread;

import android.content.Context;

import android.content.pm.UserInfo;
@@ -131,45 +132,89 @@ public abstract class SystemService { 
     */

    @SystemApi(client = Client.SYSTEM_SERVER)

    public static final class TargetUser {

        @NonNull

        private final UserInfo mUserInfo;



        // NOTE: attributes below must be immutable while ther user is running (i.e., from the

        // moment it's started until after it's shutdown).

        private final @UserIdInt int mUserId;

        private final boolean mFull;

        private final boolean mManagedProfile;

        private final boolean mPreCreated;



        /** @hide */

        public TargetUser(@NonNull UserInfo userInfo) {

            mUserInfo = userInfo;

            mUserId = userInfo.id;

            mFull = userInfo.isFull();

            mManagedProfile = userInfo.isManagedProfile();

            mPreCreated = userInfo.preCreated;

        }



        /**

         * @return The information about the user. <b>NOTE: </b> this is a "live" object

         * referenced by {@link UserManagerService} and hence should not be modified.

         * Checks if the target user is {@link UserInfo#isFull() full}.

         *

         * @hide

         */

        @NonNull

        public UserInfo getUserInfo() {

            return mUserInfo;

        public boolean isFull() {

            return mFull;

        }



        /**

         * @return the target {@link UserHandle}.

         * Checks if the target user is a managed profile.

         *

         * @hide

         */

        public boolean isManagedProfile() {

            return mManagedProfile;

        }



        /**

         * Checks if the target user is a pre-created user.

         *

         * @hide

         */

        public boolean isPreCreated() {

            return mPreCreated;

        }



        /**

         * Gets the target user's {@link UserHandle}.

         */

        @NonNull

        public UserHandle getUserHandle() {

            return mUserInfo.getUserHandle();

            return UserHandle.of(mUserId);

        }



        /**

         * @return the integer user id

         * Gets the target user's id.

         *

         * @hide

         */

        public int getUserIdentifier() {

            return mUserInfo.id;

        public @UserIdInt int getUserIdentifier() {

            return mUserId;

        }



        @Override

        public String toString() {

            return Integer.toString(getUserIdentifier());

            return Integer.toString(mUserId);

        }



        /**

         * @hide

         */

        public void dump(@NonNull StringBuilder builder) {

            builder.append(getUserIdentifier());



            if (!isFull() && !isManagedProfile()) return;



            builder.append('(');

            boolean addComma = false;

            if (isFull()) {

                builder.append("full");

            }

            if (isManagedProfile()) {

                if (addComma) builder.append(',');

                builder.append("mp");

            }

            builder.append(')');

        }

    }

services/core/java/com/android/server/SystemServiceManager.java

+39 −11
Original line number Diff line number Diff line
@@ -27,7 +27,10 @@ import android.os.UserHandle; 
import android.os.UserManagerInternal;

import android.util.ArrayMap;

import android.util.Slog;

import android.util.SparseArray;



import com.android.internal.annotations.GuardedBy;

import com.android.internal.util.Preconditions;

import com.android.server.SystemService.TargetUser;

import com.android.server.utils.TimingsTraceAndSlog;
@@ -74,6 +77,13 @@ public class SystemServiceManager { 


    private UserManagerInternal mUserManagerInternal;



    /**

     * Map of started {@link TargetUser TargetUsers} by user id; users are added on start and

     * removed after they're completely shut down.

     */

    @GuardedBy("mTargetUsers")

    private final SparseArray<TargetUser> mTargetUsers = new SparseArray<>();



    SystemServiceManager(Context context) {

        mContext = context;

    }
@@ -240,21 +250,26 @@ public class SystemServiceManager { 
        mUserManagerInternal = LocalServices.getService(UserManagerInternal.class);

    }



    private @NonNull UserInfo getUserInfo(@UserIdInt int userHandle) {

        if (mUserManagerInternal == null) {

            throw new IllegalStateException("mUserManagerInternal not set yet");

    private @NonNull TargetUser getTargetUser(@UserIdInt int userHandle) {

        final TargetUser targetUser;

        synchronized (mTargetUsers) {

            targetUser = mTargetUsers.get(userHandle);

        }

        final UserInfo userInfo = mUserManagerInternal.getUserInfo(userHandle);

        if (userInfo == null) {

            throw new IllegalStateException("No UserInfo for " + userHandle);

        }

        return userInfo;

        Preconditions.checkState(targetUser != null, "No TargetUser for " + userHandle);

        return targetUser;

    }



    /**

     * Starts the given user.

     */

    public void startUser(final @NonNull TimingsTraceAndSlog t, final @UserIdInt int userHandle) {

        // Create cached TargetUser

        final UserInfo userInfo = mUserManagerInternal.getUserInfo(userHandle);

        Preconditions.checkState(userInfo != null, "No UserInfo for " + userHandle);

        synchronized (mTargetUsers) {

            mTargetUsers.put(userHandle, new TargetUser(userInfo));

        }



        onUser(t, START, userHandle);

    }
@@ -291,6 +306,11 @@ public class SystemServiceManager { 
     */

    public void cleanupUser(final @UserIdInt int userHandle) {

        onUser(CLEANUP, userHandle);



        // Remove cached TargetUser

        synchronized (mTargetUsers) {

            mTargetUsers.remove(userHandle);

        }

    }



    private void onUser(@NonNull String onWhat, @UserIdInt int userHandle) {
@@ -306,9 +326,9 @@ public class SystemServiceManager { 
            @UserIdInt int curUserId, @UserIdInt int prevUserId) {

        t.traceBegin("ssm." + onWhat + "User-" + curUserId);

        Slog.i(TAG, "Calling on" + onWhat + "User " + curUserId);

        final TargetUser curUser = new TargetUser(getUserInfo(curUserId));

        final TargetUser curUser = getTargetUser(curUserId);

        final TargetUser prevUser = prevUserId == UserHandle.USER_NULL ? null

                : new TargetUser(getUserInfo(prevUserId));

                : getTargetUser(prevUserId);

        final int serviceLen = mServices.size();

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

            final SystemService service = mServices.get(i);
@@ -437,7 +457,7 @@ public class SystemServiceManager { 
     */

    public void dump() {

        StringBuilder builder = new StringBuilder();

        builder.append("Current phase: ").append(mCurrentPhase).append("\n");

        builder.append("Current phase: ").append(mCurrentPhase).append('\n');

        builder.append("Services:\n");

        final int startedLen = mServices.size();

        for (int i = 0; i < startedLen; i++) {
@@ -447,6 +467,14 @@ public class SystemServiceManager { 
                    .append("\n");

        }



        builder.append("Target users: ");

        final int targetUsersSize = mTargetUsers.size();

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

            mTargetUsers.valueAt(i).dump(builder);

            if (i != targetUsersSize - 1) builder.append(',');

        }

        builder.append('\n');



        Slog.e(TAG, builder.toString());

    }

}