From d9aa57e156c354ad2e7905c0d522a54109d80ecf Mon Sep 17 00:00:00 2001 From: Winson Date: Fri, 18 Feb 2022 14:50:08 -0800 Subject: [PATCH] Migrate AppDataHelper reconcileAppsData to snapshot To avoid lock contention when starting a user, migrate the reconcile calls to use the snapshot and hopefully avoid taking the lock. Bug: 220249728 Test: presubmit Change-Id: I7d893fedbada66d911310c0feed87a77facd10ce --- .../com/android/server/pm/AppDataHelper.java | 56 ++++++++----------- .../java/com/android/server/pm/Computer.java | 3 + .../com/android/server/pm/ComputerEngine.java | 11 ++++ .../java/com/android/server/pm/Settings.java | 4 +- .../android/server/pm/StorageEventHelper.java | 13 +++-- 5 files changed, 45 insertions(+), 42 deletions(-) diff --git a/services/core/java/com/android/server/pm/AppDataHelper.java b/services/core/java/com/android/server/pm/AppDataHelper.java index 5013570fa6b8..66f71a37e0a3 100644 --- a/services/core/java/com/android/server/pm/AppDataHelper.java +++ b/services/core/java/com/android/server/pm/AppDataHelper.java @@ -386,6 +386,7 @@ final class AppDataHelper { final File ceDir = Environment.getDataUserCeDirectory(volumeUuid, userId); final File deDir = Environment.getDataUserDeDirectory(volumeUuid, userId); + final Computer snapshot = mPm.snapshotComputer(); // First look for stale data that doesn't belong, and check if things // have changed since we did our last restorecon if ((flags & StorageManager.FLAG_STORAGE_CE) != 0) { @@ -400,7 +401,7 @@ final class AppDataHelper { for (File file : files) { final String packageName = file.getName(); try { - assertPackageStorageValid(volumeUuid, packageName, userId); + assertPackageStorageValid(snapshot, volumeUuid, packageName, userId); } catch (PackageManagerException e) { logCriticalInfo(Log.WARN, "Destroying " + file + " due to: " + e); try { @@ -417,7 +418,7 @@ final class AppDataHelper { for (File file : files) { final String packageName = file.getName(); try { - assertPackageStorageValid(volumeUuid, packageName, userId); + assertPackageStorageValid(snapshot, volumeUuid, packageName, userId); } catch (PackageManagerException e) { logCriticalInfo(Log.WARN, "Destroying " + file + " due to: " + e); try { @@ -434,12 +435,9 @@ final class AppDataHelper { // installed for this volume and user Trace.traceBegin(TRACE_TAG_PACKAGE_MANAGER, "prepareAppDataAndMigrate"); Installer.Batch batch = new Installer.Batch(); - final List packages; - synchronized (mPm.mLock) { - packages = mPm.mSettings.getVolumePackagesLPr(volumeUuid); - } + List packages = snapshot.getVolumePackages(volumeUuid); int preparedCount = 0; - for (PackageSetting ps : packages) { + for (PackageStateInternal ps : packages) { final String packageName = ps.getPackageName(); if (ps.getPkg() == null) { Slog.w(TAG, "Odd, missing scanned package " + packageName); @@ -453,7 +451,7 @@ final class AppDataHelper { continue; } - if (ps.getInstalled(userId)) { + if (ps.getUserStateOrDefault(userId).isInstalled()) { prepareAppDataAndMigrate(batch, ps.getPkg(), userId, flags, migrateAppData); preparedCount++; } @@ -469,35 +467,25 @@ final class AppDataHelper { * Asserts that storage path is valid by checking that {@code packageName} is present, * installed for the given {@code userId} and can have app data. */ - private void assertPackageStorageValid(String volumeUuid, String packageName, int userId) - throws PackageManagerException { - synchronized (mPm.mLock) { - // Normalize package name to handle renamed packages - packageName = normalizePackageNameLPr(packageName); - - final PackageSetting ps = mPm.mSettings.getPackageLPr(packageName); - if (ps == null) { - throw new PackageManagerException("Package " + packageName + " is unknown"); - } else if (!TextUtils.equals(volumeUuid, ps.getVolumeUuid())) { - throw new PackageManagerException( - "Package " + packageName + " found on unknown volume " + volumeUuid - + "; expected volume " + ps.getVolumeUuid()); - } else if (!ps.getInstalled(userId)) { - throw new PackageManagerException( - "Package " + packageName + " not installed for user " + userId); - } else if (ps.getPkg() != null && !shouldHaveAppStorage(ps.getPkg())) { - throw new PackageManagerException( - "Package " + packageName + " shouldn't have storage"); - } + private void assertPackageStorageValid(@NonNull Computer snapshot, String volumeUuid, + String packageName, int userId) throws PackageManagerException { + final PackageStateInternal packageState = snapshot.getPackageStateInternal(packageName); + if (packageState == null) { + throw new PackageManagerException("Package " + packageName + " is unknown"); + } else if (!TextUtils.equals(volumeUuid, packageState.getVolumeUuid())) { + throw new PackageManagerException( + "Package " + packageName + " found on unknown volume " + volumeUuid + + "; expected volume " + packageState.getVolumeUuid()); + } else if (!packageState.getUserStateOrDefault(userId).isInstalled()) { + throw new PackageManagerException( + "Package " + packageName + " not installed for user " + userId); + } else if (packageState.getPkg() != null + && !shouldHaveAppStorage(packageState.getPkg())) { + throw new PackageManagerException( + "Package " + packageName + " shouldn't have storage"); } } - @GuardedBy("mPm.mLock") - private String normalizePackageNameLPr(String packageName) { - String normalizedPackageName = mPm.mSettings.getRenamedPackageLPr(packageName); - return normalizedPackageName != null ? normalizedPackageName : packageName; - } - /** * Prepare storage for system user really early during boot, * since core system apps like SettingsProvider and SystemUI diff --git a/services/core/java/com/android/server/pm/Computer.java b/services/core/java/com/android/server/pm/Computer.java index 3e204b6e6e4c..c259797942a7 100644 --- a/services/core/java/com/android/server/pm/Computer.java +++ b/services/core/java/com/android/server/pm/Computer.java @@ -599,4 +599,7 @@ public interface Computer extends PackageDataSnapshot { void dumpPackagesProto(@NonNull ProtoOutputStream proto); void dumpSharedLibrariesProto(@NonNull ProtoOutputStream protoOutputStream); + + @NonNull + List getVolumePackages(@NonNull String volumeUuid); } diff --git a/services/core/java/com/android/server/pm/ComputerEngine.java b/services/core/java/com/android/server/pm/ComputerEngine.java index 8432f48e269e..df7c3ec6f260 100644 --- a/services/core/java/com/android/server/pm/ComputerEngine.java +++ b/services/core/java/com/android/server/pm/ComputerEngine.java @@ -354,6 +354,11 @@ public class ComputerEngine implements Computer { public void dumpSharedUsersProto(ProtoOutputStream proto) { mSettings.dumpSharedUsersProto(proto); } + + public List getVolumePackages( + @NonNull String volumeUuid) { + return mSettings.getVolumePackagesLPr(volumeUuid); + } } private static final Comparator sProviderInitOrderSorter = (p1, p2) -> { @@ -5812,4 +5817,10 @@ public class ComputerEngine implements Computer { public void dumpSharedLibrariesProto(@NonNull ProtoOutputStream proto) { mSharedLibraries.dumpProto(proto); } + + @NonNull + @Override + public List getVolumePackages(@NonNull String volumeUuid) { + return mSettings.getVolumePackages(volumeUuid); + } } diff --git a/services/core/java/com/android/server/pm/Settings.java b/services/core/java/com/android/server/pm/Settings.java index 698dbe945e6f..b53cfc558f6e 100644 --- a/services/core/java/com/android/server/pm/Settings.java +++ b/services/core/java/com/android/server/pm/Settings.java @@ -4342,8 +4342,8 @@ public final class Settings implements Watchable, Snappable { * Return all {@link PackageSetting} that are actively installed on the * given {@link VolumeInfo#fsUuid}. */ - List getVolumePackagesLPr(String volumeUuid) { - ArrayList res = new ArrayList<>(); + List getVolumePackagesLPr(String volumeUuid) { + ArrayList res = new ArrayList<>(); for (int i = 0; i < mPackages.size(); i++) { final PackageSetting setting = mPackages.valueAt(i); if (Objects.equals(volumeUuid, setting.getVolumeUuid())) { diff --git a/services/core/java/com/android/server/pm/StorageEventHelper.java b/services/core/java/com/android/server/pm/StorageEventHelper.java index 8c6b19b3361c..666776b4161e 100644 --- a/services/core/java/com/android/server/pm/StorageEventHelper.java +++ b/services/core/java/com/android/server/pm/StorageEventHelper.java @@ -109,8 +109,9 @@ public final class StorageEventHelper extends StorageEventListener { // Remove any apps installed on the forgotten volume synchronized (mPm.mLock) { - final List packages = mPm.mSettings.getVolumePackagesLPr(fsUuid); - for (PackageSetting ps : packages) { + final List packages = + mPm.mSettings.getVolumePackagesLPr(fsUuid); + for (PackageStateInternal ps : packages) { Slog.d(TAG, "Destroying " + ps.getPackageName() + " because volume was forgotten"); mPm.deletePackageVersioned(new VersionedPackage(ps.getPackageName(), @@ -145,14 +146,14 @@ public final class StorageEventHelper extends StorageEventListener { final int parseFlags = mPm.getDefParseFlags() | ParsingPackageUtils.PARSE_EXTERNAL_STORAGE; final Settings.VersionInfo ver; - final List packages; + final List packages; final InstallPackageHelper installPackageHelper = new InstallPackageHelper(mPm); synchronized (mPm.mLock) { ver = mPm.mSettings.findOrCreateVersion(volumeUuid); packages = mPm.mSettings.getVolumePackagesLPr(volumeUuid); } - for (PackageSetting ps : packages) { + for (PackageStateInternal ps : packages) { freezers.add(mPm.freezePackage(ps.getPackageName(), "loadPrivatePackagesInner")); synchronized (mPm.mInstallLock) { final AndroidPackage pkg; @@ -243,9 +244,9 @@ public final class StorageEventHelper extends StorageEventListener { final ArrayList unloaded = new ArrayList<>(); synchronized (mPm.mInstallLock) { synchronized (mPm.mLock) { - final List packages = + final List packages = mPm.mSettings.getVolumePackagesLPr(volumeUuid); - for (PackageSetting ps : packages) { + for (PackageStateInternal ps : packages) { if (ps.getPkg() == null) continue; final AndroidPackage pkg = ps.getPkg();