From a30b2fc5b3391ffdd9e27f0a55253a0f2bc903b0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mat=C3=ADas=20Hern=C3=A1ndez?= Date: Thu, 26 Jan 2023 22:59:18 +0100 Subject: [PATCH] Improve thread safety of ManagedServices.java This resolves all GuardedBy ErrorProne warnings in this class. Fixes: 242346339 Test: N/A Change-Id: Ibc191e9b4a70b59b609abc7d61cc0cedc4c8fd25 --- .../server/notification/ManagedServices.java | 21 +++++++++++++------ 1 file changed, 15 insertions(+), 6 deletions(-) diff --git a/services/core/java/com/android/server/notification/ManagedServices.java b/services/core/java/com/android/server/notification/ManagedServices.java index 39acaeec89264..25fefad8042f6 100644 --- a/services/core/java/com/android/server/notification/ManagedServices.java +++ b/services/core/java/com/android/server/notification/ManagedServices.java @@ -132,6 +132,7 @@ abstract public class ManagedServices { // contains connections to all connected services, including app services // and system services + @GuardedBy("mMutex") private final ArrayList mServices = new ArrayList<>(); /** * The services that have been bound by us. If the service is also connected, it will also @@ -157,7 +158,8 @@ abstract public class ManagedServices { // List of approved packages or components (by user, then by primary/secondary) that are // allowed to be bound as managed services. A package or component appearing in this list does // not mean that we are currently bound to said package/component. - protected ArrayMap>> mApproved = new ArrayMap<>(); + protected final ArrayMap>> mApproved = + new ArrayMap<>(); // List of packages or components (by user) that are configured to be enabled/disabled // explicitly by the user @@ -316,6 +318,7 @@ abstract public class ManagedServices { return changes; } + @GuardedBy("mApproved") private boolean clearUserSetFlagLocked(ComponentName component, int userId) { String approvedValue = getApprovedValue(component.flattenToString()); ArraySet userSet = mUserSetServices.get(userId); @@ -376,8 +379,8 @@ abstract public class ManagedServices { pw.println(" " + cmpt); } - pw.println(" Live " + getCaption() + "s (" + mServices.size() + "):"); synchronized (mMutex) { + pw.println(" Live " + getCaption() + "s (" + mServices.size() + "):"); for (ManagedServiceInfo info : mServices) { if (filter != null && !filter.matches(info.component)) continue; pw.println(" " + info.component @@ -1011,10 +1014,12 @@ abstract public class ManagedServices { return null; } final IBinder token = service.asBinder(); - final int N = mServices.size(); - for (int i = 0; i < N; i++) { - final ManagedServiceInfo info = mServices.get(i); - if (info.service.asBinder() == token) return info; + synchronized (mMutex) { + final int nServices = mServices.size(); + for (int i = 0; i < nServices; i++) { + final ManagedServiceInfo info = mServices.get(i); + if (info.service.asBinder() == token) return info; + } } return null; } @@ -1488,10 +1493,12 @@ abstract public class ManagedServices { } } + @GuardedBy("mMutex") private void registerServiceLocked(final ComponentName name, final int userid) { registerServiceLocked(name, userid, false /* isSystem */); } + @GuardedBy("mMutex") private void registerServiceLocked(final ComponentName name, final int userid, final boolean isSystem) { if (DEBUG) Slog.v(TAG, "registerService: " + name + " u=" + userid); @@ -1622,6 +1629,7 @@ abstract public class ManagedServices { } } + @GuardedBy("mMutex") private void unregisterServiceLocked(ComponentName name, int userid) { final int N = mServices.size(); for (int i = N - 1; i >= 0; i--) { @@ -1656,6 +1664,7 @@ abstract public class ManagedServices { return serviceInfo; } + @GuardedBy("mMutex") private ManagedServiceInfo removeServiceLocked(int i) { final ManagedServiceInfo info = mServices.remove(i); onServiceRemovedLocked(info);