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

Commit 95c129e5 authored by Felka Chang's avatar Felka Chang
Browse files

Declare mCommitted as AtomicBoolean in PackageInstallerSession 

mCommitted is declared as boolean and is accessed with holding mLock.
mCommitted is set to be true only in streamValidateAndCommit(). After
that, the value of mCommitted shouldn't set back to false.

In order to reduce the chance to make a deadlock situation happen,
this patch changes mCommitted declaration from boolean to
AtomicBoolean.

Test: atest \
                CtsStagedInstallHostTestCases \
                CtsPackageInstallTestCases \
                CtsAtomicInstallTestCases
Test: Presubmit Treehugger

Fix: 197088926
Change-Id: I7e7e952e26c07518539a3e3f587f699c7ecef782
parent b103b8b3

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

+20 −15
Original line number Diff line number Diff line
@@ -348,8 +348,9 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub { 
    private boolean mSealed = false;

    @GuardedBy("mLock")

    private boolean mShouldBeSealed = false;

    @GuardedBy("mLock")

    private boolean mCommitted = false;



    private final AtomicBoolean mCommitted = new AtomicBoolean(false);



    @GuardedBy("mLock")

    private boolean mRelinquished = false;
@@ -692,11 +693,10 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub { 
                    return;

                }

                mDestroyed = true;

                boolean isCommitted = mCommitted;

                List<PackageInstallerSession> childSessions = getChildSessionsLocked();

                r = () -> {

                    assertNotLocked("abandonStaged");

                    if (isCommitted) {

                    if (mCommitted.get()) {

                        mStagingManager.abortCommittedSession(this);

                    }

                    cleanStageDir(childSessions);
@@ -1069,7 +1069,7 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub { 
        }



        mPrepared = prepared;

        mCommitted = committed;

        mCommitted.set(committed);

        mDestroyed = destroyed;

        mStagedSession = params.isStaged ? new StagedSession(isReady, isApplied, isFailed,

                stagedSessionErrorCode, stagedSessionErrorMessage) : null;
@@ -1139,7 +1139,7 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub { 
                    mResolvedBaseFile.getAbsolutePath() : null;

            info.progress = mProgress;

            info.sealed = mSealed;

            info.isCommitted = mCommitted;

            info.isCommitted = mCommitted.get();

            info.active = mActiveCount.get() > 0;



            info.mode = params.mode;
@@ -1195,9 +1195,7 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub { 


    /** {@hide} */

    boolean isCommitted() {

        synchronized (mLock) {

            return mCommitted;

        }

        return mCommitted.get();

    }



    /** {@hide} */
@@ -1235,7 +1233,7 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub { 
    @GuardedBy("mLock")

    private void assertPreparedAndNotCommittedOrDestroyedLocked(String cookie) {

        assertPreparedAndNotDestroyedLocked(cookie);

        if (mCommitted) {

        if (mCommitted.get()) {

            throw new SecurityException(cookie + " not allowed after commit");

        }

    }
@@ -1276,7 +1274,7 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub { 


    @GuardedBy("mProgressLock")

    private void computeProgressLocked(boolean forcePublish) {

        if (!isIncrementalInstallation() || !mCommitted) {

        if (!isIncrementalInstallation() || !mCommitted.get()) {

            mProgress = MathUtils.constrain(mClientProgress * 0.8f, 0f, 0.8f)

                    + MathUtils.constrain(mInternalProgress * 0.2f, 0f, 0.2f);

        } else {
@@ -2004,7 +2002,7 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub { 
        //             single / child sessions.

        try {

            synchronized (mLock) {

                if (mCommitted) {

                if (mCommitted.get()) {

                    return true;

                }

                // Read transfers from the original owner stay open, but as the session's data
@@ -2037,7 +2035,13 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub { 
                // will probably close their end.

                mActiveCount.incrementAndGet();



                mCommitted = true;

                if (!mCommitted.compareAndSet(false /*expect*/, true /*update*/)) {

                    throw new PackageManagerException(

                            INSTALL_FAILED_INTERNAL_ERROR,

                            TextUtils.formatSimple(

                                    "The mCommitted of session %d should be false originally",

                                    sessionId));

                }

                committedMillis = System.currentTimeMillis();

            }

            return true;
@@ -4097,7 +4101,8 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub { 
    private boolean canBeAddedAsChild(int parentCandidate) {

        synchronized (mLock) {

            return (!hasParentSessionId() || mParentSessionId == parentCandidate)

                    && !mCommitted && !mDestroyed;

                    && !mCommitted.get()

                    && !mDestroyed;

        }

    }
@@ -4541,7 +4546,7 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub { 
                writeStringAttribute(out, ATTR_SESSION_STAGE_CID, stageCid);

            }

            writeBooleanAttribute(out, ATTR_PREPARED, mPrepared);

            writeBooleanAttribute(out, ATTR_COMMITTED, mCommitted);

            writeBooleanAttribute(out, ATTR_COMMITTED, mCommitted.get());

            writeBooleanAttribute(out, ATTR_DESTROYED, mDestroyed);

            writeBooleanAttribute(out, ATTR_SEALED, mSealed);