From 7fe57d25c892ad206b58c99c587fb28289830b80 Mon Sep 17 00:00:00 2001 From: Andrey Epin Date: Wed, 19 Jun 2024 20:33:45 -0700 Subject: Destroy all profile list adapters Fix: 346671041 Test: launch media projection selection activity on a device with multiple profiles; check that the activity is leaking before the fid and is not leaking after the fix. Flag: android.service.chooser.fix_resolver_memory_leak (cherry picked from https://googleplex-android-review.googlesource.com/q/commit:1f513aa2c90ec36d8b641251fcd5d11a23256876) Merged-In: I5055957464d905c2d8a638743658461d9680230c Change-Id: I5055957464d905c2d8a638743658461d9680230c --- core/java/android/service/chooser/flags.aconfig | 11 +++++++++++ core/java/com/android/internal/app/ResolverActivity.java | 16 +++++++++++++--- 2 files changed, 24 insertions(+), 3 deletions(-) diff --git a/core/java/android/service/chooser/flags.aconfig b/core/java/android/service/chooser/flags.aconfig index d6425c397bbb..ed20207cfb0b 100644 --- a/core/java/android/service/chooser/flags.aconfig +++ b/core/java/android/service/chooser/flags.aconfig @@ -32,3 +32,14 @@ flag { description: "Provides additional callbacks with information about user actions in ChooserResult" bug: "263474465" } + +flag { + name: "fix_resolver_memory_leak" + is_exported: true + namespace: "intentresolver" + description: "ResolverActivity memory leak (through the AppPredictor callback) fix" + bug: "346671041" + metadata { + purpose: PURPOSE_BUGFIX + } +} diff --git a/core/java/com/android/internal/app/ResolverActivity.java b/core/java/com/android/internal/app/ResolverActivity.java index 920981e2b8fe..a1945352ae09 100644 --- a/core/java/com/android/internal/app/ResolverActivity.java +++ b/core/java/com/android/internal/app/ResolverActivity.java @@ -1209,9 +1209,19 @@ public class ResolverActivity extends Activity implements if (!isChangingConfigurations() && mPickOptionRequest != null) { mPickOptionRequest.cancel(); } - if (mMultiProfilePagerAdapter != null - && mMultiProfilePagerAdapter.getActiveListAdapter() != null) { - mMultiProfilePagerAdapter.getActiveListAdapter().onDestroy(); + if (mMultiProfilePagerAdapter != null) { + ResolverListAdapter activeAdapter = + mMultiProfilePagerAdapter.getActiveListAdapter(); + if (activeAdapter != null) { + activeAdapter.onDestroy(); + } + if (android.service.chooser.Flags.fixResolverMemoryLeak()) { + ResolverListAdapter inactiveAdapter = + mMultiProfilePagerAdapter.getInactiveListAdapter(); + if (inactiveAdapter != null) { + inactiveAdapter.onDestroy(); + } + } } } -- cgit v1.2.3-59-g8ed1b From cff690835753343ee3d2ca15e78ebf46a49bde89 Mon Sep 17 00:00:00 2001 From: Beatrice Marchegiani Date: Mon, 8 Jul 2024 11:08:14 +0000 Subject: Fix NPE when reading the allowlist Bug: 351750747 Test: atest -v TarBackupReaderTest, manual test Flag: EXEMPT bugfix (cherry picked from https://googleplex-android-review.googlesource.com/q/commit:a8cac82ccdd9efdd88a62133e4eec9570d50b7f0) Merged-In: Icf4fe058e7d261c197e04ccad940595413bd4a99 Change-Id: Icf4fe058e7d261c197e04ccad940595413bd4a99 --- .../backup/java/com/android/server/backup/utils/TarBackupReader.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/services/backup/java/com/android/server/backup/utils/TarBackupReader.java b/services/backup/java/com/android/server/backup/utils/TarBackupReader.java index 4860a274cfa8..8abbe5666d58 100644 --- a/services/backup/java/com/android/server/backup/utils/TarBackupReader.java +++ b/services/backup/java/com/android/server/backup/utils/TarBackupReader.java @@ -792,10 +792,11 @@ public class TarBackupReader { } private String getVToUAllowlist(Context context, int userId) { - return Settings.Secure.getStringForUser( + String allowlist = Settings.Secure.getStringForUser( context.getContentResolver(), Settings.Secure.V_TO_U_RESTORE_ALLOWLIST, userId); + return (allowlist == null) ? "" : allowlist; } private static long extractRadix(byte[] data, int offset, int maxChars, int radix) -- cgit v1.2.3-59-g8ed1b From b81f7b8da44293aeed40ca4e829423c22e7228b3 Mon Sep 17 00:00:00 2001 From: Lucas Silva Date: Tue, 9 Jul 2024 10:58:42 -0400 Subject: Unregister task fragment organizer when dream is destroyed. Fixes: 350734809 Test: android studio profiler with heap dumps Flag: EXEMPT bugfix (cherry picked from https://googleplex-android-review.googlesource.com/q/commit:11169ab775550ac71f2e020bd5600b1bdd9118db) Merged-In: I34e85d9decc13c173840137cd3783215c64203d4 Change-Id: I34e85d9decc13c173840137cd3783215c64203d4 --- .../com/android/systemui/dreams/homecontrols/TaskFragmentComponent.kt | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/SystemUI/src/com/android/systemui/dreams/homecontrols/TaskFragmentComponent.kt b/packages/SystemUI/src/com/android/systemui/dreams/homecontrols/TaskFragmentComponent.kt index 297ad847caa6..befd822e14cd 100644 --- a/packages/SystemUI/src/com/android/systemui/dreams/homecontrols/TaskFragmentComponent.kt +++ b/packages/SystemUI/src/com/android/systemui/dreams/homecontrols/TaskFragmentComponent.kt @@ -161,5 +161,6 @@ constructor( TASK_FRAGMENT_TRANSIT_CLOSE, false ) + organizer.unregisterOrganizer() } } -- cgit v1.2.3-59-g8ed1b From 02df3c61779ab5e05298eb7d4b6f492af6c4ba92 Mon Sep 17 00:00:00 2001 From: Julia Reynolds Date: Mon, 8 Jul 2024 09:14:24 -0400 Subject: Synchronize access to notif settings And fix an instance where we were calling into PM with a lock held. Flag: EXEMPT bugfix Bug: 343447769 Test: PreferenceHelperTest (cherry picked from https://googleplex-android-review.googlesource.com/q/commit:490c3ee65e26f0a3db5819510eef2355a149dc4b) (cherry picked from https://googleplex-android-review.googlesource.com/q/commit:83e6ba7dc06fbd852da588082b33246fa4b70b64) Merged-In: I1fec0503bf347f176305744b77ad27d78fe8161c Change-Id: I1fec0503bf347f176305744b77ad27d78fe8161c --- .../server/notification/PreferencesHelper.java | 215 +++++++++++---------- .../server/notification/PreferencesHelperTest.java | 58 ++++++ 2 files changed, 168 insertions(+), 105 deletions(-) diff --git a/services/core/java/com/android/server/notification/PreferencesHelper.java b/services/core/java/com/android/server/notification/PreferencesHelper.java index 309e9450a01d..bc19b995bc14 100644 --- a/services/core/java/com/android/server/notification/PreferencesHelper.java +++ b/services/core/java/com/android/server/notification/PreferencesHelper.java @@ -189,9 +189,12 @@ public class PreferencesHelper implements RankingConfig { int USER_LOCKED_BUBBLE = 0x00000002; } + private final Object mLock = new Object(); // pkg|uid => PackagePreferences + @GuardedBy("mLock") private final ArrayMap mPackagePreferences = new ArrayMap<>(); // pkg|userId => PackagePreferences + @GuardedBy("mLock") private final ArrayMap mRestoredWithoutUids = new ArrayMap<>(); private final Context mContext; @@ -263,7 +266,7 @@ public class PreferencesHelper implements RankingConfig { Settings.Global.REVIEW_PERMISSIONS_NOTIFICATION_STATE, NotificationManagerService.REVIEW_NOTIF_STATE_SHOULD_SHOW); } - synchronized (mPackagePreferences) { + synchronized (mLock) { while ((type = parser.next()) != XmlPullParser.END_DOCUMENT) { tag = parser.getName(); if (type == XmlPullParser.END_TAG && TAG_RANKING.equals(tag)) { @@ -485,6 +488,7 @@ public class PreferencesHelper implements RankingConfig { DEFAULT_BUBBLE_PREFERENCE, mClock.millis()); } + @GuardedBy("mLock") private PackagePreferences getOrCreatePackagePreferencesLocked(String pkg, @UserIdInt int userId, int uid, int importance, int priority, int visibility, boolean showBadge, int bubblePreference, long creationTime) { @@ -616,7 +620,7 @@ public class PreferencesHelper implements RankingConfig { notifPermissions = mPermissionHelper.getNotificationPermissionValues(userId); } - synchronized (mPackagePreferences) { + synchronized (mLock) { final int N = mPackagePreferences.size(); for (int i = 0; i < N; i++) { final PackagePreferences r = mPackagePreferences.valueAt(i); @@ -625,11 +629,10 @@ public class PreferencesHelper implements RankingConfig { } writePackageXml(r, out, notifPermissions, forBackup); } - } - if (Flags.persistIncompleteRestoreData() && !forBackup) { - synchronized (mRestoredWithoutUids) { - final int N = mRestoredWithoutUids.size(); - for (int i = 0; i < N; i++) { + + if (Flags.persistIncompleteRestoreData() && !forBackup) { + final int M = mRestoredWithoutUids.size(); + for (int i = 0; i < M; i++) { final PackagePreferences r = mRestoredWithoutUids.valueAt(i); writePackageXml(r, out, notifPermissions, false); } @@ -732,7 +735,7 @@ public class PreferencesHelper implements RankingConfig { */ public void setBubblesAllowed(String pkg, int uid, int bubblePreference) { boolean changed; - synchronized (mPackagePreferences) { + synchronized (mLock) { PackagePreferences p = getOrCreatePackagePreferencesLocked(pkg, uid); changed = p.bubblePreference != bubblePreference; p.bubblePreference = bubblePreference; @@ -752,20 +755,20 @@ public class PreferencesHelper implements RankingConfig { */ @Override public int getBubblePreference(String pkg, int uid) { - synchronized (mPackagePreferences) { + synchronized (mLock) { return getOrCreatePackagePreferencesLocked(pkg, uid).bubblePreference; } } public int getAppLockedFields(String pkg, int uid) { - synchronized (mPackagePreferences) { + synchronized (mLock) { return getOrCreatePackagePreferencesLocked(pkg, uid).lockedAppFields; } } @Override public boolean canShowBadge(String packageName, int uid) { - synchronized (mPackagePreferences) { + synchronized (mLock) { return getOrCreatePackagePreferencesLocked(packageName, uid).showBadge; } } @@ -773,7 +776,7 @@ public class PreferencesHelper implements RankingConfig { @Override public void setShowBadge(String packageName, int uid, boolean showBadge) { boolean changed = false; - synchronized (mPackagePreferences) { + synchronized (mLock) { PackagePreferences pkgPrefs = getOrCreatePackagePreferencesLocked(packageName, uid); if (pkgPrefs.showBadge != showBadge) { pkgPrefs.showBadge = showBadge; @@ -786,28 +789,28 @@ public class PreferencesHelper implements RankingConfig { } public boolean isInInvalidMsgState(String packageName, int uid) { - synchronized (mPackagePreferences) { + synchronized (mLock) { PackagePreferences r = getOrCreatePackagePreferencesLocked(packageName, uid); return r.hasSentInvalidMessage && !r.hasSentValidMessage; } } public boolean hasUserDemotedInvalidMsgApp(String packageName, int uid) { - synchronized (mPackagePreferences) { + synchronized (mLock) { PackagePreferences r = getOrCreatePackagePreferencesLocked(packageName, uid); return isInInvalidMsgState(packageName, uid) ? r.userDemotedMsgApp : false; } } public void setInvalidMsgAppDemoted(String packageName, int uid, boolean isDemoted) { - synchronized (mPackagePreferences) { + synchronized (mLock) { PackagePreferences r = getOrCreatePackagePreferencesLocked(packageName, uid); r.userDemotedMsgApp = isDemoted; } } public boolean setInvalidMessageSent(String packageName, int uid) { - synchronized (mPackagePreferences) { + synchronized (mLock) { PackagePreferences r = getOrCreatePackagePreferencesLocked(packageName, uid); boolean valueChanged = r.hasSentInvalidMessage == false; r.hasSentInvalidMessage = true; @@ -817,7 +820,7 @@ public class PreferencesHelper implements RankingConfig { } public boolean setValidMessageSent(String packageName, int uid) { - synchronized (mPackagePreferences) { + synchronized (mLock) { PackagePreferences r = getOrCreatePackagePreferencesLocked(packageName, uid); boolean valueChanged = r.hasSentValidMessage == false; r.hasSentValidMessage = true; @@ -828,7 +831,7 @@ public class PreferencesHelper implements RankingConfig { @VisibleForTesting boolean hasSentInvalidMsg(String packageName, int uid) { - synchronized (mPackagePreferences) { + synchronized (mLock) { PackagePreferences r = getOrCreatePackagePreferencesLocked(packageName, uid); return r.hasSentInvalidMessage; } @@ -836,7 +839,7 @@ public class PreferencesHelper implements RankingConfig { @VisibleForTesting boolean hasSentValidMsg(String packageName, int uid) { - synchronized (mPackagePreferences) { + synchronized (mLock) { PackagePreferences r = getOrCreatePackagePreferencesLocked(packageName, uid); return r.hasSentValidMessage; } @@ -844,7 +847,7 @@ public class PreferencesHelper implements RankingConfig { @VisibleForTesting boolean didUserEverDemoteInvalidMsgApp(String packageName, int uid) { - synchronized (mPackagePreferences) { + synchronized (mLock) { PackagePreferences r = getOrCreatePackagePreferencesLocked(packageName, uid); return r.userDemotedMsgApp; } @@ -852,7 +855,7 @@ public class PreferencesHelper implements RankingConfig { /** Sets whether this package has sent a notification with valid bubble metadata. */ public boolean setValidBubbleSent(String packageName, int uid) { - synchronized (mPackagePreferences) { + synchronized (mLock) { PackagePreferences r = getOrCreatePackagePreferencesLocked(packageName, uid); boolean valueChanged = !r.hasSentValidBubble; r.hasSentValidBubble = true; @@ -861,14 +864,14 @@ public class PreferencesHelper implements RankingConfig { } boolean hasSentValidBubble(String packageName, int uid) { - synchronized (mPackagePreferences) { + synchronized (mLock) { PackagePreferences r = getOrCreatePackagePreferencesLocked(packageName, uid); return r.hasSentValidBubble; } } boolean isImportanceLocked(String pkg, int uid) { - synchronized (mPackagePreferences) { + synchronized (mLock) { PackagePreferences r = getOrCreatePackagePreferencesLocked(pkg, uid); return r.fixedImportance || r.defaultAppLockedImportance; } @@ -879,7 +882,7 @@ public class PreferencesHelper implements RankingConfig { if (groupId == null) { return false; } - synchronized (mPackagePreferences) { + synchronized (mLock) { PackagePreferences r = getOrCreatePackagePreferencesLocked(packageName, uid); NotificationChannelGroup group = r.groups.get(groupId); if (group == null) { @@ -890,13 +893,13 @@ public class PreferencesHelper implements RankingConfig { } int getPackagePriority(String pkg, int uid) { - synchronized (mPackagePreferences) { + synchronized (mLock) { return getOrCreatePackagePreferencesLocked(pkg, uid).priority; } } int getPackageVisibility(String pkg, int uid) { - synchronized (mPackagePreferences) { + synchronized (mLock) { return getOrCreatePackagePreferencesLocked(pkg, uid).visibility; } } @@ -911,7 +914,7 @@ public class PreferencesHelper implements RankingConfig { throw new IllegalArgumentException("group.getName() can't be empty"); } boolean needsDndChange = false; - synchronized (mPackagePreferences) { + synchronized (mLock) { PackagePreferences r = getOrCreatePackagePreferencesLocked(pkg, uid); if (r == null) { throw new IllegalArgumentException("Invalid package"); @@ -965,7 +968,7 @@ public class PreferencesHelper implements RankingConfig { && channel.getImportance() <= IMPORTANCE_MAX, "Invalid importance level"); boolean needsPolicyFileChange = false, wasUndeleted = false, needsDndChange = false; - synchronized (mPackagePreferences) { + synchronized (mLock) { PackagePreferences r = getOrCreatePackagePreferencesLocked(pkg, uid); if (r == null) { throw new IllegalArgumentException("Invalid package"); @@ -1109,7 +1112,7 @@ public class PreferencesHelper implements RankingConfig { void unlockNotificationChannelImportance(String pkg, int uid, String updatedChannelId) { Objects.requireNonNull(updatedChannelId); - synchronized (mPackagePreferences) { + synchronized (mLock) { PackagePreferences r = getOrCreatePackagePreferencesLocked(pkg, uid); if (r == null) { throw new IllegalArgumentException("Invalid package"); @@ -1131,7 +1134,7 @@ public class PreferencesHelper implements RankingConfig { Objects.requireNonNull(updatedChannel.getId()); boolean changed = false; boolean needsDndChange = false; - synchronized (mPackagePreferences) { + synchronized (mLock) { PackagePreferences r = getOrCreatePackagePreferencesLocked(pkg, uid); if (r == null) { throw new IllegalArgumentException("Invalid package"); @@ -1306,7 +1309,7 @@ public class PreferencesHelper implements RankingConfig { String channelId, String conversationId, boolean returnParentIfNoConversationChannel, boolean includeDeleted) { Preconditions.checkNotNull(pkg); - synchronized (mPackagePreferences) { + synchronized (mLock) { PackagePreferences r = getOrCreatePackagePreferencesLocked(pkg, uid); if (r == null) { return null; @@ -1347,7 +1350,7 @@ public class PreferencesHelper implements RankingConfig { Preconditions.checkNotNull(pkg); Preconditions.checkNotNull(conversationId); List channels = new ArrayList<>(); - synchronized (mPackagePreferences) { + synchronized (mLock) { PackagePreferences r = getOrCreatePackagePreferencesLocked(pkg, uid); if (r == null) { return channels; @@ -1367,7 +1370,7 @@ public class PreferencesHelper implements RankingConfig { int callingUid, boolean fromSystemOrSystemUi) { boolean deletedChannel = false; boolean channelBypassedDnd = false; - synchronized (mPackagePreferences) { + synchronized (mLock) { PackagePreferences r = getPackagePreferencesLocked(pkg, uid); if (r == null) { return false; @@ -1403,7 +1406,7 @@ public class PreferencesHelper implements RankingConfig { public void permanentlyDeleteNotificationChannel(String pkg, int uid, String channelId) { Objects.requireNonNull(pkg); Objects.requireNonNull(channelId); - synchronized (mPackagePreferences) { + synchronized (mLock) { PackagePreferences r = getPackagePreferencesLocked(pkg, uid); if (r == null) { return; @@ -1415,7 +1418,7 @@ public class PreferencesHelper implements RankingConfig { @Override public void permanentlyDeleteNotificationChannels(String pkg, int uid) { Objects.requireNonNull(pkg); - synchronized (mPackagePreferences) { + synchronized (mLock) { PackagePreferences r = getPackagePreferencesLocked(pkg, uid); if (r == null) { return; @@ -1446,7 +1449,7 @@ public class PreferencesHelper implements RankingConfig { boolean fixed = mPermissionHelper.isPermissionFixed( pi.packageName, user.getUserHandle().getIdentifier()); if (fixed) { - synchronized (mPackagePreferences) { + synchronized (mLock) { PackagePreferences p = getOrCreatePackagePreferencesLocked( pi.packageName, pi.applicationInfo.uid); p.fixedImportance = true; @@ -1461,7 +1464,7 @@ public class PreferencesHelper implements RankingConfig { public void updateDefaultApps(int userId, ArraySet toRemove, ArraySet> toAdd) { - synchronized (mPackagePreferences) { + synchronized (mLock) { for (PackagePreferences p : mPackagePreferences.values()) { if (userId == UserHandle.getUserId(p.uid)) { if (toRemove != null && toRemove.contains(p.pkg)) { @@ -1491,7 +1494,7 @@ public class PreferencesHelper implements RankingConfig { public NotificationChannelGroup getNotificationChannelGroupWithChannels(String pkg, int uid, String groupId, boolean includeDeleted) { Objects.requireNonNull(pkg); - synchronized (mPackagePreferences) { + synchronized (mLock) { PackagePreferences r = getPackagePreferencesLocked(pkg, uid); if (r == null || groupId == null || !r.groups.containsKey(groupId)) { return null; @@ -1514,7 +1517,7 @@ public class PreferencesHelper implements RankingConfig { public NotificationChannelGroup getNotificationChannelGroup(String groupId, String pkg, int uid) { Objects.requireNonNull(pkg); - synchronized (mPackagePreferences) { + synchronized (mLock) { PackagePreferences r = getPackagePreferencesLocked(pkg, uid); if (r == null) { return null; @@ -1528,7 +1531,7 @@ public class PreferencesHelper implements RankingConfig { boolean includeBlocked, Set activeChannelFilter) { Objects.requireNonNull(pkg); Map groups = new ArrayMap<>(); - synchronized (mPackagePreferences) { + synchronized (mLock) { PackagePreferences r = getPackagePreferencesLocked(pkg, uid); if (r == null) { return ParceledListSlice.emptyList(); @@ -1575,7 +1578,7 @@ public class PreferencesHelper implements RankingConfig { String groupId, int callingUid, boolean fromSystemOrSystemUi) { List deletedChannels = new ArrayList<>(); boolean groupBypassedDnd = false; - synchronized (mPackagePreferences) { + synchronized (mLock) { PackagePreferences r = getPackagePreferencesLocked(pkg, uid); if (r == null || TextUtils.isEmpty(groupId)) { return deletedChannels; @@ -1607,7 +1610,7 @@ public class PreferencesHelper implements RankingConfig { public Collection getNotificationChannelGroups(String pkg, int uid) { List groups = new ArrayList<>(); - synchronized (mPackagePreferences) { + synchronized (mLock) { PackagePreferences r = getPackagePreferencesLocked(pkg, uid); if (r == null) { return groups; @@ -1618,7 +1621,7 @@ public class PreferencesHelper implements RankingConfig { } public NotificationChannelGroup getGroupForChannel(String pkg, int uid, String channelId) { - synchronized (mPackagePreferences) { + synchronized (mLock) { PackagePreferences p = getPackagePreferencesLocked(pkg, uid); if (p != null) { NotificationChannel nc = p.channels.get(channelId); @@ -1637,7 +1640,7 @@ public class PreferencesHelper implements RankingConfig { public ArrayList getConversations(IntArray userIds, boolean onlyImportant) { - synchronized (mPackagePreferences) { + synchronized (mLock) { ArrayList conversations = new ArrayList<>(); for (PackagePreferences p : mPackagePreferences.values()) { if (userIds.binarySearch(UserHandle.getUserId(p.uid)) >= 0) { @@ -1681,7 +1684,7 @@ public class PreferencesHelper implements RankingConfig { public ArrayList getConversations(String pkg, int uid) { Objects.requireNonNull(pkg); - synchronized (mPackagePreferences) { + synchronized (mLock) { PackagePreferences r = getPackagePreferencesLocked(pkg, uid); if (r == null) { return new ArrayList<>(); @@ -1723,7 +1726,7 @@ public class PreferencesHelper implements RankingConfig { public @NonNull List deleteConversations(String pkg, int uid, Set conversationIds, int callingUid, boolean fromSystemOrSystemUi) { List deletedChannelIds = new ArrayList<>(); - synchronized (mPackagePreferences) { + synchronized (mLock) { PackagePreferences r = getPackagePreferencesLocked(pkg, uid); if (r == null) { return deletedChannelIds; @@ -1756,7 +1759,7 @@ public class PreferencesHelper implements RankingConfig { boolean includeDeleted) { Objects.requireNonNull(pkg); List channels = new ArrayList<>(); - synchronized (mPackagePreferences) { + synchronized (mLock) { PackagePreferences r = getPackagePreferencesLocked(pkg, uid); if (r == null) { return ParceledListSlice.emptyList(); @@ -1778,7 +1781,7 @@ public class PreferencesHelper implements RankingConfig { public ParceledListSlice getNotificationChannelsBypassingDnd(String pkg, int uid) { List channels = new ArrayList<>(); - synchronized (mPackagePreferences) { + synchronized (mLock) { final PackagePreferences r = mPackagePreferences.get( packagePreferencesKey(pkg, uid)); if (r != null) { @@ -1799,7 +1802,7 @@ public class PreferencesHelper implements RankingConfig { * upgrades. */ public boolean onlyHasDefaultChannel(String pkg, int uid) { - synchronized (mPackagePreferences) { + synchronized (mLock) { PackagePreferences r = getOrCreatePackagePreferencesLocked(pkg, uid); if (r.channels.size() == 1 && r.channels.containsKey(NotificationChannel.DEFAULT_CHANNEL_ID)) { @@ -1812,7 +1815,7 @@ public class PreferencesHelper implements RankingConfig { public int getDeletedChannelCount(String pkg, int uid) { Objects.requireNonNull(pkg); int deletedCount = 0; - synchronized (mPackagePreferences) { + synchronized (mLock) { PackagePreferences r = getPackagePreferencesLocked(pkg, uid); if (r == null) { return deletedCount; @@ -1831,7 +1834,7 @@ public class PreferencesHelper implements RankingConfig { public int getBlockedChannelCount(String pkg, int uid) { Objects.requireNonNull(pkg); int blockedCount = 0; - synchronized (mPackagePreferences) { + synchronized (mLock) { PackagePreferences r = getPackagePreferencesLocked(pkg, uid); if (r == null) { return blockedCount; @@ -1874,7 +1877,7 @@ public class PreferencesHelper implements RankingConfig { ArraySet> candidatePkgs = new ArraySet<>(); final IntArray currentUserIds = mUserProfiles.getCurrentProfileIds(); - synchronized (mPackagePreferences) { + synchronized (mLock) { final int numPackagePreferences = mPackagePreferences.size(); for (int i = 0; i < numPackagePreferences; i++) { final PackagePreferences r = mPackagePreferences.valueAt(i); @@ -1943,7 +1946,7 @@ public class PreferencesHelper implements RankingConfig { * considered for sentiment adjustments (and thus never show a blocking helper). */ public void setAppImportanceLocked(String packageName, int uid) { - synchronized (mPackagePreferences) { + synchronized (mLock) { PackagePreferences prefs = getOrCreatePackagePreferencesLocked(packageName, uid); if ((prefs.lockedAppFields & LockableAppFields.USER_LOCKED_IMPORTANCE) != 0) { return; @@ -1959,7 +1962,7 @@ public class PreferencesHelper implements RankingConfig { * Returns the delegate for a given package, if it's allowed by the package and the user. */ public @Nullable String getNotificationDelegate(String sourcePkg, int sourceUid) { - synchronized (mPackagePreferences) { + synchronized (mLock) { PackagePreferences prefs = getPackagePreferencesLocked(sourcePkg, sourceUid); if (prefs == null || prefs.delegate == null) { @@ -1977,7 +1980,7 @@ public class PreferencesHelper implements RankingConfig { */ public void setNotificationDelegate(String sourcePkg, int sourceUid, String delegatePkg, int delegateUid) { - synchronized (mPackagePreferences) { + synchronized (mLock) { PackagePreferences prefs = getOrCreatePackagePreferencesLocked(sourcePkg, sourceUid); prefs.delegate = new Delegate(delegatePkg, delegateUid, true); } @@ -1987,7 +1990,7 @@ public class PreferencesHelper implements RankingConfig { * Used by an app to turn off its notification delegate. */ public void revokeNotificationDelegate(String sourcePkg, int sourceUid) { - synchronized (mPackagePreferences) { + synchronized (mLock) { PackagePreferences prefs = getPackagePreferencesLocked(sourcePkg, sourceUid); if (prefs != null && prefs.delegate != null) { prefs.delegate.mEnabled = false; @@ -2001,7 +2004,7 @@ public class PreferencesHelper implements RankingConfig { */ public boolean isDelegateAllowed(String sourcePkg, int sourceUid, String potentialDelegatePkg, int potentialDelegateUid) { - synchronized (mPackagePreferences) { + synchronized (mLock) { PackagePreferences prefs = getPackagePreferencesLocked(sourcePkg, sourceUid); return prefs != null && prefs.isValidDelegate(potentialDelegatePkg, @@ -2047,24 +2050,25 @@ public class PreferencesHelper implements RankingConfig { pw.println("per-package config version: " + XML_VERSION); pw.println("PackagePreferences:"); - synchronized (mPackagePreferences) { + synchronized (mLock) { dumpPackagePreferencesLocked(pw, prefix, filter, mPackagePreferences, pkgPermissions); + pw.println("Restored without uid:"); + dumpPackagePreferencesLocked(pw, prefix, filter, mRestoredWithoutUids, null); } - pw.println("Restored without uid:"); - dumpPackagePreferencesLocked(pw, prefix, filter, mRestoredWithoutUids, null); } public void dump(ProtoOutputStream proto, @NonNull NotificationManagerService.DumpFilter filter, ArrayMap, Pair> pkgPermissions) { - synchronized (mPackagePreferences) { + synchronized (mLock) { dumpPackagePreferencesLocked(proto, RankingHelperProto.RECORDS, filter, mPackagePreferences, pkgPermissions); + dumpPackagePreferencesLocked(proto, RankingHelperProto.RECORDS_RESTORED_WITHOUT_UID, + filter, mRestoredWithoutUids, null); } - dumpPackagePreferencesLocked(proto, RankingHelperProto.RECORDS_RESTORED_WITHOUT_UID, filter, - mRestoredWithoutUids, null); } + @GuardedBy("mLock") private void dumpPackagePreferencesLocked(PrintWriter pw, String prefix, @NonNull NotificationManagerService.DumpFilter filter, ArrayMap packagePreferences, @@ -2249,7 +2253,7 @@ public class PreferencesHelper implements RankingConfig { pkgsWithPermissionsToHandle = pkgPermissions.keySet(); } int pulledEvents = 0; - synchronized (mPackagePreferences) { + synchronized (mLock) { for (int i = 0; i < mPackagePreferences.size(); i++) { if (pulledEvents > NOTIFICATION_PREFERENCES_PULL_LIMIT) { break; @@ -2329,7 +2333,7 @@ public class PreferencesHelper implements RankingConfig { * {@link StatsEvent}. */ public void pullPackageChannelPreferencesStats(List events) { - synchronized (mPackagePreferences) { + synchronized (mLock) { int totalChannelsPulled = 0; for (int i = 0; i < mPackagePreferences.size(); i++) { if (totalChannelsPulled > NOTIFICATION_CHANNEL_PULL_LIMIT) { @@ -2365,7 +2369,7 @@ public class PreferencesHelper implements RankingConfig { * {@link StatsEvent}. */ public void pullPackageChannelGroupPreferencesStats(List events) { - synchronized (mPackagePreferences) { + synchronized (mLock) { int totalGroupsPulled = 0; for (int i = 0; i < mPackagePreferences.size(); i++) { if (totalGroupsPulled > NOTIFICATION_CHANNEL_GROUP_PULL_LIMIT) { @@ -2394,10 +2398,12 @@ public class PreferencesHelper implements RankingConfig { ArrayMap, Pair> pkgPermissions) { JSONObject ranking = new JSONObject(); JSONArray PackagePreferencess = new JSONArray(); - try { - ranking.put("noUid", mRestoredWithoutUids.size()); - } catch (JSONException e) { - // pass + synchronized (mLock) { + try { + ranking.put("noUid", mRestoredWithoutUids.size()); + } catch (JSONException e) { + // pass + } } // Track data that we've handled from the permissions-based list @@ -2406,7 +2412,7 @@ public class PreferencesHelper implements RankingConfig { pkgsWithPermissionsToHandle = pkgPermissions.keySet(); } - synchronized (mPackagePreferences) { + synchronized (mLock) { final int N = mPackagePreferences.size(); for (int i = 0; i < N; i++) { final PackagePreferences r = mPackagePreferences.valueAt(i); @@ -2512,7 +2518,7 @@ public class PreferencesHelper implements RankingConfig { } public Map getPackageBans() { - synchronized (mPackagePreferences) { + synchronized (mLock) { final int N = mPackagePreferences.size(); ArrayMap packageBans = new ArrayMap<>(N); for (int i = 0; i < N; i++) { @@ -2571,7 +2577,7 @@ public class PreferencesHelper implements RankingConfig { private Map getPackageChannels() { ArrayMap packageChannels = new ArrayMap<>(); - synchronized (mPackagePreferences) { + synchronized (mLock) { for (int i = 0; i < mPackagePreferences.size(); i++) { final PackagePreferences r = mPackagePreferences.valueAt(i); int channelCount = 0; @@ -2587,7 +2593,7 @@ public class PreferencesHelper implements RankingConfig { } public void onUserRemoved(int userId) { - synchronized (mPackagePreferences) { + synchronized (mLock) { int N = mPackagePreferences.size(); for (int i = N - 1; i >= 0; i--) { PackagePreferences PackagePreferences = mPackagePreferences.valueAt(i); @@ -2599,7 +2605,7 @@ public class PreferencesHelper implements RankingConfig { } protected void onLocaleChanged(Context context, int userId) { - synchronized (mPackagePreferences) { + synchronized (mLock) { int N = mPackagePreferences.size(); for (int i = 0; i < N; i++) { PackagePreferences PackagePreferences = mPackagePreferences.valueAt(i); @@ -2628,22 +2634,24 @@ public class PreferencesHelper implements RankingConfig { for (int i = 0; i < size; i++) { final String pkg = pkgList[i]; final int uid = uidList[i]; - synchronized (mPackagePreferences) { + synchronized (mLock) { mPackagePreferences.remove(packagePreferencesKey(pkg, uid)); + mRestoredWithoutUids.remove(unrestoredPackageKey(pkg, changeUserId)); } - mRestoredWithoutUids.remove(unrestoredPackageKey(pkg, changeUserId)); updated = true; } } else { for (String pkg : pkgList) { - // Package install - final PackagePreferences r = - mRestoredWithoutUids.get(unrestoredPackageKey(pkg, changeUserId)); - if (r != null) { - try { - r.uid = mPm.getPackageUidAsUser(r.pkg, changeUserId); - mRestoredWithoutUids.remove(unrestoredPackageKey(pkg, changeUserId)); - synchronized (mPackagePreferences) { + try { + // Package install + int uid = mPm.getPackageUidAsUser(pkg, changeUserId); + PackagePermission p = null; + synchronized (mLock) { + final PackagePreferences r = + mRestoredWithoutUids.get(unrestoredPackageKey(pkg, changeUserId)); + if (r != null) { + r.uid = uid; + mRestoredWithoutUids.remove(unrestoredPackageKey(pkg, changeUserId)); mPackagePreferences.put(packagePreferencesKey(r.pkg, r.uid), r); // Try to restore any unrestored sound resources @@ -2665,32 +2673,29 @@ public class PreferencesHelper implements RankingConfig { channel.setSound(restoredUri, channel.getAudioAttributes()); } } - } - if (r.migrateToPm) { - try { - PackagePermission p = new PackagePermission( + + if (r.migrateToPm) { + p = new PackagePermission( r.pkg, UserHandle.getUserId(r.uid), r.importance != IMPORTANCE_NONE, hasUserConfiguredSettings(r)); - mPermissionHelper.setNotificationPermission(p); - } catch (Exception e) { - Slog.e(TAG, "could not migrate setting for " + r.pkg, e); } + updated = true; } - updated = true; - } catch (Exception e) { - Slog.e(TAG, "could not restore " + r.pkg, e); } + if (p != null) { + mPermissionHelper.setNotificationPermission(p); + } + } catch (Exception e) { + Slog.e(TAG, "could not restore " + pkg, e); } // Package upgrade try { - synchronized (mPackagePreferences) { - PackagePreferences fullPackagePreferences = getPackagePreferencesLocked(pkg, - mPm.getPackageUidAsUser(pkg, changeUserId)); - if (fullPackagePreferences != null) { - updated |= createDefaultChannelIfNeededLocked(fullPackagePreferences); - updated |= deleteDefaultChannelIfNeededLocked(fullPackagePreferences); - } + PackagePreferences fullPackagePreferences = getPackagePreferencesLocked(pkg, + mPm.getPackageUidAsUser(pkg, changeUserId)); + if (fullPackagePreferences != null) { + updated |= createDefaultChannelIfNeededLocked(fullPackagePreferences); + updated |= deleteDefaultChannelIfNeededLocked(fullPackagePreferences); } } catch (PackageManager.NameNotFoundException e) { } @@ -2704,7 +2709,7 @@ public class PreferencesHelper implements RankingConfig { } public void clearData(String pkg, int uid) { - synchronized (mPackagePreferences) { + synchronized (mLock) { PackagePreferences p = getPackagePreferencesLocked(pkg, uid); if (p != null) { p.channels = new ArrayMap<>(); @@ -2891,7 +2896,7 @@ public class PreferencesHelper implements RankingConfig { } public void unlockAllNotificationChannels() { - synchronized (mPackagePreferences) { + synchronized (mLock) { final int numPackagePreferences = mPackagePreferences.size(); for (int i = 0; i < numPackagePreferences; i++) { final PackagePreferences r = mPackagePreferences.valueAt(i); @@ -2908,7 +2913,7 @@ public class PreferencesHelper implements RankingConfig { PackageManager.PackageInfoFlags.of(PackageManager.MATCH_ALL), user.getUserHandle().getIdentifier()); for (PackageInfo pi : packages) { - synchronized (mPackagePreferences) { + synchronized (mLock) { PackagePreferences p = getOrCreatePackagePreferencesLocked( pi.packageName, pi.applicationInfo.uid); if (p.migrateToPm && p.uid != UNKNOWN_UID) { diff --git a/services/tests/uiservicestests/src/com/android/server/notification/PreferencesHelperTest.java b/services/tests/uiservicestests/src/com/android/server/notification/PreferencesHelperTest.java index d1880d2f3528..c95d19539486 100644 --- a/services/tests/uiservicestests/src/com/android/server/notification/PreferencesHelperTest.java +++ b/services/tests/uiservicestests/src/com/android/server/notification/PreferencesHelperTest.java @@ -180,6 +180,7 @@ import java.util.List; import java.util.Map; import java.util.Objects; import java.util.Set; +import java.util.concurrent.CountDownLatch; import java.util.concurrent.ThreadLocalRandom; @SmallTest @@ -480,6 +481,34 @@ public class PreferencesHelperTest extends UiServiceTestCase { when(mPm.getPackageUidAsUser(eq(packageName), anyInt())).thenReturn(uid); } + private static void testThreadSafety(Runnable operationToTest, int nThreads, + int nRunsPerThread) throws Exception { + final CountDownLatch startLatch = new CountDownLatch(1); + final CountDownLatch doneLatch = new CountDownLatch(nThreads); + + for (int i = 0; i < nThreads; i++) { + Runnable threadRunnable = () -> { + try { + startLatch.await(); + for (int j = 0; j < nRunsPerThread; j++) { + operationToTest.run(); + } + } catch (InterruptedException e) { + e.printStackTrace(); + } finally { + doneLatch.countDown(); + } + }; + new Thread(threadRunnable, "Test Thread #" + i).start(); + } + + // Ready set go + startLatch.countDown(); + + // Wait for all test threads to be done. + doneLatch.await(); + } + @Test public void testWriteXml_onlyBackupsTargetUser() throws Exception { // Setup package notifications. @@ -6044,6 +6073,35 @@ public class PreferencesHelperTest extends UiServiceTestCase { assertEquals(0, actual.getChannels().stream().filter(c -> c.getId().equals("id2")).count()); } + @Test + public void testRestoredWithoutUid_threadSafety() throws Exception { + when(mPm.getPackageUidAsUser(anyString(), anyInt())).thenReturn(UNKNOWN_UID); + when(mPm.getApplicationInfoAsUser(anyString(), anyInt(), anyInt())).thenThrow( + new PackageManager.NameNotFoundException()); + when(mClock.millis()).thenReturn(System.currentTimeMillis()); + testThreadSafety(() -> { + String id = "id"; + String xml = "\n" + + "\n" + + "\n" + + "\n" + + "\n" + + "\n" + + "\n"; + + try { + loadByteArrayXml(xml.getBytes(), true, USER_SYSTEM); + } catch (Exception e) { + throw new RuntimeException(e); + } + + // trigger a removal from the list + mXmlHelper.onPackagesChanged(true, USER_SYSTEM, new String[]{PKG_P}, + new int[]{UNKNOWN_UID}); + }, 20, 50); + } + private static NotificationChannel cloneChannel(NotificationChannel original) { Parcel parcel = Parcel.obtain(); try { -- cgit v1.2.3-59-g8ed1b