Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 2b8622c5 authored by Calin Juravle's avatar Calin Juravle
Browse files

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
parent bf16b6d3

services/core/java/com/android/server/pm/dex/DexManager.java

+23 −8
Original line number Diff line number Diff line
@@ -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;

services/core/java/com/android/server/pm/dex/PackageDexUsage.java

+8 −5
Original line number Diff line number Diff line
@@ -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.

services/tests/servicestests/src/com/android/server/pm/dex/DexManagerTests.java

+21 −0
Original line number Diff line number Diff line
@@ -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);
@@ -521,6 +524,24 @@ public class DexManagerTests { 
                expectedContexts);

    }



    @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();

services/tests/servicestests/src/com/android/server/pm/dex/OWNERS

0 → 100644
+1 −0
Original line number Diff line number Diff line 
include /services/core/java/com/android/server/pm/dex/OWNERS

services/tests/servicestests/src/com/android/server/pm/dex/PackageDexUsageTests.java

+32 −8
Original line number Diff line number Diff line
@@ -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);
@@ -460,6 +460,22 @@ public class PackageDexUsageTests { 
        assertPackageDexUsage(null, expectedContext);

    }



    @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;
@@ -643,7 +659,8 @@ 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.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() {