From c6540daf8388ce1ebd2e4157ec70b9651ae14b4f Mon Sep 17 00:00:00 2001 From: Calin Juravle Date: Wed, 15 Nov 2017 16:28:01 -0800 Subject: [PATCH] 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 --- .../server/pm/PackageDexOptimizer.java | 7 ++ .../server/pm/PackageManagerService.java | 93 ++++++++++--------- 2 files changed, 56 insertions(+), 44 deletions(-) diff --git a/services/core/java/com/android/server/pm/PackageDexOptimizer.java b/services/core/java/com/android/server/pm/PackageDexOptimizer.java index 00cfa3104b0a7..730a9fdad420c 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 @@ public class PackageDexOptimizer { 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 class PackageDexOptimizer { */ 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 2d5f7c7124c48..e4fe1d6fd682b 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 @@ public class PackageManagerService extends IPackageManager.Stub 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 @@ public class PackageManagerService extends IPackageManager.Stub } } + // 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) {