Reconcile app data twice to pick up packages installed during unlock
Tries to reconcile app data inside UserManagerService twice in case
something like a carrier app was enabled during the user unlock phase.
This tries to fix a race condition between the user unlock state being
set and PackageManagerService attempting to read the state to determine
whether user encrypted credential storage should be created.
This is a re-attempt of this fix from the reverted
If086f5126d508739d1079662776f4951ea339f43.
Bug: 187103629
Test: manual, install app during reconcilation
Change-Id: I7cc18c11eccfdc6eea4d50e1880e9c675c59db28
diff --git a/services/core/java/com/android/server/am/UserController.java b/services/core/java/com/android/server/am/UserController.java
index fa7eae3..256c472 100644
--- a/services/core/java/com/android/server/am/UserController.java
+++ b/services/core/java/com/android/server/am/UserController.java
@@ -594,7 +594,8 @@
final TimingsTraceAndSlog t = new TimingsTraceAndSlog();
t.traceBegin("UM.onBeforeUnlockUser-" + userId);
- mInjector.getUserManager().onBeforeUnlockUser(userId);
+ final ArraySet<String> reconciledPackages =
+ mInjector.getUserManager().onBeforeUnlockUser(userId);
t.traceEnd();
synchronized (mLock) {
// Do not proceed if unexpected state
@@ -603,6 +604,9 @@
}
}
mInjector.getUserManagerInternal().setUserState(userId, uss.state);
+ t.traceBegin("UM.onUserStateRunningUnlocking-" + userId);
+ mInjector.getUserManager().onUserStateRunningUnlocking(userId, reconciledPackages);
+ t.traceEnd();
uss.mUnlockProgress.setProgress(20);
diff --git a/services/core/java/com/android/server/pm/PackageManagerService.java b/services/core/java/com/android/server/pm/PackageManagerService.java
index b44e2ed..bf3bbbb 100644
--- a/services/core/java/com/android/server/pm/PackageManagerService.java
+++ b/services/core/java/com/android/server/pm/PackageManagerService.java
@@ -7593,7 +7593,7 @@
}
List<String> deferPackages = reconcileAppsDataLI(StorageManager.UUID_PRIVATE_INTERNAL,
UserHandle.USER_SYSTEM, storageFlags, true /* migrateAppData */,
- true /* onlyCoreApps */);
+ true /* onlyCoreApps */, null);
mPrepareAppDataFuture = SystemServerInitThreadPool.submit(() -> {
TimingsTraceLog traceLog = new TimingsTraceLog("SystemServerTimingAsync",
Trace.TRACE_TAG_PACKAGE_MANAGER);
@@ -21438,7 +21438,8 @@
try {
sm.prepareUserStorage(volumeUuid, user.id, user.serialNumber, flags);
synchronized (mInstallLock) {
- reconcileAppsDataLI(volumeUuid, user.id, flags, true /* migrateAppData */);
+ reconcileAppsDataLI(volumeUuid, user.id, flags, true /* migrateAppData */,
+ null);
}
} catch (IllegalStateException e) {
// Device was probably ejected, and we'll process that event momentarily
@@ -21627,21 +21628,32 @@
* <p>
* Verifies that directories exist and that ownership and labeling is
* correct for all installed apps on all mounted volumes.
+ *
+ * @param reconciledPackages A set that will be populated with package names that have
+ * successfully had their data reconciled. Any package names already
+ * contained will be skipped. Because this must be mutable when
+ * non-null, it is typed {@link ArraySet} to prevent accidental
+ * usage of {@link Collections#emptySet()}. Null can be passed if the
+ * caller doesn't need this functionality.
*/
- void reconcileAppsData(int userId, int flags, boolean migrateAppsData) {
+ @NonNull
+ void reconcileAppsData(int userId, int flags, boolean migrateAppsData,
+ @Nullable ArraySet<String> reconciledPackages) {
final StorageManager storage = mInjector.getSystemService(StorageManager.class);
for (VolumeInfo vol : storage.getWritablePrivateVolumes()) {
final String volumeUuid = vol.getFsUuid();
synchronized (mInstallLock) {
- reconcileAppsDataLI(volumeUuid, userId, flags, migrateAppsData);
+ reconcileAppsDataLI(volumeUuid, userId, flags, migrateAppsData,
+ reconciledPackages);
}
}
}
@GuardedBy("mInstallLock")
private void reconcileAppsDataLI(String volumeUuid, int userId, int flags,
- boolean migrateAppData) {
- reconcileAppsDataLI(volumeUuid, userId, flags, migrateAppData, false /* onlyCoreApps */);
+ boolean migrateAppData, @Nullable ArraySet<String> reconciledPackages) {
+ reconcileAppsDataLI(volumeUuid, userId, flags, migrateAppData, false /* onlyCoreApps */,
+ reconciledPackages);
}
/**
@@ -21656,7 +21668,8 @@
*/
@GuardedBy("mInstallLock")
private List<String> reconcileAppsDataLI(String volumeUuid, int userId, int flags,
- boolean migrateAppData, boolean onlyCoreApps) {
+ boolean migrateAppData, boolean onlyCoreApps,
+ @Nullable ArraySet<String> reconciledPackages) {
Slog.v(TAG, "reconcileAppsData for " + volumeUuid + " u" + userId + " 0x"
+ Integer.toHexString(flags) + " migrateAppData=" + migrateAppData);
List<String> result = onlyCoreApps ? new ArrayList<>() : null;
@@ -21719,6 +21732,9 @@
int preparedCount = 0;
for (PackageSetting ps : packages) {
final String packageName = ps.name;
+ if (reconciledPackages != null && reconciledPackages.contains(packageName)) {
+ continue;
+ }
if (ps.pkg == null) {
Slog.w(TAG, "Odd, missing scanned package " + packageName);
// TODO: might be due to legacy ASEC apps; we should circle back
@@ -21734,6 +21750,10 @@
if (ps.getInstalled(userId)) {
prepareAppDataAndMigrate(batch, ps.pkg, userId, flags, migrateAppData);
preparedCount++;
+
+ if (reconciledPackages != null) {
+ reconciledPackages.add(packageName);
+ }
}
}
executeBatchLI(batch);
diff --git a/services/core/java/com/android/server/pm/UserManagerService.java b/services/core/java/com/android/server/pm/UserManagerService.java
index 0d909a9..7f91a81 100644
--- a/services/core/java/com/android/server/pm/UserManagerService.java
+++ b/services/core/java/com/android/server/pm/UserManagerService.java
@@ -4805,7 +4805,7 @@
mUserDataPreparer.prepareUserData(userId, userSerial, StorageManager.FLAG_STORAGE_DE);
t.traceEnd();
t.traceBegin("reconcileAppsData");
- mPm.reconcileAppsData(userId, StorageManager.FLAG_STORAGE_DE, migrateAppsData);
+ mPm.reconcileAppsData(userId, StorageManager.FLAG_STORAGE_DE, migrateAppsData, null);
t.traceEnd();
if (userId != UserHandle.USER_SYSTEM) {
@@ -4821,11 +4821,14 @@
/**
* Called right before a user is unlocked. This gives us a chance to prepare
* app storage.
+ *
+ * @return set of packages that reconciled app data
*/
- public void onBeforeUnlockUser(@UserIdInt int userId) {
+ @NonNull public ArraySet<String> onBeforeUnlockUser(@UserIdInt int userId) {
UserInfo userInfo = getUserInfo(userId);
if (userInfo == null) {
- return;
+ // PMS requires mutable set, so the API uses ArraySet to prevent Collections.emptySet()
+ return new ArraySet<>();
}
final int userSerial = userInfo.serialNumber;
// Migrate only if build fingerprints mismatch
@@ -4836,8 +4839,33 @@
mUserDataPreparer.prepareUserData(userId, userSerial, StorageManager.FLAG_STORAGE_CE);
t.traceEnd();
- t.traceBegin("reconcileAppsData-" + userId);
- mPm.reconcileAppsData(userId, StorageManager.FLAG_STORAGE_CE, migrateAppsData);
+ final ArraySet<String> reconciledPackages = new ArraySet<>();
+ t.traceBegin("reconcileAppsDataFirstPass-" + userId);
+ mPm.reconcileAppsData(userId, StorageManager.FLAG_STORAGE_CE, migrateAppsData,
+ reconciledPackages);
+ t.traceEnd();
+ return reconciledPackages;
+ }
+
+ /**
+ * Called right after a user state is moved to {@link UserState#STATE_RUNNING_UNLOCKING}. This
+ * gives us a chance to reconcile app data for apps installed since
+ * {@link #onBeforeUnlockUser(int)} was called.
+ *
+ * @param previouslyReconciledPackages the result from {@link #onBeforeUnlockUser(int)}
+ */
+ public void onUserStateRunningUnlocking(@UserIdInt int userId,
+ @NonNull ArraySet<String> previouslyReconciledPackages) {
+ final UserInfo userInfo = getUserInfo(userId);
+ if (userInfo == null) {
+ return;
+ }
+ // Migrate only if build fingerprints mismatch
+ boolean migrateAppsData = !Build.FINGERPRINT.equals(userInfo.lastLoggedInFingerprint);
+ final TimingsTraceAndSlog t = new TimingsTraceAndSlog();
+ t.traceBegin("reconcileAppsDataSecondPass-" + userId);
+ mPm.reconcileAppsData(userId, StorageManager.FLAG_STORAGE_CE, migrateAppsData,
+ previouslyReconciledPackages);
t.traceEnd();
}
