Merge "Prevent a deadlock in #abandon" into rvc-dev
diff --git a/services/core/java/com/android/server/pm/PackageInstallerSession.java b/services/core/java/com/android/server/pm/PackageInstallerSession.java
index de8ad6b..994cec2 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 @@
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 @@
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 @@
*
* <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 @@
return;
}
}
- List<PackageInstallerSession> childSessions = getChildSessions();
+ List<PackageInstallerSession> childSessions = getChildSessionsNotLocked();
synchronized (mLock) {
try {
sealLocked(childSessions);
@@ -1649,7 +1654,7 @@
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 @@
// 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 @@
"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 @@
mCallback.onStagedSessionChanged(this);
return;
}
- cleanStageDir();
+ cleanStageDir(childSessions);
}
if (mRelinquished) {
@@ -3055,7 +3062,7 @@
mStagedSessionErrorMessage = errorMessage;
Slog.d(TAG, "Marking session " + sessionId + " as failed: " + errorMessage);
}
- cleanStageDir();
+ cleanStageDirNotLocked();
mCallback.onStagedSessionChanged(this);
}
@@ -3070,7 +3077,7 @@
mStagedSessionErrorMessage = "";
Slog.d(TAG, "Marking session " + sessionId + " as applied");
}
- cleanStageDir();
+ cleanStageDirNotLocked();
mCallback.onStagedSessionChanged(this);
}
@@ -3128,20 +3135,37 @@
}
}
- 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) {
}
}