diff options
5 files changed, 85 insertions, 21 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 cc6d80a2aeec..6d3de968b036 100644 --- a/services/core/java/com/android/server/pm/dex/DexManager.java +++ b/services/core/java/com/android/server/pm/dex/DexManager.java @@ -215,7 +215,7 @@ public class DexManager { searchResult.mOutcome == DEX_SEARCH_FOUND_SPLIT; if (primaryOrSplit && !isUsedByOtherApps - && !PLATFORM_PACKAGE_NAME.equals(searchResult.mOwningPackageName)) { + && !isPlatformPackage(searchResult.mOwningPackageName)) { // If the dex file is the primary apk (or a split) and not isUsedByOtherApps // do not record it. This case does not bring any new usable information // and can be safely skipped. @@ -232,15 +232,24 @@ public class DexManager { } String classLoaderContext = mapping.getValue(); + + // Overwrite the class loader context for system server (instead of merging it). + // We expect system server jars to only change contexts in between OTAs and to + // otherwise be stable. + // Instead of implementing a complex clear-context logic post OTA, it is much + // simpler to always override the context for system server. This way, the context + // will always be up to date and we will avoid merging which could lead to the + // the context being marked as variable and thus making dexopt non-optimal. + boolean overwriteCLC = isPlatformPackage(searchResult.mOwningPackageName); + if (classLoaderContext != null && VMRuntime.isValidClassLoaderContext(classLoaderContext)) { // Record dex file usage. If the current usage is a new pattern (e.g. new // secondary, or UsedByOtherApps), record will return true and we trigger an // async write to disk to make sure we don't loose the data in case of a reboot. - if (mPackageDexUsage.record(searchResult.mOwningPackageName, dexPath, loaderUserId, loaderIsa, primaryOrSplit, - loadingAppInfo.packageName, classLoaderContext)) { + loadingAppInfo.packageName, classLoaderContext, overwriteCLC)) { mPackageDexUsage.maybeWriteAsync(); } } @@ -474,7 +483,7 @@ public class DexManager { * because they don't need to be compiled).. */ public boolean dexoptSecondaryDex(DexoptOptions options) { - if (PLATFORM_PACKAGE_NAME.equals(options.getPackageName())) { + if (isPlatformPackage(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. @@ -534,7 +543,7 @@ public class DexManager { * <p>PackageDexOptimizer.DEX_OPT_PERFORMED if all dexopt operations succeeded. */ public int dexoptSystemServer(DexoptOptions options) { - if (!PLATFORM_PACKAGE_NAME.equals(options.getPackageName())) { + if (!isPlatformPackage(options.getPackageName())) { Slog.wtf(TAG, "Non system server package used when trying to dexopt system server:" + options.getPackageName()); return PackageDexOptimizer.DEX_OPT_FAILED; @@ -662,7 +671,7 @@ public class DexManager { // 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 (isPlatformPackage(packageName)) { if (!Files.exists(Paths.get(dexPath))) { if (DEBUG) { Slog.w(TAG, "A dex file previously loaded by System Server does not exist " @@ -739,7 +748,8 @@ public class DexManager { boolean newUpdate = mPackageDexUsage.record(searchResult.mOwningPackageName, dexPath, userId, isa, /*primaryOrSplit*/ false, loadingPackage, - PackageDexUsage.VARIABLE_CLASS_LOADER_CONTEXT); + PackageDexUsage.VARIABLE_CLASS_LOADER_CONTEXT, + /*overwriteCLC*/ false); update |= newUpdate; } if (update) { @@ -809,7 +819,7 @@ public class DexManager { // Note: We don't have any way to detect which code paths are actually // owned by system server. We can only assume that such paths are on // system partitions. - if (PLATFORM_PACKAGE_NAME.equals(loadingAppInfo.packageName)) { + if (isPlatformPackage(loadingAppInfo.packageName)) { if (isSystemServerDexPathSupportedForOdex(dexPath)) { // We record system server dex files as secondary dex files. // The reason is that we only record the class loader context for secondary dex @@ -842,6 +852,11 @@ public class DexManager { return new DexSearchResult(null, DEX_SEARCH_NOT_FOUND); } + /** Returns true if this is the platform package .*/ + private static boolean isPlatformPackage(String packageName) { + return PLATFORM_PACKAGE_NAME.equals(packageName); + } + private static <K,V> V putIfAbsent(Map<K,V> map, K key, V newValue) { V existingValue = map.putIfAbsent(key, newValue); return existingValue == null ? newValue : existingValue; diff --git a/services/core/java/com/android/server/pm/dex/PackageDexUsage.java b/services/core/java/com/android/server/pm/dex/PackageDexUsage.java index 10760f52a02b..3d63b75c5da1 100644 --- a/services/core/java/com/android/server/pm/dex/PackageDexUsage.java +++ b/services/core/java/com/android/server/pm/dex/PackageDexUsage.java @@ -111,17 +111,18 @@ public class PackageDexUsage extends AbstractStatsBase<Void> { * @param dexPath the path of the dex files being loaded * @param ownerUserId the user id which runs the code loading the dex files * @param loaderIsa the ISA of the app loading the dex files - * @param isUsedByOtherApps whether or not this dex file was not loaded by its owning package * @param primaryOrSplit whether or not the dex file is a primary/split dex. True indicates * the file is either primary or a split. False indicates the file is secondary dex. * @param loadingPackageName the package performing the load. Recorded only if it is different * than {@param owningPackageName}. + * @param overwriteCLC if true, the class loader context will be overwritten instead of being + * merged * @return true if the dex load constitutes new information, or false if this information * has been seen before. */ /* package */ boolean record(String owningPackageName, String dexPath, int ownerUserId, String loaderIsa, boolean primaryOrSplit, - String loadingPackageName, String classLoaderContext) { + String loadingPackageName, String classLoaderContext, boolean overwriteCLC) { if (!PackageManagerServiceUtils.checkISA(loaderIsa)) { throw new IllegalArgumentException("loaderIsa " + loaderIsa + " is unsupported"); } @@ -193,7 +194,7 @@ public class PackageDexUsage extends AbstractStatsBase<Void> { } // Merge the information into the existing data. // Returns true if there was an update. - return existingData.merge(newData) || updateLoadingPackages; + return existingData.merge(newData, overwriteCLC) || updateLoadingPackages; } } } @@ -809,14 +810,16 @@ public class PackageDexUsage extends AbstractStatsBase<Void> { mLoadingPackages = new HashSet<>(other.mLoadingPackages); } - private boolean merge(DexUseInfo dexUseInfo) { + private boolean merge(DexUseInfo dexUseInfo, boolean overwriteCLC) { boolean oldIsUsedByOtherApps = mIsUsedByOtherApps; mIsUsedByOtherApps = mIsUsedByOtherApps || dexUseInfo.mIsUsedByOtherApps; boolean updateIsas = mLoaderIsas.addAll(dexUseInfo.mLoaderIsas); boolean updateLoadingPackages = mLoadingPackages.addAll(dexUseInfo.mLoadingPackages); String oldClassLoaderContext = mClassLoaderContext; - if (isUnsupportedContext(mClassLoaderContext)) { + if (overwriteCLC) { + mClassLoaderContext = dexUseInfo.mClassLoaderContext; + } else if (isUnsupportedContext(mClassLoaderContext)) { mClassLoaderContext = dexUseInfo.mClassLoaderContext; } else if (!Objects.equals(mClassLoaderContext, dexUseInfo.mClassLoaderContext)) { // We detected a context change. diff --git a/services/tests/servicestests/src/com/android/server/pm/dex/DexManagerTests.java b/services/tests/servicestests/src/com/android/server/pm/dex/DexManagerTests.java index ff43da6370e8..ee0a16a70265 100644 --- a/services/tests/servicestests/src/com/android/server/pm/dex/DexManagerTests.java +++ b/services/tests/servicestests/src/com/android/server/pm/dex/DexManagerTests.java @@ -86,6 +86,7 @@ public class DexManagerTests { private TestData mBarUser0DelegateLastClassLoader; private TestData mSystemServerJar; + private TestData mSystemServerJarUpdatedContext; private TestData mSystemServerJarInvalid; private int mUser0; @@ -113,6 +114,8 @@ public class DexManagerTests { mSystemServerJar = new TestData("android", isa, mUser0, PATH_CLASS_LOADER_NAME); mSystemServerJarInvalid = new TestData("android", isa, mUser0, PATH_CLASS_LOADER_NAME); + mSystemServerJarUpdatedContext = new TestData("android", isa, mUser0, + DELEGATE_LAST_CLASS_LOADER_NAME); mDexManager = new DexManager(/*Context*/ null, mPM, /*PackageDexOptimizer*/ null, mInstaller, mInstallLock); @@ -522,6 +525,24 @@ public class DexManagerTests { } @Test + public void testSystemServerOverwritesContext() { + // Record bar secondaries with the default PathClassLoader. + List<String> secondaries = mSystemServerJar.getSecondaryDexPaths(); + + notifyDexLoad(mSystemServerJar, secondaries, mUser0); + PackageUseInfo pui = getPackageUseInfo(mSystemServerJar); + assertSecondaryUse(mSystemServerJar, pui, secondaries, /*isUsedByOtherApps*/false, mUser0); + + // Record bar secondaries again with a different class loader. This will change the context. + notifyDexLoad(mSystemServerJarUpdatedContext, secondaries, mUser0); + + pui = getPackageUseInfo(mSystemServerJar); + // We expect that all the contexts to be updated according to the last notify. + assertSecondaryUse(mSystemServerJarUpdatedContext, pui, secondaries, + /*isUsedByOtherApps*/false, mUser0); + } + + @Test public void testNotifyUnsupportedClassLoaderDoesNotChangeExisting() { List<String> secondaries = mBarUser0.getSecondaryDexPaths(); diff --git a/services/tests/servicestests/src/com/android/server/pm/dex/OWNERS b/services/tests/servicestests/src/com/android/server/pm/dex/OWNERS new file mode 100644 index 000000000000..66ef75d6c823 --- /dev/null +++ b/services/tests/servicestests/src/com/android/server/pm/dex/OWNERS @@ -0,0 +1 @@ +include /services/core/java/com/android/server/pm/dex/OWNERS diff --git a/services/tests/servicestests/src/com/android/server/pm/dex/PackageDexUsageTests.java b/services/tests/servicestests/src/com/android/server/pm/dex/PackageDexUsageTests.java index adf4551e79a8..3450710f60a0 100644 --- a/services/tests/servicestests/src/com/android/server/pm/dex/PackageDexUsageTests.java +++ b/services/tests/servicestests/src/com/android/server/pm/dex/PackageDexUsageTests.java @@ -451,7 +451,7 @@ public class PackageDexUsageTests { "PCL[new_context.dex]"); assertTrue(record(fooSecondary1User0NewContext)); - // Not check that the context was switch to variable. + // Now check that the context was switch to variable. TestData expectedContext = mFooSecondary1User0.updateClassLoaderContext( PackageDexUsage.VARIABLE_CLASS_LOADER_CONTEXT); @@ -461,6 +461,22 @@ public class PackageDexUsageTests { } @Test + public void testRecordClassLoaderContextOverwritten() { + // Record a secondary dex file. + assertTrue(record(mFooSecondary1User0)); + // Now update its context. + TestData fooSecondary1User0NewContext = mFooSecondary1User0.updateClassLoaderContext( + "PCL[new_context.dex]", true); + assertTrue(record(fooSecondary1User0NewContext)); + + // Now check that the context was overwritten. + TestData expectedContext = mFooSecondary1User0.updateClassLoaderContext( + "PCL[new_context.dex]", true); + + assertPackageDexUsage(null, expectedContext); + } + + @Test public void testDexUsageClassLoaderContext() { final boolean isUsedByOtherApps = false; final int userId = 0; @@ -642,8 +658,9 @@ public class PackageDexUsageTests { private boolean record(TestData testData) { return mPackageDexUsage.record(testData.mPackageName, testData.mDexFile, - testData.mOwnerUserId, testData.mLoaderIsa, - testData.mPrimaryOrSplit, testData.mUsedBy, testData.mClassLoaderContext); + testData.mOwnerUserId, testData.mLoaderIsa, + testData.mPrimaryOrSplit, testData.mUsedBy, testData.mClassLoaderContext, + testData.mOverwriteCLC); } private boolean record(PackageDexUsage packageDexUsage, TestData testData, Set<String> users) { @@ -651,7 +668,8 @@ public class PackageDexUsageTests { for (String user : users) { result = result && packageDexUsage.record(testData.mPackageName, testData.mDexFile, testData.mOwnerUserId, testData.mLoaderIsa, - testData.mPrimaryOrSplit, user, testData.mClassLoaderContext); + testData.mPrimaryOrSplit, user, testData.mClassLoaderContext, + testData.mOverwriteCLC); } return result; } @@ -682,15 +700,16 @@ public class PackageDexUsageTests { private final boolean mPrimaryOrSplit; private final String mUsedBy; private final String mClassLoaderContext; + private final boolean mOverwriteCLC; private TestData(String packageName, String dexFile, int ownerUserId, String loaderIsa, boolean primaryOrSplit, String usedBy) { this(packageName, dexFile, ownerUserId, loaderIsa, primaryOrSplit, - usedBy, "PCL[" + dexFile + "]"); + usedBy, "PCL[" + dexFile + "]", false); } private TestData(String packageName, String dexFile, int ownerUserId, String loaderIsa, boolean primaryOrSplit, String usedBy, - String classLoaderContext) { + String classLoaderContext, boolean overwriteCLC) { mPackageName = packageName; mDexFile = dexFile; mOwnerUserId = ownerUserId; @@ -698,16 +717,21 @@ public class PackageDexUsageTests { mPrimaryOrSplit = primaryOrSplit; mUsedBy = usedBy; mClassLoaderContext = classLoaderContext; + mOverwriteCLC = overwriteCLC; } private TestData updateClassLoaderContext(String newContext) { + return updateClassLoaderContext(newContext, mOverwriteCLC); + } + + private TestData updateClassLoaderContext(String newContext, boolean overwriteCLC) { return new TestData(mPackageName, mDexFile, mOwnerUserId, mLoaderIsa, - mPrimaryOrSplit, mUsedBy, newContext); + mPrimaryOrSplit, mUsedBy, newContext, overwriteCLC); } private TestData updateUsedBy(String newUsedBy) { return new TestData(mPackageName, mDexFile, mOwnerUserId, mLoaderIsa, - mPrimaryOrSplit, newUsedBy, mClassLoaderContext); + mPrimaryOrSplit, newUsedBy, mClassLoaderContext, mOverwriteCLC); } private boolean isUsedByOtherApps() { |