From 4ae13e08bcadda77b9cc8d6d0c06d1cdf27fc842 Mon Sep 17 00:00:00 2001
From: Martin Stjernholm <mast@google.com>
Date: Sat, 18 Mar 2023 00:51:41 +0000
Subject: [PATCH] Fix init order problem between PackageManagerService and
 ArtManagerLocal.

PackageManagerService calls initSystemApps in its constructor, which
may try to call ArtManagerLocal.clearAppProfiles, but ArtManagerLocal
hasn't yet been registered at that point (it depends on PMS).

Work around the problem by skipping the clearing of profiles in that
case, since both ART Service and the ART runtime ignore invalid
profiles.

Test: atest apex_apkinapex_tests
  with dalvik.vm.useartservice=true
Bug: 273923424
Change-Id: I8571c960b78cff3622f8bd5d05cd2ed8b9b6f4f1
---
 .../com/android/server/pm/AppDataHelper.java     | 16 ++++++++++++----
 .../java/com/android/server/pm/DexOptHelper.java | 12 ++++++++++++
 2 files changed, 24 insertions(+), 4 deletions(-)

diff --git a/services/core/java/com/android/server/pm/AppDataHelper.java b/services/core/java/com/android/server/pm/AppDataHelper.java
index 2e86df89f63eb..f95f7bc0d165d 100644
--- a/services/core/java/com/android/server/pm/AppDataHelper.java
+++ b/services/core/java/com/android/server/pm/AppDataHelper.java
@@ -18,7 +18,6 @@ package com.android.server.pm;
 
 import static android.os.Trace.TRACE_TAG_PACKAGE_MANAGER;
 
-import static com.android.server.pm.DexOptHelper.useArtService;
 import static com.android.server.pm.PackageManagerService.TAG;
 import static com.android.server.pm.PackageManagerServiceUtils.getPackageManagerLocal;
 import static com.android.server.pm.PackageManagerServiceUtils.logCriticalInfo;
@@ -245,7 +244,7 @@ public class AppDataHelper {
                 }
             }
 
-            if (!useArtService()) { // ART Service handles this on demand instead.
+            if (!DexOptHelper.useArtService()) { // ART Service handles this on demand instead.
                 // Prepare the application profiles only for upgrades and
                 // first boot (so that we don't repeat the same operation at
                 // each boot).
@@ -591,7 +590,7 @@ public class AppDataHelper {
             Slog.wtf(TAG, "Package was null!", new Throwable());
             return;
         }
-        if (useArtService()) {
+        if (DexOptHelper.useArtService()) {
             destroyAppProfilesWithArtService(pkg);
         } else {
             try {
@@ -637,7 +636,7 @@ public class AppDataHelper {
     }
 
     private void destroyAppProfilesLeafLIF(AndroidPackage pkg) {
-        if (useArtService()) {
+        if (DexOptHelper.useArtService()) {
             destroyAppProfilesWithArtService(pkg);
         } else {
             try {
@@ -651,6 +650,15 @@ public class AppDataHelper {
     }
 
     private void destroyAppProfilesWithArtService(AndroidPackage pkg) {
+        if (!DexOptHelper.artManagerLocalIsInitialized()) {
+            // This function may get called while PackageManagerService is constructed (via e.g.
+            // InitAppsHelper.initSystemApps), and ART Service hasn't yet been started then (it
+            // requires a registered PackageManagerLocal instance). We can skip clearing any stale
+            // app profiles in this case, because ART Service and the runtime will ignore stale or
+            // otherwise invalid ref and cur profiles.
+            return;
+        }
+
         try (PackageManagerLocal.FilteredSnapshot snapshot =
                         getPackageManagerLocal().withFilteredSnapshot()) {
             try {
diff --git a/services/core/java/com/android/server/pm/DexOptHelper.java b/services/core/java/com/android/server/pm/DexOptHelper.java
index a9d4115b4b79f..064be7c5ddc7c 100644
--- a/services/core/java/com/android/server/pm/DexOptHelper.java
+++ b/services/core/java/com/android/server/pm/DexOptHelper.java
@@ -99,6 +99,8 @@ import java.util.function.Predicate;
 public final class DexOptHelper {
     private static final long SEVEN_DAYS_IN_MILLISECONDS = 7 * 24 * 60 * 60 * 1000;
 
+    private static boolean sArtManagerLocalIsInitialized = false;
+
     private final PackageManagerService mPm;
 
     // Start time for the boot dexopt in performPackageDexOptUpgradeIfNeeded when ART Service is
@@ -1035,6 +1037,7 @@ public final class DexOptHelper {
         artManager.addDexoptDoneCallback(false /* onlyIncludeUpdates */, Runnable::run,
                 pm.getDexOptHelper().new DexoptDoneHandler());
         LocalManagerRegistry.addManager(ArtManagerLocal.class, artManager);
+        sArtManagerLocalIsInitialized = true;
 
         // Schedule the background job when boot is complete. This decouples us from when
         // JobSchedulerService is initialized.
@@ -1047,6 +1050,15 @@ public final class DexOptHelper {
         }, new IntentFilter(Intent.ACTION_BOOT_COMPLETED));
     }
 
+    /**
+     * Returns true if an {@link ArtManagerLocal} instance has been created.
+     *
+     * Avoid this function if at all possible, because it may hide initialization order problems.
+     */
+    public static boolean artManagerLocalIsInitialized() {
+        return sArtManagerLocalIsInitialized;
+    }
+
     /**
      * Returns the registered {@link ArtManagerLocal} instance, or else throws an unchecked error.
      */