From 6dba50d63375b6738c6e1ad8a8758f181d295d8b Mon Sep 17 00:00:00 2001 From: Alan Stokes Date: Tue, 30 Oct 2018 15:05:36 +0000 Subject: [PATCH] Stop recording unsupported class loaders. Don't record anything if any of the classloaders in the chain are unsupported. (This is slightly unsubtle, but matchs the existing behavior - see below.) We can't correctly DexOpt secondary dex files in this case anyway. Also discard any existing records of unsupported classloaders on read. Currently this is almost a no-op: BaseDexClassLoader doesn't notify if there are any unknown class loaders, since 5ac512c07cfa80160e240c359349c1390a20a981, so the new check is reachable only if there's some sort of mismatch. But I'm going to change that soon. Tightened up visibility a bit too. Bug: 111336847 Test: atest --test-mapping services/core/java/com/android/server/pm/dex Change-Id: I6af5620a73be7b6440cbafdf6a5c1da1082cbdd4 --- .../com/android/server/pm/dex/DexManager.java | 21 ++++++--- .../server/pm/dex/PackageDexUsage.java | 44 ++++++++++--------- .../server/pm/dex/DexManagerTests.java | 41 +++++------------ .../server/pm/dex/PackageDexUsageTests.java | 44 +++++++++---------- 4 files changed, 71 insertions(+), 79 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 392d4d839c455..753c2833b0568 100644 --- a/services/core/java/com/android/server/pm/dex/DexManager.java +++ b/services/core/java/com/android/server/pm/dex/DexManager.java @@ -30,6 +30,7 @@ import android.os.storage.StorageManager; import android.os.SystemProperties; import android.os.UserHandle; import android.provider.Settings.Global; +import android.util.Log; import android.util.Slog; import android.util.jar.StrictJarFile; @@ -74,7 +75,7 @@ public class DexManager { private static final String PROPERTY_NAME_PM_DEXOPT_PRIV_APPS_OOB_LIST = "pm.dexopt.priv-apps-oob-list"; - private static final boolean DEBUG = false; + private static final boolean DEBUG = Log.isLoggable(TAG, Log.DEBUG); private final Context mContext; @@ -192,6 +193,16 @@ public class DexManager { String[] classLoaderContexts = DexoptUtils.processContextForDexLoad( classLoaderNames, classPaths); + // A null classLoaderContexts means that there are unsupported class loaders in the + // chain. + if (classLoaderContexts == null) { + if (DEBUG) { + Slog.i(TAG, loadingAppInfo.packageName + + " uses unsupported class loader in " + classLoaderNames); + } + return; + } + int dexPathIndex = 0; for (String dexPath : dexPathsToRegister) { // Find the owning package name. @@ -219,14 +230,10 @@ public class DexManager { } // Record dex file usage. If the current usage is a new pattern (e.g. new secondary, - // or UsedBytOtherApps), record will return true and we trigger an async write + // 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. - // A null classLoaderContexts means that there are unsupported class loaders in the - // chain. - String classLoaderContext = classLoaderContexts == null - ? PackageDexUsage.UNSUPPORTED_CLASS_LOADER_CONTEXT - : classLoaderContexts[dexPathIndex]; + String classLoaderContext = classLoaderContexts[dexPathIndex]; if (mPackageDexUsage.record(searchResult.mOwningPackageName, dexPath, loaderUserId, loaderIsa, isUsedByOtherApps, primaryOrSplit, loadingAppInfo.packageName, classLoaderContext)) { 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 602ce3b2a0c10..86f7380e331bf 100644 --- a/services/core/java/com/android/server/pm/dex/PackageDexUsage.java +++ b/services/core/java/com/android/server/pm/dex/PackageDexUsage.java @@ -21,6 +21,7 @@ import android.util.Slog; import android.os.Build; import com.android.internal.annotations.GuardedBy; +import com.android.internal.annotations.VisibleForTesting; import com.android.internal.util.FastPrintWriter; import com.android.server.pm.AbstractStatsBase; import com.android.server.pm.PackageManagerServiceUtils; @@ -78,14 +79,16 @@ public class PackageDexUsage extends AbstractStatsBase { // skip optimizations on that dex files. /*package*/ static final String VARIABLE_CLASS_LOADER_CONTEXT = "=VariableClassLoaderContext="; - // The marker used for unsupported class loader contexts. - /*package*/ static final String UNSUPPORTED_CLASS_LOADER_CONTEXT = - "=UnsupportedClassLoaderContext="; // The markers used for unknown class loader contexts. This can happen if the dex file was // recorded in a previous version and we didn't have a chance to update its usage. /*package*/ static final String UNKNOWN_CLASS_LOADER_CONTEXT = "=UnknownClassLoaderContext="; + // The marker used for unsupported class loader contexts (no longer written, may occur in old + // files so discarded on read). + private static final String UNSUPPORTED_CLASS_LOADER_CONTEXT = + "=UnsupportedClassLoaderContext="; + // Map which structures the information we have on a package. // Maps package name to package data (which stores info about UsedByOtherApps and // secondary dex files.). @@ -365,6 +368,12 @@ public class PackageDexUsage extends AbstractStatsBase { Set loadingPackages = maybeReadLoadingPackages(in, version); String classLoaderContext = maybeReadClassLoaderContext(in, version); + if (UNSUPPORTED_CLASS_LOADER_CONTEXT.equals(classLoaderContext)) { + // We used to record use of unsupported class loaders, but we no longer do. + // Discard such entries; they will be deleted when we next write the file. + continue; + } + int ownerUserId = Integer.parseInt(elems[0]); boolean isUsedByOtherApps = readBoolean(elems[1]); DexUseInfo dexUseInfo = new DexUseInfo(isUsedByOtherApps, ownerUserId, @@ -709,13 +718,13 @@ public class PackageDexUsage extends AbstractStatsBase { // the compiled code will be private. private boolean mUsedByOtherAppsBeforeUpgrade; - public PackageUseInfo() { + /*package*/ PackageUseInfo() { mCodePathsUsedByOtherApps = new HashMap<>(); mDexUseInfoMap = new HashMap<>(); } // Creates a deep copy of the `other`. - public PackageUseInfo(PackageUseInfo other) { + private PackageUseInfo(PackageUseInfo other) { mCodePathsUsedByOtherApps = new HashMap<>(); for (Map.Entry> e : other.mCodePathsUsedByOtherApps.entrySet()) { mCodePathsUsedByOtherApps.put(e.getKey(), new HashSet<>(e.getValue())); @@ -796,8 +805,9 @@ public class PackageDexUsage extends AbstractStatsBase { // Packages who load this dex file. private final Set mLoadingPackages; - public DexUseInfo(boolean isUsedByOtherApps, int ownerUserId, String classLoaderContext, - String loaderIsa) { + @VisibleForTesting + /* package */ DexUseInfo(boolean isUsedByOtherApps, int ownerUserId, + String classLoaderContext, String loaderIsa) { mIsUsedByOtherApps = isUsedByOtherApps; mOwnerUserId = ownerUserId; mClassLoaderContext = classLoaderContext; @@ -809,7 +819,7 @@ public class PackageDexUsage extends AbstractStatsBase { } // Creates a deep copy of the `other`. - public DexUseInfo(DexUseInfo other) { + private DexUseInfo(DexUseInfo other) { mIsUsedByOtherApps = other.mIsUsedByOtherApps; mOwnerUserId = other.mOwnerUserId; mClassLoaderContext = other.mClassLoaderContext; @@ -827,11 +837,7 @@ public class PackageDexUsage extends AbstractStatsBase { if (UNKNOWN_CLASS_LOADER_CONTEXT.equals(mClassLoaderContext)) { // Can happen if we read a previous version. mClassLoaderContext = dexUseInfo.mClassLoaderContext; - } else if (UNSUPPORTED_CLASS_LOADER_CONTEXT.equals(dexUseInfo.mClassLoaderContext)) { - // We detected an unsupported context. - mClassLoaderContext = UNSUPPORTED_CLASS_LOADER_CONTEXT; - } else if (!UNSUPPORTED_CLASS_LOADER_CONTEXT.equals(mClassLoaderContext) && - !Objects.equals(mClassLoaderContext, dexUseInfo.mClassLoaderContext)) { + } else if (!Objects.equals(mClassLoaderContext, dexUseInfo.mClassLoaderContext)) { // We detected a context change. mClassLoaderContext = VARIABLE_CLASS_LOADER_CONTEXT; } @@ -846,7 +852,7 @@ public class PackageDexUsage extends AbstractStatsBase { return mIsUsedByOtherApps; } - public int getOwnerUserId() { + /* package */ int getOwnerUserId() { return mOwnerUserId; } @@ -860,17 +866,15 @@ public class PackageDexUsage extends AbstractStatsBase { public String getClassLoaderContext() { return mClassLoaderContext; } - public boolean isUnsupportedClassLoaderContext() { - return UNSUPPORTED_CLASS_LOADER_CONTEXT.equals(mClassLoaderContext); - } - - public boolean isUnknownClassLoaderContext() { + @VisibleForTesting + /* package */ boolean isUnknownClassLoaderContext() { // The class loader context may be unknown if we loaded the data from a previous version // which didn't save the context. return UNKNOWN_CLASS_LOADER_CONTEXT.equals(mClassLoaderContext); } - public boolean isVariableClassLoaderContext() { + @VisibleForTesting + /* package */ boolean isVariableClassLoaderContext() { return VARIABLE_CLASS_LOADER_CONTEXT.equals(mClassLoaderContext); } } 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 147347d474448..030f9cca265ea 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 @@ -402,15 +402,7 @@ public class DexManagerTests { List secondaries = mBarUser0UnsupportedClassLoader.getSecondaryDexPaths(); notifyDexLoad(mBarUser0UnsupportedClassLoader, secondaries, mUser0); - PackageUseInfo pui = getPackageUseInfo(mBarUser0UnsupportedClassLoader); - assertIsUsedByOtherApps(mBarUser0UnsupportedClassLoader, pui, false); - assertEquals(secondaries.size(), pui.getDexUseInfoMap().size()); - // We expect that all the contexts are unsupported. - String[] expectedContexts = - Collections.nCopies(secondaries.size(), - PackageDexUsage.UNSUPPORTED_CLASS_LOADER_CONTEXT).toArray(new String[0]); - assertSecondaryUse(mBarUser0UnsupportedClassLoader, pui, secondaries, - /*isUsedByOtherApps*/false, mUser0, expectedContexts); + assertNoUseInfo(mBarUser0UnsupportedClassLoader); } @Test @@ -439,27 +431,18 @@ public class DexManagerTests { } @Test - public void testNotifyUnsupportedClassLoaderDoesNotChange() { - List secondaries = mBarUser0UnsupportedClassLoader.getSecondaryDexPaths(); + public void testNotifyUnsupportedClassLoaderDoesNotChangeExisting() { + List secondaries = mBarUser0.getSecondaryDexPaths(); + + notifyDexLoad(mBarUser0, secondaries, mUser0); + PackageUseInfo pui = getPackageUseInfo(mBarUser0); + assertSecondaryUse(mBarUser0, pui, secondaries, /*isUsedByOtherApps*/false, mUser0); + + // Record bar secondaries again with an unsupported class loader. This should not change the + // context. notifyDexLoad(mBarUser0UnsupportedClassLoader, secondaries, mUser0); - - PackageUseInfo pui = getPackageUseInfo(mBarUser0UnsupportedClassLoader); - assertIsUsedByOtherApps(mBarUser0UnsupportedClassLoader, pui, false); - assertEquals(secondaries.size(), pui.getDexUseInfoMap().size()); - // We expect that all the contexts are unsupported. - String[] expectedContexts = - Collections.nCopies(secondaries.size(), - PackageDexUsage.UNSUPPORTED_CLASS_LOADER_CONTEXT).toArray(new String[0]); - assertSecondaryUse(mBarUser0UnsupportedClassLoader, pui, secondaries, - /*isUsedByOtherApps*/false, mUser0, expectedContexts); - - // Record bar secondaries again with a different class loader. This will change the context. - // However, because the context was already marked as unsupported we should not chage it. - notifyDexLoad(mBarUser0DelegateLastClassLoader, secondaries, mUser0); - pui = getPackageUseInfo(mBarUser0UnsupportedClassLoader); - assertSecondaryUse(mBarUser0UnsupportedClassLoader, pui, secondaries, - /*isUsedByOtherApps*/false, mUser0, expectedContexts); - + pui = getPackageUseInfo(mBarUser0); + assertSecondaryUse(mBarUser0, pui, secondaries, /*isUsedByOtherApps*/false, mUser0); } @Test 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 69a148db8b638..0de0503aa85f9 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 @@ -397,20 +397,6 @@ public class PackageDexUsageTests { assertPackageDexUsage(null, expectedContext); } - @Test - public void testRecordClassLoaderContextUnsupportedContext() { - // Record a secondary dex file. - assertTrue(record(mFooSecondary1User0)); - // Now update its context. - TestData unsupportedContext = mFooSecondary1User0.updateClassLoaderContext( - PackageDexUsage.UNSUPPORTED_CLASS_LOADER_CONTEXT); - assertTrue(record(unsupportedContext)); - - assertPackageDexUsage(null, unsupportedContext); - writeAndReadBack(); - assertPackageDexUsage(null, unsupportedContext); - } - @Test public void testRecordClassLoaderContextTransitionFromUnknown() { // Record a secondary dex file. @@ -439,28 +425,40 @@ public class PackageDexUsageTests { PackageDexUsage.DexUseInfo validContext = new DexUseInfo(isUsedByOtherApps, userId, "valid_context", "arm"); assertFalse(validContext.isUnknownClassLoaderContext()); - assertFalse(validContext.isUnsupportedClassLoaderContext()); assertFalse(validContext.isVariableClassLoaderContext()); - PackageDexUsage.DexUseInfo unsupportedContext = new DexUseInfo(isUsedByOtherApps, userId, - PackageDexUsage.UNSUPPORTED_CLASS_LOADER_CONTEXT, "arm"); - assertFalse(unsupportedContext.isUnknownClassLoaderContext()); - assertTrue(unsupportedContext.isUnsupportedClassLoaderContext()); - assertFalse(unsupportedContext.isVariableClassLoaderContext()); - PackageDexUsage.DexUseInfo variableContext = new DexUseInfo(isUsedByOtherApps, userId, PackageDexUsage.VARIABLE_CLASS_LOADER_CONTEXT, "arm"); assertFalse(variableContext.isUnknownClassLoaderContext()); - assertFalse(variableContext.isUnsupportedClassLoaderContext()); assertTrue(variableContext.isVariableClassLoaderContext()); PackageDexUsage.DexUseInfo unknownContext = new DexUseInfo(isUsedByOtherApps, userId, PackageDexUsage.UNKNOWN_CLASS_LOADER_CONTEXT, "arm"); assertTrue(unknownContext.isUnknownClassLoaderContext()); - assertFalse(unknownContext.isUnsupportedClassLoaderContext()); assertFalse(unknownContext.isVariableClassLoaderContext()); } + @Test + public void testUnsupportedClassLoaderDiscardedOnRead() throws Exception { + String content = "PACKAGE_MANAGER__PACKAGE_DEX_USAGE__2\n" + + mBarSecondary1User0.mPackageName + "\n" + + "#" + mBarSecondary1User0.mDexFile + "\n" + + "0,0," + mBarSecondary1User0.mLoaderIsa + "\n" + + "@\n" + + "=UnsupportedClassLoaderContext=\n" + + + mFooSecondary1User0.mPackageName + "\n" + + "#" + mFooSecondary1User0.mDexFile + "\n" + + "0,0," + mFooSecondary1User0.mLoaderIsa + "\n" + + "@\n" + + mFooSecondary1User0.mClassLoaderContext + "\n"; + + mPackageDexUsage.read(new StringReader(content)); + + assertPackageDexUsage(mFooBaseUser0, mFooSecondary1User0); + assertPackageDexUsage(mBarBaseUser0); + } + @Test public void testReadVersion1() { String isa = VMRuntime.getInstructionSet(Build.SUPPORTED_ABIS[0]);