Fix thread-safety issues with mRequestedNotificationListeners (a83b60f6) · Commits · e / os / android_frameworks_base

services/core/java/com/android/server/notification/NotificationManagerService.java

+89 −72

Original line number	Diff line number	Diff line
		@@ -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,17 +10915,20 @@ public class NotificationManagerService extends SystemService {
		@Override
		public void onUserRemoved(int user) {
		super.onUserRemoved(user);
		synchronized (mRequestedNotificationListeners) {
		for (int i = mRequestedNotificationListeners.size() - 1; i >= 0; i--) {
		if (mRequestedNotificationListeners.keyAt(i).second == user) {
		mRequestedNotificationListeners.removeAt(i);
		}
		}
		}
		}

		@Override
		public void onPackagesChanged(boolean removingPackage, String[] pkgList, int[] uidList) {
		super.onPackagesChanged(removingPackage, pkgList, uidList);

		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.
		@@ -10933,7 +10937,8 @@ public class NotificationManagerService extends SystemService {
		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);
		Pair<ComponentName, Integer> key =
		mRequestedNotificationListeners.keyAt(j);
		if (key.second == userId && key.first.getPackageName().equals(pkg)) {
		mRequestedNotificationListeners.removeAt(j);
		}
		@@ -10944,15 +10949,16 @@ public class NotificationManagerService extends SystemService {
		// 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);
		NotificationListenerFilter nlf =
		mRequestedNotificationListeners.valueAt(j);

		VersionedPackage ai = new VersionedPackage(pkg, uidList[i]);
		nlf.removePackage(ai);
		}
		}
		}
		}

		@Override
		protected String getRequiredPermission() {
		@@ -10997,15 +11003,19 @@ public class NotificationManagerService extends SystemService {
		}
		NotificationListenerFilter nlf =
		new NotificationListenerFilter(approved, disallowedPkgs);
		synchronized (mRequestedNotificationListeners) {
		mRequestedNotificationListeners.put(Pair.create(cn, userId), nlf);
		}
		}
		}
		}

		@Override
		protected void writeExtraXmlTags(TypedXmlSerializer out) throws IOException {
		out.startTag(null, TAG_REQUESTED_LISTENERS);
		for (Pair<ComponentName, Integer> listener : mRequestedNotificationListeners.keySet()) {
		synchronized (mRequestedNotificationListeners) {
		for (Pair<ComponentName, Integer> listener :
		mRequestedNotificationListeners.keySet()) {
		NotificationListenerFilter nlf = mRequestedNotificationListeners.get(listener);
		out.startTag(null, TAG_REQUESTED_LISTENER);
		XmlUtils.writeStringAttribute(
		@@ -11027,23 +11037,29 @@ public class NotificationManagerService extends SystemService {

		out.endTag(null, TAG_REQUESTED_LISTENER);
		}
		}

		out.endTag(null, TAG_REQUESTED_LISTENERS);
		}

		protected @Nullable NotificationListenerFilter getNotificationListenerFilter(
		@Nullable protected NotificationListenerFilter getNotificationListenerFilter(
		Pair<ComponentName, Integer> pair) {
		synchronized (mRequestedNotificationListeners) {
		return mRequestedNotificationListeners.get(pair);
		}
		}

		protected void setNotificationListenerFilter(Pair<ComponentName, Integer> pair,
		NotificationListenerFilter nlf) {
		synchronized (mRequestedNotificationListeners) {
		mRequestedNotificationListeners.put(pair, nlf);
		}
		}

		@Override
		protected void ensureFilters(ServiceInfo si, int userId) {
		Pair listener = Pair.create(si.getComponentName(), userId);
		Pair<ComponentName, Integer> listener = Pair.create(si.getComponentName(), userId);
		synchronized (mRequestedNotificationListeners) {
		NotificationListenerFilter existingNlf =
		mRequestedNotificationListeners.get(listener);
		if (si.metaData != null) {
		@@ -11075,6 +11091,7 @@ public class NotificationManagerService extends SystemService {
		}
		}
		}
		}

		private int getTypesFromStringList(String typeList) {
		int types = 0;

services/tests/uiservicestests/src/com/android/server/notification/NotificationListenersTest.java

+55 −2

Original line number	Diff line number	Diff line
		@@ -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 {