commit a62c25ab87bb8a1f8b7f2b1d16337bd2dece4f50
author TYM Tsai <tymtsai@google.com> Mon Apr 28 12:35:49 2025 +0000
committer TYM Tsai <tymtsai@google.com> Thu May 08 07:05:53 2025 +0000
tree 637d9d1bfa2d136d808c8eea5e6859f30bfd3a17
parent a090aa24177b9099d44ede50ccab5791bd9e7b66

Fix deadlock between PMS and UMS threads

Resolve circular waiting in PMS side. It holds mLock in
setEnabledSettings(), then calls
getComponentEnabledSettingInternal() -> UMS to ask mUsersLock.
To fix, move the method out of mLock block

Symptom:
PMS a: hold mSnapshotLock ->  ask PMS:mLock
PMS b: hold mLock         ->  ask UMS:mUsersLock
UMS: hold mUsersLock      ->  ask PMS:mSnapshotLock

Bug: 414214527
Flag: EXEMPT bug fix
Test: atest CtsPackageInstallTestCases
Test: atest CtsPackageManagerTestCases
Test: atest CtsPackageManagerStatsHostTestCases
Change-Id: Ibad1f72477bc5b04fc1df129f593dc087272f3ee

services/core/java/com/android/server/pm/PackageManagerService.java

diff --git a/services/core/java/com/android/server/pm/PackageManagerService.java b/services/core/java/com/android/server/pm/PackageManagerService.java
index 2035dda..444158b 100644
--- a/services/core/java/com/android/server/pm/PackageManagerService.java
+++ b/services/core/java/com/android/server/pm/PackageManagerService.java
@@ -169,6 +169,7 @@
 import android.util.DisplayMetrics;
 import android.util.EventLog;
 import android.util.ExceptionUtils;
+import android.util.IntArray;
 import android.util.Log;
 import android.util.Pair;
 import android.util.Slog;
@@ -4060,14 +4061,13 @@
 
         // packageName -> list of components to send broadcasts now
         final ArrayMap<String, ArrayList<String>> sendNowBroadcasts = new ArrayMap<>(targetSize);
-        final List<PackageMetrics.ComponentStateMetrics> componentStateMetricsList =
-                new ArrayList<PackageMetrics.ComponentStateMetrics>();
+        final IntArray changedComponentIndices = new IntArray();
         boolean scheduleBroadcastMessage = false;
         boolean isSynchronous = false;
-        synchronized (mLock) {
-            Computer computer = snapshotComputer();
-            boolean anyChanged = false;
 
+        Computer computer = snapshotComputer();
+        boolean anyChanged = false;
+        synchronized (mLock) {
             for (int i = 0; i < targetSize; i++) {
                 if (!updateAllowed[i]) {
                     continue;
@@ -4076,18 +4076,13 @@
                 final ComponentEnabledSetting setting = settings.get(i);
                 final String packageName = setting.getPackageName();
                 final PackageSetting packageSetting = pkgSettings.get(packageName);
-                final PackageMetrics.ComponentStateMetrics componentStateMetrics =
-                        new PackageMetrics.ComponentStateMetrics(setting,
-                                UserHandle.getUid(userId, packageSetting.getAppId()),
-                                setting.isComponent() ? computer.getComponentEnabledSettingInternal(
-                                        setting.getComponentName(), callingUid, userId)
-                                        : packageSetting.getEnabled(userId), callingUid);
                 if (!setEnabledSettingInternalLocked(computer, packageSetting, setting, userId,
                         callingPackage)) {
                     continue;
                 }
+
                 anyChanged = true;
-                componentStateMetricsList.add(componentStateMetrics);
+                changedComponentIndices.add(i);
 
                 if ((setting.getEnabledFlags() & PackageManager.SYNCHRONOUS) != 0) {
                     isSynchronous = true;
@@ -4108,7 +4103,7 @@
                 } else {
                     mPendingBroadcasts.addComponent(userId, packageName, componentName);
                     Trace.instant(Trace.TRACE_TAG_PACKAGE_MANAGER, "setEnabledSetting broadcast: "
-                                   + componentName + ": " + setting.getEnabledState());
+                            + componentName + ": " + setting.getEnabledState());
                     scheduleBroadcastMessage = true;
                 }
             }
@@ -4140,6 +4135,27 @@
             }
         }
 
+        final List<PackageMetrics.ComponentStateMetrics> componentStateMetricsList =
+                new ArrayList<PackageMetrics.ComponentStateMetrics>();
+        final int listSize = changedComponentIndices.size();
+        for (int i = 0; i < listSize; i++) {
+            final int index = changedComponentIndices.get(i);
+            final ComponentEnabledSetting setting = settings.get(index);
+            final String packageName = setting.getPackageName();
+            final PackageStateInternal psi = computer.getPackageStateInternal(packageName);
+            // Gets component state from the old snapshot which was taken before the state change,
+            // because the metrics want to use the old state
+            final int componentOldState =
+                    setting.isComponent() ? computer.getComponentEnabledSettingInternal(
+                            setting.getComponentName(), callingUid, userId)
+                            : psi.getUserStateOrDefault(userId).getEnabledState();
+            final PackageMetrics.ComponentStateMetrics componentStateMetrics =
+                    new PackageMetrics.ComponentStateMetrics(setting,
+                            UserHandle.getUid(userId, psi.getAppId()),
+                            componentOldState, callingUid);
+            componentStateMetricsList.add(componentStateMetrics);
+        }
+
         // Log the metrics when the component state is changed.
         PackageMetrics.reportComponentStateChanged(snapshotComputer(), componentStateMetricsList,
                 userId);