From ebe2a0baeb2c1be6da55ac0cbd9528ea9a0e9288 Mon Sep 17 00:00:00 2001 From: Lee Shombert Date: Thu, 23 Jan 2020 10:02:15 -0800 Subject: [PATCH] Binder cache for isUserUnlocked(). Bug: 140788621 Test: A special build that puts the PropertyInvalidatedCache in verification mode was loaded on the device. Then one iteration of MPTS was executed. No cache inconsistencies were found and no SELinux violations (associated with the binder cache) were found. The number of cache misses was approximately 10% of the total binder calls. All binder calls went through the cache. Added a new user and later deleted the same user; verified that caches were invalidated as expected and no errors were detected by the PropertyInvalidatedCache verification mode. Passed the CtsMultiUserHostTestCases test. Change-Id: I32e3e39768e288101c82b02e63a7eb20d5ed7f93 --- core/java/android/os/UserManager.java | 33 +++++++++++--- .../android/server/StorageManagerService.java | 44 ++++++++++++++++--- .../android/server/pm/UserManagerService.java | 41 ++++++++++++++++- 3 files changed, 107 insertions(+), 11 deletions(-) diff --git a/core/java/android/os/UserManager.java b/core/java/android/os/UserManager.java index b202053b5d65b..6d1f646f943b8 100644 --- a/core/java/android/os/UserManager.java +++ b/core/java/android/os/UserManager.java @@ -34,6 +34,7 @@ import android.annotation.WorkerThread; import android.app.Activity; import android.app.ActivityManager; import android.app.admin.DevicePolicyManager; +import android.app.PropertyInvalidatedCache; import android.compat.annotation.UnsupportedAppUsage; import android.content.ComponentName; import android.content.Context; @@ -2150,16 +2151,38 @@ public class UserManager { return isUserUnlocked(user.getIdentifier()); } + private static final String CACHE_KEY_IS_USER_UNLOCKED_PROPERTY = + "cache_key.is_user_unlocked"; + + private final PropertyInvalidatedCache mIsUserUnlockedCache = + new PropertyInvalidatedCache( + 32, CACHE_KEY_IS_USER_UNLOCKED_PROPERTY) { + @Override + protected Boolean recompute(Integer query) { + try { + return mService.isUserUnlocked(query); + } catch (RemoteException re) { + throw re.rethrowFromSystemServer(); + } + } + }; + /** {@hide} */ @UnsupportedAppUsage @RequiresPermission(anyOf = {Manifest.permission.MANAGE_USERS, Manifest.permission.INTERACT_ACROSS_USERS}, conditional = true) public boolean isUserUnlocked(@UserIdInt int userId) { - try { - return mService.isUserUnlocked(userId); - } catch (RemoteException re) { - throw re.rethrowFromSystemServer(); - } + return mIsUserUnlockedCache.query(userId); + } + + /** {@hide} */ + public void disableIsUserUnlockedCache() { + mIsUserUnlockedCache.disableLocal(); + } + + /** {@hide} */ + public static final void invalidateIsUserUnlockedCache() { + PropertyInvalidatedCache.invalidateCache(CACHE_KEY_IS_USER_UNLOCKED_PROPERTY); } /** diff --git a/services/core/java/com/android/server/StorageManagerService.java b/services/core/java/com/android/server/StorageManagerService.java index a08bdb23dd22b..7bb9adf315144 100644 --- a/services/core/java/com/android/server/StorageManagerService.java +++ b/services/core/java/com/android/server/StorageManagerService.java @@ -342,9 +342,43 @@ class StorageManagerService extends IStorageManager.Stub */ private final Object mPackagesLock = new Object(); + /** + * mLocalUnlockedUsers affects the return value of isUserUnlocked. If + * any value in the array changes, then the binder cache for + * isUserUnlocked must be invalidated. When adding mutating methods to + * WatchedLockedUsers, be sure to invalidate the cache in the new + * methods. + */ + private class WatchedLockedUsers { + private int[] users = EmptyArray.INT; + public WatchedLockedUsers() { + } + public void append(int userId) { + users = ArrayUtils.appendInt(users, userId); + invalidateIsUserUnlockedCache(); + } + public void remove(int userId) { + users = ArrayUtils.removeInt(users, userId); + invalidateIsUserUnlockedCache(); + } + public boolean contains(int userId) { + return ArrayUtils.contains(users, userId); + } + public int[] all() { + return users; + } + @Override + public String toString() { + return Arrays.toString(users); + } + private void invalidateIsUserUnlockedCache() { + UserManager.invalidateIsUserUnlockedCache(); + } + } + /** Set of users that we know are unlocked. */ @GuardedBy("mLock") - private int[] mLocalUnlockedUsers = EmptyArray.INT; + private WatchedLockedUsers mLocalUnlockedUsers = new WatchedLockedUsers(); /** Set of users that system knows are unlocked. */ @GuardedBy("mLock") private int[] mSystemUnlockedUsers = EmptyArray.INT; @@ -3042,7 +3076,7 @@ class StorageManagerService extends IStorageManager.Stub } synchronized (mLock) { - mLocalUnlockedUsers = ArrayUtils.appendInt(mLocalUnlockedUsers, userId); + mLocalUnlockedUsers.append(userId); } } @@ -3058,14 +3092,14 @@ class StorageManagerService extends IStorageManager.Stub } synchronized (mLock) { - mLocalUnlockedUsers = ArrayUtils.removeInt(mLocalUnlockedUsers, userId); + mLocalUnlockedUsers.remove(userId); } } @Override public boolean isUserKeyUnlocked(int userId) { synchronized (mLock) { - return ArrayUtils.contains(mLocalUnlockedUsers, userId); + return mLocalUnlockedUsers.contains(userId); } } @@ -4177,7 +4211,7 @@ class StorageManagerService extends IStorageManager.Stub } pw.println(); - pw.println("Local unlocked users: " + Arrays.toString(mLocalUnlockedUsers)); + pw.println("Local unlocked users: " + mLocalUnlockedUsers); pw.println("System unlocked users: " + Arrays.toString(mSystemUnlockedUsers)); final ContentResolver cr = mContext.getContentResolver(); diff --git a/services/core/java/com/android/server/pm/UserManagerService.java b/services/core/java/com/android/server/pm/UserManagerService.java index 1c02161c5b96c..6e4ec7e4b7d0c 100644 --- a/services/core/java/com/android/server/pm/UserManagerService.java +++ b/services/core/java/com/android/server/pm/UserManagerService.java @@ -462,8 +462,42 @@ public class UserManagerService extends IUserManager.Stub { @GuardedBy("mUsersLock") private boolean mForceEphemeralUsers; + /** + * The member mUserStates affects the return value of isUserUnlocked. + * If any value in mUserStates changes, then the binder cache for + * isUserUnlocked must be invalidated. When adding mutating methods to + * WatchedUserStates, be sure to invalidate the cache in the new + * methods. + */ + private class WatchedUserStates { + final SparseIntArray states; + public WatchedUserStates() { + states = new SparseIntArray(); + } + public int get(int userId) { + return states.get(userId); + } + public int get(int userId, int fallback) { + return states.indexOfKey(userId) >= 0 ? states.get(userId) : fallback; + } + public void put(int userId, int state) { + states.put(userId, state); + invalidateIsUserUnlockedCache(); + } + public void delete(int userId) { + states.delete(userId); + invalidateIsUserUnlockedCache(); + } + @Override + public String toString() { + return states.toString(); + } + private void invalidateIsUserUnlockedCache() { + UserManager.invalidateIsUserUnlockedCache(); + } + } @GuardedBy("mUserStates") - private final SparseIntArray mUserStates = new SparseIntArray(); + private final WatchedUserStates mUserStates = new WatchedUserStates(); private static UserManagerService sInstance; @@ -4801,6 +4835,11 @@ public class UserManagerService extends IUserManager.Stub { || (state == UserState.STATE_RUNNING_UNLOCKED); } + /** + * The return values of this method are cached in clients. If the + * logic in this function changes then the cache invalidation code + * may need to be revisited. + */ @Override public boolean isUserUnlocked(@UserIdInt int userId) { int state;