commit 072bc7d6a808c91f38eb41adb392cf46e57476ad
author panhaihui <panhaihui@xiaomi.com> Thu Sep 12 19:25:38 2024 +0800
committer Sudheer Shanka <sudheersai@google.com> Mon Sep 16 17:57:27 2024 +0000
tree f3b73c41cbe542349d8359a44cf4cbfbfd68f50c
parent 71a4fd543828c6fc562dee00670168e8481e296b

[Bugfix] avoid deadlock caused by AppBatteryTracker.dump

AppBatteryTracker.dump -> BaseAppStatePolicy.shouldExemptUid -> getBackgroundRestrictionExemptionReason -> getPotentialSystemExemptionReason -> isProfileOwner
Getting mlock first in AppBatteryTracker.dump() and then getting AMS lock in isProfileOwner() is an inappropriate lock order in the above call chain. This is inconsistent with the method of getting AMS lock first and then mlock lock in most method calls, so it will cause deadlock and cause watchdog.

Similar bug cases include 366150181, 358274041

Bug:366150181
Test: Manual test

Change-Id: I8a1b509a1e95ad8570f3d4c248f9e0ed346962ad
Signed-off-by: panhaihui <panhaihui@xiaomi.com>

services/core/java/com/android/server/am/ActivityManagerService.java

diff --git a/services/core/java/com/android/server/am/ActivityManagerService.java b/services/core/java/com/android/server/am/ActivityManagerService.java
index 995c176..e0db1f6 100644
--- a/services/core/java/com/android/server/am/ActivityManagerService.java
+++ b/services/core/java/com/android/server/am/ActivityManagerService.java
@@ -749,12 +749,14 @@
     /**
      * Map userId to its companion app uids.
      */
+    @GuardedBy("mCompanionAppUidsMap")
     private final Map<Integer, Set<Integer>> mCompanionAppUidsMap = new ArrayMap<>();
 
     /**
      * The profile owner UIDs.
      */
-    private ArraySet<Integer> mProfileOwnerUids = null;
+    @GuardedBy("mProfileOwnerUids")
+    private final ArraySet<Integer> mProfileOwnerUids = new ArraySet<>();
 
     final UserController mUserController;
     @VisibleForTesting
@@ -19604,32 +19606,35 @@
 
         @Override
         public void setProfileOwnerUid(ArraySet<Integer> profileOwnerUids) {
-            synchronized (ActivityManagerService.this) {
-                mProfileOwnerUids = profileOwnerUids;
+            synchronized (mProfileOwnerUids) {
+                mProfileOwnerUids.clear();
+                mProfileOwnerUids.addAll(profileOwnerUids);
             }
         }
 
         @Override
         public boolean isProfileOwner(int uid) {
-            synchronized (ActivityManagerService.this) {
-                return mProfileOwnerUids != null && mProfileOwnerUids.indexOf(uid) >= 0;
+            synchronized (mProfileOwnerUids) {
+                return mProfileOwnerUids.indexOf(uid) >= 0;
             }
         }
 
         @Override
         public void setCompanionAppUids(int userId, Set<Integer> companionAppUids) {
-            synchronized (ActivityManagerService.this) {
+            synchronized (mCompanionAppUidsMap) {
                 mCompanionAppUidsMap.put(userId, companionAppUids);
             }
         }
 
         @Override
         public boolean isAssociatedCompanionApp(int userId, int uid) {
-            final Set<Integer> allUids = mCompanionAppUidsMap.get(userId);
-            if (allUids == null) {
-                return false;
+            synchronized (mCompanionAppUidsMap) {
+                final Set<Integer> allUids = mCompanionAppUidsMap.get(userId);
+                if (allUids == null) {
+                    return false;
+                }
+                return allUids.contains(uid);
             }
-            return allUids.contains(uid);
         }
 
         @Override