From ce8a2d50933169e5cbba8ff1e48651843a8d3bf7 Mon Sep 17 00:00:00 2001
From: Patrick Baumann <patb@google.com>
Date: Fri, 12 Apr 2019 12:48:03 -0700
Subject: [PATCH] Don't hold mPackages when clearing package state

This change stops locking on mPackages when making calls to
clearPackageForUserLIF to avoid the potential for lock contention and
address error logs emitted by installd.

Test: atest AppSecurityTests
Fixes: 122530620
Change-Id: If3372fc38140ede0c05e49dfafd761d56790df44
---
 .../server/pm/PackageManagerService.java      | 39 +++++++++++--------
 1 file changed, 22 insertions(+), 17 deletions(-)

diff --git a/services/core/java/com/android/server/pm/PackageManagerService.java b/services/core/java/com/android/server/pm/PackageManagerService.java
index d3aa746f75969..0718bbfd42652 100644
--- a/services/core/java/com/android/server/pm/PackageManagerService.java
+++ b/services/core/java/com/android/server/pm/PackageManagerService.java
@@ -19138,22 +19138,22 @@ public class PackageManagerService extends IPackageManager.Stub
         final UserHandle user = action.user;
         final int flags = action.flags;
         final boolean systemApp = isSystemApp(ps);
-        synchronized (mPackages) {
 
-            if (ps.parentPackageName != null
-                    && (!systemApp || (flags & PackageManager.DELETE_SYSTEM_APP) != 0)) {
-                if (DEBUG_REMOVE) {
-                    Slog.d(TAG, "Uninstalled child package:" + packageName + " for user:"
-                            + ((user == null) ? UserHandle.USER_ALL : user));
-                }
-                final int removedUserId = (user != null) ? user.getIdentifier()
-                        : UserHandle.USER_ALL;
+        if (ps.parentPackageName != null
+                && (!systemApp || (flags & PackageManager.DELETE_SYSTEM_APP) != 0)) {
+            if (DEBUG_REMOVE) {
+                Slog.d(TAG, "Uninstalled child package:" + packageName + " for user:"
+                        + ((user == null) ? UserHandle.USER_ALL : user));
+            }
+            final int removedUserId = (user != null) ? user.getIdentifier()
+                    : UserHandle.USER_ALL;
 
-                clearPackageStateForUserLIF(ps, removedUserId, outInfo, flags);
+            clearPackageStateForUserLIF(ps, removedUserId, outInfo, flags);
+            synchronized (mPackages) {
                 markPackageUninstalledForUserLPw(ps, user);
                 scheduleWritePackageRestrictionsLocked(user);
-                return;
             }
+            return;
         }
 
         final int userId = user == null ? UserHandle.USER_ALL : user.getIdentifier();
@@ -19167,6 +19167,7 @@ public class PackageManagerService extends IPackageManager.Stub
             // its data. If this is a system app, we only allow this to happen if
             // they have set the special DELETE_SYSTEM_APP which requests different
             // semantics than normal for uninstalling system apps.
+            final boolean clearPackageStateAndReturn;
             synchronized (mPackages) {
                 markPackageUninstalledForUserLPw(ps, user);
                 if (!systemApp) {
@@ -19177,15 +19178,14 @@ public class PackageManagerService extends IPackageManager.Stub
                         // we need to do is clear this user's data and save that
                         // it is uninstalled.
                         if (DEBUG_REMOVE) Slog.d(TAG, "Still installed by other users");
-                        clearPackageStateForUserLIF(ps, userId, outInfo, flags);
-                        scheduleWritePackageRestrictionsLocked(user);
-                        return;
+                        clearPackageStateAndReturn = true;
                     } else {
                         // We need to set it back to 'installed' so the uninstall
                         // broadcasts will be sent correctly.
                         if (DEBUG_REMOVE) Slog.d(TAG, "Not installed by other users, full delete");
                         ps.setInstalled(true, userId);
                         mSettings.writeKernelMappingLPr(ps);
+                        clearPackageStateAndReturn = false;
                     }
                 } else {
                     // This is a system app, so we assume that the
@@ -19193,11 +19193,16 @@ public class PackageManagerService extends IPackageManager.Stub
                     // we need to do is clear this user's data and save that
                     // it is uninstalled.
                     if (DEBUG_REMOVE) Slog.d(TAG, "Deleting system app");
-                    clearPackageStateForUserLIF(ps, userId, outInfo, flags);
-                    scheduleWritePackageRestrictionsLocked(user);
-                    return;
+                    clearPackageStateAndReturn = true;
                 }
             }
+            if (clearPackageStateAndReturn) {
+                clearPackageStateForUserLIF(ps, userId, outInfo, flags);
+                synchronized (mPackages) {
+                    scheduleWritePackageRestrictionsLocked(user);
+                }
+                return;
+            }
         }
 
         // If we are deleting a composite package for all users, keep track