diff options
| -rw-r--r-- | services/core/java/com/android/server/pm/PackageInstallerSession.java | 64 |
1 files changed, 44 insertions, 20 deletions
diff --git a/services/core/java/com/android/server/pm/PackageInstallerSession.java b/services/core/java/com/android/server/pm/PackageInstallerSession.java index de8ad6b7db13..994cec2b1e59 100644 --- a/services/core/java/com/android/server/pm/PackageInstallerSession.java +++ b/services/core/java/com/android/server/pm/PackageInstallerSession.java @@ -401,6 +401,7 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub { private boolean mDataLoaderFinished = false; + // TODO(b/159663586): should be protected by mLock private IncrementalFileStorages mIncrementalFileStorages; 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) { Objects.requireNonNull(statusReceiver); - List<PackageInstallerSession> childSessions = getChildSessions(); + List<PackageInstallerSession> childSessions = getChildSessionsNotLocked(); synchronized (mLock) { assertCallerIsOwnerOrRootLocked(); @@ -1436,7 +1437,11 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub { * * <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; if (isMultiPackage()) { final int[] childSessionIds = getChildSessionIds(); @@ -1605,7 +1610,7 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub { return; } } - List<PackageInstallerSession> childSessions = getChildSessions(); + List<PackageInstallerSession> childSessions = getChildSessionsNotLocked(); synchronized (mLock) { try { sealLocked(childSessions); @@ -1649,7 +1654,7 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub { throw new SecurityException("Can only transfer sessions that use public options"); } - List<PackageInstallerSession> childSessions = getChildSessions(); + List<PackageInstallerSession> childSessions = getChildSessionsNotLocked(); synchronized (mLock) { assertCallerIsOwnerOrRootLocked(); @@ -1701,7 +1706,7 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub { // outside of the lock, because reading the child // sessions with the lock held could lead to deadlock // (b/123391593). - List<PackageInstallerSession> childSessions = getChildSessions(); + List<PackageInstallerSession> childSessions = getChildSessionsNotLocked(); try { synchronized (mLock) { @@ -2602,6 +2607,8 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub { "Session " + sessionId + " is a child of multi-package session " + mParentSessionId + " and may not be abandoned directly."); } + + List<PackageInstallerSession> childSessions = getChildSessionsNotLocked(); synchronized (mLock) { if (params.isStaged && mDestroyed) { // 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); return; } - cleanStageDir(); + cleanStageDir(childSessions); } if (mRelinquished) { @@ -3055,7 +3062,7 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub { mStagedSessionErrorMessage = errorMessage; Slog.d(TAG, "Marking session " + sessionId + " as failed: " + errorMessage); } - cleanStageDir(); + cleanStageDirNotLocked(); mCallback.onStagedSessionChanged(this); } @@ -3070,7 +3077,7 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub { mStagedSessionErrorMessage = ""; Slog.d(TAG, "Marking session " + sessionId + " as applied"); } - cleanStageDir(); + cleanStageDirNotLocked(); mCallback.onStagedSessionChanged(this); } @@ -3128,20 +3135,37 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub { } } - private void cleanStageDir() { - if (isMultiPackage()) { - for (int childSessionId : getChildSessionIds()) { - mSessionProvider.getSession(childSessionId).cleanStageDir(); + /** + * <b>must not hold {@link #mLock}</b> + */ + 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 { - if (mIncrementalFileStorages != null) { - mIncrementalFileStorages.cleanUp(); - mIncrementalFileStorages = null; - } - try { - mPm.mInstaller.rmPackageDir(stageDir.getAbsolutePath()); - } catch (InstallerException ignored) { - } + cleanStageDir(); + } + } + + private void cleanStageDir() { + if (mIncrementalFileStorages != null) { + mIncrementalFileStorages.cleanUp(); + mIncrementalFileStorages = null; + } + try { + mPm.mInstaller.rmPackageDir(stageDir.getAbsolutePath()); + } catch (InstallerException ignored) { } } |