From d2a82123ec613d9947dd5ffb1afe85a95ce2ec1e Mon Sep 17 00:00:00 2001 From: Martin Stjernholm Date: Sun, 19 Mar 2023 23:35:24 +0000 Subject: [PATCH] Make registerDexModule a nop regardless ART Service is enabled or not. ART Service doesn't support that method, but the callback should still be called, for compatibility. This also disables the method in the legacy code, to align behaviour when ART Service is enabled or not. The main problem with this API is that it doesn't take a classloader context, so it is hard to produce dexopt artifacts that can be succesfully loaded by the runtime later. This problem exists in the legacy code as well. Test: atest DexModuleRegistrationTest with and without dalvik.vm.useartservice=true Bug: 274096060 Bug: 37290820 Bug: 251903639 Change-Id: Ie1510268a721e846408b40819cd681063cfbb485 --- .../android/content/pm/PackageManager.java | 20 +--- .../server/pm/PackageManagerService.java | 38 ++----- .../com/android/server/pm/dex/DexManager.java | 57 ---------- .../server/pm/PackageManagerTests.java | 105 ------------------ 4 files changed, 14 insertions(+), 206 deletions(-) diff --git a/core/java/android/content/pm/PackageManager.java b/core/java/android/content/pm/PackageManager.java index b9c671a8f3ea2..21e2a131bcac0 100644 --- a/core/java/android/content/pm/PackageManager.java +++ b/core/java/android/content/pm/PackageManager.java @@ -10134,25 +10134,9 @@ public abstract class PackageManager { /** * Register an application dex module with the package manager. - * The package manager will keep track of the given module for future optimizations. * - * Dex module optimizations will disable the classpath checking at runtime. The client bares - * the responsibility to ensure that the static assumptions on classes in the optimized code - * hold at runtime (e.g. there's no duplicate classes in the classpath). - * - * Note that the package manager already keeps track of dex modules loaded with - * {@link dalvik.system.DexClassLoader} and {@link dalvik.system.PathClassLoader}. - * This can be called for an eager registration. - * - * The call might take a while and the results will be posted on the main thread, using - * the given callback. - * - * If the module is intended to be shared with other apps, make sure that the file - * permissions allow for it. - * If at registration time the permissions allow for others to read it, the module would - * be marked as a shared module which might undergo a different optimization strategy. - * (usually shared modules will generated larger optimizations artifacts, - * taking more disk space). + * This call no longer does anything. If a callback is given it is called with a false success + * value. * * @param dexModulePath the absolute path of the dex module. * @param callback if not null, {@link DexModuleRegisterCallback#onDexModuleRegistered} will diff --git a/services/core/java/com/android/server/pm/PackageManagerService.java b/services/core/java/com/android/server/pm/PackageManagerService.java index 6bc876037cfb8..b92e3c0ff428e 100644 --- a/services/core/java/com/android/server/pm/PackageManagerService.java +++ b/services/core/java/com/android/server/pm/PackageManagerService.java @@ -5561,32 +5561,18 @@ public class PackageManagerService implements PackageSender, TestUtilityService public void registerDexModule(String packageName, String dexModulePath, boolean isSharedModule, IDexModuleRegisterCallback callback) { - if (useArtService()) { - // ART Service currently doesn't support this explicit dexopting and instead relies - // on background dexopt for secondary dex files. This API is problematic since it - // doesn't provide the correct classloader context. - Slog.i(TAG, - "Ignored unsupported registerDexModule call for " + dexModulePath + " in " - + packageName); - return; - } - - int userId = UserHandle.getCallingUserId(); - ApplicationInfo ai = snapshot().getApplicationInfo(packageName, /*flags*/ 0, userId); - DexManager.RegisterDexModuleResult result; - if (ai == null) { - Slog.w(PackageManagerService.TAG, - "Registering a dex module for a package that does not exist for the" + - " calling user. package=" + packageName + ", user=" + userId); - result = new DexManager.RegisterDexModuleResult(false, "Package not installed"); - } else { - try { - result = mDexManager.registerDexModule( - ai, dexModulePath, isSharedModule, userId); - } catch (LegacyDexoptDisabledException e) { - throw new RuntimeException(e); - } - } + // ART Service doesn't support this explicit dexopting and instead relies on background + // dexopt for secondary dex files. For compat parity between ART Service and the legacy + // code it's disabled for both. + // + // Also, this API is problematic anyway since it doesn't provide the correct classloader + // context, so it is hard to produce dexopt artifacts that the runtime can load + // successfully. + Slog.i(TAG, + "Ignored unsupported registerDexModule call for " + dexModulePath + " in " + + packageName); + DexManager.RegisterDexModuleResult result = new DexManager.RegisterDexModuleResult( + false, "registerDexModule call not supported since Android U"); if (callback != null) { mHandler.post(() -> { 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 7f0c3f9f4f065..6e738daf93158 100644 --- a/services/core/java/com/android/server/pm/dex/DexManager.java +++ b/services/core/java/com/android/server/pm/dex/DexManager.java @@ -16,7 +16,6 @@ package com.android.server.pm.dex; -import static com.android.server.pm.InstructionSets.getAppDexInstructionSets; import static com.android.server.pm.PackageManagerService.PLATFORM_PACKAGE_NAME; import static com.android.server.pm.dex.PackageDexUsage.DexUseInfo; import static com.android.server.pm.dex.PackageDexUsage.PackageUseInfo; @@ -659,62 +658,6 @@ public class DexManager { } } - // TODO(calin): questionable API in the presence of class loaders context. Needs amends as the - // compilation happening here will use a pessimistic context. - public RegisterDexModuleResult registerDexModule(ApplicationInfo info, String dexPath, - boolean isSharedModule, int userId) throws LegacyDexoptDisabledException { - // Find the owning package record. - DexSearchResult searchResult = getDexPackage(info, dexPath, userId); - - if (searchResult.mOutcome == DEX_SEARCH_NOT_FOUND) { - return new RegisterDexModuleResult(false, "Package not found"); - } - if (!info.packageName.equals(searchResult.mOwningPackageName)) { - return new RegisterDexModuleResult(false, "Dex path does not belong to package"); - } - if (searchResult.mOutcome == DEX_SEARCH_FOUND_PRIMARY || - searchResult.mOutcome == DEX_SEARCH_FOUND_SPLIT) { - return new RegisterDexModuleResult(false, "Main apks cannot be registered"); - } - - // We found the package. Now record the usage for all declared ISAs. - boolean update = false; - // If this is a shared module set the loading package to an arbitrary package name - // so that we can mark that module as usedByOthers. - String loadingPackage = isSharedModule ? ".shared.module" : searchResult.mOwningPackageName; - for (String isa : getAppDexInstructionSets(info.primaryCpuAbi, info.secondaryCpuAbi)) { - boolean newUpdate = mPackageDexUsage.record(searchResult.mOwningPackageName, - dexPath, userId, isa, /*primaryOrSplit*/ false, - loadingPackage, - PackageDexUsage.VARIABLE_CLASS_LOADER_CONTEXT, - /*overwriteCLC=*/ false); - update |= newUpdate; - } - if (update) { - mPackageDexUsage.maybeWriteAsync(); - } - - DexUseInfo dexUseInfo = mPackageDexUsage.getPackageUseInfo(searchResult.mOwningPackageName) - .getDexUseInfoMap().get(dexPath); - - // Try to optimize the package according to the install reason. - DexoptOptions options = new DexoptOptions(info.packageName, - PackageManagerService.REASON_INSTALL, /*flags*/0); - - int result = mPackageDexOptimizer.dexOptSecondaryDexPath(info, dexPath, dexUseInfo, - options); - - // If we fail to optimize the package log an error but don't propagate the error - // back to the app. The app cannot do much about it and the background job - // will rety again when it executes. - // TODO(calin): there might be some value to return the error here but it may - // cause red herrings since that doesn't mean the app cannot use the module. - if (result != PackageDexOptimizer.DEX_OPT_FAILED) { - Slog.e(TAG, "Failed to optimize dex module " + dexPath); - } - return new RegisterDexModuleResult(true, "Dex module registered successfully"); - } - /** * Return all packages that contain records of secondary dex files. */ diff --git a/services/tests/PackageManagerServiceTests/server/src/com/android/server/pm/PackageManagerTests.java b/services/tests/PackageManagerServiceTests/server/src/com/android/server/pm/PackageManagerTests.java index fd31b2211b7ad..d559b67218cae 100644 --- a/services/tests/PackageManagerServiceTests/server/src/com/android/server/pm/PackageManagerTests.java +++ b/services/tests/PackageManagerServiceTests/server/src/com/android/server/pm/PackageManagerTests.java @@ -84,9 +84,6 @@ import java.io.FileInputStream; import java.io.IOException; import java.io.InputStream; import java.io.OutputStream; -import java.nio.file.Files; -import java.nio.file.Paths; -import java.nio.file.StandardCopyOption; import java.util.Collections; import java.util.HashSet; import java.util.List; @@ -2904,108 +2901,6 @@ public class PackageManagerTests extends AndroidTestCase { PackageInfo.INSTALL_LOCATION_UNSPECIFIED); } - private static class TestDexModuleRegisterCallback - extends PackageManager.DexModuleRegisterCallback { - private String mDexModulePath = null; - private boolean mSuccess = false; - private String mMessage = null; - CountDownLatch doneSignal = new CountDownLatch(1); - - @Override - public void onDexModuleRegistered(String dexModulePath, boolean success, String message) { - mDexModulePath = dexModulePath; - mSuccess = success; - mMessage = message; - doneSignal.countDown(); - } - - boolean waitTillDone() { - long startTime = System.currentTimeMillis(); - while (System.currentTimeMillis() - startTime < MAX_WAIT_TIME) { - try { - return doneSignal.await(MAX_WAIT_TIME, TimeUnit.MILLISECONDS); - } catch (InterruptedException e) { - Log.i(TAG, "Interrupted during sleep", e); - } - } - return false; - } - - } - - // Verify that the base code path cannot be registered. - public void testRegisterDexModuleBaseCode() throws Exception { - PackageManager pm = getPm(); - ApplicationInfo info = getContext().getApplicationInfo(); - TestDexModuleRegisterCallback callback = new TestDexModuleRegisterCallback(); - pm.registerDexModule(info.getBaseCodePath(), callback); - assertTrue(callback.waitTillDone()); - assertEquals(info.getBaseCodePath(), callback.mDexModulePath); - assertFalse("BaseCodePath should not be registered", callback.mSuccess); - } - - // Verify that modules which are not own by the calling package are not registered. - public void testRegisterDexModuleNotOwningModule() throws Exception { - TestDexModuleRegisterCallback callback = new TestDexModuleRegisterCallback(); - String moduleBelongingToOtherPackage = "/data/user/0/com.google.android.gms/module.apk"; - getPm().registerDexModule(moduleBelongingToOtherPackage, callback); - assertTrue(callback.waitTillDone()); - assertEquals(moduleBelongingToOtherPackage, callback.mDexModulePath); - assertTrue(callback.waitTillDone()); - assertFalse("Only modules belonging to the calling package can be registered", - callback.mSuccess); - } - - // Verify that modules owned by the package are successfully registered. - public void testRegisterDexModuleSuccessfully() throws Exception { - ApplicationInfo info = getContext().getApplicationInfo(); - // Copy the main apk into the data folder and use it as a "module". - File dexModuleDir = new File(info.dataDir, "module-dir"); - File dexModule = new File(dexModuleDir, "module.apk"); - try { - assertNotNull(FileUtils.createDir( - dexModuleDir.getParentFile(), dexModuleDir.getName())); - Files.copy(Paths.get(info.getBaseCodePath()), dexModule.toPath(), - StandardCopyOption.REPLACE_EXISTING); - TestDexModuleRegisterCallback callback = new TestDexModuleRegisterCallback(); - getPm().registerDexModule(dexModule.toString(), callback); - assertTrue(callback.waitTillDone()); - assertEquals(dexModule.toString(), callback.mDexModulePath); - assertTrue(callback.waitTillDone()); - assertTrue(callback.mMessage, callback.mSuccess); - - // NOTE: - // This actually verifies internal behaviour which might change. It's not - // ideal but it's the best we can do since there's no other place we can currently - // write a better test. - for(String isa : getAppDexInstructionSets(info)) { - Files.exists(Paths.get(dexModuleDir.toString(), "oat", isa, "module.odex")); - Files.exists(Paths.get(dexModuleDir.toString(), "oat", isa, "module.vdex")); - } - } finally { - FileUtils.deleteContentsAndDir(dexModuleDir); - } - } - - // If the module does not exist on disk we should get a failure. - public void testRegisterDexModuleNotExists() throws Exception { - ApplicationInfo info = getContext().getApplicationInfo(); - String nonExistentApk = Paths.get(info.dataDir, "non-existent.apk").toString(); - TestDexModuleRegisterCallback callback = new TestDexModuleRegisterCallback(); - getPm().registerDexModule(nonExistentApk, callback); - assertTrue(callback.waitTillDone()); - assertEquals(nonExistentApk, callback.mDexModulePath); - assertTrue(callback.waitTillDone()); - assertFalse("DexModule registration should fail", callback.mSuccess); - } - - // If the module does not exist on disk we should get a failure. - public void testRegisterDexModuleNotExistsNoCallback() throws Exception { - ApplicationInfo info = getContext().getApplicationInfo(); - String nonExistentApk = Paths.get(info.dataDir, "non-existent.apk").toString(); - getPm().registerDexModule(nonExistentApk, null); - } - @LargeTest public void testMinInstallableTargetSdkPass() throws Exception { // Test installing a package that meets the minimum installable sdk requirement