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

Commit bc6593cf authored by Felka Chang's avatar Felka Chang
Browse files

Solve error prone issues GuardBy, Permission etc. 

GuardedBy
* To access mProgress and mClientProgress should hold mProgressLock
* To access mInstallSource, mSessionApplied, mSessionFailed,
  mCommitted should hold mLock
* remove unnecessary @GuardBy for assertNotChildLocked and change
  function name to assertNotChild.
* Doesn't hold two locks as possible.
  as TODO(b/197071589)
* Suppress GuardedBy for mPm.mInstaller without holding
  mPm.mInstallerLock

UnusedNestedClass
* remove UnusedNestedClass that is StreamingException

AndroidFrameworkEfficientCollections
* change List<Integer> to IntArray

AndroidFrameworkEfficientStrings
* change from String.format to TextUtils.formatSimp

Test: ./build/soong/soong_ui.bash --make-mode services \
      RUN_ERROR_PRONE=true
Test: atest \
            CtsStagedInstallHostTestCases \
            CtsPackageInstallTestCases \
            CtsAtomicInstallTestCases
Test: atest com.android.server.pm.StagingManagerTest

Bug: 196675234
Change-Id: I9302115901f0298cdf48fe01d926f9edb96ae23e
parent 291d2b44

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

+60 −39
Original line number Diff line number Diff line
@@ -128,6 +128,7 @@ import android.text.TextUtils; 
import android.util.ArrayMap;

import android.util.ArraySet;

import android.util.ExceptionUtils;

import android.util.IntArray;

import android.util.MathUtils;

import android.util.Slog;

