commit 04918fe02715d330cbefa16d055d5766264273c3
author Todd Kennedy <toddke@google.com> Tue Jul 12 14:07:40 2016 -0700
committer Todd Kennedy <toddke@google.com> Tue Jul 12 14:07:40 2016 -0700
tree 268846443c586ad49de09c302ac424f094af38dd
parent 0e792f567ac1b37a75e94e1ff1017f2f152494ef

Don't hold lock calling into PackageMgr

There's a subtle deadlock; when dumping state, we obtain the package manager
lock before trying to obtain the package installer session lock. Meanwhile,
creating a new session obtains the locks in the reverse -- the package
installer takes the package installer session lock and then tries to obtain
the package manager lock.

Here, we avoid holding the package installer session lock before we call
into the package manager. Alternatively, we could consider not holding the
package manager lock when dumping package installer state. But, given the
scope of the dumping logic, that's a bigger change with other, unknown
side effects.

Change-Id: I11f08484cd335bb7ad3bc557808eb48d14bd29cf
Fixes: 30089638

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

diff --git a/services/core/java/com/android/server/pm/PackageInstallerService.java b/services/core/java/com/android/server/pm/PackageInstallerService.java
index 83af017..a3cf9c8 100644
--- a/services/core/java/com/android/server/pm/PackageInstallerService.java
+++ b/services/core/java/com/android/server/pm/PackageInstallerService.java
@@ -666,23 +666,26 @@
                         "Too many historical sessions for UID " + callingUid);
             }
 
-            final long createdMillis = System.currentTimeMillis();
             sessionId = allocateSessionIdLocked();
+        }
 
-            // We're staging to exactly one location
-            File stageDir = null;
-            String stageCid = null;
-            if ((params.installFlags & PackageManager.INSTALL_INTERNAL) != 0) {
-                final boolean isEphemeral =
-                        (params.installFlags & PackageManager.INSTALL_EPHEMERAL) != 0;
-                stageDir = buildStageDir(params.volumeUuid, sessionId, isEphemeral);
-            } else {
-                stageCid = buildExternalStageCid(sessionId);
-            }
+        final long createdMillis = System.currentTimeMillis();
+        // We're staging to exactly one location
+        File stageDir = null;
+        String stageCid = null;
+        if ((params.installFlags & PackageManager.INSTALL_INTERNAL) != 0) {
+            final boolean isEphemeral =
+                    (params.installFlags & PackageManager.INSTALL_EPHEMERAL) != 0;
+            stageDir = buildStageDir(params.volumeUuid, sessionId, isEphemeral);
+        } else {
+            stageCid = buildExternalStageCid(sessionId);
+        }
 
-            session = new PackageInstallerSession(mInternalCallback, mContext, mPm,
-                    mInstallThread.getLooper(), sessionId, userId, installerPackageName, callingUid,
-                    params, createdMillis, stageDir, stageCid, false, false);
+        session = new PackageInstallerSession(mInternalCallback, mContext, mPm,
+                mInstallThread.getLooper(), sessionId, userId, installerPackageName, callingUid,
+                params, createdMillis, stageDir, stageCid, false, false);
+
+        synchronized (mSessions) {
             mSessions.put(sessionId, session);
         }