From 9d681f1d97b38086062a358841609053cd66afb1 Mon Sep 17 00:00:00 2001 From: Alex Buynytskyy Date: Wed, 23 Feb 2022 10:18:32 -0800 Subject: [PATCH] Reduce lock contention. Remove lock for cached state in ApplicationPackageManager. Short-circut the instant app check for internal API calls. Bug: 215305214 Test: presubmit Change-Id: Ie122d76f6ea0e950fa25b22bea65f9938a3bf5f1 --- .../app/ApplicationPackageManager.java | 89 +++++++------------ .../server/pm/PackageManagerService.java | 5 ++ .../server/pm/PackageManagerServiceUtils.java | 11 ++- 3 files changed, 48 insertions(+), 57 deletions(-) diff --git a/core/java/android/app/ApplicationPackageManager.java b/core/java/android/app/ApplicationPackageManager.java index bda2e459f52a7..dca5c542af172 100644 --- a/core/java/android/app/ApplicationPackageManager.java +++ b/core/java/android/app/ApplicationPackageManager.java @@ -165,50 +165,35 @@ public class ApplicationPackageManager extends PackageManager { public static final String PERMISSION_CONTROLLER_RESOURCE_PACKAGE = "com.android.permissioncontroller"; - private final Object mLock = new Object(); - - @GuardedBy("mLock") - private UserManager mUserManager; - @GuardedBy("mLock") - private PermissionManager mPermissionManager; - @GuardedBy("mLock") - private PackageInstaller mInstaller; - @GuardedBy("mLock") - private ArtManager mArtManager; - @GuardedBy("mLock") - private DevicePolicyManager mDevicePolicyManager; + private volatile UserManager mUserManager; + private volatile PermissionManager mPermissionManager; + private volatile PackageInstaller mInstaller; + private volatile ArtManager mArtManager; + private volatile DevicePolicyManager mDevicePolicyManager; + private volatile String mPermissionsControllerPackageName; @GuardedBy("mDelegates") private final ArrayList mDelegates = new ArrayList<>(); - @GuardedBy("mLock") - private String mPermissionsControllerPackageName; - UserManager getUserManager() { - synchronized (mLock) { - if (mUserManager == null) { - mUserManager = UserManager.get(mContext); - } - return mUserManager; + if (mUserManager == null) { + mUserManager = UserManager.get(mContext); } + return mUserManager; } DevicePolicyManager getDevicePolicyManager() { - synchronized (mLock) { - if (mDevicePolicyManager == null) { - mDevicePolicyManager = mContext.getSystemService(DevicePolicyManager.class); - } - return mDevicePolicyManager; + if (mDevicePolicyManager == null) { + mDevicePolicyManager = mContext.getSystemService(DevicePolicyManager.class); } + return mDevicePolicyManager; } private PermissionManager getPermissionManager() { - synchronized (mLock) { - if (mPermissionManager == null) { - mPermissionManager = mContext.getSystemService(PermissionManager.class); - } - return mPermissionManager; + if (mPermissionManager == null) { + mPermissionManager = mContext.getSystemService(PermissionManager.class); } + return mPermissionManager; } @Override @@ -851,16 +836,14 @@ public class ApplicationPackageManager extends PackageManager { */ @Override public String getPermissionControllerPackageName() { - synchronized (mLock) { - if (mPermissionsControllerPackageName == null) { - try { - mPermissionsControllerPackageName = mPM.getPermissionControllerPackageName(); - } catch (RemoteException e) { - throw e.rethrowFromSystemServer(); - } + if (mPermissionsControllerPackageName == null) { + try { + mPermissionsControllerPackageName = mPM.getPermissionControllerPackageName(); + } catch (RemoteException e) { + throw e.rethrowFromSystemServer(); } - return mPermissionsControllerPackageName; } + return mPermissionsControllerPackageName; } /** @@ -3235,17 +3218,15 @@ public class ApplicationPackageManager extends PackageManager { @Override public PackageInstaller getPackageInstaller() { - synchronized (mLock) { - if (mInstaller == null) { - try { - mInstaller = new PackageInstaller(mPM.getPackageInstaller(), - mContext.getPackageName(), mContext.getAttributionTag(), getUserId()); - } catch (RemoteException e) { - throw e.rethrowFromSystemServer(); - } + if (mInstaller == null) { + try { + mInstaller = new PackageInstaller(mPM.getPackageInstaller(), + mContext.getPackageName(), mContext.getAttributionTag(), getUserId()); + } catch (RemoteException e) { + throw e.rethrowFromSystemServer(); } - return mInstaller; } + return mInstaller; } @Override @@ -3583,16 +3564,14 @@ public class ApplicationPackageManager extends PackageManager { @Override public ArtManager getArtManager() { - synchronized (mLock) { - if (mArtManager == null) { - try { - mArtManager = new ArtManager(mContext, mPM.getArtManager()); - } catch (RemoteException e) { - throw e.rethrowFromSystemServer(); - } + if (mArtManager == null) { + try { + mArtManager = new ArtManager(mContext, mPM.getArtManager()); + } catch (RemoteException e) { + throw e.rethrowFromSystemServer(); } - return mArtManager; } + return mArtManager; } @Override diff --git a/services/core/java/com/android/server/pm/PackageManagerService.java b/services/core/java/com/android/server/pm/PackageManagerService.java index 0602f3e72031c..9452e968a1f68 100644 --- a/services/core/java/com/android/server/pm/PackageManagerService.java +++ b/services/core/java/com/android/server/pm/PackageManagerService.java @@ -6675,6 +6675,11 @@ public class PackageManagerService extends IPackageManager.Stub @Override public IPackageInstaller getPackageInstaller() { + // Return installer service for internal calls. + if (PackageManagerServiceUtils.isSystemOrRoot()) { + return mInstallerService; + } + // Return null for InstantApps. if (getInstantAppPackageName(Binder.getCallingUid()) != null) { return null; } diff --git a/services/core/java/com/android/server/pm/PackageManagerServiceUtils.java b/services/core/java/com/android/server/pm/PackageManagerServiceUtils.java index a9471cf57070d..8d3fbf7fc6798 100644 --- a/services/core/java/com/android/server/pm/PackageManagerServiceUtils.java +++ b/services/core/java/com/android/server/pm/PackageManagerServiceUtils.java @@ -1249,6 +1249,14 @@ public class PackageManagerServiceUtils { } } + /** + * Check if the Binder caller is system UID or root's UID. + */ + public static boolean isSystemOrRoot() { + final int uid = Binder.getCallingUid(); + return uid == Process.SYSTEM_UID || uid == Process.ROOT_UID; + } + /** * Enforces that only the system UID or root's UID can call a method exposed * via Binder. @@ -1257,8 +1265,7 @@ public class PackageManagerServiceUtils { * @throws SecurityException if the caller is not system or root */ public static void enforceSystemOrRoot(String message) { - final int uid = Binder.getCallingUid(); - if (uid != Process.SYSTEM_UID && uid != Process.ROOT_UID) { + if (!isSystemOrRoot()) { throw new SecurityException(message); } }