From d5e295f9192522176796271cb29558a53d37c875 Mon Sep 17 00:00:00 2001 From: Todd Kennedy Date: Tue, 26 Apr 2016 17:06:38 -0700 Subject: [PATCH] Don't hold lock when calling externally Wound up in a deadlock. We held mPackages lock and called the mount service. This called into the AppOps service which tried to obtain its lock. On another thread, we held the AppOps service lock which called into PackageManager service and tried to take mPackages. Ideally, we'd never hold an internal lock when calling externally. In practice, that's not always fixable [for example, code change would be too great to not hold the AppOps lock when calling into the PackageManager service]. Bug: 28169075 Change-Id: If0a7f1361c7be9385b1f629f68ebc0556b08ab06 --- .../server/pm/PackageManagerService.java | 23 ++++++++++--------- 1 file changed, 12 insertions(+), 11 deletions(-) diff --git a/services/core/java/com/android/server/pm/PackageManagerService.java b/services/core/java/com/android/server/pm/PackageManagerService.java index 38144e476cca6..94fd1ef9d7b7a 100644 --- a/services/core/java/com/android/server/pm/PackageManagerService.java +++ b/services/core/java/com/android/server/pm/PackageManagerService.java @@ -2050,17 +2050,18 @@ public class PackageManagerService extends IPackageManager.Stub { final int immutableFlags = PackageManager.FLAG_PERMISSION_SYSTEM_FIXED | PackageManager.FLAG_PERMISSION_POLICY_FIXED; - synchronized (mPackages) { - for (String permission : pkg.requestedPermissions) { - BasePermission bp = mSettings.mPermissions.get(permission); - if (bp != null && (bp.isRuntime() || bp.isDevelopment()) - && (grantedPermissions == null - || ArrayUtils.contains(grantedPermissions, permission))) { - final int flags = permissionsState.getPermissionFlags(permission, userId); - // Installer cannot change immutable permissions. - if ((flags & immutableFlags) == 0) { - grantRuntimePermission(pkg.packageName, permission, userId); - } + for (String permission : pkg.requestedPermissions) { + final BasePermission bp; + synchronized (mPackages) { + bp = mSettings.mPermissions.get(permission); + } + if (bp != null && (bp.isRuntime() || bp.isDevelopment()) + && (grantedPermissions == null + || ArrayUtils.contains(grantedPermissions, permission))) { + final int flags = permissionsState.getPermissionFlags(permission, userId); + // Installer cannot change immutable permissions. + if ((flags & immutableFlags) == 0) { + grantRuntimePermission(pkg.packageName, permission, userId); } } }