From 30a4adc0c08cd8b976d021564aafec8ee3e51c30 Mon Sep 17 00:00:00 2001 From: Kevin Han Date: Mon, 25 Oct 2021 20:23:50 -0700 Subject: [PATCH] Exempt hibernating apps from dex optimization We previously only checked whether an app could be optimized in PackageManagerService#getOptimizablePackages. This missed edge cases where the app would still generate vdex/odex such as when the app is updated. We fix this by considering any hibernating app as not optimizable in all cases. To support unit testing, we also change canOptimizePackage to a non-static method and enumerate PackageDexOptimizer's dependencies clearly with an injector. Bug: 203693378 Test: Reproduce steps in bug, confirm app does not have vdex/odex Test: atest PackageManagerServiceHibernationTests Test: forrest run of asit/dexoptota/self_full Merged-In: I6b88713529a570f285422e6716ad797e2148ce2c Change-Id: I6b88713529a570f285422e6716ad797e2148ce2c (cherry picked from commit 7cd8c2535045877ec2d5a31b486acb5dd09d39ea) --- .../AppHibernationManagerInternal.java | 5 ++ .../apphibernation/AppHibernationService.java | 13 +++++ .../android/server/pm/OtaDexoptService.java | 2 +- .../server/pm/PackageDexOptimizer.java | 53 +++++++++++++++++-- .../server/pm/PackageManagerService.java | 12 ++--- .../PackageManagerServiceHibernationTests.kt | 22 ++++++++ 6 files changed, 92 insertions(+), 15 deletions(-) diff --git a/services/core/java/com/android/server/apphibernation/AppHibernationManagerInternal.java b/services/core/java/com/android/server/apphibernation/AppHibernationManagerInternal.java index b0335fe404f42..a3c9612e0e2c8 100644 --- a/services/core/java/com/android/server/apphibernation/AppHibernationManagerInternal.java +++ b/services/core/java/com/android/server/apphibernation/AppHibernationManagerInternal.java @@ -43,4 +43,9 @@ public abstract class AppHibernationManagerInternal { * @see AppHibernationService#setHibernatingGlobally */ public abstract void setHibernatingGlobally(String packageName, boolean isHibernating); + + /** + * @see AppHibernationService#isOatArtifactDeletionEnabled + */ + public abstract boolean isOatArtifactDeletionEnabled(); } diff --git a/services/core/java/com/android/server/apphibernation/AppHibernationService.java b/services/core/java/com/android/server/apphibernation/AppHibernationService.java index 19dcee4828ddd..5cfcb4c5f8a81 100644 --- a/services/core/java/com/android/server/apphibernation/AppHibernationService.java +++ b/services/core/java/com/android/server/apphibernation/AppHibernationService.java @@ -199,6 +199,14 @@ public final class AppHibernationService extends SystemService { } } + /** + * Whether global hibernation should delete ART ahead-of-time compilation artifacts and prevent + * package manager from re-optimizing the APK. + */ + private boolean isOatArtifactDeletionEnabled() { + return mOatArtifactDeletionEnabled; + } + /** * Whether a package is hibernating for a given user. * @@ -742,6 +750,11 @@ public final class AppHibernationService extends SystemService { public boolean isHibernatingGlobally(String packageName) { return mService.isHibernatingGlobally(packageName); } + + @Override + public boolean isOatArtifactDeletionEnabled() { + return mService.isOatArtifactDeletionEnabled(); + } } private final AppHibernationServiceStub mServiceStub = new AppHibernationServiceStub(this); diff --git a/services/core/java/com/android/server/pm/OtaDexoptService.java b/services/core/java/com/android/server/pm/OtaDexoptService.java index d6400f3c879ef..b500c9930e004 100644 --- a/services/core/java/com/android/server/pm/OtaDexoptService.java +++ b/services/core/java/com/android/server/pm/OtaDexoptService.java @@ -381,7 +381,7 @@ public class OtaDexoptService extends IOtaDexopt.Stub { } // Does the package have code? If not, there won't be any artifacts. - if (!PackageDexOptimizer.canOptimizePackage(pkg)) { + if (!mPackageManagerService.mPackageDexOptimizer.canOptimizePackage(pkg)) { continue; } if (pkg.getPath() == null) { diff --git a/services/core/java/com/android/server/pm/PackageDexOptimizer.java b/services/core/java/com/android/server/pm/PackageDexOptimizer.java index 44f7d88693223..040457bf90001 100644 --- a/services/core/java/com/android/server/pm/PackageDexOptimizer.java +++ b/services/core/java/com/android/server/pm/PackageDexOptimizer.java @@ -63,7 +63,10 @@ import android.util.Slog; import android.util.SparseArray; import com.android.internal.annotations.GuardedBy; +import com.android.internal.annotations.VisibleForTesting; import com.android.internal.util.IndentingPrintWriter; +import com.android.server.LocalServices; +import com.android.server.apphibernation.AppHibernationManagerInternal; import com.android.server.pm.Installer.InstallerException; import com.android.server.pm.dex.ArtManagerService; import com.android.server.pm.dex.ArtStatsLogUtils; @@ -107,16 +110,24 @@ public class PackageDexOptimizer { private volatile boolean mSystemReady; private final ArtStatsLogger mArtStatsLogger = new ArtStatsLogger(); + private final Injector mInjector; + private static final Random sRandom = new Random(); PackageDexOptimizer(Installer installer, Object installLock, Context context, String wakeLockTag) { - this.mInstaller = installer; - this.mInstallLock = installLock; + this(new Injector() { + @Override + public AppHibernationManagerInternal getAppHibernationManagerInternal() { + return LocalServices.getService(AppHibernationManagerInternal.class); + } - PowerManager powerManager = (PowerManager)context.getSystemService(Context.POWER_SERVICE); - mDexoptWakeLock = powerManager.newWakeLock(PowerManager.PARTIAL_WAKE_LOCK, wakeLockTag); + @Override + public PowerManager getPowerManager(Context context) { + return context.getSystemService(PowerManager.class); + } + }, installer, installLock, context, wakeLockTag); } protected PackageDexOptimizer(PackageDexOptimizer from) { @@ -124,9 +135,21 @@ public class PackageDexOptimizer { this.mInstallLock = from.mInstallLock; this.mDexoptWakeLock = from.mDexoptWakeLock; this.mSystemReady = from.mSystemReady; + this.mInjector = from.mInjector; } - static boolean canOptimizePackage(AndroidPackage pkg) { + @VisibleForTesting + PackageDexOptimizer(@NonNull Injector injector, Installer installer, Object installLock, + Context context, String wakeLockTag) { + this.mInstaller = installer; + this.mInstallLock = installLock; + + PowerManager powerManager = injector.getPowerManager(context); + mDexoptWakeLock = powerManager.newWakeLock(PowerManager.PARTIAL_WAKE_LOCK, wakeLockTag); + mInjector = injector; + } + + boolean canOptimizePackage(AndroidPackage pkg) { // We do not dexopt a package with no code. // Note that the system package is marked as having no code, however we can // still optimize it via dexoptSystemServerPath. @@ -134,6 +157,17 @@ public class PackageDexOptimizer { return false; } + // We do not dexopt unused packages. + // It's possible for this to be called before app hibernation service is ready due to + // an OTA dexopt. In this case, we ignore the hibernation check here. This is fine since + // a hibernating app should have no artifacts to copy in the first place. + AppHibernationManagerInternal ahm = mInjector.getAppHibernationManagerInternal(); + if (ahm != null + && ahm.isHibernatingGlobally(pkg.getPackageName()) + && ahm.isOatArtifactDeletionEnabled()) { + return false; + } + return true; } @@ -921,4 +955,13 @@ public class PackageDexOptimizer { return flags | DEXOPT_FORCE; } } + + /** + * Injector for {@link PackageDexOptimizer} dependencies + */ + interface Injector { + AppHibernationManagerInternal getAppHibernationManagerInternal(); + + PowerManager getPowerManager(Context context); + } } diff --git a/services/core/java/com/android/server/pm/PackageManagerService.java b/services/core/java/com/android/server/pm/PackageManagerService.java index 821900bf57fea..979b77f050b4c 100644 --- a/services/core/java/com/android/server/pm/PackageManagerService.java +++ b/services/core/java/com/android/server/pm/PackageManagerService.java @@ -367,7 +367,6 @@ import com.android.server.SystemConfig; import com.android.server.SystemServerInitThreadPool; import com.android.server.Watchdog; import com.android.server.apphibernation.AppHibernationManagerInternal; -import com.android.server.apphibernation.AppHibernationService; import com.android.server.compat.CompatChange; import com.android.server.compat.PlatformCompat; import com.android.server.net.NetworkPolicyManagerInternal; @@ -1458,7 +1457,7 @@ public class PackageManagerService extends IPackageManager.Stub final ArtManagerService mArtManagerService; - private final PackageDexOptimizer mPackageDexOptimizer; + final PackageDexOptimizer mPackageDexOptimizer; // DexManager handles the usage of dex files (e.g. secondary files, whether or not a package // is used by other apps). private final DexManager mDexManager; @@ -12741,7 +12740,7 @@ public class PackageManagerService extends IPackageManager.Stub } } - if (!PackageDexOptimizer.canOptimizePackage(pkg)) { + if (!mPackageDexOptimizer.canOptimizePackage(pkg)) { if (DEBUG_DEXOPT) { Log.i(TAG, "Skipping update of non-optimizable app " + pkg.getPackageName()); } @@ -12992,16 +12991,11 @@ public class PackageManagerService extends IPackageManager.Stub ArraySet pkgs = new ArraySet<>(); synchronized (mLock) { for (AndroidPackage p : mPackages.values()) { - if (PackageDexOptimizer.canOptimizePackage(p)) { + if (mPackageDexOptimizer.canOptimizePackage(p)) { pkgs.add(p.getPackageName()); } } } - if (AppHibernationService.isAppHibernationEnabled()) { - AppHibernationManagerInternal appHibernationManager = - mInjector.getLocalService(AppHibernationManagerInternal.class); - pkgs.removeIf(pkgName -> appHibernationManager.isHibernatingGlobally(pkgName)); - } return pkgs; } diff --git a/services/tests/mockingservicestests/src/com/android/server/pm/PackageManagerServiceHibernationTests.kt b/services/tests/mockingservicestests/src/com/android/server/pm/PackageManagerServiceHibernationTests.kt index 72bc77ebb9eff..fd97557480bbf 100644 --- a/services/tests/mockingservicestests/src/com/android/server/pm/PackageManagerServiceHibernationTests.kt +++ b/services/tests/mockingservicestests/src/com/android/server/pm/PackageManagerServiceHibernationTests.kt @@ -16,8 +16,10 @@ package com.android.server.pm +import android.content.Context import android.os.Build import android.os.Handler +import android.os.PowerManager import android.provider.DeviceConfig import android.provider.DeviceConfig.NAMESPACE_APP_HIBERNATION import android.testing.AndroidTestingRunner @@ -55,6 +57,8 @@ class PackageManagerServiceHibernationTests { @Mock lateinit var appHibernationManager: AppHibernationManagerInternal + @Mock + lateinit var powerManager: PowerManager @Before @Throws(Exception::class) @@ -68,6 +72,24 @@ class PackageManagerServiceHibernationTests { .thenReturn(appHibernationManager) whenever(rule.mocks().injector.handler) .thenReturn(Handler(TestableLooper.get(this).looper)) + val injector = object : PackageDexOptimizer.Injector { + override fun getAppHibernationManagerInternal(): AppHibernationManagerInternal { + return appHibernationManager + } + + override fun getPowerManager(context: Context?): PowerManager { + return powerManager + } + } + val packageDexOptimizer = PackageDexOptimizer( + injector, + rule.mocks().installer, + rule.mocks().installLock, + rule.mocks().context, + "*dexopt*") + whenever(rule.mocks().injector.packageDexOptimizer) + .thenReturn(packageDexOptimizer) + whenever(appHibernationManager.isOatArtifactDeletionEnabled).thenReturn(true) } @Test