From 321f0bd44f510c1b13d5f533d61ac0b8c194a51a Mon Sep 17 00:00:00 2001 From: Makoto Onuki Date: Tue, 25 May 2021 17:03:24 -0700 Subject: [PATCH] `cmd deviceidle tempwhitelist -r` should clear FGS tempallowlist too Also clean up FgsTempAllowList, since the key is always UID. Bug: 188789296 Test: atest FgsTempAllowListTest Test: Manual test with `cmd deviceidle tempwhitelist` with and without `-r`, while monitoring the AMS dumpsys with the following command. $ watch -n 1 "adb shell dumpsys activity processes | sed -n '/mFgsStartTempAllowList/,\$p'" Change-Id: I0e113b1614653c85e57a37579921c3baa7dedc78 --- .../android/server/DeviceIdleController.java | 2 +- .../server/am/ActivityManagerService.java | 26 ++++--- .../android/server/am/FgsTempAllowList.java | 69 ++++++++++++------- .../com/android/server/am/ProcessList.java | 2 +- .../server/am/FgsTempAllowListTest.java | 56 ++++++++++++++- 5 files changed, 116 insertions(+), 39 deletions(-) diff --git a/apex/jobscheduler/service/java/com/android/server/DeviceIdleController.java b/apex/jobscheduler/service/java/com/android/server/DeviceIdleController.java index 60f5769a46f71..cef065ddac9e5 100644 --- a/apex/jobscheduler/service/java/com/android/server/DeviceIdleController.java +++ b/apex/jobscheduler/service/java/com/android/server/DeviceIdleController.java @@ -547,7 +547,7 @@ public class DeviceIdleController extends SystemService private int[] mPowerSaveWhitelistUserAppIdArray = new int[0]; /** - * List of end times for UIDs that are temporarily marked as being allowed to access + * List of end times for app-IDs that are temporarily marked as being allowed to access * the network and acquire wakelocks. Times are in milliseconds. */ private final SparseArray> mTempWhitelistAppIdEndTimes diff --git a/services/core/java/com/android/server/am/ActivityManagerService.java b/services/core/java/com/android/server/am/ActivityManagerService.java index b44fe9fdaad14..3e6a0a8ec80dc 100644 --- a/services/core/java/com/android/server/am/ActivityManagerService.java +++ b/services/core/java/com/android/server/am/ActivityManagerService.java @@ -1236,7 +1236,7 @@ public class ActivityManagerService extends IActivityManager.Stub * The temp-allowlist that is allowed to start FGS from background. */ @CompositeRWLock({"this", "mProcLock"}) - final FgsTempAllowList mFgsStartTempAllowList = + final FgsTempAllowList mFgsStartTempAllowList = new FgsTempAllowList(); static final FgsTempAllowListItem FAKE_TEMP_ALLOW_LIST_ITEM = new FgsTempAllowListItem( @@ -1246,7 +1246,7 @@ public class ActivityManagerService extends IActivityManager.Stub * List of uids that are allowed to have while-in-use permission when FGS is started from * background. */ - private final FgsTempAllowList mFgsWhileInUseTempAllowList = + private final FgsTempAllowList mFgsWhileInUseTempAllowList = new FgsTempAllowList(); /** @@ -9372,22 +9372,17 @@ public class ActivityManagerService extends IActivityManager.Stub pw.println(" mFgsStartTempAllowList:"); final long currentTimeNow = System.currentTimeMillis(); final long elapsedRealtimeNow = SystemClock.elapsedRealtime(); - final Set uids = new ArraySet<>(mFgsStartTempAllowList.keySet()); - for (Integer uid : uids) { - final Pair entry = mFgsStartTempAllowList.get(uid); - if (entry == null) { - continue; - } + mFgsStartTempAllowList.forEach((uid, entry) -> { pw.print(" " + UserHandle.formatUid(uid) + ": "); - entry.second.dump(pw); pw.println(); - pw.print("ms expiration="); + entry.second.dump(pw); + pw.print(" expiration="); // Convert entry.mExpirationTime, which is an elapsed time since boot, // to a time since epoch (i.e. System.currentTimeMillis()-based time.) final long expirationInCurrentTime = currentTimeNow - elapsedRealtimeNow + entry.first; TimeUtils.dumpTimeWithDelta(pw, expirationInCurrentTime, currentTimeNow); pw.println(); - } + }); } if (mDebugApp != null || mOrigDebugApp != null || mDebugTransient || mOrigWaitForDebugger) { @@ -15345,10 +15340,19 @@ public class ActivityManagerService extends IActivityManager.Stub mDeviceIdleTempAllowlist = appids; if (adding) { if (type == TEMPORARY_ALLOW_LIST_TYPE_FOREGROUND_SERVICE_ALLOWED) { + // Note, the device idle temp-allowlist are by app-ids, but here + // mFgsStartTempAllowList contains UIDs. mFgsStartTempAllowList.add(changingUid, durationMs, new FgsTempAllowListItem(durationMs, reasonCode, reason, callingUid)); } + } else { + // Note in the removing case, we need to remove all the UIDs matching + // the appId, because DeviceIdle's temp-allowlist are based on AppIds, + // not UIDs. + // For eacmple, "cmd deviceidle tempallowlist -r PACKAGE" will + // not only remove this app for user 0, but for all users. + mFgsStartTempAllowList.removeAppId(UserHandle.getAppId(changingUid)); } setAppIdTempAllowlistStateLSP(changingUid, adding); } diff --git a/services/core/java/com/android/server/am/FgsTempAllowList.java b/services/core/java/com/android/server/am/FgsTempAllowList.java index 847e82f078998..c28655655765a 100644 --- a/services/core/java/com/android/server/am/FgsTempAllowList.java +++ b/services/core/java/com/android/server/am/FgsTempAllowList.java @@ -20,11 +20,12 @@ import static com.android.server.am.ActivityManagerDebugConfig.TAG_AM; import android.annotation.Nullable; import android.os.SystemClock; -import android.util.ArrayMap; +import android.os.UserHandle; import android.util.Pair; import android.util.Slog; +import android.util.SparseArray; -import java.util.Set; +import java.util.function.BiConsumer; /** * List of keys that have expiration time. @@ -33,19 +34,18 @@ import java.util.Set; * *

This is used for both FGS-BG-start restriction, and FGS-while-in-use permissions check.

* - *

Note: the underlying data structure is an {@link ArrayMap}, for performance reason, it is only - * suitable to hold up to hundreds of entries.

- * @param type of the key. + *

Note: the underlying data structure is an {@link SparseArray}, for performance reason, + * it is only suitable to hold up to hundreds of entries.

* @param type of the additional optional info. */ -public class FgsTempAllowList { +public class FgsTempAllowList { private static final int DEFAULT_MAX_SIZE = 100; /** * The value is Pair type, Pair.first is the expirationTime(an elapsedRealtime), * Pair.second is the optional information entry about this key. */ - private final ArrayMap> mTempAllowList = new ArrayMap<>(); + private final SparseArray> mTempAllowList = new SparseArray<>(); private int mMaxSize = DEFAULT_MAX_SIZE; private final Object mLock = new Object(); @@ -70,15 +70,14 @@ public class FgsTempAllowList { /** * Add a key and its duration with optional info into the temp allowlist. - * @param key * @param durationMs temp-allowlisted duration in milliseconds. * @param entry additional optional information of this key, could be null. */ - public void add(K key, long durationMs, @Nullable E entry) { + public void add(int uid, long durationMs, @Nullable E entry) { synchronized (mLock) { if (durationMs <= 0) { Slog.e(TAG_AM, "FgsTempAllowList bad duration:" + durationMs + " key: " - + key); + + uid); return; } // The temp allowlist should be a short list with only a few entries in it. @@ -94,10 +93,10 @@ public class FgsTempAllowList { } } } - final Pair existing = mTempAllowList.get(key); + final Pair existing = mTempAllowList.get(uid); final long expirationTime = now + durationMs; if (existing == null || existing.first < expirationTime) { - mTempAllowList.put(key, new Pair(expirationTime, entry)); + mTempAllowList.put(uid, new Pair(expirationTime, entry)); } } } @@ -105,13 +104,12 @@ public class FgsTempAllowList { /** * If the key has not expired (AKA allowed), return its non-null value. * If the key has expired, return null. - * @param key * @return */ @Nullable - public Pair get(K key) { + public Pair get(int uid) { synchronized (mLock) { - final int index = mTempAllowList.indexOfKey(key); + final int index = mTempAllowList.indexOfKey(uid); if (index < 0) { return null; } else if (mTempAllowList.valueAt(index).first < SystemClock.elapsedRealtime()) { @@ -126,23 +124,48 @@ public class FgsTempAllowList { /** * If the key has not expired (AKA allowed), return true. * If the key has expired, return false. - * @param key - * @return */ - public boolean isAllowed(K key) { - Pair entry = get(key); + public boolean isAllowed(int uid) { + Pair entry = get(uid); return entry != null; } - public void remove(K key) { + /** + * Remove a given UID. + */ + public void removeUid(int uid) { synchronized (mLock) { - mTempAllowList.remove(key); + mTempAllowList.remove(uid); } } - public Set keySet() { + /** + * Remove by appId. + */ + public void removeAppId(int appId) { synchronized (mLock) { - return mTempAllowList.keySet(); + // Find all UIDs matching the appId. + for (int i = mTempAllowList.size() - 1; i >= 0; i--) { + final int uid = mTempAllowList.keyAt(i); + if (UserHandle.getAppId(uid) == appId) { + mTempAllowList.removeAt(i); + } + } + } + } + + /** + * Iterate over the entries. + */ + public void forEach(BiConsumer> callback) { + synchronized (mLock) { + for (int i = 0; i < mTempAllowList.size(); i++) { + final int uid = mTempAllowList.keyAt(i); + final Pair entry = mTempAllowList.valueAt(i); + if (entry != null) { + callback.accept(uid, entry); + } + } } } } diff --git a/services/core/java/com/android/server/am/ProcessList.java b/services/core/java/com/android/server/am/ProcessList.java index 0ffaccfa0e894..457fe0f88aa7c 100644 --- a/services/core/java/com/android/server/am/ProcessList.java +++ b/services/core/java/com/android/server/am/ProcessList.java @@ -3074,7 +3074,7 @@ public final class ProcessList { UidRecord.CHANGE_GONE); EventLogTags.writeAmUidStopped(uid); mActiveUids.remove(uid); - mService.mFgsStartTempAllowList.remove(record.info.uid); + mService.mFgsStartTempAllowList.removeUid(record.info.uid); mService.noteUidProcessState(uid, ActivityManager.PROCESS_STATE_NONEXISTENT, ActivityManager.PROCESS_CAPABILITY_NONE); } diff --git a/services/tests/servicestests/src/com/android/server/am/FgsTempAllowListTest.java b/services/tests/servicestests/src/com/android/server/am/FgsTempAllowListTest.java index f85f0f8437897..50d4d8475395e 100644 --- a/services/tests/servicestests/src/com/android/server/am/FgsTempAllowListTest.java +++ b/services/tests/servicestests/src/com/android/server/am/FgsTempAllowListTest.java @@ -29,6 +29,9 @@ import android.util.Pair; import org.junit.Test; +import java.util.concurrent.atomic.AtomicInteger; +import java.util.function.Supplier; + /** * Build/Install/Run: * atest FrameworksServicesTests:TempAllowListTest @@ -41,7 +44,7 @@ public class FgsTempAllowListTest { */ @Test public void testIsAllowed() { - FgsTempAllowList allowList = new FgsTempAllowList(); + FgsTempAllowList allowList = new FgsTempAllowList(); allowList.add(10001, 2000, "description1"); allowList.add(10002, 2000, "description2"); @@ -55,7 +58,7 @@ public class FgsTempAllowListTest { assertNotNull(entry2); assertEquals(entry2.second, "description2"); - allowList.remove(10001); + allowList.removeUid(10001); assertFalse(allowList.isAllowed(10001)); assertNull(allowList.get(10001)); } @@ -65,7 +68,7 @@ public class FgsTempAllowListTest { */ @Test public void testExpired() { - FgsTempAllowList allowList = new FgsTempAllowList(); + FgsTempAllowList allowList = new FgsTempAllowList(); // temp allow for 2000ms. allowList.add(10001, 2000, "uid1-2000ms"); // sleep for 3000ms. @@ -74,4 +77,51 @@ public class FgsTempAllowListTest { assertFalse(allowList.isAllowed(10001)); assertNull(allowList.get(10001)); } + + @Test + public void testRemoveAppId() { + FgsTempAllowList allowList = new FgsTempAllowList(); + allowList.add(10001, 2000, "description1"); + allowList.add(10002, 2000, "description2"); + allowList.add(10_10001, 2000, "description3"); + + assertTrue(allowList.isAllowed(10001)); + assertTrue(allowList.isAllowed(10002)); + assertTrue(allowList.isAllowed(10_10001)); + + allowList.removeAppId(10001); + + assertFalse(allowList.isAllowed(10001)); + assertTrue(allowList.isAllowed(10002)); + assertFalse(allowList.isAllowed(10_10001)); + } + + @Test + public void testForEach() { + final FgsTempAllowList allowList = new FgsTempAllowList(); + + + // Call forEach(), return the sum of all the UIDs, and make sure the item is + // "uid" + uid. + final Supplier callForEach = () -> { + final AtomicInteger sum = new AtomicInteger(); + sum.set(0); + allowList.forEach((uid, entry) -> { + sum.set(sum.get() + uid); + assertEquals(entry.second, "uid" + uid); + }); + return sum.get(); + }; + + // Call on th empty list. + assertEquals(0, (int) callForEach.get()); + + // Add one item. + allowList.add(1, 2000, "uid1"); + assertEquals(1, (int) callForEach.get()); + + // Add one more item. + allowList.add(10, 2000, "uid10"); + assertEquals(11, (int) callForEach.get()); + } }