From b1cdba13f08a0230821b1b2fd0936f586cb61c29 Mon Sep 17 00:00:00 2001 From: Calin Juravle Date: Thu, 9 Apr 2020 19:58:08 -0700 Subject: [PATCH 1/3] Add special path for system server dex file reconciliation We need a special path for system server dex file reconcilliation. - we cannot use the application info to extract the arguments for the installd call - but we can use normal file checks because the system server has the right permission. Test: DexManagerTests, manual Bug: 148774920 Change-Id: Ic63aee129d10f6905cecbdba1feeb0320ee0ca1c --- .../com/android/server/pm/dex/DexManager.java | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/services/core/java/com/android/server/pm/dex/DexManager.java b/services/core/java/com/android/server/pm/dex/DexManager.java index 6dcf71e9fbf03..7538e86c8e9e0 100644 --- a/services/core/java/com/android/server/pm/dex/DexManager.java +++ b/services/core/java/com/android/server/pm/dex/DexManager.java @@ -600,6 +600,23 @@ public class DexManager { packageName, dexUseInfo.getOwnerUserId()) || updated; continue; } + + // Special handle system server files. + // We don't need an installd call because we have permissions to check if the file + // exists. + if (PLATFORM_PACKAGE_NAME.equals(packageName)) { + if (!Files.exists(Paths.get(dexPath))) { + if (DEBUG) { + Slog.w(TAG, "A dex file previously loaded by System Server does not exist " + + " anymore: " + dexPath); + } + updated = mPackageDexUsage.removeUserPackage( + packageName, dexUseInfo.getOwnerUserId()) || updated; + } + continue; + } + + // This is a regular application. ApplicationInfo info = pkg.applicationInfo; int flags = 0; if (info.deviceProtectedDataDir != null && From ffcd7a586bcea521a6d47f9f36fc5c08764eb9a7 Mon Sep 17 00:00:00 2001 From: Calin Juravle Date: Thu, 9 Apr 2020 20:05:33 -0700 Subject: [PATCH 2/3] Ensure that system server compiler filter is set to verify It is useless to try a higher filter because system server cannot load executable code. For now this is hard coded but we should consider making it a system property to facilitate experiments. Test: manual, DexManagerTests Bug: 148774920 Change-Id: I10326823a1737b879f61eb4e1886f697807f1184 --- .../com/android/server/pm/dex/DexManager.java | 19 ++++++++++++++++--- .../android/server/pm/dex/DexoptOptions.java | 13 +++++++++++++ 2 files changed, 29 insertions(+), 3 deletions(-) diff --git a/services/core/java/com/android/server/pm/dex/DexManager.java b/services/core/java/com/android/server/pm/dex/DexManager.java index 7538e86c8e9e0..1f41724da7e2b 100644 --- a/services/core/java/com/android/server/pm/dex/DexManager.java +++ b/services/core/java/com/android/server/pm/dex/DexManager.java @@ -79,6 +79,10 @@ public class DexManager { private static final String PROPERTY_NAME_PM_DEXOPT_PRIV_APPS_OOB_LIST = "pm.dexopt.priv-apps-oob-list"; + // System server cannot load executable code outside system partitions. + // However it can load verification data - thus we pick the "verify" compiler filter. + private static final String SYSTEM_SERVER_COMPILER_FILTER = "verify"; + private final Context mContext; // Maps package name to code locations. @@ -501,8 +505,17 @@ public class DexManager { return PackageDexOptimizer.DEX_OPT_FAILED; } - PackageDexOptimizer pdo = getPackageDexOptimizer(options); - String packageName = options.getPackageName(); + // Override compiler filter for system server to the expected one. + // + // We could let the caller do this every time the invoke PackageManagerServer#dexopt. + // However, there are a few places were this will need to be done which creates + // redundancy and the danger of overlooking the config (and thus generating code that will + // waste storage and time). + DexoptOptions overriddenOptions = options.overrideCompilerFilter( + SYSTEM_SERVER_COMPILER_FILTER); + + PackageDexOptimizer pdo = getPackageDexOptimizer(overriddenOptions); + String packageName = overriddenOptions.getPackageName(); PackageUseInfo useInfo = getPackageUseInfoOrDefault(packageName); if (useInfo.getDexUseInfoMap().isEmpty()) { if (DEBUG) { @@ -527,7 +540,7 @@ public class DexManager { continue; } - int newResult = pdo.dexoptSystemServerPath(dexPath, dexUseInfo, options); + int newResult = pdo.dexoptSystemServerPath(dexPath, dexUseInfo, overriddenOptions); // The end result is: // - FAILED if any path failed, diff --git a/services/core/java/com/android/server/pm/dex/DexoptOptions.java b/services/core/java/com/android/server/pm/dex/DexoptOptions.java index de3c9f28218db..b453c898ded8d 100644 --- a/services/core/java/com/android/server/pm/dex/DexoptOptions.java +++ b/services/core/java/com/android/server/pm/dex/DexoptOptions.java @@ -166,4 +166,17 @@ public final class DexoptOptions { public int getCompilationReason() { return mCompilationReason; } + + /** + * Creates a new set of DexoptOptions which are the same with the exception of the compiler + * filter (set to the given value). + */ + public DexoptOptions overrideCompilerFilter(String newCompilerFilter) { + return new DexoptOptions( + mPackageName, + mCompilationReason, + newCompilerFilter, + mSplitName, + mFlags); + } } From 5ebac36ca983f9d682cd45aedd1f89775687da85 Mon Sep 17 00:00:00 2001 From: Calin Juravle Date: Thu, 9 Apr 2020 20:03:26 -0700 Subject: [PATCH 3/3] Add SystemServer package to the list of optimizable packages This will allow the normal jobs to optimize system server dex files when needed. These include: the bakground dexopt job and the upgrade job. Test: manual, DexManagerTests Bug: 148774920 Change-Id: I70f8ffa06affd06c8d25031b81cf294f2689a52f --- .../com/android/server/pm/BackgroundDexOptService.java | 7 +++++-- .../java/com/android/server/pm/PackageDexOptimizer.java | 9 ++++++++- .../core/java/com/android/server/pm/dex/DexManager.java | 8 ++++++++ 3 files changed, 21 insertions(+), 3 deletions(-) diff --git a/services/core/java/com/android/server/pm/BackgroundDexOptService.java b/services/core/java/com/android/server/pm/BackgroundDexOptService.java index dab4bfd4df5a6..5415967c3bdcd 100644 --- a/services/core/java/com/android/server/pm/BackgroundDexOptService.java +++ b/services/core/java/com/android/server/pm/BackgroundDexOptService.java @@ -17,6 +17,7 @@ package com.android.server.pm; import static com.android.server.pm.PackageManagerService.DEBUG_DEXOPT; +import static com.android.server.pm.PackageManagerService.PLATFORM_PACKAGE_NAME; import android.annotation.Nullable; import android.app.job.JobInfo; @@ -434,7 +435,7 @@ public class BackgroundDexOptService extends JobService { | DexoptOptions.DEXOPT_DOWNGRADE; long package_size_before = getPackageSize(pm, pkg); - if (isForPrimaryDex) { + if (isForPrimaryDex || PLATFORM_PACKAGE_NAME.equals(pkg)) { // This applies for system apps or if packages location is not a directory, i.e. // monolithic install. if (!pm.canHaveOatDir(pkg)) { @@ -486,7 +487,9 @@ public class BackgroundDexOptService extends JobService { | DexoptOptions.DEXOPT_BOOT_COMPLETE | DexoptOptions.DEXOPT_IDLE_BACKGROUND_JOB; - return isForPrimaryDex + // System server share the same code path as primary dex files. + // PackageManagerService will select the right optimization path for it. + return (isForPrimaryDex || PLATFORM_PACKAGE_NAME.equals(pkg)) ? performDexOptPrimary(pm, pkg, reason, dexoptFlags) : performDexOptSecondary(pm, pkg, reason, dexoptFlags); } diff --git a/services/core/java/com/android/server/pm/PackageDexOptimizer.java b/services/core/java/com/android/server/pm/PackageDexOptimizer.java index 65b7cf3eabd16..1951e7417b2c5 100644 --- a/services/core/java/com/android/server/pm/PackageDexOptimizer.java +++ b/services/core/java/com/android/server/pm/PackageDexOptimizer.java @@ -32,6 +32,7 @@ import static com.android.server.pm.Installer.DEXOPT_STORAGE_CE; import static com.android.server.pm.Installer.DEXOPT_STORAGE_DE; import static com.android.server.pm.InstructionSets.getAppDexInstructionSets; import static com.android.server.pm.InstructionSets.getDexCodeInstructionSets; +import static com.android.server.pm.PackageManagerService.PLATFORM_PACKAGE_NAME; import static com.android.server.pm.PackageManagerService.WATCHDOG_TIMEOUT; import static com.android.server.pm.PackageManagerServiceCompilerMapping.getReasonName; @@ -115,7 +116,9 @@ public class PackageDexOptimizer { static boolean canOptimizePackage(AndroidPackage pkg) { // We do not dexopt a package with no code. - if (!pkg.isHasCode()) { + // Note that the system package is marked as having no code, however we can + // still optimize it via dexoptSystemServerPath. + if (!PLATFORM_PACKAGE_NAME.equals(pkg.getPackageName()) && !pkg.isHasCode()) { return false; } @@ -132,6 +135,10 @@ public class PackageDexOptimizer { int performDexOpt(AndroidPackage pkg, @NonNull PackageSetting pkgSetting, String[] instructionSets, CompilerStats.PackageStats packageStats, PackageDexUsage.PackageUseInfo packageUseInfo, DexoptOptions options) { + if (PLATFORM_PACKAGE_NAME.equals(pkg.getPackageName())) { + throw new IllegalArgumentException("System server dexopting should be done via " + + " DexManager and PackageDexOptimizer#dexoptSystemServerPath"); + } if (pkg.getUid() == -1) { throw new IllegalArgumentException("Dexopt for " + pkg.getPackageName() + " has invalid uid."); diff --git a/services/core/java/com/android/server/pm/dex/DexManager.java b/services/core/java/com/android/server/pm/dex/DexManager.java index 1f41724da7e2b..f7bf1d985786d 100644 --- a/services/core/java/com/android/server/pm/dex/DexManager.java +++ b/services/core/java/com/android/server/pm/dex/DexManager.java @@ -447,6 +447,14 @@ public class DexManager { * because they don't need to be compiled).. */ public boolean dexoptSecondaryDex(DexoptOptions options) { + if (PLATFORM_PACKAGE_NAME.equals(options.getPackageName())) { + // We could easily redirect to #dexoptSystemServer in this case. But there should be + // no-one calling this method directly for system server. + // As such we prefer to abort in this case. + Slog.wtf(TAG, "System server jars should be optimized with dexoptSystemServer"); + return false; + } + PackageDexOptimizer pdo = getPackageDexOptimizer(options); String packageName = options.getPackageName(); PackageUseInfo useInfo = getPackageUseInfoOrDefault(packageName);