From 450ce9fc8a1522819aec151433e6f509dfe60690 Mon Sep 17 00:00:00 2001 From: Adrian Roos Date: Wed, 29 Oct 2014 14:30:37 +0100 Subject: [PATCH] Disable LockPatternUtilsCache Also fix a bug where hasPattern and hasPassword were not invalidated properly. Bug: 18163444 Change-Id: I5bd8cc4e7c0d00497ee7a42f3c34449aa3f95a6c --- .../widget/ILockSettingsObserver.aidl | 9 ++++++ .../internal/widget/LockPatternUtils.java | 16 ++++++++-- .../widget/LockPatternUtilsCache.java | 29 +++++++++++++++---- .../android/server/LockSettingsService.java | 3 ++ 4 files changed, 50 insertions(+), 7 deletions(-) diff --git a/core/java/com/android/internal/widget/ILockSettingsObserver.aidl b/core/java/com/android/internal/widget/ILockSettingsObserver.aidl index 6c354d8917a99..edf8f0e27d52f 100644 --- a/core/java/com/android/internal/widget/ILockSettingsObserver.aidl +++ b/core/java/com/android/internal/widget/ILockSettingsObserver.aidl @@ -18,5 +18,14 @@ package com.android.internal.widget; /** {@hide} */ oneway interface ILockSettingsObserver { + /** + * Called when a lock setting has changed. + * + * Note: Impementations of this should do as little work as possible, because this may be + * called synchronously while writing a setting. + * + * @param key the key of the setting that has changed or {@code null} if any may have changed. + * @param userId the user whose setting has changed. + */ void onLockSettingChanged(in String key, in int userId); } diff --git a/core/java/com/android/internal/widget/LockPatternUtils.java b/core/java/com/android/internal/widget/LockPatternUtils.java index d6885da2b1691..a4b8380d730a3 100644 --- a/core/java/com/android/internal/widget/LockPatternUtils.java +++ b/core/java/com/android/internal/widget/LockPatternUtils.java @@ -64,6 +64,13 @@ public class LockPatternUtils { private static final String TAG = "LockPatternUtils"; private static final boolean DEBUG = false; + /** + * If true, LockPatternUtils will cache its values in-process. While this leads to faster reads, + * it can cause problems because writes to to the settings are no longer synchronous + * across all processes. + */ + private static final boolean ENABLE_CLIENT_CACHE = false; + /** * The maximum number of incorrect attempts before the user is prevented * from trying again for {@link #FAILED_ATTEMPT_TIMEOUT_MS}. @@ -207,8 +214,13 @@ public class LockPatternUtils { private ILockSettings getLockSettings() { if (mLockSettingsService == null) { - mLockSettingsService = LockPatternUtilsCache.getInstance( - ILockSettings.Stub.asInterface(ServiceManager.getService("lock_settings"))); + ILockSettings service = ILockSettings.Stub.asInterface( + ServiceManager.getService("lock_settings")); + if (ENABLE_CLIENT_CACHE) { + mLockSettingsService = LockPatternUtilsCache.getInstance(service); + } else { + mLockSettingsService = service; + } } return mLockSettingsService; } diff --git a/core/java/com/android/internal/widget/LockPatternUtilsCache.java b/core/java/com/android/internal/widget/LockPatternUtilsCache.java index 624f67c0f729b..a9524fffff878 100644 --- a/core/java/com/android/internal/widget/LockPatternUtilsCache.java +++ b/core/java/com/android/internal/widget/LockPatternUtilsCache.java @@ -18,7 +18,9 @@ package com.android.internal.widget; import android.os.IBinder; import android.os.RemoteException; +import android.os.UserHandle; import android.util.ArrayMap; +import android.util.Log; /** * A decorator for {@link ILockSettings} that caches the key-value responses in memory. @@ -28,9 +30,11 @@ import android.util.ArrayMap; */ public class LockPatternUtilsCache implements ILockSettings { - private static final String HAS_LOCK_PATTERN_CACHE_KEY + private static final String TAG = "LockPatternUtilsCache"; + + public static final String HAS_LOCK_PATTERN_CACHE_KEY = "LockPatternUtils.Cache.HasLockPatternCacheKey"; - private static final String HAS_LOCK_PASSWORD_CACHE_KEY + public static final String HAS_LOCK_PASSWORD_CACHE_KEY = "LockPatternUtils.Cache.HasLockPasswordCacheKey"; private static LockPatternUtilsCache sInstance; @@ -53,7 +57,7 @@ public class LockPatternUtilsCache implements ILockSettings { // ILockSettings - private LockPatternUtilsCache(ILockSettings service) { + public LockPatternUtilsCache(ILockSettings service) { mService = service; try { service.registerObserver(mObserver); @@ -186,6 +190,7 @@ public class LockPatternUtilsCache implements ILockSettings { // Caching private Object peekCache(String key, int userId) { + if (!validateUserId(userId)) return null; synchronized (mCache) { // Safe to reuse mCacheKey, because it is not stored in the map. return mCache.get(mCacheKey.set(key, userId)); @@ -193,6 +198,7 @@ public class LockPatternUtilsCache implements ILockSettings { } private void putCache(String key, int userId, Object value) { + if (!validateUserId(userId)) return; synchronized (mCache) { // Create a new key, because this will be stored in the map. mCache.put(new CacheKey().set(key, userId), value); @@ -200,9 +206,14 @@ public class LockPatternUtilsCache implements ILockSettings { } private void invalidateCache(String key, int userId) { + if (!validateUserId(userId)) return; synchronized (mCache) { - // Safe to reuse mCacheKey, because it is not stored in the map. - mCache.remove(mCacheKey.set(key, userId)); + if (key != null) { + // Safe to reuse mCacheKey, because it is not stored in the map. + mCache.remove(mCacheKey.set(key, userId)); + } else { + mCache.clear(); + } } } @@ -213,6 +224,14 @@ public class LockPatternUtilsCache implements ILockSettings { } }; + private final boolean validateUserId(int userId) { + if (userId < UserHandle.USER_OWNER) { + Log.e(TAG, "User " + userId + " not supported: Must be a concrete user."); + return false; + } + return true; + } + private static final class CacheKey { String key; int userId; diff --git a/services/core/java/com/android/server/LockSettingsService.java b/services/core/java/com/android/server/LockSettingsService.java index 11ba8e8114b3a..3c7d85d5980e4 100644 --- a/services/core/java/com/android/server/LockSettingsService.java +++ b/services/core/java/com/android/server/LockSettingsService.java @@ -49,6 +49,7 @@ import android.util.Slog; import com.android.internal.widget.ILockSettings; import com.android.internal.widget.ILockSettingsObserver; import com.android.internal.widget.LockPatternUtils; +import com.android.internal.widget.LockPatternUtilsCache; import java.util.ArrayList; import java.util.Arrays; @@ -348,6 +349,7 @@ public class LockSettingsService extends ILockSettings.Stub { final byte[] hash = LockPatternUtils.patternToHash( LockPatternUtils.stringToPattern(pattern)); mStorage.writePatternHash(hash, userId); + notifyObservers(LockPatternUtilsCache.HAS_LOCK_PATTERN_CACHE_KEY, userId); } @Override @@ -357,6 +359,7 @@ public class LockSettingsService extends ILockSettings.Stub { maybeUpdateKeystore(password, userId); mStorage.writePasswordHash(mLockPatternUtils.passwordToHash(password, userId), userId); + notifyObservers(LockPatternUtilsCache.HAS_LOCK_PASSWORD_CACHE_KEY, userId); } @Override