Android/frameworks_base
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Merge "Prevent a deadlock in #abandon" into rvc-dev

Browse Source
This commit is contained in:
JW Wang
2020-06-25 01:40:48 +00:00
committed by Android (Google) Code Review
parent d06ebaeeed 8a207c40f8
commit 69c5cac4c1
1 changed files with 44 additions and 20 deletions
Download Patch File Download Diff File Expand all files Collapse all files

64
services/core/java/com/android/server/pm/PackageInstallerSession.java
View File

@@ -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,20 +3135,37 @@ 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 {
if (mIncrementalFileStorages != null) { cleanStageDir();
mIncrementalFileStorages.cleanUp(); }
mIncrementalFileStorages = null; }
}
try { private void cleanStageDir() {
mPm.mInstaller.rmPackageDir(stageDir.getAbsolutePath()); if (mIncrementalFileStorages != null) {
} catch (InstallerException ignored) { mIncrementalFileStorages.cleanUp();
} mIncrementalFileStorages = null;
}
try {
mPm.mInstaller.rmPackageDir(stageDir.getAbsolutePath());
} catch (InstallerException ignored) {
} }
} }