diff options
2 files changed, 144 insertions, 74 deletions
diff --git a/services/core/java/com/android/server/notification/NotificationManagerService.java b/services/core/java/com/android/server/notification/NotificationManagerService.java index ebcbfed93ac7..07891f30abd0 100755 --- a/services/core/java/com/android/server/notification/NotificationManagerService.java +++ b/services/core/java/com/android/server/notification/NotificationManagerService.java @@ -10796,7 +10796,8 @@ public class NotificationManagerService extends SystemService { static final String FLAG_SEPARATOR = "\\|"; private final ArraySet<ManagedServiceInfo> mLightTrimListeners = new ArraySet<>(); - ArrayMap<Pair<ComponentName, Integer>, NotificationListenerFilter> + @GuardedBy("mRequestedNotificationListeners") + private final ArrayMap<Pair<ComponentName, Integer>, NotificationListenerFilter> mRequestedNotificationListeners = new ArrayMap<>(); private final boolean mIsHeadlessSystemUserMode; @@ -10914,9 +10915,11 @@ public class NotificationManagerService extends SystemService { @Override public void onUserRemoved(int user) { super.onUserRemoved(user); - for (int i = mRequestedNotificationListeners.size() - 1; i >= 0; i--) { - if (mRequestedNotificationListeners.keyAt(i).second == user) { - mRequestedNotificationListeners.removeAt(i); + synchronized (mRequestedNotificationListeners) { + for (int i = mRequestedNotificationListeners.size() - 1; i >= 0; i--) { + if (mRequestedNotificationListeners.keyAt(i).second == user) { + mRequestedNotificationListeners.removeAt(i); + } } } } @@ -10925,31 +10928,34 @@ public class NotificationManagerService extends SystemService { public void onPackagesChanged(boolean removingPackage, String[] pkgList, int[] uidList) { super.onPackagesChanged(removingPackage, pkgList, uidList); - // Since the default behavior is to allow everything, we don't need to explicitly - // handle package add or update. they will be added to the xml file on next boot or - // when the user tries to change the settings. - if (removingPackage) { - for (int i = 0; i < pkgList.length; i++) { - String pkg = pkgList[i]; - int userId = UserHandle.getUserId(uidList[i]); - for (int j = mRequestedNotificationListeners.size() - 1; j >= 0; j--) { - Pair<ComponentName, Integer> key = mRequestedNotificationListeners.keyAt(j); - if (key.second == userId && key.first.getPackageName().equals(pkg)) { - mRequestedNotificationListeners.removeAt(j); + synchronized (mRequestedNotificationListeners) { + // Since the default behavior is to allow everything, we don't need to explicitly + // handle package add or update. they will be added to the xml file on next boot or + // when the user tries to change the settings. + if (removingPackage) { + for (int i = 0; i < pkgList.length; i++) { + String pkg = pkgList[i]; + int userId = UserHandle.getUserId(uidList[i]); + for (int j = mRequestedNotificationListeners.size() - 1; j >= 0; j--) { + Pair<ComponentName, Integer> key = + mRequestedNotificationListeners.keyAt(j); + if (key.second == userId && key.first.getPackageName().equals(pkg)) { + mRequestedNotificationListeners.removeAt(j); + } } } } - } - // clean up anything in the disallowed pkgs list - for (int i = 0; i < pkgList.length; i++) { - String pkg = pkgList[i]; - int userId = UserHandle.getUserId(uidList[i]); - for (int j = mRequestedNotificationListeners.size() - 1; j >= 0; j--) { - NotificationListenerFilter nlf = mRequestedNotificationListeners.valueAt(j); + // clean up anything in the disallowed pkgs list + for (int i = 0; i < pkgList.length; i++) { + String pkg = pkgList[i]; + for (int j = mRequestedNotificationListeners.size() - 1; j >= 0; j--) { + NotificationListenerFilter nlf = + mRequestedNotificationListeners.valueAt(j); - VersionedPackage ai = new VersionedPackage(pkg, uidList[i]); - nlf.removePackage(ai); + VersionedPackage ai = new VersionedPackage(pkg, uidList[i]); + nlf.removePackage(ai); + } } } } @@ -10997,7 +11003,9 @@ public class NotificationManagerService extends SystemService { } NotificationListenerFilter nlf = new NotificationListenerFilter(approved, disallowedPkgs); - mRequestedNotificationListeners.put(Pair.create(cn, userId), nlf); + synchronized (mRequestedNotificationListeners) { + mRequestedNotificationListeners.put(Pair.create(cn, userId), nlf); + } } } } @@ -11005,72 +11013,81 @@ public class NotificationManagerService extends SystemService { @Override protected void writeExtraXmlTags(TypedXmlSerializer out) throws IOException { out.startTag(null, TAG_REQUESTED_LISTENERS); - for (Pair<ComponentName, Integer> listener : mRequestedNotificationListeners.keySet()) { - NotificationListenerFilter nlf = mRequestedNotificationListeners.get(listener); - out.startTag(null, TAG_REQUESTED_LISTENER); - XmlUtils.writeStringAttribute( - out, ATT_COMPONENT, listener.first.flattenToString()); - XmlUtils.writeIntAttribute(out, ATT_USER_ID, listener.second); - - out.startTag(null, TAG_APPROVED); - XmlUtils.writeIntAttribute(out, ATT_TYPES, nlf.getTypes()); - out.endTag(null, TAG_APPROVED); - - for (VersionedPackage ai : nlf.getDisallowedPackages()) { - if (!TextUtils.isEmpty(ai.getPackageName())) { - out.startTag(null, TAG_DISALLOWED); - XmlUtils.writeStringAttribute(out, ATT_PKG, ai.getPackageName()); - XmlUtils.writeIntAttribute(out, ATT_UID, ai.getVersionCode()); - out.endTag(null, TAG_DISALLOWED); + synchronized (mRequestedNotificationListeners) { + for (Pair<ComponentName, Integer> listener : + mRequestedNotificationListeners.keySet()) { + NotificationListenerFilter nlf = mRequestedNotificationListeners.get(listener); + out.startTag(null, TAG_REQUESTED_LISTENER); + XmlUtils.writeStringAttribute( + out, ATT_COMPONENT, listener.first.flattenToString()); + XmlUtils.writeIntAttribute(out, ATT_USER_ID, listener.second); + + out.startTag(null, TAG_APPROVED); + XmlUtils.writeIntAttribute(out, ATT_TYPES, nlf.getTypes()); + out.endTag(null, TAG_APPROVED); + + for (VersionedPackage ai : nlf.getDisallowedPackages()) { + if (!TextUtils.isEmpty(ai.getPackageName())) { + out.startTag(null, TAG_DISALLOWED); + XmlUtils.writeStringAttribute(out, ATT_PKG, ai.getPackageName()); + XmlUtils.writeIntAttribute(out, ATT_UID, ai.getVersionCode()); + out.endTag(null, TAG_DISALLOWED); + } } - } - out.endTag(null, TAG_REQUESTED_LISTENER); + out.endTag(null, TAG_REQUESTED_LISTENER); + } } out.endTag(null, TAG_REQUESTED_LISTENERS); } - protected @Nullable NotificationListenerFilter getNotificationListenerFilter( + @Nullable protected NotificationListenerFilter getNotificationListenerFilter( Pair<ComponentName, Integer> pair) { - return mRequestedNotificationListeners.get(pair); + synchronized (mRequestedNotificationListeners) { + return mRequestedNotificationListeners.get(pair); + } } protected void setNotificationListenerFilter(Pair<ComponentName, Integer> pair, NotificationListenerFilter nlf) { - mRequestedNotificationListeners.put(pair, nlf); + synchronized (mRequestedNotificationListeners) { + mRequestedNotificationListeners.put(pair, nlf); + } } @Override protected void ensureFilters(ServiceInfo si, int userId) { - Pair listener = Pair.create(si.getComponentName(), userId); - NotificationListenerFilter existingNlf = - mRequestedNotificationListeners.get(listener); - if (si.metaData != null) { - if (existingNlf == null) { - // no stored filters for this listener; see if they provided a default - if (si.metaData.containsKey(META_DATA_DEFAULT_FILTER_TYPES)) { - String typeList = - si.metaData.get(META_DATA_DEFAULT_FILTER_TYPES).toString(); - if (typeList != null) { - int types = getTypesFromStringList(typeList); - NotificationListenerFilter nlf = - new NotificationListenerFilter(types, new ArraySet<>()); - mRequestedNotificationListeners.put(listener, nlf); + Pair<ComponentName, Integer> listener = Pair.create(si.getComponentName(), userId); + synchronized (mRequestedNotificationListeners) { + NotificationListenerFilter existingNlf = + mRequestedNotificationListeners.get(listener); + if (si.metaData != null) { + if (existingNlf == null) { + // no stored filters for this listener; see if they provided a default + if (si.metaData.containsKey(META_DATA_DEFAULT_FILTER_TYPES)) { + String typeList = + si.metaData.get(META_DATA_DEFAULT_FILTER_TYPES).toString(); + if (typeList != null) { + int types = getTypesFromStringList(typeList); + NotificationListenerFilter nlf = + new NotificationListenerFilter(types, new ArraySet<>()); + mRequestedNotificationListeners.put(listener, nlf); + } } } - } - // also check the types they never want bridged - if (si.metaData.containsKey(META_DATA_DISABLED_FILTER_TYPES)) { - int neverBridge = getTypesFromStringList(si.metaData.get( - META_DATA_DISABLED_FILTER_TYPES).toString()); - if (neverBridge != 0) { - NotificationListenerFilter nlf = - mRequestedNotificationListeners.getOrDefault( - listener, new NotificationListenerFilter()); - nlf.setTypes(nlf.getTypes() & ~neverBridge); - mRequestedNotificationListeners.put(listener, nlf); + // also check the types they never want bridged + if (si.metaData.containsKey(META_DATA_DISABLED_FILTER_TYPES)) { + int neverBridge = getTypesFromStringList(si.metaData.get( + META_DATA_DISABLED_FILTER_TYPES).toString()); + if (neverBridge != 0) { + NotificationListenerFilter nlf = + mRequestedNotificationListeners.getOrDefault( + listener, new NotificationListenerFilter()); + nlf.setTypes(nlf.getTypes() & ~neverBridge); + mRequestedNotificationListeners.put(listener, nlf); + } } } } diff --git a/services/tests/uiservicestests/src/com/android/server/notification/NotificationListenersTest.java b/services/tests/uiservicestests/src/com/android/server/notification/NotificationListenersTest.java index 90d148856fff..4406d831dd93 100644 --- a/services/tests/uiservicestests/src/com/android/server/notification/NotificationListenersTest.java +++ b/services/tests/uiservicestests/src/com/android/server/notification/NotificationListenersTest.java @@ -71,10 +71,10 @@ import android.service.notification.StatusBarNotification; import android.testing.TestableContext; import android.util.ArraySet; import android.util.Pair; -import com.android.modules.utils.TypedXmlPullParser; -import com.android.modules.utils.TypedXmlSerializer; import android.util.Xml; +import com.android.modules.utils.TypedXmlPullParser; +import com.android.modules.utils.TypedXmlSerializer; import com.android.server.UiServiceTestCase; import com.google.common.collect.ImmutableList; @@ -92,6 +92,7 @@ import java.io.ByteArrayInputStream; import java.io.ByteArrayOutputStream; import java.util.Arrays; import java.util.List; +import java.util.concurrent.CountDownLatch; public class NotificationListenersTest extends UiServiceTestCase { @@ -626,6 +627,58 @@ public class NotificationListenersTest extends UiServiceTestCase { .onNotificationChannelGroupModification(anyString(), any(), any(), anyInt()); } + @Test + public void testNotificationListenerFilter_threadSafety() throws Exception { + testThreadSafety(() -> { + mListeners.setNotificationListenerFilter( + new Pair<>(new ComponentName("pkg1", "cls1"), 0), + new NotificationListenerFilter()); + mListeners.setNotificationListenerFilter( + new Pair<>(new ComponentName("pkg2", "cls2"), 10), + new NotificationListenerFilter()); + mListeners.setNotificationListenerFilter( + new Pair<>(new ComponentName("pkg3", "cls3"), 11), + new NotificationListenerFilter()); + + mListeners.onUserRemoved(10); + mListeners.onPackagesChanged(true, new String[]{"pkg1", "pkg2"}, new int[]{0, 0}); + }, 20, 50); + } + + /** + * Helper method to test the thread safety of some operations. + * + * <p>Runs the supplied {@code operationToTest}, {@code nRunsPerThread} times, + * concurrently using {@code nThreads} threads, and waits for all of them to finish. + */ + private static void testThreadSafety(Runnable operationToTest, int nThreads, + int nRunsPerThread) throws InterruptedException { + 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(); + } + private ManagedServices.ManagedServiceInfo getParcelingListener( final NotificationChannelGroup toParcel) throws RemoteException { |