Fix package install flow w.r.t. dexopt
Calling dexopt before the applicationInfo gets the uid is wrong.
Dexopt needs to be able to set the GID of the odex file to the
UserHandle.getSharedAppGid(pkg.applicationInfo.uid) and that is
possible only with a valid uid.
Move the dexopt logic after installNewPackageLIF/replacePackageLIF
to ensure that we get a valid uid.
Bug: 69331247
Test: adb install & check the GID of the compiler artifacts
Change-Id: I2434a1a0b9015091a9af2009b3f785b7a16e1256
diff --git a/services/core/java/com/android/server/pm/PackageDexOptimizer.java b/services/core/java/com/android/server/pm/PackageDexOptimizer.java
index 00cfa31..730a9fd 100644
--- a/services/core/java/com/android/server/pm/PackageDexOptimizer.java
+++ b/services/core/java/com/android/server/pm/PackageDexOptimizer.java
@@ -128,6 +128,10 @@
int performDexOpt(PackageParser.Package pkg, String[] sharedLibraries,
String[] instructionSets, CompilerStats.PackageStats packageStats,
PackageDexUsage.PackageUseInfo packageUseInfo, DexoptOptions options) {
+ if (pkg.applicationInfo.uid == -1) {
+ throw new IllegalArgumentException("Dexopt for " + pkg.packageName
+ + " has invalid uid.");
+ }
if (!canOptimizePackage(pkg)) {
return DEX_OPT_SKIPPED;
}
@@ -299,6 +303,9 @@
*/
public int dexOptSecondaryDexPath(ApplicationInfo info, String path,
PackageDexUsage.DexUseInfo dexUseInfo, DexoptOptions options) {
+ if (info.uid == -1) {
+ throw new IllegalArgumentException("Dexopt for path " + path + " has invalid uid.");
+ }
synchronized (mInstallLock) {
final long acquireTime = acquireWakeLockLI(info.uid);
try {
diff --git a/services/core/java/com/android/server/pm/PackageManagerService.java b/services/core/java/com/android/server/pm/PackageManagerService.java
index 2d5f7c7..e4fe1d6 100644
--- a/services/core/java/com/android/server/pm/PackageManagerService.java
+++ b/services/core/java/com/android/server/pm/PackageManagerService.java
@@ -16508,50 +16508,6 @@
return;
}
- // Verify if we need to dexopt the app.
- //
- // NOTE: it is *important* to call dexopt after doRename which will sync the
- // package data from PackageParser.Package and its corresponding ApplicationInfo.
- //
- // We only need to dexopt if the package meets ALL of the following conditions:
- // 1) it is not forward locked.
- // 2) it is not on on an external ASEC container.
- // 3) it is not an instant app or if it is then dexopt is enabled via gservices.
- //
- // Note that we do not dexopt instant apps by default. dexopt can take some time to
- // complete, so we skip this step during installation. Instead, we'll take extra time
- // the first time the instant app starts. It's preferred to do it this way to provide
- // continuous progress to the useur instead of mysteriously blocking somewhere in the
- // middle of running an instant app. The default behaviour can be overridden
- // via gservices.
- final boolean performDexopt = !forwardLocked
- && !pkg.applicationInfo.isExternalAsec()
- && (!instantApp || Global.getInt(mContext.getContentResolver(),
- Global.INSTANT_APP_DEXOPT_ENABLED, 0) != 0);
-
- if (performDexopt) {
- Trace.traceBegin(TRACE_TAG_PACKAGE_MANAGER, "dexopt");
- // Do not run PackageDexOptimizer through the local performDexOpt
- // method because `pkg` may not be in `mPackages` yet.
- //
- // Also, don't fail application installs if the dexopt step fails.
- DexoptOptions dexoptOptions = new DexoptOptions(pkg.packageName,
- REASON_INSTALL,
- DexoptOptions.DEXOPT_BOOT_COMPLETE);
- mPackageDexOptimizer.performDexOpt(pkg, pkg.usesLibraryFiles,
- null /* instructionSets */,
- getOrCreateCompilerPackageStats(pkg),
- mDexManager.getPackageUseInfoOrDefault(pkg.packageName),
- dexoptOptions);
- Trace.traceEnd(TRACE_TAG_PACKAGE_MANAGER);
- }
-
- // Notify BackgroundDexOptService that the package has been changed.
- // If this is an update of a package which used to fail to compile,
- // BackgroundDexOptService will remove it from its blacklist.
- // TODO: Layering violation
- BackgroundDexOptService.notifyPackageChanged(pkg.packageName);
-
if (!instantApp) {
startIntentFilterVerifications(args.user.getIdentifier(), replace, pkg);
} else {
@@ -16583,6 +16539,55 @@
}
}
+ // Check whether we need to dexopt the app.
+ //
+ // NOTE: it is IMPORTANT to call dexopt:
+ // - after doRename which will sync the package data from PackageParser.Package and its
+ // corresponding ApplicationInfo.
+ // - after installNewPackageLIF or replacePackageLIF which will update result with the
+ // uid of the application (pkg.applicationInfo.uid).
+ // This update happens in place!
+ //
+ // We only need to dexopt if the package meets ALL of the following conditions:
+ // 1) it is not forward locked.
+ // 2) it is not on on an external ASEC container.
+ // 3) it is not an instant app or if it is then dexopt is enabled via gservices.
+ //
+ // Note that we do not dexopt instant apps by default. dexopt can take some time to
+ // complete, so we skip this step during installation. Instead, we'll take extra time
+ // the first time the instant app starts. It's preferred to do it this way to provide
+ // continuous progress to the useur instead of mysteriously blocking somewhere in the
+ // middle of running an instant app. The default behaviour can be overridden
+ // via gservices.
+ final boolean performDexopt = (res.returnCode == PackageManager.INSTALL_SUCCEEDED)
+ && !forwardLocked
+ && !pkg.applicationInfo.isExternalAsec()
+ && (!instantApp || Global.getInt(mContext.getContentResolver(),
+ Global.INSTANT_APP_DEXOPT_ENABLED, 0) != 0);
+
+ if (performDexopt) {
+ Trace.traceBegin(TRACE_TAG_PACKAGE_MANAGER, "dexopt");
+ // Do not run PackageDexOptimizer through the local performDexOpt
+ // method because `pkg` may not be in `mPackages` yet.
+ //
+ // Also, don't fail application installs if the dexopt step fails.
+ DexoptOptions dexoptOptions = new DexoptOptions(pkg.packageName,
+ REASON_INSTALL,
+ DexoptOptions.DEXOPT_BOOT_COMPLETE);
+ mPackageDexOptimizer.performDexOpt(pkg, pkg.usesLibraryFiles,
+ null /* instructionSets */,
+ getOrCreateCompilerPackageStats(pkg),
+ mDexManager.getPackageUseInfoOrDefault(pkg.packageName),
+ dexoptOptions);
+ Trace.traceEnd(TRACE_TAG_PACKAGE_MANAGER);
+ }
+
+ // Notify BackgroundDexOptService that the package has been changed.
+ // If this is an update of a package which used to fail to compile,
+ // BackgroundDexOptService will remove it from its blacklist.
+ // TODO: Layering violation
+ BackgroundDexOptService.notifyPackageChanged(pkg.packageName);
+
synchronized (mPackages) {
final PackageSetting ps = mSettings.mPackages.get(pkgName);
if (ps != null) {