Merge "Make ManagedServices more thread-safe" into udc-dev am: 0491f3d984 (dd9d8447) · Commits · e / os / android_frameworks_base

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

+69 −42

Original line number	Original line	Diff line number	Diff line
	@@ -140,34 +140,39 @@ abstract public class ManagedServices {
	* The services that have been bound by us. If the service is also connected, it will also		* The services that have been bound by us. If the service is also connected, it will also
	* be in {@link #mServices}.		* be in {@link #mServices}.
	*/		*/
			@GuardedBy("mMutex")
	private final ArrayList<Pair<ComponentName, Integer>> mServicesBound = new ArrayList<>();		private final ArrayList<Pair<ComponentName, Integer>> mServicesBound = new ArrayList<>();
			@GuardedBy("mMutex")
	private final ArraySet<Pair<ComponentName, Integer>> mServicesRebinding = new ArraySet<>();		private final ArraySet<Pair<ComponentName, Integer>> mServicesRebinding = new ArraySet<>();
	// we need these packages to be protected because classes that inherit from it need to see it		// we need these packages to be protected because classes that inherit from it need to see it
	protected final Object mDefaultsLock = new Object();		protected final Object mDefaultsLock = new Object();
			@GuardedBy("mDefaultsLock")
	protected final ArraySet<ComponentName> mDefaultComponents = new ArraySet<>();		protected final ArraySet<ComponentName> mDefaultComponents = new ArraySet<>();
			@GuardedBy("mDefaultsLock")
	protected final ArraySet<String> mDefaultPackages = new ArraySet<>();		protected final ArraySet<String> mDefaultPackages = new ArraySet<>();

	// lists the component names of all enabled (and therefore potentially connected)		// lists the component names of all enabled (and therefore potentially connected)
	// app services for current profiles.		// app services for current profiles.
	private ArraySet<ComponentName> mEnabledServicesForCurrentProfiles		@GuardedBy("mMutex")
	= new ArraySet<>();		private final ArraySet<ComponentName> mEnabledServicesForCurrentProfiles = new ArraySet<>();
	// Just the packages from mEnabledServicesForCurrentProfiles		// Just the packages from mEnabledServicesForCurrentProfiles
	private ArraySet<String> mEnabledServicesPackageNames = new ArraySet<>();		@GuardedBy("mMutex")
			private final ArraySet<String> mEnabledServicesPackageNames = new ArraySet<>();
	// Per user id, list of enabled packages that have nevertheless asked not to be run		// Per user id, list of enabled packages that have nevertheless asked not to be run
	private final android.util.SparseSetArray<ComponentName> mSnoozing =		@GuardedBy("mSnoozing")
	new android.util.SparseSetArray<>();		private final SparseSetArray<ComponentName> mSnoozing = new SparseSetArray<>();

	// List of approved packages or components (by user, then by primary/secondary) that are		// 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		// 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.		// not mean that we are currently bound to said package/component.
			@GuardedBy("mApproved")
	protected final ArrayMap<Integer, ArrayMap<Boolean, ArraySet<String>>> mApproved =		protected final ArrayMap<Integer, ArrayMap<Boolean, ArraySet<String>>> mApproved =
	new ArrayMap<>();		new ArrayMap<>();

	// List of packages or components (by user) that are configured to be enabled/disabled		// List of packages or components (by user) that are configured to be enabled/disabled
	// explicitly by the user		// explicitly by the user
	@GuardedBy("mApproved")		@GuardedBy("mApproved")
	protected ArrayMap<Integer, ArraySet<String>> mUserSetServices = new ArrayMap<>();		protected ArrayMap<Integer, ArraySet<String>> mUserSetServices = new ArrayMap<>();
			@GuardedBy("mApproved")
	protected ArrayMap<Integer, Boolean> mIsUserChanged = new ArrayMap<>();		protected ArrayMap<Integer, Boolean> mIsUserChanged = new ArrayMap<>();

	// True if approved services are stored in xml, not settings.		// True if approved services are stored in xml, not settings.
	@@ -262,20 +267,18 @@ abstract public class ManagedServices {
	@NonNull		@NonNull
	ArrayMap<Boolean, ArrayList<ComponentName>> resetComponents(String packageName, int userId) {		ArrayMap<Boolean, ArrayList<ComponentName>> resetComponents(String packageName, int userId) {
	// components that we want to enable		// components that we want to enable
	ArrayList<ComponentName> componentsToEnable =		ArrayList<ComponentName> componentsToEnable;
	new ArrayList<>(mDefaultComponents.size());

	// components that were removed		// components that were removed
	ArrayList<ComponentName> disabledComponents =		ArrayList<ComponentName> disabledComponents;
	new ArrayList<>(mDefaultComponents.size());

	// all components that are enabled now		// all components that are enabled now
	ArraySet<ComponentName> enabledComponents =		ArraySet<ComponentName> enabledComponents = new ArraySet<>(getAllowedComponents(userId));
	new ArraySet<>(getAllowedComponents(userId));

	boolean changed = false;		boolean changed = false;

	synchronized (mDefaultsLock) {		synchronized (mDefaultsLock) {
			componentsToEnable = new ArrayList<>(mDefaultComponents.size());
			disabledComponents = new ArrayList<>(mDefaultComponents.size());

	// record all components that are enabled but should not be by default		// record all components that are enabled but should not be by default
	for (int i = 0; i < mDefaultComponents.size() && enabledComponents.size() > 0; i++) {		for (int i = 0; i < mDefaultComponents.size() && enabledComponents.size() > 0; i++) {
	ComponentName currentDefault = mDefaultComponents.valueAt(i);		ComponentName currentDefault = mDefaultComponents.valueAt(i);
	@@ -374,6 +377,7 @@ abstract public class ManagedServices {
	}		}
	}		}

			synchronized (mMutex) {
	pw.println(" All " + getCaption() + "s (" + mEnabledServicesForCurrentProfiles.size()		pw.println(" All " + getCaption() + "s (" + mEnabledServicesForCurrentProfiles.size()
	+ ") enabled for current profiles:");		+ ") enabled for current profiles:");
	for (ComponentName cmpt : mEnabledServicesForCurrentProfiles) {		for (ComponentName cmpt : mEnabledServicesForCurrentProfiles) {
	@@ -381,7 +385,6 @@ abstract public class ManagedServices {
	pw.println(" " + cmpt);		pw.println(" " + cmpt);
	}		}

	synchronized (mMutex) {
	pw.println(" Live " + getCaption() + "s (" + mServices.size() + "):");		pw.println(" Live " + getCaption() + "s (" + mServices.size() + "):");
	for (ManagedServiceInfo info : mServices) {		for (ManagedServiceInfo info : mServices) {
	if (filter != null && !filter.matches(info.component)) continue;		if (filter != null && !filter.matches(info.component)) continue;
	@@ -434,12 +437,12 @@ abstract public class ManagedServices {
	}		}
	}		}


			synchronized (mMutex) {
	for (ComponentName cmpt : mEnabledServicesForCurrentProfiles) {		for (ComponentName cmpt : mEnabledServicesForCurrentProfiles) {
	if (filter != null && !filter.matches(cmpt)) continue;		if (filter != null && !filter.matches(cmpt)) continue;
	cmpt.dumpDebug(proto, ManagedServicesProto.ENABLED);		cmpt.dumpDebug(proto, ManagedServicesProto.ENABLED);
	}		}

	synchronized (mMutex) {
	for (ManagedServiceInfo info : mServices) {		for (ManagedServiceInfo info : mServices) {
	if (filter != null && !filter.matches(info.component)) continue;		if (filter != null && !filter.matches(info.component)) continue;
	info.dumpDebug(proto, ManagedServicesProto.LIVE_SERVICES, this);		info.dumpDebug(proto, ManagedServicesProto.LIVE_SERVICES, this);
	@@ -677,7 +680,9 @@ abstract public class ManagedServices {
	if (isUserChanged == null) { //NLS		if (isUserChanged == null) { //NLS
	userSetComponent = TextUtils.emptyIfNull(userSetComponent);		userSetComponent = TextUtils.emptyIfNull(userSetComponent);
	} else { //NAS		} else { //NAS
			synchronized (mApproved) {
	mIsUserChanged.put(resolvedUserId, Boolean.valueOf(isUserChanged));		mIsUserChanged.put(resolvedUserId, Boolean.valueOf(isUserChanged));
			}
	userSetComponent = Boolean.valueOf(isUserChanged) ? approved : "";		userSetComponent = Boolean.valueOf(isUserChanged) ? approved : "";
	}		}
	} else {		} else {
	@@ -688,7 +693,9 @@ abstract public class ManagedServices {
	if (isUserChanged_Old != null && Boolean.valueOf(isUserChanged_Old)) {		if (isUserChanged_Old != null && Boolean.valueOf(isUserChanged_Old)) {
	//user_set = true		//user_set = true
	userSetComponent = approved;		userSetComponent = approved;
			synchronized (mApproved) {
	mIsUserChanged.put(resolvedUserId, true);		mIsUserChanged.put(resolvedUserId, true);
			}
	needUpgradeUserset = false;		needUpgradeUserset = false;
	} else {		} else {
	userSetComponent = "";		userSetComponent = "";
	@@ -724,8 +731,11 @@ abstract public class ManagedServices {
	}		}

	void upgradeDefaultsXmlVersion() {		void upgradeDefaultsXmlVersion() {
			int defaultsSize;
			synchronized (mDefaultsLock) {
	// check if any defaults are loaded		// check if any defaults are loaded
	int defaultsSize = mDefaultComponents.size() + mDefaultPackages.size();		defaultsSize = mDefaultComponents.size() + mDefaultPackages.size();
			}
	if (defaultsSize == 0) {		if (defaultsSize == 0) {
	// load defaults from current allowed		// load defaults from current allowed
	if (this.mApprovalLevel == APPROVAL_BY_COMPONENT) {		if (this.mApprovalLevel == APPROVAL_BY_COMPONENT) {
	@@ -741,8 +751,10 @@ abstract public class ManagedServices {
	}		}
	}		}
	}		}
	// if no defaults are loaded, then load from config		synchronized (mDefaultsLock) {
	defaultsSize = mDefaultComponents.size() + mDefaultPackages.size();		defaultsSize = mDefaultComponents.size() + mDefaultPackages.size();
			}
			// if no defaults are loaded, then load from config
	if (defaultsSize == 0) {		if (defaultsSize == 0) {
	loadDefaultsFromConfig();		loadDefaultsFromConfig();
	}		}
	@@ -806,8 +818,10 @@ abstract public class ManagedServices {
	}		}

	protected boolean isComponentEnabledForPackage(String pkg) {		protected boolean isComponentEnabledForPackage(String pkg) {
			synchronized (mMutex) {
	return mEnabledServicesPackageNames.contains(pkg);		return mEnabledServicesPackageNames.contains(pkg);
	}		}
			}

	protected void setPackageOrComponentEnabled(String pkgOrComponent, int userId,		protected void setPackageOrComponentEnabled(String pkgOrComponent, int userId,
	boolean isPrimary, boolean enabled) {		boolean isPrimary, boolean enabled) {
	@@ -959,9 +973,13 @@ abstract public class ManagedServices {
	}		}

	public void onPackagesChanged(boolean removingPackage, String[] pkgList, int[] uidList) {		public void onPackagesChanged(boolean removingPackage, String[] pkgList, int[] uidList) {
	if (DEBUG) Slog.d(TAG, "onPackagesChanged removingPackage=" + removingPackage		if (DEBUG) {
			synchronized (mMutex) {
			Slog.d(TAG, "onPackagesChanged removingPackage=" + removingPackage
	+ " pkgList=" + (pkgList == null ? null : Arrays.asList(pkgList))		+ " pkgList=" + (pkgList == null ? null : Arrays.asList(pkgList))
	+ " mEnabledServicesPackageNames=" + mEnabledServicesPackageNames);		+ " mEnabledServicesPackageNames=" + mEnabledServicesPackageNames);
			}
			}

	if (pkgList != null && (pkgList.length > 0)) {		if (pkgList != null && (pkgList.length > 0)) {
	boolean anyServicesInvolved = false;		boolean anyServicesInvolved = false;
	@@ -975,7 +993,7 @@ abstract public class ManagedServices {
	}		}
	}		}
	for (String pkgName : pkgList) {		for (String pkgName : pkgList) {
	if (mEnabledServicesPackageNames.contains(pkgName)) {		if (isComponentEnabledForPackage(pkgName)) {
	anyServicesInvolved = true;		anyServicesInvolved = true;
	}		}
	if (uidList != null && uidList.length > 0) {		if (uidList != null && uidList.length > 0) {
	@@ -1299,10 +1317,12 @@ abstract public class ManagedServices {
	}		}

	final Set<ComponentName> add = new HashSet<>(userComponents);		final Set<ComponentName> add = new HashSet<>(userComponents);
			synchronized (mSnoozing) {
	ArraySet<ComponentName> snoozed = mSnoozing.get(userId);		ArraySet<ComponentName> snoozed = mSnoozing.get(userId);
	if (snoozed != null) {		if (snoozed != null) {
	add.removeAll(snoozed);		add.removeAll(snoozed);
	}		}
			}

	componentsToBind.put(userId, add);		componentsToBind.put(userId, add);

	@@ -1605,10 +1625,13 @@ abstract public class ManagedServices {
	}		}
	}		}

			@VisibleForTesting
	boolean isBound(ComponentName cn, int userId) {		boolean isBound(ComponentName cn, int userId) {
	final Pair<ComponentName, Integer> servicesBindingTag = Pair.create(cn, userId);		final Pair<ComponentName, Integer> servicesBindingTag = Pair.create(cn, userId);
			synchronized (mMutex) {
	return mServicesBound.contains(servicesBindingTag);		return mServicesBound.contains(servicesBindingTag);
	}		}
			}

	protected boolean isBoundOrRebinding(final ComponentName cn, final int userId) {		protected boolean isBoundOrRebinding(final ComponentName cn, final int userId) {
	synchronized (mMutex) {		synchronized (mMutex) {
	@@ -1833,8 +1856,10 @@ abstract public class ManagedServices {
	public boolean isEnabledForCurrentProfiles() {		public boolean isEnabledForCurrentProfiles() {
	if (this.isSystem) return true;		if (this.isSystem) return true;
	if (this.connection == null) return false;		if (this.connection == null) return false;
			synchronized (mMutex) {
	return mEnabledServicesForCurrentProfiles.contains(this.component);		return mEnabledServicesForCurrentProfiles.contains(this.component);
	}		}
			}

	/**		/**
	* Returns true if this service is allowed to receive events for the given userId. A		* Returns true if this service is allowed to receive events for the given userId. A
	@@ -1877,8 +1902,10 @@ abstract public class ManagedServices {

	/** convenience method for looking in mEnabledServicesForCurrentProfiles */		/** convenience method for looking in mEnabledServicesForCurrentProfiles */
	public boolean isComponentEnabledForCurrentProfiles(ComponentName component) {		public boolean isComponentEnabledForCurrentProfiles(ComponentName component) {
			synchronized (mMutex) {
	return mEnabledServicesForCurrentProfiles.contains(component);		return mEnabledServicesForCurrentProfiles.contains(component);
	}		}
			}

	public static class UserProfiles {		public static class UserProfiles {
	// Profiles of the current user.		// Profiles of the current user.

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

+47 −0

Original line number	Original line	Diff line number	Diff line
	@@ -96,6 +96,7 @@ import java.util.Collection;
	import java.util.Collections;		import java.util.Collections;
	import java.util.List;		import java.util.List;
	import java.util.Set;		import java.util.Set;
			import java.util.concurrent.CountDownLatch;

	public class ManagedServicesTest extends UiServiceTestCase {		public class ManagedServicesTest extends UiServiceTestCase {

	@@ -1920,6 +1921,18 @@ public class ManagedServicesTest extends UiServiceTestCase {
	assertTrue(service.isBound(cn_disallowed, 0));		assertTrue(service.isBound(cn_disallowed, 0));
	}		}

			@Test
			public void isComponentEnabledForCurrentProfiles_isThreadSafe() throws InterruptedException {
			for (UserInfo userInfo : mUm.getUsers()) {
			mService.addApprovedList("pkg1/cmp1:pkg2/cmp2:pkg3/cmp3", userInfo.id, true);
			}
			testThreadSafety(() -> {
			mService.rebindServices(false, 0);
			assertThat(mService.isComponentEnabledForCurrentProfiles(
			new ComponentName("pkg1", "cmp1"))).isTrue();
			}, 20, 30);
			}

	private void mockServiceInfoWithMetaData(List<ComponentName> componentNames,		private void mockServiceInfoWithMetaData(List<ComponentName> componentNames,
	ManagedServices service, ArrayMap<ComponentName, Bundle> metaDatas)		ManagedServices service, ArrayMap<ComponentName, Bundle> metaDatas)
	throws RemoteException {		throws RemoteException {
	@@ -2298,4 +2311,38 @@ public class ManagedServicesTest extends UiServiceTestCase {
	return false;		return false;
	}		}
	}		}

			/**
			* 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();
			}
	}		}