Don't get child sessions while holding session lock.

To avoid a deadlock that happens from inconsistent lock ordering between
the session lock and the session provider lock.

Bug: 123391593
Bug: 123886893
Test: atest RollbackTest
Change-Id: If4e6a8a7e6399e99ebc5c2822c2979678cd15096

1 files changed, 42 insertions, 25 deletions

diff --git a/services/core/java/com/android/server/pm/PackageInstallerSession.java b/services/core/java/com/android/server/pm/PackageInstallerSession.java
index d75d6537f45c..56ef33ac4c31 100644
--- a/services/core/java/com/android/server/pm/PackageInstallerSession.java
+++ b/services/core/java/com/android/server/pm/PackageInstallerSession.java
@@ -322,18 +322,7 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub {
         public boolean handleMessage(Message msg) {
             switch (msg.what) {
                 case MSG_COMMIT:
-                    synchronized (mLock) {
-                        try {
-                            commitLocked();
-                        } catch (PackageManagerException e) {
-                            final String completeMsg = ExceptionUtils.getCompleteMessage(e);
-                            Slog.e(TAG,
-                                    "Commit of session " + sessionId + " failed: " + completeMsg);
-                            destroyInternal();
-                            dispatchSessionFinished(e.error, completeMsg, null);
-                        }
-                    }
-
+                    handleCommit();
                     break;
                 case MSG_ON_PACKAGE_INSTALLED:
                     final SomeArgs args = (SomeArgs) msg.obj;
@@ -1073,38 +1062,66 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub {
         mCallback.onSessionSealedBlocking(this);
     }
 
-    @GuardedBy("mLock")
-    private void commitLocked()
-            throws PackageManagerException {
+    private void handleCommit() {
         if (params.isStaged) {
             mStagingManager.commitSession(this);
             destroyInternal();
             dispatchSessionFinished(PackageManager.INSTALL_SUCCEEDED, "Session staged", null);
             return;
         }
+
         if ((params.installFlags & PackageManager.INSTALL_APEX) != 0) {
-            throw new PackageManagerException(
-                PackageManager.INSTALL_FAILED_INTERNAL_ERROR,
-                "APEX packages can only be installed using staged sessions.");
+            destroyInternal();
+            dispatchSessionFinished(PackageManager.INSTALL_FAILED_INTERNAL_ERROR,
+                    "APEX packages can only be installed using staged sessions.", null);
+            return;
+        }
+
+        // For a multiPackage session, read the child sessions
+        // outside of the lock, because reading the child
+        // sessions with the lock held could lead to deadlock
+        // (b/123391593).
+        List<PackageInstallerSession> childSessions = null;
+        if (isMultiPackage()) {
+            final int[] childSessionIds = getChildSessionIds();
+            childSessions = new ArrayList<>(childSessionIds.length);
+            for (int childSessionId : childSessionIds) {
+                childSessions.add(mSessionProvider.getSession(childSessionId));
+            }
+        }
+
+        try {
+            synchronized (mLock) {
+                commitNonStagedLocked(childSessions);
+            }
+        } catch (PackageManagerException e) {
+            final String completeMsg = ExceptionUtils.getCompleteMessage(e);
+            Slog.e(TAG, "Commit of session " + sessionId + " failed: " + completeMsg);
+            destroyInternal();
+            dispatchSessionFinished(e.error, completeMsg, null);
         }
+    }
+
+    @GuardedBy("mLock")
+    private void commitNonStagedLocked(List<PackageInstallerSession> childSessions)
+            throws PackageManagerException {
         final PackageManagerService.ActiveInstallSession committingSession =
                 makeSessionActiveLocked();
         if (committingSession == null) {
             return;
         }
         if (isMultiPackage()) {
-            final int[] childSessionIds = getChildSessionIds();
-            List<PackageManagerService.ActiveInstallSession> childSessions =
-                    new ArrayList<>(childSessionIds.length);
+            List<PackageManagerService.ActiveInstallSession> activeChildSessions =
+                    new ArrayList<>(childSessions.size());
             boolean success = true;
             PackageManagerException failure = null;
-            for (int childSessionId : getChildSessionIds()) {
-                final PackageInstallerSession session = mSessionProvider.getSession(childSessionId);
+            for (int i = 0; i < childSessions.size(); ++i) {
+                final PackageInstallerSession session = childSessions.get(i);
                 try {
                     final PackageManagerService.ActiveInstallSession activeSession =
                             session.makeSessionActiveLocked();
                     if (activeSession != null) {
-                        childSessions.add(activeSession);
+                        activeChildSessions.add(activeSession);
                     }
                 } catch (PackageManagerException e) {
                     failure = e;
@@ -1119,7 +1136,7 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub {
                 }
                 return;
             }
-            mPm.installStage(childSessions);
+            mPm.installStage(activeChildSessions);
         } else {
             mPm.installStage(committingSession);
         }