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

Store object references along with child session Ids (1/n)

Browse Source 
1. Null-check is no longer needed in removeChildSessionId()
   since we never store null in child sessions.

2. Simplify the lock model for we no longer need
   mSessionProvider.getSession() to retrieve a child session which
   must be called outside mLock.

Bug: 162386287
Test: atest StagedInstallTest

Change-Id: I05580968a6ed97647c841f7c47fc1cb1a94ead5a
This commit is contained in:
JW Wang
2020-07-29 11:33:30 +08:00
parent 5605ab66d7
commit 4a2bbeab93
2 changed files with 93 additions and 87 deletions
Download Patch File Download Diff File Expand all files Collapse all files

2
services/core/java/com/android/server/pm/PackageInstallerService.java
View File

@@ -442,7 +442,7 @@ public class PackageInstallerService extends IPackageInstaller.Stub implements
// After reboot housekeeping.
for (int i = 0; i < mSessions.size(); ++i) {
PackageInstallerSession session = mSessions.valueAt(i);
session.onAfterSessionRead();
session.onAfterSessionRead(mSessions);
}
}

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

@@ -122,7 +122,7 @@ import android.util.ArraySet;
import android.util.ExceptionUtils;
import android.util.MathUtils;
import android.util.Slog;
import android.util.SparseIntArray;
import android.util.SparseArray;
import android.util.apk.ApkSignatureVerifier;
import com.android.internal.R;
@@ -336,7 +336,7 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub {
@GuardedBy("mLock")
private PackageParser.SigningDetails mSigningDetails;
@GuardedBy("mLock")
private SparseIntArray mChildSessionIds = new SparseIntArray();
private SparseArray<PackageInstallerSession> mChildSessions = new SparseArray<>();
@GuardedBy("mLock")
private int mParentSessionId;
@@ -589,7 +589,9 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub {
this.mShouldBeSealed = sealed;
if (childSessionIds != null) {
for (int childSessionId : childSessionIds) {
mChildSessionIds.put(childSessionId, 0);
// Null values will be resolved to actual object references in
// #onAfterSessionRead later.
mChildSessions.put(childSessionId, null);
}
}
this.mParentSessionId = parentSessionId;
@@ -708,10 +710,7 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub {
info.isStaged = params.isStaged;
info.rollbackDataPolicy = params.rollbackDataPolicy;
info.parentSessionId = mParentSessionId;
info.childSessionIds = mChildSessionIds.copyKeys();
if (info.childSessionIds == null) {
info.childSessionIds = EMPTY_CHILD_SESSION_ARRAY;
}
info.childSessionIds = getChildSessionIdsLocked();
info.isStagedSessionApplied = mStagedSessionApplied;
info.isStagedSessionReady = mStagedSessionReady;
info.isStagedSessionFailed = mStagedSessionFailed;
@@ -1159,27 +1158,22 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub {
return;
}
if (isMultiPackage()) {
final SparseIntArray remainingSessions;
final int[] childSessionIds;
synchronized (mLock) {
remainingSessions = mChildSessionIds.clone();
childSessionIds = mChildSessionIds.copyKeys();
}
final IntentSender childIntentSender =
new ChildStatusIntentReceiver(remainingSessions, statusReceiver)
.getIntentSender();
boolean sealFailed = false;
for (int i = childSessionIds.length - 1; i >= 0; --i) {
final int childSessionId = childSessionIds[i];
// seal all children, regardless if any of them fail; we'll throw/return
// as appropriate once all children have been processed
if (!mSessionProvider.getSession(childSessionId)
.markAsSealed(childIntentSender, forTransfer)) {
sealFailed = true;
final IntentSender childIntentSender =
new ChildStatusIntentReceiver(mChildSessions.clone(), statusReceiver)
.getIntentSender();
boolean sealFailed = false;
for (int i = mChildSessions.size() - 1; i >= 0; --i) {
// seal all children, regardless if any of them fail; we'll throw/return
// as appropriate once all children have been processed
if (!mChildSessions.valueAt(i)
.markAsSealed(childIntentSender, forTransfer)) {
sealFailed = true;
}
}
if (sealFailed) {
return;
}
}
if (sealFailed) {
return;
}
}
@@ -1218,21 +1212,20 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub {
}
if (isMultiPackage()) {
final int[] childSessionIds;
final List<PackageInstallerSession> childSessions;
synchronized (mLock) {
childSessionIds = mChildSessionIds.copyKeys();
childSessions = getChildSessionsLocked();
}
int childCount = childSessionIds.length;
int childCount = childSessions.size();
// This will contain all child sessions that do not encounter an unrecoverable failure
ArrayList<PackageInstallerSession> nonFailingSessions = new ArrayList<>(childCount);
for (int i = childCount - 1; i >= 0; --i) {
final int childSessionId = childSessionIds[i];
// commit all children, regardless if any of them fail; we'll throw/return
// as appropriate once all children have been processed
try {
PackageInstallerSession session = mSessionProvider.getSession(childSessionId);
PackageInstallerSession session = childSessions.get(i);
allSessionsReady &= session.streamValidateAndCommit();
nonFailingSessions.add(session);
} catch (PackageManagerException e) {
@@ -1293,7 +1286,7 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub {
}
private class ChildStatusIntentReceiver {
private final SparseIntArray mChildSessionsRemaining;
private final SparseArray<PackageInstallerSession> mChildSessionsRemaining;
private final IntentSender mStatusReceiver;
private final IIntentSender.Stub mLocalSender = new IIntentSender.Stub() {
@Override
@@ -1303,7 +1296,7 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub {
}
};
private ChildStatusIntentReceiver(SparseIntArray remainingSessions,
private ChildStatusIntentReceiver(SparseArray<PackageInstallerSession> remainingSessions,
IntentSender statusReceiver) {
this.mChildSessionsRemaining = remainingSessions;
this.mStatusReceiver = statusReceiver;
@@ -1413,8 +1406,6 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub {
private boolean markAsSealed(@NonNull IntentSender statusReceiver, boolean forTransfer) {
Objects.requireNonNull(statusReceiver);
List<PackageInstallerSession> childSessions = getChildSessionsNotLocked();
synchronized (mLock) {
assertCallerIsOwnerOrRootLocked();
assertPreparedAndNotDestroyedLocked("commit");
@@ -1446,7 +1437,7 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub {
}
try {
sealLocked(childSessions);
sealLocked(getChildSessionsLocked());
} catch (PackageManagerException e) {
return false;
}
@@ -1507,6 +1498,19 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub {
return childSessions;
}
@GuardedBy("mLock")
private @Nullable List<PackageInstallerSession> getChildSessionsLocked() {
List<PackageInstallerSession> childSessions = null;
if (isMultiPackage()) {
int size = mChildSessions.size();
childSessions = new ArrayList<>(size);
for (int i = 0; i < size; ++i) {
childSessions.add(mChildSessions.valueAt(i));
}
}
return childSessions;
}
/**
* Assert multipackage install has consistent sessions.
*
@@ -1657,17 +1661,32 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub {
*
* <p> This is meant to be called after all of the sessions are loaded and added to
* PackageInstallerService
*
* @param allSessions All sessions loaded by PackageInstallerService, guaranteed to be
* immutable by the caller during the method call. Used to resolve child
* sessions Ids to actual object reference.
*/
void onAfterSessionRead() {
void onAfterSessionRead(SparseArray<PackageInstallerSession> allSessions) {
synchronized (mLock) {
// Resolve null values to actual object references
for (int i = mChildSessions.size() - 1; i >= 0; --i) {
int childSessionId = mChildSessions.keyAt(i);
PackageInstallerSession childSession = allSessions.get(childSessionId);
if (childSession != null) {
mChildSessions.setValueAt(i, childSession);
} else {
Slog.e(TAG, "Child session not existed: " + childSessionId);
mChildSessions.removeAt(i);
}
}
if (!mShouldBeSealed || isStagedAndInTerminalState()) {
return;
}
}
List<PackageInstallerSession> childSessions = getChildSessionsNotLocked();
synchronized (mLock) {
try {
sealLocked(childSessions);
sealLocked(getChildSessionsLocked());
if (isApexInstallation()) {
// APEX installations rely on certain fields to be populated after reboot.
@@ -1708,14 +1727,12 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub {
throw new SecurityException("Can only transfer sessions that use public options");
}
List<PackageInstallerSession> childSessions = getChildSessionsNotLocked();
synchronized (mLock) {
assertCallerIsOwnerOrRootLocked();
assertPreparedAndNotSealedLocked("transfer");
try {
sealLocked(childSessions);
sealLocked(getChildSessionsLocked());
} catch (PackageManagerException e) {
throw new IllegalArgumentException("Package is not valid", e);
}
@@ -1746,11 +1763,10 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub {
return;
}
// For a multiPackage session, read the child sessions
// outside of the lock, because reading the child
// sessions with the lock held could lead to deadlock
// (b/123391593).
List<PackageInstallerSession> childSessions = getChildSessionsNotLocked();
final List<PackageInstallerSession> childSessions;
synchronized (mLock) {
childSessions = getChildSessionsLocked();
}
try {
verifyNonStaged(childSessions);
@@ -2741,15 +2757,6 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub {
}
}
/**
* Adds a child session ID without any safety / sanity checks. This should only be used to
* build a session from XML or similar.
*/
@GuardedBy("mLock")
void addChildSessionIdLocked(int sessionId) {
mChildSessionIds.put(sessionId, 0);
}
public void open() throws IOException {
if (mActiveCount.getAndIncrement() == 0) {
mCallback.onSessionActiveChanged(this, true);
@@ -2804,7 +2811,6 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub {
+ getParentSessionId() + " 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
@@ -2828,7 +2834,7 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub {
mCallback.onStagedSessionChanged(this);
return;
}
cleanStageDir(childSessions);
cleanStageDir(getChildSessionsLocked());
}
if (mRelinquished) {
@@ -3149,8 +3155,15 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub {
@GuardedBy("mLock")
private int[] getChildSessionIdsLocked() {
final int[] childSessionIds = mChildSessionIds.copyKeys();
return childSessionIds != null ? childSessionIds : EMPTY_CHILD_SESSION_ARRAY;
int size = mChildSessions.size();
if (size == 0) {
return EMPTY_CHILD_SESSION_ARRAY;
}
final int[] childSessionIds = new int[size];
for (int i = 0; i < size; ++i) {
childSessionIds[i] = mChildSessions.keyAt(i);
}
return childSessionIds;
}
@Override
@@ -3205,12 +3218,12 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub {
assertCallerIsOwnerOrRootLocked();
assertPreparedAndNotSealedLocked("addChildSessionId");
final int indexOfSession = mChildSessionIds.indexOfKey(childSessionId);
final int indexOfSession = mChildSessions.indexOfKey(childSessionId);
if (indexOfSession >= 0) {
return;
}
childSession.setParentSessionId(this.sessionId);
addChildSessionIdLocked(childSessionId);
mChildSessions.put(childSessionId, childSession);
}
} finally {
releaseTransactionLock();
@@ -3220,30 +3233,23 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub {
@Override
public void removeChildSessionId(int sessionId) {
final PackageInstallerSession session = mSessionProvider.getSession(sessionId);
try {
acquireTransactionLock();
if (session != null) {
synchronized (mLock) {
assertCallerIsOwnerOrRootLocked();
assertPreparedAndNotSealedLocked("removeChildSessionId");
final int indexOfSession = mChildSessions.indexOfKey(sessionId);
if (indexOfSession < 0) {
// not added in the first place; no-op
return;
}
PackageInstallerSession session = mChildSessions.valueAt(indexOfSession);
try {
acquireTransactionLock();
session.acquireTransactionLock();
}
synchronized (mLock) {
assertCallerIsOwnerOrRootLocked();
assertPreparedAndNotSealedLocked("removeChildSessionId");
final int indexOfSession = mChildSessionIds.indexOfKey(sessionId);
if (indexOfSession < 0) {
// not added in the first place; no-op
return;
}
if (session != null) {
session.setParentSessionId(SessionInfo.INVALID_ID);
}
mChildSessionIds.removeAt(indexOfSession);
}
} finally {
releaseTransactionLock();
if (session != null) {
session.setParentSessionId(SessionInfo.INVALID_ID);
mChildSessions.removeAt(indexOfSession);
} finally {
releaseTransactionLock();
session.releaseTransactionLock();
}
}
@@ -3504,7 +3510,7 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub {
pw.printPair("params.isMultiPackage", params.isMultiPackage);
pw.printPair("params.isStaged", params.isStaged);
pw.printPair("mParentSessionId", mParentSessionId);
pw.printPair("mChildSessionIds", mChildSessionIds);
pw.printPair("mChildSessionIds", getChildSessionIdsLocked());
pw.printPair("mStagedSessionApplied", mStagedSessionApplied);
pw.printPair("mStagedSessionFailed", mStagedSessionFailed);
pw.printPair("mStagedSessionReady", mStagedSessionReady);