From 04918fe02715d330cbefa16d055d5766264273c3 Mon Sep 17 00:00:00 2001 From: Todd Kennedy Date: Tue, 12 Jul 2016 14:07:40 -0700 Subject: [PATCH] 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 --- .../server/pm/PackageInstallerService.java | 31 ++++++++++--------- 1 file changed, 17 insertions(+), 14 deletions(-) diff --git a/services/core/java/com/android/server/pm/PackageInstallerService.java b/services/core/java/com/android/server/pm/PackageInstallerService.java index 83af0173c514b..a3cf9c856ae5b 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 @@ public class PackageInstallerService extends IPackageInstaller.Stub { "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); }