Always use the latest CLC for system server
Overwrite the class loader context for system server (instead of merging
it). We expect system server jars to change contexts only in between OTAs
and otherwise to be stable. So it is ok to take the latest context all the
time instead of trying to merge, which will lead to the context being
marked as variable and thus making dexopt sub-optimal.
Test: PackageDexUsageTests DexManagerTests
Bug: 179512101
Change-Id: Iaf8368e4ededa6eab074dfe9589fbdaac24491d1
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 cc6d80a..6d3de96 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 @@
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 @@
}
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 @@
* 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 @@
* <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 @@
// 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 @@
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 @@
// 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 @@
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 10760f5..3d63b75 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 @@
* @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 @@
}
// 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 @@
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 ff43da6..ee0a16a 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 @@
private TestData mBarUser0DelegateLastClassLoader;
private TestData mSystemServerJar;
+ private TestData mSystemServerJarUpdatedContext;
private TestData mSystemServerJarInvalid;
private int mUser0;
@@ -113,6 +114,8 @@
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 @@
}
@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 0000000..66ef75d
--- /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 adf4551..3450710 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 @@
"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 @@
}
@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 @@
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 @@
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 @@
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 @@
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() {
