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

Commit d5d21230 authored by Jiakai Zhang's avatar Jiakai Zhang
Browse files

Support creating a filtered snapshot from an existing snapshot computer. 

It is a bit awkward that, in some places in Package Manager, for calling
ART Service methods, we create a filtered snapshot from a new snapshot
computer while we already have a computer. It leads to insistency when
there's a race.

This CL reuses the existing snapshot computer to avoid the awkwardness.

Bug: 258223472
Test: Presubmit
Flag: android.content.pm.alternative_for_dexopt_cleanup
Change-Id: If9968708e5e32f83c3f043527cf77157d986ac58
parent 3058334a

services/core/java/com/android/server/pm/ComputerEngine.java

+1 −1
Original line number Diff line number Diff line
@@ -3118,7 +3118,7 @@ public class ComputerEngine implements Computer { 
                }

                ipw.println("Dexopt state:");

                ipw.increaseIndent();

                DexOptHelper.dumpDexoptState(ipw, packageName);

                DexOptHelper.dumpDexoptState(ipw, this, packageName);

                ipw.decreaseIndent();

                break;

            }

services/core/java/com/android/server/pm/DexOptHelper.java

+16 −7
Original line number Diff line number Diff line
@@ -35,6 +35,7 @@ import android.content.Context; 
import android.content.Intent;

import android.content.IntentFilter;

import android.content.pm.ApexStagedEvent;

import android.content.pm.Flags;

import android.content.pm.IPackageManagerNative;

import android.content.pm.IStagedApexObserver;

import android.content.pm.PackageManager;
@@ -63,6 +64,7 @@ import com.android.server.pinner.PinnerService; 
import com.android.server.pm.dex.InstallScenarioHelper;

import com.android.server.pm.local.PackageManagerLocalImpl;

import com.android.server.pm.pkg.AndroidPackage;

import com.android.server.pm.snapshot.PackageDataSnapshot;



import java.io.File;

