Merge "Properly handle stopping related users" into main (4a35a467) · Commits · e / os / android_frameworks_base

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

+71 −28

Original line number	Diff line number	Diff line
		@@ -276,7 +276,15 @@ class UserController implements Handler.Callback {
		private final SparseArray<UserState> mStartedUsers = new SparseArray<>();

		/**
		* LRU list of history of current users. Most recently current is at the end.
		* LRU list of history of running users, in order of when we last needed to start them.
		*
		* Switching to a user will move it towards the end. Attempting to start a user/profile (even
		* if it was already running) will move it towards the end.
		*
		* <p>Guarantees (by the end of startUser):
		* <li>The current user will always be at the end, even if background users were started
		* subsequently.
		* <li>Parents always come later than (but not necessarily adjacent to) their profiles.
		*/
		@GuardedBy("mLock")
		private final ArrayList<Integer> mUserLru = new ArrayList<>();
		@@ -299,6 +307,9 @@ class UserController implements Handler.Callback {
		/**
		* Mapping from each known user ID to the profile group ID it is associated with.
		* <p>Users not present in this array have a profile group of NO_PROFILE_GROUP_ID.
		*
		* <p>For better or worse, this class sometimes assumes that the profileGroupId of a parent user
		* is always identical with its userId. If that ever becomes false, this class needs updating.
		*/
		@GuardedBy("mLock")
		private final SparseIntArray mUserProfileGroupIds = new SparseIntArray();
		@@ -499,6 +510,23 @@ class UserController implements Handler.Callback {
		});
		}

		/** Adds a user to mUserLru, moving it to the end of the list if it was already present. */
		private void addUserToUserLru(@UserIdInt int userId) {
		synchronized (mLock) {
		final Integer userIdObj = userId;
		mUserLru.remove(userIdObj);
		mUserLru.add(userIdObj);

		// Now also move the user's parent to the end (if applicable).
		Integer parentIdObj = mUserProfileGroupIds.get(userId, UserInfo.NO_PROFILE_GROUP_ID);
		if (parentIdObj != UserInfo.NO_PROFILE_GROUP_ID && !parentIdObj.equals(userIdObj)
		&& mUserLru.remove(parentIdObj)) {
		mUserLru.add(parentIdObj);
		}
		}
		}

		/** Returns a list of running users, in order of when they were started (oldest first). */
		@GuardedBy("mLock")
		@VisibleForTesting
		List<Integer> getRunningUsersLU() {
		@@ -536,9 +564,9 @@ class UserController implements Handler.Callback {

		@GuardedBy("mLock")
		private void stopExcessRunningUsersLU(int maxRunningUsers, ArraySet<Integer> exemptedUsers) {
		List<Integer> currentlyRunning = getRunningUsersLU();
		Iterator<Integer> iterator = currentlyRunning.iterator();
		while (currentlyRunning.size() > maxRunningUsers && iterator.hasNext()) {
		List<Integer> currentlyRunningLru = getRunningUsersLU();
		Iterator<Integer> iterator = currentlyRunningLru.iterator();
		while (currentlyRunningLru.size() > maxRunningUsers && iterator.hasNext()) {
		Integer userId = iterator.next();
		if (userId == UserHandle.USER_SYSTEM
		\|\| userId == mCurrentUserId
		@@ -551,6 +579,10 @@ class UserController implements Handler.Callback {
		if (stopUsersLU(userId, /* force= / false, / allowDelayedLocking= */ true,
		/* stopUserCallback= / null, / keyEvictedCallback= */ null)
		== USER_OP_SUCCESS) {
		// Technically, stopUsersLU can remove more than one user when stopping a parent.
		// But mUserLru is designed so that profiles always precede their parent, so this
		// normally won't happen here, and therefore won't cause underestimating the number
		// removed.
		iterator.remove();
		}
		}
		@@ -947,7 +979,7 @@ class UserController implements Handler.Callback {
		}

		/**
		* Stops the user along with its related users. The method calls
		* Stops the user along with its profiles. The method calls
		* {@link #getUsersToStopLU(int)} to determine the list of users that should be stopped.
		*/
		@GuardedBy("mLock")
		@@ -992,7 +1024,12 @@ class UserController implements Handler.Callback {
		}

		/**
		* Stops a single User. This can also trigger locking user data out depending on device's
		* Stops a single User.
		*
		* This should only ever be called by {@link #stopUsersLU},
		* which is responsible to making sure any associated users are appropriately stopped too.
		*
		* This can also trigger locking user data out depending on device's
		* config ({@code mDelayUserDataLocking}) and arguments.
		*
		* In the default configuration for most device and users, users will be locked when stopping.
		@@ -1425,7 +1462,8 @@ class UserController implements Handler.Callback {

		/**
		* Determines the list of users that should be stopped together with the specified
		* {@code userId}. The returned list includes {@code userId}.
		* {@code userId}, i.e. the user and its profiles (if the given user is a parent).
		* The returned list includes {@code userId}.
		*/
		@GuardedBy("mLock")
		private @NonNull int[] getUsersToStopLU(@UserIdInt int userId) {
		@@ -1433,6 +1471,8 @@ class UserController implements Handler.Callback {
		IntArray userIds = new IntArray();
		userIds.add(userId);
		int userGroupId = mUserProfileGroupIds.get(userId, UserInfo.NO_PROFILE_GROUP_ID);
		if (userGroupId == userId) {
		// The user is the parent of the profile group. Stop its profiles too.
		for (int i = 0; i < startedUsersSize; i++) {
		UserState uss = mStartedUsers.valueAt(i);
		int startedUserId = uss.mHandle.getIdentifier();
		@@ -1448,6 +1488,7 @@ class UserController implements Handler.Callback {
		}
		userIds.add(startedUserId);
		}
		}
		return userIds.toArray();
		}

		@@ -1519,6 +1560,7 @@ class UserController implements Handler.Callback {
		});
		}

		/** Starts all applicable profiles of the current user. */
		private void startProfiles() {
		int currentUserId = getCurrentUserId();
		if (DEBUG_MU) Slogf.i(TAG, "startProfilesLocked");
		@@ -1711,6 +1753,7 @@ class UserController implements Handler.Callback {
		t.traceBegin("getStartedUserState");
		final int oldUserId = getCurrentUserId();
		if (oldUserId == userId) {
		// The user we're requested to start is already the current user.
		final UserState state = getStartedUserState(userId);
		if (state == null) {
		Slogf.wtf(TAG, "Current user has no UserState");
		@@ -1793,10 +1836,12 @@ class UserController implements Handler.Callback {
		t.traceEnd(); // updateStartedUserArrayStarting
		return true;
		}
		final Integer userIdInt = userId;
		mUserLru.remove(userIdInt);
		mUserLru.add(userIdInt);
		}

		// No matter what, the fact that we're requested to start the user (even if it is
		// already running) puts it towards the end of the mUserLru list.
		addUserToUserLru(userId);

		if (unlockListener != null) {
		uss.mUnlockProgress.addListener(unlockListener);
		}
		@@ -1835,12 +1880,10 @@ class UserController implements Handler.Callback {
		}

		} else {
		final Integer currentUserIdInt = mCurrentUserId;
		updateProfileRelatedCaches();
		synchronized (mLock) {
		mUserLru.remove(currentUserIdInt);
		mUserLru.add(currentUserIdInt);
		}
		// We are starting a non-foreground user. They have already been added to the end
		// of mUserLru, so we need to ensure that the foreground user isn't displaced.
		addUserToUserLru(mCurrentUserId);
		}
		t.traceEnd();

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

+169 −15

Original line number	Diff line number	Diff line
		@@ -111,6 +111,8 @@ import com.android.server.pm.UserTypeFactory;
		import com.android.server.wm.ActivityTaskManagerInternal;
		import com.android.server.wm.WindowManagerService;

		import com.google.common.collect.Range;

		import org.junit.After;
		import org.junit.Before;
		import org.junit.Rule;
		@@ -155,6 +157,7 @@ public class UserControllerTest {
		private UserController mUserController;
		private TestInjector mInjector;
		private final HashMap<Integer, UserState> mUserStates = new HashMap<>();
		private final HashMap<Integer, UserInfo> mUserInfos = new HashMap<>();

		private final KeyEvictedCallback mKeyEvictedCallback = (userId) -> { /* ignore */ };

		@@ -587,25 +590,25 @@ public class UserControllerTest {

		setUpUser(TEST_USER_ID1, 0);
		setUpUser(TEST_USER_ID2, 0);
		int numerOfUserSwitches = 1;
		int numberOfUserSwitches = 1;
		addForegroundUserAndContinueUserSwitch(TEST_USER_ID, UserHandle.USER_SYSTEM,
		numerOfUserSwitches, false);
		numberOfUserSwitches, false);
		// running: user 0, USER_ID
		assertTrue(mUserController.canStartMoreUsers());
		assertEquals(Arrays.asList(new Integer[] {0, TEST_USER_ID}),
		mUserController.getRunningUsersLU());

		numerOfUserSwitches++;
		numberOfUserSwitches++;
		addForegroundUserAndContinueUserSwitch(TEST_USER_ID1, TEST_USER_ID,
		numerOfUserSwitches, false);
		numberOfUserSwitches, false);
		// running: user 0, USER_ID, USER_ID1
		assertFalse(mUserController.canStartMoreUsers());
		assertEquals(Arrays.asList(new Integer[] {0, TEST_USER_ID, TEST_USER_ID1}),
		mUserController.getRunningUsersLU());

		numerOfUserSwitches++;
		numberOfUserSwitches++;
		addForegroundUserAndContinueUserSwitch(TEST_USER_ID2, TEST_USER_ID1,
		numerOfUserSwitches, false);
		numberOfUserSwitches, false);
		UserState ussUser2 = mUserStates.get(TEST_USER_ID2);
		// skip middle step and call this directly.
		mUserController.finishUserSwitch(ussUser2);
		@@ -631,20 +634,20 @@ public class UserControllerTest {

		setUpUser(TEST_USER_ID1, 0);
		setUpUser(TEST_USER_ID2, 0);
		int numerOfUserSwitches = 1;
		int numberOfUserSwitches = 1;
		addForegroundUserAndContinueUserSwitch(TEST_USER_ID, UserHandle.USER_SYSTEM,
		numerOfUserSwitches, false);
		numberOfUserSwitches, false);
		// running: user 0, USER_ID
		assertTrue(mUserController.canStartMoreUsers());
		assertEquals(Arrays.asList(new Integer[] {0, TEST_USER_ID}),
		mUserController.getRunningUsersLU());
		numerOfUserSwitches++;
		numberOfUserSwitches++;

		addForegroundUserAndContinueUserSwitch(TEST_USER_ID1, TEST_USER_ID,
		numerOfUserSwitches, true);
		numberOfUserSwitches, true);
		// running: user 0, USER_ID1
		// stopped + unlocked: USER_ID
		numerOfUserSwitches++;
		numberOfUserSwitches++;
		assertTrue(mUserController.canStartMoreUsers());
		assertEquals(Arrays.asList(new Integer[] {0, TEST_USER_ID1}),
		mUserController.getRunningUsersLU());
		@@ -659,7 +662,7 @@ public class UserControllerTest {
		.lockCeStorage(anyInt());

		addForegroundUserAndContinueUserSwitch(TEST_USER_ID2, TEST_USER_ID1,
		numerOfUserSwitches, true);
		numberOfUserSwitches, true);
		// running: user 0, USER_ID2
		// stopped + unlocked: USER_ID1
		// stopped + locked: USER_ID
		@@ -674,6 +677,105 @@ public class UserControllerTest {
		.lockCeStorage(TEST_USER_ID);
		}

		/**
		* Test that, in getRunningUsersLU, parents come after their profile, even if the profile was
		* started afterwards.
		*/
		@Test
		public void testRunningUsersListOrder_parentAfterProfile() {
		mUserController.setInitialConfig(/* userSwitchUiEnabled= */ true,
		/* maxRunningUsers= / 7, / delayUserDataLocking= */ false);

		final int PARENT_ID = 200;
		final int PROFILE1_ID = 201;
		final int PROFILE2_ID = 202;
		final int FG_USER_ID = 300;
		final int BG_USER_ID = 400;

		setUpUser(PARENT_ID, 0).profileGroupId = PARENT_ID;
		setUpUser(PROFILE1_ID, UserInfo.FLAG_PROFILE).profileGroupId = PARENT_ID;
		setUpUser(PROFILE2_ID, UserInfo.FLAG_PROFILE).profileGroupId = PARENT_ID;
		setUpUser(FG_USER_ID, 0).profileGroupId = FG_USER_ID;
		setUpUser(BG_USER_ID, 0).profileGroupId = UserInfo.NO_PROFILE_GROUP_ID;
		mUserController.onSystemReady(); // To set the profileGroupIds in UserController.

		assertEquals(Arrays.asList(
		new Integer[] {SYSTEM_USER_ID}),
		mUserController.getRunningUsersLU());

		int numberOfUserSwitches = 1;
		addForegroundUserAndContinueUserSwitch(PARENT_ID, UserHandle.USER_SYSTEM,
		numberOfUserSwitches, false);
		assertEquals(Arrays.asList(
		new Integer[] {SYSTEM_USER_ID, PARENT_ID}),
		mUserController.getRunningUsersLU());

		assertThat(mUserController.startProfile(PROFILE1_ID, true, null)).isTrue();
		assertEquals(Arrays.asList(
		new Integer[] {SYSTEM_USER_ID, PROFILE1_ID, PARENT_ID}),
		mUserController.getRunningUsersLU());

		numberOfUserSwitches++;
		addForegroundUserAndContinueUserSwitch(FG_USER_ID, PARENT_ID,
		numberOfUserSwitches, false);
		assertEquals(Arrays.asList(
		new Integer[] {SYSTEM_USER_ID, PROFILE1_ID, PARENT_ID, FG_USER_ID}),
		mUserController.getRunningUsersLU());

		mUserController.startUser(BG_USER_ID, USER_START_MODE_BACKGROUND);
		assertEquals(Arrays.asList(
		new Integer[] {SYSTEM_USER_ID, PROFILE1_ID, PARENT_ID, BG_USER_ID, FG_USER_ID}),
		mUserController.getRunningUsersLU());

		assertThat(mUserController.startProfile(PROFILE2_ID, true, null)).isTrue();
		// Note for the future:
		// It is not absolutely essential that PROFILE1 come before PROFILE2,
		// nor that PROFILE1 come before BG_USER. We can change that policy later if we'd like.
		// The important thing is that PROFILE1 and PROFILE2 precede PARENT,
		// and that everything precedes OTHER.
		assertEquals(Arrays.asList(new Integer[] {
		SYSTEM_USER_ID, PROFILE1_ID, BG_USER_ID, PROFILE2_ID, PARENT_ID, FG_USER_ID}),
		mUserController.getRunningUsersLU());
		}

		/**
		* Test that, in getRunningUsersLU, the current user is always at the end, even if background
		* users were started subsequently.
		*/
		@Test
		public void testRunningUsersListOrder_currentAtEnd() {
		mUserController.setInitialConfig(/* userSwitchUiEnabled= */ true,
		/* maxRunningUsers= / 7, / delayUserDataLocking= */ false);

		final int CURRENT_ID = 200;
		final int PROFILE_ID = 201;
		final int BG_USER_ID = 400;

		setUpUser(CURRENT_ID, 0).profileGroupId = CURRENT_ID;
		setUpUser(PROFILE_ID, UserInfo.FLAG_PROFILE).profileGroupId = CURRENT_ID;
		setUpUser(BG_USER_ID, 0).profileGroupId = BG_USER_ID;
		mUserController.onSystemReady(); // To set the profileGroupIds in UserController.

		assertEquals(Arrays.asList(
		new Integer[] {SYSTEM_USER_ID}),
		mUserController.getRunningUsersLU());

		addForegroundUserAndContinueUserSwitch(CURRENT_ID, UserHandle.USER_SYSTEM, 1, false);
		assertEquals(Arrays.asList(
		new Integer[] {SYSTEM_USER_ID, CURRENT_ID}),
		mUserController.getRunningUsersLU());

		mUserController.startUser(BG_USER_ID, USER_START_MODE_BACKGROUND);
		assertEquals(Arrays.asList(
		new Integer[] {SYSTEM_USER_ID, BG_USER_ID, CURRENT_ID}),
		mUserController.getRunningUsersLU());

		assertThat(mUserController.startProfile(PROFILE_ID, true, null)).isTrue();
		assertEquals(Arrays.asList(
		new Integer[] {SYSTEM_USER_ID, BG_USER_ID, PROFILE_ID, CURRENT_ID}),
		mUserController.getRunningUsersLU());
		}

		/**
		* Test locking user with mDelayUserDataLocking false.
		*/
		@@ -785,6 +887,52 @@ public class UserControllerTest {
		verifyUserUnassignedFromDisplayNeverCalled(TEST_USER_ID);
		}

		/** Test that stopping a profile doesn't also stop its parent, even if it's in background. */
		@Test
		public void testStopProfile_doesNotStopItsParent() throws Exception {
		mUserController.setInitialConfig(/* userSwitchUiEnabled= */ true,
		/* maxRunningUsers= / 5, / delayUserDataLocking= */ false);

		final Range<Integer> RUNNING_RANGE =
		Range.closed(UserState.STATE_BOOTING, UserState.STATE_RUNNING_UNLOCKED);

		final int PARENT_ID = TEST_USER_ID1;
		final int PROFILE_ID = TEST_USER_ID2;
		final int OTHER_ID = TEST_USER_ID3;

		setUpUser(PARENT_ID, 0).profileGroupId = PARENT_ID;
		setUpUser(PROFILE_ID, UserInfo.FLAG_PROFILE).profileGroupId = PARENT_ID;
		setUpUser(OTHER_ID, 0).profileGroupId = OTHER_ID;
		mUserController.onSystemReady(); // To set the profileGroupIds in UserController.

		// Start the parent in the background
		boolean started = mUserController.startUser(PARENT_ID, USER_START_MODE_BACKGROUND);
		assertWithMessage("startUser(%s)", PARENT_ID).that(started).isTrue();
		assertThat(mUserController.getStartedUserState(PARENT_ID).state).isIn(RUNNING_RANGE);

		// Start the profile
		started = mUserController.startProfile(PROFILE_ID, true, null);
		assertWithMessage("startProfile(%s)", PROFILE_ID).that(started).isTrue();
		assertThat(mUserController.getStartedUserState(PARENT_ID).state).isIn(RUNNING_RANGE);
		assertThat(mUserController.getStartedUserState(PROFILE_ID).state).isIn(RUNNING_RANGE);

		// Start an unrelated user
		started = mUserController.startUser(OTHER_ID, USER_START_MODE_FOREGROUND);
		assertWithMessage("startUser(%s)", OTHER_ID).that(started).isTrue();
		assertThat(mUserController.getStartedUserState(PARENT_ID).state).isIn(RUNNING_RANGE);
		assertThat(mUserController.getStartedUserState(PROFILE_ID).state).isIn(RUNNING_RANGE);
		assertThat(mUserController.getStartedUserState(OTHER_ID).state).isIn(RUNNING_RANGE);

		// Stop the profile and assert that its (background) parent didn't stop too
		boolean stopped = mUserController.stopProfile(PROFILE_ID);
		assertWithMessage("stopProfile(%s)", PROFILE_ID).that(stopped).isTrue();
		if (mUserController.getStartedUserState(PROFILE_ID) != null) {
		assertThat(mUserController.getStartedUserState(PROFILE_ID).state)
		.isNotIn(RUNNING_RANGE);
		}
		assertThat(mUserController.getStartedUserState(PARENT_ID).state).isIn(RUNNING_RANGE);
		}

		@Test
		public void testStartProfile_disabledProfileFails() {
		setUpUser(TEST_USER_ID1, UserInfo.FLAG_PROFILE \| UserInfo.FLAG_DISABLED, /* preCreated= */
		@@ -1152,11 +1300,11 @@ public class UserControllerTest {
		continueUserSwitchAssertions(oldUserId, newUserId, expectOldUserStopping);
		}

		private void setUpUser(@UserIdInt int userId, @UserInfoFlag int flags) {
		setUpUser(userId, flags, /* preCreated= / false, / userType */ null);
		private UserInfo setUpUser(@UserIdInt int userId, @UserInfoFlag int flags) {
		return setUpUser(userId, flags, /* preCreated= / false, / userType */ null);
		}

		private void setUpUser(@UserIdInt int userId, @UserInfoFlag int flags, boolean preCreated,
		private UserInfo setUpUser(@UserIdInt int userId, @UserInfoFlag int flags, boolean preCreated,
		@Nullable String userType) {
		if (userType == null) {
		userType = UserInfo.getDefaultUserType(flags);
		@@ -1171,6 +1319,12 @@ public class UserControllerTest {
		assertThat(userTypeDetails).isNotNull();
		when(mInjector.mUserManagerInternalMock.getUserProperties(eq(userId)))
		.thenReturn(userTypeDetails.getDefaultUserPropertiesReference());

		mUserInfos.put(userId, userInfo);
		when(mInjector.mUserManagerMock.getUsers(anyBoolean()))
		.thenReturn(mUserInfos.values().stream().toList());

		return userInfo;
		}

		private static List<String> getActions(List<Intent> intents) {