import android.util.SparseArray;
@@ -280,6 +281,7 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub { 
    private final PackageInstallerService.InternalCallback mCallback;

    private final Context mContext;

    private final PackageManagerService mPm;

    private final Installer mInstaller;

    private final Handler mHandler;

    private final PackageSessionProvider mSessionProvider;

    private final SilentUpdatePolicy mSilentUpdatePolicy;
@@ -587,7 +589,7 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub { 
        @Override

        public void installSession(IntentSender statusReceiver) {

            assertCallerIsOwnerOrRootOrSystem();

            assertNotChildLocked("StagedSession#installSession");

            assertNotChild("StagedSession#installSession");

            Preconditions.checkArgument(isCommitted() && isSessionReady());



            // Since staged sessions are installed during boot, the original reference to status
@@ -599,12 +601,12 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub { 


        private void updateRemoteStatusReceiver(IntentSender remoteStatusReceiver) {

            synchronized (mLock) {

                mRemoteStatusReceiver = remoteStatusReceiver;

                setRemoteStatusReceiver(remoteStatusReceiver);

                if (isMultiPackage()) {

                    final IntentSender childIntentSender = new ChildStatusIntentReceiver(

                            mChildSessions.clone(), remoteStatusReceiver).getIntentSender();

                    for (int i = mChildSessions.size() - 1; i >= 0; --i) {

                        mChildSessions.valueAt(i).mRemoteStatusReceiver = childIntentSender;

                        mChildSessions.valueAt(i).setRemoteStatusReceiver(childIntentSender);

                    }

                }

            }
@@ -684,7 +686,7 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub { 
        public void abandon() {

            final Runnable r;

            synchronized (mLock) {

                assertNotChildLocked("StagedSession#abandon");

                assertNotChild("StagedSession#abandon");

                assertCallerIsOwnerOrRoot();

                if (isInTerminalState()) {

                    // We keep the session in the database if it's in a finalized state. It will be
@@ -754,7 +756,7 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub { 
        public void verifySession() {

            assertCallerIsOwnerOrRootOrSystem();

            Preconditions.checkArgument(isCommitted());

            Preconditions.checkArgument(!mSessionApplied && !mSessionFailed);

            Preconditions.checkArgument(!isInTerminalState());

            notifyStartPreRebootVerification();

            verify();

        }
@@ -918,8 +920,10 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub { 
        }

        DevicePolicyManagerInternal dpmi =

                LocalServices.getService(DevicePolicyManagerInternal.class);

        // It may wait for a long time to finish {@code dpmi.canSilentlyInstallPackage}.

        // Please don't acquire mLock before calling {@code dpmi.canSilentlyInstallPackage}.

        return dpmi != null && dpmi.canSilentlyInstallPackage(

                mInstallSource.installerPackageName, mInstallerUid);

                getInstallSource().installerPackageName, mInstallerUid);

    }



    private static final int USER_ACTION_NOT_NEEDED = 0;
@@ -1008,8 +1012,10 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub { 
        return USER_ACTION_REQUIRED;

    }



    @SuppressWarnings("GuardedBy" /*mPm.mInstaller is {@code final} field*/)

    public PackageInstallerSession(PackageInstallerService.InternalCallback callback,

            Context context, PackageManagerService pm, PackageSessionProvider sessionProvider,

            Context context, PackageManagerService pm,

            PackageSessionProvider sessionProvider,

            SilentUpdatePolicy silentUpdatePolicy, Looper looper, StagingManager stagingManager,

            int sessionId, int userId, int installerUid, @NonNull InstallSource installSource,

            SessionParams params, long createdMillis, long committedMillis,
@@ -1022,6 +1028,7 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub { 
        mCallback = callback;

        mContext = context;

        mPm = pm;

        mInstaller = (mPm != null) ? mPm.mInstaller : null;

        mSessionProvider = sessionProvider;

        mSilentUpdatePolicy = silentUpdatePolicy;

        mHandler = new Handler(looper, mHandlerCallback);
@@ -1131,6 +1138,10 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub { 


    private SessionInfo generateInfoInternal(boolean includeIcon, boolean scrubData) {

        final SessionInfo info = new SessionInfo();

        final float progress;

        synchronized (mProgressLock) {

            progress = mProgress;

        }

        synchronized (mLock) {

            info.sessionId = sessionId;

            info.userId = userId;
@@ -1138,7 +1149,7 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub { 
            info.installerAttributionTag = mInstallSource.installerAttributionTag;

            info.resolvedBaseCodePath = (mResolvedBaseFile != null) ?

                    mResolvedBaseFile.getAbsolutePath() : null;

            info.progress = mProgress;

            info.progress = progress;

            info.sealed = mSealed;

            info.isCommitted = mCommitted.get();

            info.active = mActiveCount.get() > 0;
@@ -1364,7 +1375,7 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub { 
            return;

        }



        final String initiatingPackageName = mInstallSource.initiatingPackageName;

        final String initiatingPackageName = getInstallSource().initiatingPackageName;



        final AppOpsManager appOps = mContext.getSystemService(AppOpsManager.class);

        appOps.checkPackage(Binder.getCallingUid(), initiatingPackageName);
@@ -1585,7 +1596,7 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub { 
                                if (params.sizeBytes > 0) {

                                    final long delta = progress - last.value;

                                    last.value = progress;

                                    synchronized (mLock) {

                                    synchronized (mProgressLock) {

                                        setClientProgressLocked(mClientProgress

                                                + (float) delta / (float) params.sizeBytes);

                                    }
@@ -1897,13 +1908,6 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub { 
        }

    }



    /** {@hide} */

    private class StreamingException extends Exception {

        StreamingException(Throwable cause) {

            super(cause);

        }

    }



    /**

     * Returns whether or not a package can be installed while Secure FRP is enabled.

     * <p>
@@ -1973,7 +1977,7 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub { 
                }

            }



            mRemoteStatusReceiver = statusReceiver;

            setRemoteStatusReceiver(statusReceiver);



            // After updating the observer, we can skip re-sealing.

            if (mSealed) {
@@ -2140,7 +2144,7 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub { 


    private void onSystemDataLoaderUnrecoverable() {

        final PackageManagerService packageManagerService = mPm;

        final String packageName = mPackageName;

        final String packageName = getPackageName();

        if (TextUtils.isEmpty(packageName)) {

            // The package has not been installed.

            return;
@@ -2303,9 +2307,7 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub { 
     */

    @WorkerThread

    private boolean sendPendingUserActionIntentIfNeeded() {

        synchronized (mLock) {

            assertNotChildLocked("PackageInstallerSession#sendPendingUserActionIntentIfNeeded");

        }

        assertNotChild("PackageInstallerSession#sendPendingUserActionIntentIfNeeded");



        return sessionContains(PackageInstallerSession::checkUserActionRequirement);

    }
@@ -2315,7 +2317,7 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub { 
        if (isInstallerDeviceOwnerOrAffiliatedProfileOwner()) {

            DevicePolicyEventLogger

                    .createEvent(DevicePolicyEnums.INSTALL_PACKAGE)

                    .setAdmin(mInstallSource.installerPackageName)

                    .setAdmin(getInstallSource().installerPackageName)

                    .write();

        }
@@ -2392,6 +2394,12 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub { 
        }

    }



    private void setRemoteStatusReceiver(IntentSender remoteStatusReceiver) {

        synchronized (mLock) {

            mRemoteStatusReceiver = remoteStatusReceiver;

        }

    }



    private void verifyNonStaged()

            throws PackageManagerException {

        final VerificationParams verifyingSession = prepareForVerification();
@@ -2882,7 +2890,7 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub { 
        final List<File> addedFiles = getAddedApksLocked();

        if (addedFiles.isEmpty()) {

            throw new PackageManagerException(INSTALL_FAILED_INVALID_APK,

                    String.format("Session: %d. No packages staged in %s", sessionId,

                    TextUtils.formatSimple("Session: %d. No packages staged in %s", sessionId,

                          stageDir.getAbsolutePath()));

        }
@@ -2982,7 +2990,7 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub { 
        final List<File> addedFiles = getAddedApksLocked();

        if (addedFiles.isEmpty() && removeSplitList.size() == 0) {

            throw new PackageManagerException(INSTALL_FAILED_INVALID_APK,

                    String.format("Session: %d. No packages staged in %s", sessionId,

                    TextUtils.formatSimple("Session: %d. No packages staged in %s", sessionId,

                          stageDir.getAbsolutePath()));

        }
@@ -3316,12 +3324,19 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub { 
                DexMetadataHelper.isFsVerityRequired());

    }



    private IncrementalFileStorages getIncrementalFileStorages() {

        synchronized (mLock) {

            return mIncrementalFileStorages;

        }

    }



    private void storeBytesToInstallationFile(final String localPath, final String absolutePath,

            final byte[] bytes) throws IOException {

        if (!isIncrementalInstallation() || mIncrementalFileStorages == null) {

        final IncrementalFileStorages incrementalFileStorages = getIncrementalFileStorages();

        if (!isIncrementalInstallation() || incrementalFileStorages == null) {

            FileUtils.bytesToFile(absolutePath, bytes);

        } else {

            mIncrementalFileStorages.makeFile(localPath, bytes);

            incrementalFileStorages.makeFile(localPath, bytes);

        }

    }
@@ -3583,7 +3598,7 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub { 
            throws PackageManagerException {

        for (String instructionSet : instructionSets) {

            try {

                mPm.mInstaller.createOatDir(fromDir.getAbsolutePath(), instructionSet);

                mInstaller.createOatDir(fromDir.getAbsolutePath(), instructionSet);

            } catch (InstallerException e) {

                throw PackageManagerException.from(e);

            }
@@ -3593,11 +3608,12 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub { 
    private void linkFile(String relativePath, String fromBase, String toBase) throws IOException {

        try {

            // Try

            if (mIncrementalFileStorages != null && mIncrementalFileStorages.makeLink(relativePath,

            final IncrementalFileStorages incrementalFileStorages = getIncrementalFileStorages();

            if (incrementalFileStorages != null && incrementalFileStorages.makeLink(relativePath,

                    fromBase, toBase)) {

                return;

            }

            mPm.mInstaller.linkFile(relativePath, fromBase, toBase);

            mInstaller.linkFile(relativePath, fromBase, toBase);

        } catch (InstallerException | IOException e) {

            throw new IOException("failed linkOrCreateDir(" + relativePath + ", "

                    + fromBase + ", " + toBase + ")", e);
@@ -3767,7 +3783,7 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub { 


    private void abandonNonStaged() {

        synchronized (mLock) {

            assertNotChildLocked("abandonNonStaged");

            assertNotChild("abandonNonStaged");

            assertCallerIsOwnerOrRoot();

            if (mRelinquished) {

                if (LOGD) Slog.d(TAG, "Ignoring abandon after commit relinquished control");
@@ -3779,8 +3795,7 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub { 
        maybeCleanUpChildSessions();

    }



    @GuardedBy("mLock")

    private void assertNotChildLocked(String cookie) {

    private void assertNotChild(String cookie) {

        if (hasParentSessionId()) {

            throw new IllegalStateException(cookie + " can't be called on a child session, id="

                    + sessionId + " parentId=" + getParentSessionId());
@@ -4324,7 +4339,7 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub { 
                if (incrementalFileStorages != null) {

                    incrementalFileStorages.cleanUpAndMarkComplete();

                }

                mPm.mInstaller.rmPackageDir(stageDir.getAbsolutePath());

                mInstaller.rmPackageDir(stageDir.getAbsolutePath());

            } catch (InstallerException ignored) {

            }

        }
@@ -4350,7 +4365,7 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub { 
            if (incrementalFileStorages != null) {

                incrementalFileStorages.cleanUpAndMarkComplete();

            }

            mPm.mInstaller.rmPackageDir(stageDir.getAbsolutePath());

            mInstaller.rmPackageDir(stageDir.getAbsolutePath());

        } catch (InstallerException ignored) {

        }

    }
@@ -4382,8 +4397,14 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub { 


        params.dump(pw);



        pw.printPair("mClientProgress", mClientProgress);

        pw.printPair("mProgress", mProgress);

        final float clientProgress;

        final float progress;

        synchronized (mProgressLock) {

            clientProgress = mClientProgress;

            progress = mProgress;

        }

        pw.printPair("mClientProgress", clientProgress);

        pw.printPair("mProgress", progress);

        pw.printPair("mCommitted", mCommitted);

        pw.printPair("mSealed", mSealed);

        pw.printPair("mPermissionsManuallyAccepted", mPermissionsManuallyAccepted);
@@ -4693,7 +4714,7 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub { 
     * @param installerThread Thread to be used for callbacks of this session

     * @param sessionsDir The directory the sessions are stored in

     *

     * @param sessionProvider

     * @param sessionProvider to get the other PackageInstallerSession instance by sessionId.

     * @return The newly created session

     */

    public static PackageInstallerSession readFromXml(@NonNull TypedXmlPullParser in,
@@ -4781,7 +4802,7 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub { 
        List<String> grantedRuntimePermissions = new ArrayList<>();

        List<String> whitelistedRestrictedPermissions = new ArrayList<>();

        int autoRevokePermissionsMode = MODE_DEFAULT;

        List<Integer> childSessionIds = new ArrayList<>();

        IntArray childSessionIds = new IntArray();

        List<InstallationFile> files = new ArrayList<>();

        ArrayMap<String, List<Checksum>> checksums = new ArrayMap<>();

        ArrayMap<String, byte[]> signatures = new ArrayMap<>();

services/tests/mockingservicestests/src/com/android/server/pm/StagingManagerTest.java

+2 −1
Original line number Diff line number Diff line
@@ -87,6 +87,7 @@ public class StagingManagerTest { 
    @Mock private Context mContext;

    @Mock private IStorageManager mStorageManager;

    @Mock private ApexManager mApexManager;

    @Mock private PackageManagerService mMockPackageManagerInternal;



    private File mTmpDir;

    private StagingManager mStagingManager;
@@ -826,7 +827,7 @@ public class StagingManagerTest { 
        PackageInstallerSession session = new PackageInstallerSession(

                /* callback */ null,

                /* context */ null,

                /* pm */ null,

                /* pm */ mMockPackageManagerInternal,

                /* sessionProvider */ null,

                /* silentUpdatePolicy */ null,

                /* looper */ BackgroundThread.getHandler().getLooper(),