import java.io.IOException;
@@ -163,17 +165,24 @@ public final class DexOptHelper { 
    /**

     * Dumps the dexopt state for the given package, or all packages if it is null.

     */

    public static void dumpDexoptState(

            @NonNull IndentingPrintWriter ipw, @Nullable String packageName) {

    public static void dumpDexoptState(@NonNull IndentingPrintWriter ipw,

            @NonNull PackageDataSnapshot computer, @Nullable String packageName) {

        try (PackageManagerLocal.FilteredSnapshot snapshot =

                        getPackageManagerLocal().withFilteredSnapshot()) {

                        getPackageManagerLocal().withUnownedFilteredSnapshot(computer)) {

            if (packageName != null) {

                if (Flags.alternativeForDexoptCleanup()) {

                    // The caller has already vetted the package name against the computer, so

                    // IllegalArgumentException cannot happen.

                    DexOptHelper.getArtManagerLocal().dumpPackage(ipw, snapshot, packageName);

                } else {

                    try {

                        DexOptHelper.getArtManagerLocal().dumpPackage(ipw, snapshot, packageName);

                    } catch (IllegalArgumentException e) {

                    // Package isn't found, but that should only happen due to race.

                        // Package isn't found, but that should only happen due to race. It can

                        // happen because the snapshot is not created from the computer.

                        ipw.println(e);

                    }

                }

            } else {

                DexOptHelper.getArtManagerLocal().dump(ipw, snapshot);

            }

services/core/java/com/android/server/pm/PackageManagerLocal.java

+10 −0
Original line number Diff line number Diff line
@@ -27,6 +27,7 @@ import android.os.UserHandle; 


import com.android.server.pm.pkg.PackageState;

import com.android.server.pm.pkg.SharedUserApi;

import com.android.server.pm.snapshot.PackageDataSnapshot;



import java.io.IOException;

import java.lang.annotation.Retention;
@@ -125,6 +126,15 @@ public interface PackageManagerLocal { 
    @NonNull

    FilteredSnapshot withFilteredSnapshot(int callingUid, @NonNull UserHandle user);



    /**

     * Same as {@link #withFilteredSnapshot()}, but from an existing snapshot computer. The created

     * object does not own the computer and is not responsible for releasing the resources that the

     * computer holds (if any).

     *

     * @hide

     */

    @NonNull FilteredSnapshot withUnownedFilteredSnapshot(@NonNull PackageDataSnapshot computer);



    /**

     * Add a pair of signing details so that packages signed with {@code oldSigningDetails} will

     * behave as if they are signed by the {@code newSigningDetails}.

services/core/java/com/android/server/pm/PackageManagerService.java

+3 −6
Original line number Diff line number Diff line
@@ -5705,12 +5705,10 @@ public class PackageManagerService implements PackageSender, TestUtilityService 
                return;

            }



            UserHandle user = Binder.getCallingUserHandle();

            DexUseManagerLocal dexUseManager = DexOptHelper.getDexUseManagerLocal();

            // TODO(chiuwinson): Retrieve filtered snapshot from Computer instance instead.

            try (PackageManagerLocal.FilteredSnapshot filteredSnapshot =

                            LocalManagerRegistry.getManager(PackageManagerLocal.class)

                            .withFilteredSnapshot(callingUid, user)) {

                                    .withUnownedFilteredSnapshot(snapshot)) {

                // This is called from binder, so exceptions thrown here are caught and handled

                // by it.

                dexUseManager.notifyDexContainersLoaded(
@@ -7662,10 +7660,9 @@ public class PackageManagerService implements PackageSender, TestUtilityService 
        PackageManagerServiceUtils.enforceSystemOrRootOrShell(

                "Only the system or shell can delete oat artifacts");



        // TODO(chiuwinson): Retrieve filtered snapshot from Computer instance instead.

        try (PackageManagerLocal.FilteredSnapshot filteredSnapshot =

                        PackageManagerServiceUtils.getPackageManagerLocal()

                                .withFilteredSnapshot()) {

                                .withUnownedFilteredSnapshot(snapshot)) {

            try {

                DeleteResult res = DexOptHelper.getArtManagerLocal().deleteDexoptArtifacts(

                        filteredSnapshot, packageName);

services/core/java/com/android/server/pm/local/PackageManagerLocalImpl.java

+33 −6
Original line number Diff line number Diff line
@@ -20,6 +20,7 @@ import android.annotation.CallSuper; 
import android.annotation.NonNull;

import android.annotation.Nullable;

import android.annotation.UserIdInt;

import android.content.pm.Flags;

import android.content.pm.SigningDetails;

import android.os.Binder;

import android.os.Build;
@@ -91,7 +92,19 @@ public class PackageManagerLocalImpl implements PackageManagerLocal { 
            @Nullable PackageState uncommittedPs) {

        return new FilteredSnapshotImpl(callingUid, user,

                mService.snapshotComputer(/* allowLiveComputer= */ false),

                /* parentSnapshot= */ null, uncommittedPs);

                /* parentSnapshot= */ null, uncommittedPs, /* unowned= */ false);

    }



    @NonNull

    @Override

    public FilteredSnapshot withUnownedFilteredSnapshot(@NonNull PackageDataSnapshot computer) {

        if (Flags.alternativeForDexoptCleanup()) {

            return new FilteredSnapshotImpl(Binder.getCallingUid(), Binder.getCallingUserHandle(),

                    computer,

                    /* parentSnapshot= */ null, /* uncommittedPs= */ null, /* unowned= */ true);

        } else {

            return withFilteredSnapshot();

        }

    }



    @Override
@@ -126,14 +139,28 @@ public class PackageManagerLocalImpl implements PackageManagerLocal { 
        @NonNull

        protected Computer mSnapshot;



        private BaseSnapshotImpl(@NonNull PackageDataSnapshot snapshot) {

        // True if this object does not own the computer ({@link #mSnapshot}) and is not responsible

        // for releasing the resources that the computer holds (if any).

        private final boolean mUnowned;



        private BaseSnapshotImpl(@NonNull PackageDataSnapshot snapshot, boolean unowned) {

            mSnapshot = (Computer) snapshot;

            mUnowned = unowned;

        }



        @CallSuper

        @Override

        public void close() {

            mClosed = true;

            if (mUnowned) {

                // Short-circuit this method in the unowned case. At the time of writing, it is

                // actually fine to execute this method to the end because it doesn't change the

                // state of the computer but only drops the reference to it, but we aggressively

                // early return just in case someone in the future adds some code below to release

                // resources by changing the state of the computer and forgets to handle the unowned

                // case.

                return;

            }

            mSnapshot = null;

            // TODO: Recycle snapshots?

        }
@@ -159,13 +186,13 @@ public class PackageManagerLocalImpl implements PackageManagerLocal { 
        private Map<String, PackageState> mCachedUnmodifiableDisabledSystemPackageStates;



        private UnfilteredSnapshotImpl(@NonNull PackageDataSnapshot snapshot) {

            super(snapshot);

            super(snapshot, /* unowned= */ false);

        }



        @Override

        public FilteredSnapshot filtered(int callingUid, @NonNull UserHandle user) {

            return new FilteredSnapshotImpl(callingUid, user, mSnapshot, this,

                    /* uncommittedPs= */ null);

                    /* uncommittedPs= */ null, /* unowned= */ true);

        }



        @SuppressWarnings("RedundantSuppression")
@@ -235,8 +262,8 @@ public class PackageManagerLocalImpl implements PackageManagerLocal { 
        private FilteredSnapshotImpl(int callingUid, @NonNull UserHandle user,

                @NonNull PackageDataSnapshot snapshot,

                @Nullable UnfilteredSnapshotImpl parentSnapshot,

                @Nullable PackageState uncommittedPs) {

            super(snapshot);

                @Nullable PackageState uncommittedPs, boolean unowned) {

            super(snapshot, unowned);

            mCallingUid = callingUid;

            mUserId = user.getIdentifier();

            mParentSnapshot = parentSnapshot;