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

Merge "Remove deadlock/lock contention on Incremental progress update." into sc-dev

Browse Source
This commit is contained in:
TreeHugger Robot
2021-04-05 19:20:08 +00:00
committed by Android (Google) Code Review
parent b35f41d955 14b4c8d7dd
commit b0ccf6470f
1 changed files with 42 additions and 39 deletions
Download Patch File Download Diff File Expand all files Collapse all files

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

@@ -309,23 +309,24 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub {
private final String mOriginalInstallerPackageName; private final String mOriginalInstallerPackageName;
/** Uid of the owner of the installer session */ /** Uid of the owner of the installer session */
@GuardedBy("mLock") private volatile int mInstallerUid;
private int mInstallerUid;
/** Where this install request came from */ /** Where this install request came from */
@GuardedBy("mLock") @GuardedBy("mLock")
private InstallSource mInstallSource; private InstallSource mInstallSource;
@GuardedBy("mLock") private final Object mProgressLock = new Object();
@GuardedBy("mProgressLock")
private float mClientProgress = 0; private float mClientProgress = 0;
@GuardedBy("mLock") @GuardedBy("mProgressLock")
private float mInternalProgress = 0; private float mInternalProgress = 0;
@GuardedBy("mLock") @GuardedBy("mProgressLock")
private float mProgress = 0; private float mProgress = 0;
@GuardedBy("mLock") @GuardedBy("mProgressLock")
private float mReportedProgress = -1; private float mReportedProgress = -1;
@GuardedBy("mLock") @GuardedBy("mProgressLock")
private float mIncrementalProgress = 0; private float mIncrementalProgress = 0;
/** State of the session. */ /** State of the session. */
@@ -571,7 +572,7 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub {
*/ */
@Override @Override
public void installSession(IntentSender statusReceiver) { public void installSession(IntentSender statusReceiver) {
assertCallerIsOwnerOrRootOrSystemLocked(); assertCallerIsOwnerOrRootOrSystem();
assertNotChildLocked("StagedSession#installSession"); assertNotChildLocked("StagedSession#installSession");
Preconditions.checkArgument(isCommitted() && isSessionReady()); Preconditions.checkArgument(isCommitted() && isSessionReady());
@@ -670,7 +671,7 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub {
final Runnable r; final Runnable r;
synchronized (mLock) { synchronized (mLock) {
assertNotChildLocked("StagedSession#abandon"); assertNotChildLocked("StagedSession#abandon");
assertCallerIsOwnerOrRootLocked(); assertCallerIsOwnerOrRoot();
if (isInTerminalState()) { if (isInTerminalState()) {
// We keep the session in the database if it's in a finalized state. It will be // We keep the session in the database if it's in a finalized state. It will be
// removed by PackageInstallerService when the last update time is old enough. // removed by PackageInstallerService when the last update time is old enough.
@@ -744,7 +745,7 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub {
*/ */
@Override @Override
public void verifySession() { public void verifySession() {
assertCallerIsOwnerOrRootOrSystemLocked(); assertCallerIsOwnerOrRootOrSystem();
Preconditions.checkArgument(isCommitted()); Preconditions.checkArgument(isCommitted());
Preconditions.checkArgument(!mSessionApplied && !mSessionFailed); Preconditions.checkArgument(!mSessionApplied && !mSessionFailed);
verify(); verify();
@@ -1229,7 +1230,7 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub {
} }
} }
@GuardedBy("mLock") @GuardedBy("mProgressLock")
private void setClientProgressLocked(float progress) { private void setClientProgressLocked(float progress) {
// Always publish first staging movement // Always publish first staging movement
final boolean forcePublish = (mClientProgress == 0); final boolean forcePublish = (mClientProgress == 0);
@@ -1239,21 +1240,21 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub {
@Override @Override
public void setClientProgress(float progress) { public void setClientProgress(float progress) {
synchronized (mLock) { assertCallerIsOwnerOrRoot();
assertCallerIsOwnerOrRootLocked(); synchronized (mProgressLock) {
setClientProgressLocked(progress); setClientProgressLocked(progress);
} }
} }
@Override @Override
public void addClientProgress(float progress) { public void addClientProgress(float progress) {
synchronized (mLock) { assertCallerIsOwnerOrRoot();
assertCallerIsOwnerOrRootLocked(); synchronized (mProgressLock) {
setClientProgressLocked(mClientProgress + progress); setClientProgressLocked(mClientProgress + progress);
} }
} }
@GuardedBy("mLock") @GuardedBy("mProgressLock")
private void computeProgressLocked(boolean forcePublish) { private void computeProgressLocked(boolean forcePublish) {
if (!mCommitted) { if (!mCommitted) {
mProgress = MathUtils.constrain(mClientProgress * 0.8f, 0f, 0.8f) mProgress = MathUtils.constrain(mClientProgress * 0.8f, 0f, 0.8f)
@@ -1278,8 +1279,8 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub {
@Override @Override
public String[] getNames() { public String[] getNames() {
assertCallerIsOwnerOrRoot();
synchronized (mLock) { synchronized (mLock) {
assertCallerIsOwnerOrRootLocked();
assertPreparedAndNotCommittedOrDestroyedLocked("getNames"); assertPreparedAndNotCommittedOrDestroyedLocked("getNames");
return getNamesLocked(); return getNamesLocked();
@@ -1362,8 +1363,8 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub {
} }
} }
assertCallerIsOwnerOrRoot();
synchronized (mLock) { synchronized (mLock) {
assertCallerIsOwnerOrRootLocked();
assertPreparedAndNotCommittedOrDestroyedLocked("addChecksums"); assertPreparedAndNotCommittedOrDestroyedLocked("addChecksums");
if (mChecksums.containsKey(name)) { if (mChecksums.containsKey(name)) {
@@ -1384,8 +1385,8 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub {
throw new IllegalStateException("Must specify package name to remove a split"); throw new IllegalStateException("Must specify package name to remove a split");
} }
assertCallerIsOwnerOrRoot();
synchronized (mLock) { synchronized (mLock) {
assertCallerIsOwnerOrRootLocked();
assertPreparedAndNotCommittedOrDestroyedLocked("removeSplit"); assertPreparedAndNotCommittedOrDestroyedLocked("removeSplit");
try { try {
@@ -1431,8 +1432,8 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub {
throw new IllegalStateException( throw new IllegalStateException(
"Cannot write regular files in a data loader installation session."); "Cannot write regular files in a data loader installation session.");
} }
assertCallerIsOwnerOrRoot();
synchronized (mLock) { synchronized (mLock) {
assertCallerIsOwnerOrRootLocked();
assertPreparedAndNotSealedLocked("assertCanWrite"); assertPreparedAndNotSealedLocked("assertCanWrite");
} }
if (reverseMode) { if (reverseMode) {
@@ -1605,8 +1606,8 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub {
throw new IllegalStateException( throw new IllegalStateException(
"Cannot read regular files in a data loader installation session."); "Cannot read regular files in a data loader installation session.");
} }
assertCallerIsOwnerOrRoot();
synchronized (mLock) { synchronized (mLock) {
assertCallerIsOwnerOrRootLocked();
assertPreparedAndNotCommittedOrDestroyedLocked("openRead"); assertPreparedAndNotCommittedOrDestroyedLocked("openRead");
try { try {
return openReadInternalLocked(name); return openReadInternalLocked(name);
@@ -1634,8 +1635,7 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub {
* Check if the caller is the owner of this session. Otherwise throw a * Check if the caller is the owner of this session. Otherwise throw a
* {@link SecurityException}. * {@link SecurityException}.
*/ */
@GuardedBy("mLock") private void assertCallerIsOwnerOrRoot() {
private void assertCallerIsOwnerOrRootLocked() {
final int callingUid = Binder.getCallingUid(); final int callingUid = Binder.getCallingUid();
if (callingUid != Process.ROOT_UID && callingUid != mInstallerUid) { if (callingUid != Process.ROOT_UID && callingUid != mInstallerUid) {
throw new SecurityException("Session does not belong to uid " + callingUid); throw new SecurityException("Session does not belong to uid " + callingUid);
@@ -1646,8 +1646,7 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub {
* Check if the caller is the owner of this session. Otherwise throw a * Check if the caller is the owner of this session. Otherwise throw a
* {@link SecurityException}. * {@link SecurityException}.
*/ */
@GuardedBy("mLock") private void assertCallerIsOwnerOrRootOrSystem() {
private void assertCallerIsOwnerOrRootOrSystemLocked() {
final int callingUid = Binder.getCallingUid(); final int callingUid = Binder.getCallingUid();
if (callingUid != Process.ROOT_UID && callingUid != mInstallerUid if (callingUid != Process.ROOT_UID && callingUid != mInstallerUid
&& callingUid != Process.SYSTEM_UID) { && callingUid != Process.SYSTEM_UID) {
@@ -1929,9 +1928,9 @@ 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);
assertCallerIsOwnerOrRoot();
synchronized (mLock) { synchronized (mLock) {
assertCallerIsOwnerOrRootLocked();
assertPreparedAndNotDestroyedLocked("commit of session " + sessionId); assertPreparedAndNotDestroyedLocked("commit of session " + sessionId);
assertNoWriteFileTransfersOpenLocked(); assertNoWriteFileTransfersOpenLocked();
@@ -2004,9 +2003,11 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub {
"Session destroyed"); "Session destroyed");
} }
if (!isIncrementalInstallation()) { if (!isIncrementalInstallation()) {
// For non-incremental installs, client staging is fully done at this point synchronized (mProgressLock) {
mClientProgress = 1f; // For non-incremental installs, client staging is fully done at this point
computeProgressLocked(true); mClientProgress = 1f;
computeProgressLocked(true);
}
} }
// This ongoing commit should keep session active, even though client // This ongoing commit should keep session active, even though client
@@ -2191,7 +2192,7 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub {
} }
synchronized (mLock) { synchronized (mLock) {
assertCallerIsOwnerOrRootLocked(); assertCallerIsOwnerOrRoot();
assertPreparedAndNotSealedLocked("transfer"); assertPreparedAndNotSealedLocked("transfer");
try { try {
@@ -2376,8 +2377,10 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub {
PackageLite result = parseApkLite(); PackageLite result = parseApkLite();
if (result != null) { if (result != null) {
mPackageLite = result; mPackageLite = result;
mInternalProgress = 0.5f; synchronized (mProgressLock) {
computeProgressLocked(true); mInternalProgress = 0.5f;
computeProgressLocked(true);
}
extractNativeLibraries( extractNativeLibraries(
mPackageLite, stageDir, params.abiOverride, mayInheritNativeLibs()); mPackageLite, stageDir, params.abiOverride, mayInheritNativeLibs());
@@ -3554,7 +3557,7 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub {
int activeCount; int activeCount;
synchronized (mLock) { synchronized (mLock) {
if (checkCaller) { if (checkCaller) {
assertCallerIsOwnerOrRootLocked(); assertCallerIsOwnerOrRoot();
} }
activeCount = mActiveCount.decrementAndGet(); activeCount = mActiveCount.decrementAndGet();
@@ -3593,7 +3596,7 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub {
private void abandonNonStaged() { private void abandonNonStaged() {
synchronized (mLock) { synchronized (mLock) {
assertNotChildLocked("abandonNonStaged"); assertNotChildLocked("abandonNonStaged");
assertCallerIsOwnerOrRootLocked(); assertCallerIsOwnerOrRoot();
if (mRelinquished) { if (mRelinquished) {
if (LOGD) Slog.d(TAG, "Ignoring abandon after commit relinquished control"); if (LOGD) Slog.d(TAG, "Ignoring abandon after commit relinquished control");
return; return;
@@ -3659,7 +3662,7 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub {
} }
synchronized (mLock) { synchronized (mLock) {
assertCallerIsOwnerOrRootLocked(); assertCallerIsOwnerOrRoot();
assertPreparedAndNotSealedLocked("addFile"); assertPreparedAndNotSealedLocked("addFile");
if (!mFiles.add(new FileEntry(mFiles.size(), if (!mFiles.add(new FileEntry(mFiles.size(),
@@ -3680,7 +3683,7 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub {
} }
synchronized (mLock) { synchronized (mLock) {
assertCallerIsOwnerOrRootLocked(); assertCallerIsOwnerOrRoot();
assertPreparedAndNotSealedLocked("removeFile"); assertPreparedAndNotSealedLocked("removeFile");
if (!mFiles.add(new FileEntry(mFiles.size(), if (!mFiles.add(new FileEntry(mFiles.size(),
@@ -3893,7 +3896,7 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub {
new IPackageLoadingProgressCallback.Stub() { new IPackageLoadingProgressCallback.Stub() {
@Override @Override
public void onPackageLoadingProgressChanged(float progress) { public void onPackageLoadingProgressChanged(float progress) {
synchronized (mLock) { synchronized (mProgressLock) {
mIncrementalProgress = progress; mIncrementalProgress = progress;
computeProgressLocked(true); computeProgressLocked(true);
} }
@@ -3993,7 +3996,7 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub {
+ " as it is in an invalid state."); + " as it is in an invalid state.");
} }
synchronized (mLock) { synchronized (mLock) {
assertCallerIsOwnerOrRootLocked(); assertCallerIsOwnerOrRoot();
assertPreparedAndNotSealedLocked("addChildSessionId"); assertPreparedAndNotSealedLocked("addChildSessionId");
final int indexOfSession = mChildSessions.indexOfKey(childSessionId); final int indexOfSession = mChildSessions.indexOfKey(childSessionId);
@@ -4012,7 +4015,7 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub {
@Override @Override
public void removeChildSessionId(int sessionId) { public void removeChildSessionId(int sessionId) {
synchronized (mLock) { synchronized (mLock) {
assertCallerIsOwnerOrRootLocked(); assertCallerIsOwnerOrRoot();
assertPreparedAndNotSealedLocked("removeChildSessionId"); assertPreparedAndNotSealedLocked("removeChildSessionId");
final int indexOfSession = mChildSessions.indexOfKey(sessionId); final int indexOfSession = mChildSessions.indexOfKey(sessionId);