Limit background user starting to max running users
- startUserInBackground can allow more uses that is configured.
So restrict it inside car service. Eviction is based on LRU.
- Fix mishandling of background user in stopped state: It is auto-unlocked
and unlockUser fails. This leads into not stopping the user.
Add more logic to check such state so that such case is handled.
- Clean up confusing user lock / unlock check in CarUserService
It could go wrong if any user is unlocked before user 0.
(Not true in car now)
Bug: 130158283
Test: run unit tests, check started background users when garage mode starts
Change-Id: I2a2baa1ed1cf845d6dd10c19a4cf644939c72a75
diff --git a/service/src/com/android/car/ICarImpl.java b/service/src/com/android/car/ICarImpl.java
index f67ead0..3d183db 100644
--- a/service/src/com/android/car/ICarImpl.java
+++ b/service/src/com/android/car/ICarImpl.java
@@ -17,6 +17,7 @@
package com.android.car;
import android.annotation.MainThread;
+import android.app.ActivityManager;
import android.app.UiModeManager;
import android.car.Car;
import android.car.ICar;
@@ -24,6 +25,7 @@
import android.car.userlib.CarUserManagerHelper;
import android.content.Context;
import android.content.pm.PackageManager;
+import android.content.res.Resources;
import android.hardware.automotive.vehicle.V2_0.IVehicle;
import android.hardware.automotive.vehicle.V2_0.VehicleArea;
import android.os.Binder;
@@ -118,7 +120,11 @@
mHal = new VehicleHal(vehicle);
mVehicleInterfaceName = vehicleInterfaceName;
mUserManagerHelper = new CarUserManagerHelper(serviceContext);
- mCarUserService = new CarUserService(serviceContext, mUserManagerHelper);
+ final Resources res = mContext.getResources();
+ final int maxRunningUsers = res.getInteger(
+ com.android.internal.R.integer.config_multiuserMaxRunningUsers);
+ mCarUserService = new CarUserService(serviceContext, mUserManagerHelper,
+ ActivityManager.getService(), maxRunningUsers);
mSystemActivityMonitoringService = new SystemActivityMonitoringService(serviceContext);
mCarPowerManagementService = new CarPowerManagementService(mContext, mHal.getPowerHal(),
systemInterface, mUserManagerHelper);
diff --git a/service/src/com/android/car/user/CarUserService.java b/service/src/com/android/car/user/CarUserService.java
index a5d64dc..3ae28d2 100644
--- a/service/src/com/android/car/user/CarUserService.java
+++ b/service/src/com/android/car/user/CarUserService.java
@@ -34,6 +34,7 @@
import com.android.car.CarServiceBase;
import com.android.internal.annotations.GuardedBy;
+import com.android.internal.annotations.VisibleForTesting;
import java.io.PrintWriter;
import java.util.ArrayList;
@@ -58,8 +59,21 @@
private boolean mUser0Unlocked;
@GuardedBy("mLock")
private final ArrayList<Runnable> mUser0UnlockTasks = new ArrayList<>();
+ /**
+ * Background users that will be restarted in garage mode. This list can include the
+ * current foreground user bit the current foreground user should not be restarted.
+ */
@GuardedBy("mLock")
- private final ArrayList<Integer> mLastUnlockedUsers = new ArrayList<>();
+ private final ArrayList<Integer> mBackgroundUsersToRestart = new ArrayList<>();
+ /**
+ * Keep the list of background users started here. This is wholly for debugging purpose.
+ */
+ @GuardedBy("mLock")
+ private final ArrayList<Integer> mBackgroundUsersRestartedHere = new ArrayList<>();
+
+ private final int mMaxRunningUsers;
+
+ private final UserManager mUserManager;
private final CopyOnWriteArrayList<UserCallback> mUserCallbacks = new CopyOnWriteArrayList<>();
@@ -73,13 +87,16 @@
}
public CarUserService(
- @Nullable Context context, @Nullable CarUserManagerHelper carUserManagerHelper) {
+ @Nullable Context context, @Nullable CarUserManagerHelper carUserManagerHelper,
+ IActivityManager am, int maxRunningUsers) {
if (Log.isLoggable(TAG, Log.DEBUG)) {
Log.d(TAG, "constructed");
}
mContext = context;
mCarUserManagerHelper = carUserManagerHelper;
- mAm = ActivityManager.getService();
+ mAm = am;
+ mMaxRunningUsers = maxRunningUsers;
+ mUserManager = (UserManager) context.getSystemService(Context.USER_SERVICE);
}
@Override
@@ -104,16 +121,19 @@
@Override
public void dump(PrintWriter writer) {
writer.println(TAG);
- writer.println("Context: " + mContext);
boolean user0Unlocked;
- ArrayList<Integer> lastUnlockedUsers;
+ ArrayList<Integer> backgroundUsersToRestart;
+ ArrayList<Integer> backgroundUsersRestarted;
synchronized (mLock) {
user0Unlocked = mUser0Unlocked;
- lastUnlockedUsers = new ArrayList<>(mLastUnlockedUsers);
+ backgroundUsersToRestart = new ArrayList<>(mBackgroundUsersToRestart);
+ backgroundUsersRestarted = new ArrayList<>(mBackgroundUsersRestartedHere);
}
writer.println("User0Unlocked: " + user0Unlocked);
- writer.println("LastUnlockedUsers:" + lastUnlockedUsers);
+ writer.println("maxRunningUsers:" + mMaxRunningUsers);
+ writer.println("BackgroundUsersToRestart:" + backgroundUsersToRestart);
+ writer.println("BackgroundUsersRestarted:" + backgroundUsersRestarted);
}
private void updateDefaultUserRestriction() {
@@ -158,21 +178,36 @@
for (UserCallback callback : mUserCallbacks) {
callback.onUserLockChanged(userHandle, unlocked);
}
+ if (!unlocked) { // nothing else to do when it is locked back.
+ return;
+ }
ArrayList<Runnable> tasks = null;
synchronized (mLock) {
- if (userHandle != UserHandle.USER_SYSTEM && unlocked
- && mCarUserManagerHelper.isPersistentUser(userHandle)) {
+ if (userHandle == UserHandle.USER_SYSTEM) {
+ if (!mUser0Unlocked) { // user 0, unlocked, do this only once
+ updateDefaultUserRestriction();
+ tasks = new ArrayList<>(mUser0UnlockTasks);
+ mUser0UnlockTasks.clear();
+ mUser0Unlocked = unlocked;
+ }
+ } else { // none user0
Integer user = userHandle;
- mLastUnlockedUsers.remove(user);
- mLastUnlockedUsers.add(0, user);
- return;
- }
- // Assumes that car service need to do it only once during boot-up
- if (unlocked && !mUser0Unlocked) {
- updateDefaultUserRestriction();
- tasks = new ArrayList<>(mUser0UnlockTasks);
- mUser0UnlockTasks.clear();
- mUser0Unlocked = unlocked;
+ if (mCarUserManagerHelper.isPersistentUser(userHandle)) {
+ // current foreground user should stay in top priority.
+ if (userHandle == mCarUserManagerHelper.getCurrentForegroundUserId()) {
+ mBackgroundUsersToRestart.remove(user);
+ mBackgroundUsersToRestart.add(0, user);
+ }
+ // -1 for user 0
+ if (mBackgroundUsersToRestart.size() > (mMaxRunningUsers - 1)) {
+ final int userToDrop = mBackgroundUsersToRestart.get(
+ mBackgroundUsersToRestart.size() - 1);
+ Log.i(TAG, "New user unlocked:" + userHandle
+ + ", dropping least recently user from restart list:" + userToDrop);
+ // Drop the least recently used user.
+ mBackgroundUsersToRestart.remove(mBackgroundUsersToRestart.size() - 1);
+ }
+ }
}
}
if (tasks != null && tasks.size() > 0) {
@@ -190,7 +225,9 @@
public ArrayList<Integer> startAllBackgroundUsers() {
ArrayList<Integer> users;
synchronized (mLock) {
- users = new ArrayList<>(mLastUnlockedUsers);
+ users = new ArrayList<>(mBackgroundUsersToRestart);
+ mBackgroundUsersRestartedHere.clear();
+ mBackgroundUsersRestartedHere.addAll(mBackgroundUsersToRestart);
}
ArrayList<Integer> startedUsers = new ArrayList<>();
for (Integer user : users) {
@@ -199,14 +236,33 @@
}
try {
if (mAm.startUserInBackground(user)) {
- if (mAm.unlockUser(user, null, null, null)) {
+ if (mUserManager.isUserUnlockingOrUnlocked(user)) {
+ // already unlocked / unlocking. No need to unlock.
startedUsers.add(user);
+ } else if (mAm.unlockUser(user, null, null, null)) {
+ startedUsers.add(user);
+ } else { // started but cannot unlock
+ Log.w(TAG, "Background user started but cannot be unlocked:" + user);
+ if (mUserManager.isUserRunning(user)) {
+ // add to started list so that it can be stopped later.
+ startedUsers.add(user);
+ }
}
}
} catch (RemoteException e) {
// ignore
}
}
+ // Keep only users that were re-started in mBackgroundUsersRestartedHere
+ synchronized (mLock) {
+ ArrayList<Integer> usersToRemove = new ArrayList<>();
+ for (Integer user : mBackgroundUsersToRestart) {
+ if (!startedUsers.contains(user)) {
+ usersToRemove.add(user);
+ }
+ }
+ mBackgroundUsersRestartedHere.removeAll(usersToRemove);
+ }
return startedUsers;
}
@@ -215,13 +271,23 @@
* @return true if stopping succeeds.
*/
public boolean stopBackgroundUser(int userId) {
+ if (userId == UserHandle.USER_SYSTEM) {
+ return false;
+ }
if (userId == mCarUserManagerHelper.getCurrentForegroundUserId()) {
Log.i(TAG, "stopBackgroundUser, already a fg user:" + userId);
return false;
}
try {
int r = mAm.stopUser(userId, true, null);
- if (r != ActivityManager.USER_OP_SUCCESS) {
+ if (r == ActivityManager.USER_OP_SUCCESS) {
+ synchronized (mLock) {
+ Integer user = userId;
+ mBackgroundUsersRestartedHere.remove(user);
+ }
+ } else if (r == ActivityManager.USER_OP_IS_CURRENT) {
+ return false;
+ } else {
Log.i(TAG, "stopBackgroundUser failed, user:" + userId + " err:" + r);
return false;
}
@@ -261,6 +327,15 @@
}
}
+ @VisibleForTesting
+ protected ArrayList<Integer> getBackgroundUsersToRestart() {
+ ArrayList<Integer> backgroundUsersToRestart;
+ synchronized (mLock) {
+ backgroundUsersToRestart = new ArrayList<>(mBackgroundUsersToRestart);
+ }
+ return backgroundUsersToRestart;
+ }
+
private void setSystemUserRestrictions() {
// Disable adding accounts for system user.
mCarUserManagerHelper.setUserRestriction(mCarUserManagerHelper.getSystemUserInfo(),