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

Commit 8a207c40 authored by JW Wang's avatar JW Wang
Browse files

Prevent a deadlock in #abandon 

See b/159568595#comment1 for analysis.

1. rename cleanStageDir to cleanStageDirNotLocked to show it must be
   called without mLock held.
2. rename getChildSessions to getChildSessionsNotLocked to show it must
   be called without mLock held.
3. add cleanStageDir(List<PackageInstallerSession>) which is callable
   when mLock is held.

Bug: 159568595
Test: atest StagedInstallTest
Merged-In: I512ea07a4f8750431b306fa6e3c47a600a50b64f
Change-Id: I512ea07a4f8750431b306fa6e3c47a600a50b64f
(cherry picked from commit 4ce1273f)
parent 71f13947

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

+44 −20
Original line number Original line Diff line number Diff line
@@ -401,6 +401,7 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub { 




    private boolean mDataLoaderFinished = false;

    private boolean mDataLoaderFinished = false;





    // TODO(b/159663586): should be protected by mLock

    private IncrementalFileStorages mIncrementalFileStorages;

    private IncrementalFileStorages mIncrementalFileStorages;





    private static final FileFilter sAddedApkFilter = new FileFilter() {

    private static final FileFilter sAddedApkFilter = new FileFilter() {
@@ -1353,7 +1354,7 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub { 
    private boolean markAsSealed(@NonNull IntentSender statusReceiver, boolean forTransfer) {

    private boolean markAsSealed(@NonNull IntentSender statusReceiver, boolean forTransfer) {

        Objects.requireNonNull(statusReceiver);

        Objects.requireNonNull(statusReceiver);





        List<PackageInstallerSession> childSessions = getChildSessions();

        List<PackageInstallerSession> childSessions = getChildSessionsNotLocked();





        synchronized (mLock) {

        synchronized (mLock) {

            assertCallerIsOwnerOrRootLocked();

            assertCallerIsOwnerOrRootLocked();
@@ -1436,7 +1437,11 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub { 
     *

     *

     * <p> This method is handy to prevent potential deadlocks (b/123391593)

     * <p> This method is handy to prevent potential deadlocks (b/123391593)

     */

     */

    private @Nullable List<PackageInstallerSession> getChildSessions() {

    private @Nullable List<PackageInstallerSession> getChildSessionsNotLocked() {

        if (Thread.holdsLock(mLock)) {

            Slog.wtf(TAG, "Calling thread " + Thread.currentThread().getName()

                    + " is holding mLock", new Throwable());

        }

        List<PackageInstallerSession> childSessions = null;

        List<PackageInstallerSession> childSessions = null;

        if (isMultiPackage()) {

        if (isMultiPackage()) {

            final int[] childSessionIds = getChildSessionIds();

            final int[] childSessionIds = getChildSessionIds();
@@ -1605,7 +1610,7 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub { 
                return;

                return;

            }

            }

        }

        }

        List<PackageInstallerSession> childSessions = getChildSessions();

        List<PackageInstallerSession> childSessions = getChildSessionsNotLocked();

        synchronized (mLock) {

        synchronized (mLock) {

            try {

            try {

                sealLocked(childSessions);

                sealLocked(childSessions);
@@ -1649,7 +1654,7 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub { 
            throw new SecurityException("Can only transfer sessions that use public options");

            throw new SecurityException("Can only transfer sessions that use public options");

        }

        }





        List<PackageInstallerSession> childSessions = getChildSessions();

        List<PackageInstallerSession> childSessions = getChildSessionsNotLocked();





        synchronized (mLock) {

        synchronized (mLock) {

            assertCallerIsOwnerOrRootLocked();

            assertCallerIsOwnerOrRootLocked();
@@ -1701,7 +1706,7 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub { 
        // outside of the lock, because reading the child

        // outside of the lock, because reading the child

        // sessions with the lock held could lead to deadlock

        // sessions with the lock held could lead to deadlock

        // (b/123391593).

        // (b/123391593).

        List<PackageInstallerSession> childSessions = getChildSessions();

        List<PackageInstallerSession> childSessions = getChildSessionsNotLocked();





        try {

        try {

            synchronized (mLock) {

            synchronized (mLock) {
@@ -2602,6 +2607,8 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub { 
                    "Session " + sessionId + " is a child of multi-package session "

                    "Session " + sessionId + " is a child of multi-package session "

                            + mParentSessionId +  " and may not be abandoned directly.");

                            + mParentSessionId +  " and may not be abandoned directly.");

        }

        }



        List<PackageInstallerSession> childSessions = getChildSessionsNotLocked();

        synchronized (mLock) {

        synchronized (mLock) {

            if (params.isStaged && mDestroyed) {

            if (params.isStaged && mDestroyed) {

                // If a user abandons staged session in an unsafe state, then system will try to

                // If a user abandons staged session in an unsafe state, then system will try to
@@ -2625,7 +2632,7 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub { 
                    mCallback.onStagedSessionChanged(this);

                    mCallback.onStagedSessionChanged(this);

                    return;

                    return;

                }

                }

                cleanStageDir();

                cleanStageDir(childSessions);

            }

            }





            if (mRelinquished) {

            if (mRelinquished) {
@@ -3055,7 +3062,7 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub { 
            mStagedSessionErrorMessage = errorMessage;

            mStagedSessionErrorMessage = errorMessage;

            Slog.d(TAG, "Marking session " + sessionId + " as failed: " + errorMessage);

            Slog.d(TAG, "Marking session " + sessionId + " as failed: " + errorMessage);

        }

        }

        cleanStageDir();

        cleanStageDirNotLocked();

        mCallback.onStagedSessionChanged(this);

        mCallback.onStagedSessionChanged(this);

    }

    }
@@ -3070,7 +3077,7 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub { 
            mStagedSessionErrorMessage = "";

            mStagedSessionErrorMessage = "";

            Slog.d(TAG, "Marking session " + sessionId + " as applied");

            Slog.d(TAG, "Marking session " + sessionId + " as applied");

        }

        }

        cleanStageDir();

        cleanStageDirNotLocked();

        mCallback.onStagedSessionChanged(this);

        mCallback.onStagedSessionChanged(this);

    }

    }
@@ -3128,12 +3135,30 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub { 
        }

        }

    }

    }





    private void cleanStageDir() {

    /**

        if (isMultiPackage()) {

     * <b>must not hold {@link #mLock}</b>

            for (int childSessionId : getChildSessionIds()) {

     */

                mSessionProvider.getSession(childSessionId).cleanStageDir();

    private void cleanStageDirNotLocked() {

        if (Thread.holdsLock(mLock)) {

            Slog.wtf(TAG, "Calling thread " + Thread.currentThread().getName()

                    + " is holding mLock", new Throwable());

        }

        cleanStageDir(getChildSessionsNotLocked());

    }



    private void cleanStageDir(List<PackageInstallerSession> childSessions) {

        if (childSessions != null) {

            for (PackageInstallerSession childSession : childSessions) {

                if (childSession != null) {

                    childSession.cleanStageDir();

                }

            }

            }

        } else {

        } else {

            cleanStageDir();

        }

    }



    private void cleanStageDir() {

        if (mIncrementalFileStorages != null) {

        if (mIncrementalFileStorages != null) {

            mIncrementalFileStorages.cleanUp();

            mIncrementalFileStorages.cleanUp();

            mIncrementalFileStorages = null;

            mIncrementalFileStorages = null;
@@ -3143,7 +3168,6 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub { 
        } catch (InstallerException ignored) {

        } catch (InstallerException ignored) {

        }

        }

    }

    }

    }





    void dump(IndentingPrintWriter pw) {

    void dump(IndentingPrintWriter pw) {

        synchronized (mLock) {

        synchronized (mLock) {