Android/frameworks_base
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Merge "Fix thread-safety issues with mRequestedNotificationListeners" into udc-dev

Browse Source
This commit is contained in:
Matías Hernández
2023-04-18 12:38:39 +00:00
committed by Android (Google) Code Review
parent 17c8bbca2e a83b60f648
commit ca08538cd3
2 changed files with 142 additions and 72 deletions
Download Patch File Download Diff File Expand all files Collapse all files

157
services/core/java/com/android/server/notification/NotificationManagerService.java
View File

@@ -10796,7 +10796,8 @@ public class NotificationManagerService extends SystemService {
static final String FLAG_SEPARATOR = "\\|"; static final String FLAG_SEPARATOR = "\\|";
private final ArraySet<ManagedServiceInfo> mLightTrimListeners = new ArraySet<>(); private final ArraySet<ManagedServiceInfo> mLightTrimListeners = new ArraySet<>();
ArrayMap<Pair<ComponentName, Integer>, NotificationListenerFilter> @GuardedBy("mRequestedNotificationListeners")
private final ArrayMap<Pair<ComponentName, Integer>, NotificationListenerFilter>
mRequestedNotificationListeners = new ArrayMap<>(); mRequestedNotificationListeners = new ArrayMap<>();
private final boolean mIsHeadlessSystemUserMode; private final boolean mIsHeadlessSystemUserMode;
@@ -10914,9 +10915,11 @@ public class NotificationManagerService extends SystemService {
@Override @Override
public void onUserRemoved(int user) { public void onUserRemoved(int user) {
super.onUserRemoved(user); super.onUserRemoved(user);
for (int i = mRequestedNotificationListeners.size() - 1; i >= 0; i--) { synchronized (mRequestedNotificationListeners) {
if (mRequestedNotificationListeners.keyAt(i).second == user) { for (int i = mRequestedNotificationListeners.size() - 1; i >= 0; i--) {
mRequestedNotificationListeners.removeAt(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) { public void onPackagesChanged(boolean removingPackage, String[] pkgList, int[] uidList) {
super.onPackagesChanged(removingPackage, pkgList, uidList); super.onPackagesChanged(removingPackage, pkgList, uidList);
// Since the default behavior is to allow everything, we don't need to explicitly synchronized (mRequestedNotificationListeners) {
// handle package add or update. they will be added to the xml file on next boot or // Since the default behavior is to allow everything, we don't need to explicitly
// when the user tries to change the settings. // handle package add or update. they will be added to the xml file on next boot or
if (removingPackage) { // when the user tries to change the settings.
for (int i = 0; i < pkgList.length; i++) { if (removingPackage) {
String pkg = pkgList[i]; for (int i = 0; i < pkgList.length; i++) {
int userId = UserHandle.getUserId(uidList[i]); String pkg = pkgList[i];
for (int j = mRequestedNotificationListeners.size() - 1; j >= 0; j--) { int userId = UserHandle.getUserId(uidList[i]);
Pair<ComponentName, Integer> key = mRequestedNotificationListeners.keyAt(j); for (int j = mRequestedNotificationListeners.size() - 1; j >= 0; j--) {
if (key.second == userId && key.first.getPackageName().equals(pkg)) { Pair<ComponentName, Integer> key =
mRequestedNotificationListeners.removeAt(j); mRequestedNotificationListeners.keyAt(j);
if (key.second == userId && key.first.getPackageName().equals(pkg)) {
mRequestedNotificationListeners.removeAt(j);
}
} }
} }
} }
}
// clean up anything in the disallowed pkgs list // clean up anything in the disallowed pkgs list
for (int i = 0; i < pkgList.length; i++) { for (int i = 0; i < pkgList.length; i++) {
String pkg = pkgList[i]; String pkg = pkgList[i];
int userId = UserHandle.getUserId(uidList[i]); for (int j = mRequestedNotificationListeners.size() - 1; j >= 0; j--) {
for (int j = mRequestedNotificationListeners.size() - 1; j >= 0; j--) { NotificationListenerFilter nlf =
NotificationListenerFilter nlf = mRequestedNotificationListeners.valueAt(j); mRequestedNotificationListeners.valueAt(j);
VersionedPackage ai = new VersionedPackage(pkg, uidList[i]); VersionedPackage ai = new VersionedPackage(pkg, uidList[i]);
nlf.removePackage(ai); nlf.removePackage(ai);
}
} }
} }
} }
@@ -10997,7 +11003,9 @@ public class NotificationManagerService extends SystemService {
} }
NotificationListenerFilter nlf = NotificationListenerFilter nlf =
new NotificationListenerFilter(approved, disallowedPkgs); 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 @Override
protected void writeExtraXmlTags(TypedXmlSerializer out) throws IOException { protected void writeExtraXmlTags(TypedXmlSerializer out) throws IOException {
out.startTag(null, TAG_REQUESTED_LISTENERS); out.startTag(null, TAG_REQUESTED_LISTENERS);
for (Pair<ComponentName, Integer> listener : mRequestedNotificationListeners.keySet()) { synchronized (mRequestedNotificationListeners) {
NotificationListenerFilter nlf = mRequestedNotificationListeners.get(listener); for (Pair<ComponentName, Integer> listener :
out.startTag(null, TAG_REQUESTED_LISTENER); mRequestedNotificationListeners.keySet()) {
XmlUtils.writeStringAttribute( NotificationListenerFilter nlf = mRequestedNotificationListeners.get(listener);
out, ATT_COMPONENT, listener.first.flattenToString()); out.startTag(null, TAG_REQUESTED_LISTENER);
XmlUtils.writeIntAttribute(out, ATT_USER_ID, listener.second); XmlUtils.writeStringAttribute(
out, ATT_COMPONENT, listener.first.flattenToString());
XmlUtils.writeIntAttribute(out, ATT_USER_ID, listener.second);
out.startTag(null, TAG_APPROVED); out.startTag(null, TAG_APPROVED);
XmlUtils.writeIntAttribute(out, ATT_TYPES, nlf.getTypes()); XmlUtils.writeIntAttribute(out, ATT_TYPES, nlf.getTypes());
out.endTag(null, TAG_APPROVED); out.endTag(null, TAG_APPROVED);
for (VersionedPackage ai : nlf.getDisallowedPackages()) { for (VersionedPackage ai : nlf.getDisallowedPackages()) {
if (!TextUtils.isEmpty(ai.getPackageName())) { if (!TextUtils.isEmpty(ai.getPackageName())) {
out.startTag(null, TAG_DISALLOWED); out.startTag(null, TAG_DISALLOWED);
XmlUtils.writeStringAttribute(out, ATT_PKG, ai.getPackageName()); XmlUtils.writeStringAttribute(out, ATT_PKG, ai.getPackageName());
XmlUtils.writeIntAttribute(out, ATT_UID, ai.getVersionCode()); XmlUtils.writeIntAttribute(out, ATT_UID, ai.getVersionCode());
out.endTag(null, TAG_DISALLOWED); out.endTag(null, TAG_DISALLOWED);
}
} }
}
out.endTag(null, TAG_REQUESTED_LISTENER); out.endTag(null, TAG_REQUESTED_LISTENER);
}
} }
out.endTag(null, TAG_REQUESTED_LISTENERS); out.endTag(null, TAG_REQUESTED_LISTENERS);
} }
protected @Nullable NotificationListenerFilter getNotificationListenerFilter( @Nullable protected NotificationListenerFilter getNotificationListenerFilter(
Pair<ComponentName, Integer> pair) { Pair<ComponentName, Integer> pair) {
return mRequestedNotificationListeners.get(pair); synchronized (mRequestedNotificationListeners) {
return mRequestedNotificationListeners.get(pair);
}
} }
protected void setNotificationListenerFilter(Pair<ComponentName, Integer> pair, protected void setNotificationListenerFilter(Pair<ComponentName, Integer> pair,
NotificationListenerFilter nlf) { NotificationListenerFilter nlf) {
mRequestedNotificationListeners.put(pair, nlf); synchronized (mRequestedNotificationListeners) {
mRequestedNotificationListeners.put(pair, nlf);
}
} }
@Override @Override
protected void ensureFilters(ServiceInfo si, int userId) { protected void ensureFilters(ServiceInfo si, int userId) {
Pair listener = Pair.create(si.getComponentName(), userId); Pair<ComponentName, Integer> listener = Pair.create(si.getComponentName(), userId);
NotificationListenerFilter existingNlf = synchronized (mRequestedNotificationListeners) {
mRequestedNotificationListeners.get(listener); NotificationListenerFilter existingNlf =
if (si.metaData != null) { mRequestedNotificationListeners.get(listener);
if (existingNlf == null) { if (si.metaData != null) {
// no stored filters for this listener; see if they provided a default if (existingNlf == null) {
if (si.metaData.containsKey(META_DATA_DEFAULT_FILTER_TYPES)) { // no stored filters for this listener; see if they provided a default
String typeList = if (si.metaData.containsKey(META_DATA_DEFAULT_FILTER_TYPES)) {
si.metaData.get(META_DATA_DEFAULT_FILTER_TYPES).toString(); String typeList =
if (typeList != null) { si.metaData.get(META_DATA_DEFAULT_FILTER_TYPES).toString();
int types = getTypesFromStringList(typeList); if (typeList != null) {
NotificationListenerFilter nlf = int types = getTypesFromStringList(typeList);
new NotificationListenerFilter(types, new ArraySet<>()); NotificationListenerFilter nlf =
mRequestedNotificationListeners.put(listener, nlf); new NotificationListenerFilter(types, new ArraySet<>());
mRequestedNotificationListeners.put(listener, nlf);
}
} }
} }
}
// also check the types they never want bridged // also check the types they never want bridged
if (si.metaData.containsKey(META_DATA_DISABLED_FILTER_TYPES)) { if (si.metaData.containsKey(META_DATA_DISABLED_FILTER_TYPES)) {
int neverBridge = getTypesFromStringList(si.metaData.get( int neverBridge = getTypesFromStringList(si.metaData.get(
META_DATA_DISABLED_FILTER_TYPES).toString()); META_DATA_DISABLED_FILTER_TYPES).toString());
if (neverBridge != 0) { if (neverBridge != 0) {
NotificationListenerFilter nlf = NotificationListenerFilter nlf =
mRequestedNotificationListeners.getOrDefault( mRequestedNotificationListeners.getOrDefault(
listener, new NotificationListenerFilter()); listener, new NotificationListenerFilter());
nlf.setTypes(nlf.getTypes() & ~neverBridge); nlf.setTypes(nlf.getTypes() & ~neverBridge);
mRequestedNotificationListeners.put(listener, nlf); mRequestedNotificationListeners.put(listener, nlf);
}
} }
} }
} }

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

@@ -71,10 +71,10 @@ import android.service.notification.StatusBarNotification;
import android.testing.TestableContext; import android.testing.TestableContext;
import android.util.ArraySet; import android.util.ArraySet;
import android.util.Pair; import android.util.Pair;
import com.android.modules.utils.TypedXmlPullParser;
import com.android.modules.utils.TypedXmlSerializer;
import android.util.Xml; import android.util.Xml;
import com.android.modules.utils.TypedXmlPullParser;
import com.android.modules.utils.TypedXmlSerializer;
import com.android.server.UiServiceTestCase; import com.android.server.UiServiceTestCase;
import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableList;
@@ -92,6 +92,7 @@ import java.io.ByteArrayInputStream;
import java.io.ByteArrayOutputStream; import java.io.ByteArrayOutputStream;
import java.util.Arrays; import java.util.Arrays;
import java.util.List; import java.util.List;
import java.util.concurrent.CountDownLatch;
public class NotificationListenersTest extends UiServiceTestCase { public class NotificationListenersTest extends UiServiceTestCase {
@@ -626,6 +627,58 @@ public class NotificationListenersTest extends UiServiceTestCase {
.onNotificationChannelGroupModification(anyString(), any(), any(), anyInt()); .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( private ManagedServices.ManagedServiceInfo getParcelingListener(
final NotificationChannelGroup toParcel) final NotificationChannelGroup toParcel)
throws RemoteException { throws RemoteException {