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

Remove deadlock/lock contention on Incremental progress update.

Browse Source 
Incremental progress is updated separately, and can create deadlock or
simply wait for mLock while session is doing some heavy processing. This
in turn blocks progress update or binder thread, causing long delays for
all other packages.

Bug: 160635296
Test: atest PackageManagerShellCommandTest PackageManagerShellCommandIncrementalTest IncrementalServiceTest PackageManagerServiceTest ChecksumsTest
Change-Id: I6120443a03ec22d1ca672e5a1acf8b3752e1c3ee
Merged-In: I6120443a03ec22d1ca672e5a1acf8b3752e1c3ee
This commit is contained in:
Alex Buynytskyy
2021-03-30 21:06:15 -07:00
parent befdba3764
commit 14b4c8d7dd
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;
/** Uid of the owner of the installer session */
@GuardedBy("mLock")
private int mInstallerUid;
private volatile int mInstallerUid;
/** Where this install request came from */
@GuardedBy("mLock")
private InstallSource mInstallSource;
@GuardedBy("mLock")
private final Object mProgressLock = new Object();
@GuardedBy("mProgressLock")
private float mClientProgress = 0;
@GuardedBy("mLock")
@GuardedBy("mProgressLock")
private float mInternalProgress = 0;
@GuardedBy("mLock")
@GuardedBy("mProgressLock")
private float mProgress = 0;
@GuardedBy("mLock")
@GuardedBy("mProgressLock")
private float mReportedProgress = -1;
@GuardedBy("mLock")
@GuardedBy("mProgressLock")
private float mIncrementalProgress = 0;
/** State of the session. */
@@ -571,7 +572,7 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub {
*/
@Override
public void installSession(IntentSender statusReceiver) {
assertCallerIsOwnerOrRootOrSystemLocked();
assertCallerIsOwnerOrRootOrSystem();
assertNotChildLocked("StagedSession#installSession");
Preconditions.checkArgument(isCommitted() && isSessionReady());
@@ -670,7 +671,7 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub {
final Runnable r;
synchronized (mLock) {
assertNotChildLocked("StagedSession#abandon");
assertCallerIsOwnerOrRootLocked();
assertCallerIsOwnerOrRoot();
if (isInTerminalState()) {
// 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.
@@ -744,7 +745,7 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub {
*/
@Override
public void verifySession() {
assertCallerIsOwnerOrRootOrSystemLocked();
assertCallerIsOwnerOrRootOrSystem();
Preconditions.checkArgument(isCommitted());
Preconditions.checkArgument(!mSessionApplied && !mSessionFailed);
verify();
@@ -1229,7 +1230,7 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub {
}
}
@GuardedBy("mLock")
@GuardedBy("mProgressLock")
private void setClientProgressLocked(float progress) {
// Always publish first staging movement
final boolean forcePublish = (mClientProgress == 0);
@@ -1239,21 +1240,21 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub {
@Override
public void setClientProgress(float progress) {
synchronized (mLock) {
assertCallerIsOwnerOrRootLocked();
assertCallerIsOwnerOrRoot();
synchronized (mProgressLock) {
setClientProgressLocked(progress);
}
}
@Override
public void addClientProgress(float progress) {
synchronized (mLock) {
assertCallerIsOwnerOrRootLocked();
assertCallerIsOwnerOrRoot();
synchronized (mProgressLock) {
setClientProgressLocked(mClientProgress + progress);
}
}
@GuardedBy("mLock")
@GuardedBy("mProgressLock")
private void computeProgressLocked(boolean forcePublish) {
if (!mCommitted) {
mProgress = MathUtils.constrain(mClientProgress * 0.8f, 0f, 0.8f)
@@ -1278,8 +1279,8 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub {
@Override
public String[] getNames() {
assertCallerIsOwnerOrRoot();
synchronized (mLock) {
assertCallerIsOwnerOrRootLocked();
assertPreparedAndNotCommittedOrDestroyedLocked("getNames");
return getNamesLocked();
@@ -1362,8 +1363,8 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub {
}
}
assertCallerIsOwnerOrRoot();
synchronized (mLock) {
assertCallerIsOwnerOrRootLocked();
assertPreparedAndNotCommittedOrDestroyedLocked("addChecksums");
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");
}
assertCallerIsOwnerOrRoot();
synchronized (mLock) {
assertCallerIsOwnerOrRootLocked();
assertPreparedAndNotCommittedOrDestroyedLocked("removeSplit");
try {
@@ -1431,8 +1432,8 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub {
throw new IllegalStateException(
"Cannot write regular files in a data loader installation session.");
}
assertCallerIsOwnerOrRoot();
synchronized (mLock) {
assertCallerIsOwnerOrRootLocked();
assertPreparedAndNotSealedLocked("assertCanWrite");
}
if (reverseMode) {
@@ -1605,8 +1606,8 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub {
throw new IllegalStateException(
"Cannot read regular files in a data loader installation session.");
}
assertCallerIsOwnerOrRoot();
synchronized (mLock) {
assertCallerIsOwnerOrRootLocked();
assertPreparedAndNotCommittedOrDestroyedLocked("openRead");
try {
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
* {@link SecurityException}.
*/
@GuardedBy("mLock")
private void assertCallerIsOwnerOrRootLocked() {
private void assertCallerIsOwnerOrRoot() {
final int callingUid = Binder.getCallingUid();
if (callingUid != Process.ROOT_UID && callingUid != mInstallerUid) {
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
* {@link SecurityException}.
*/
@GuardedBy("mLock")
private void assertCallerIsOwnerOrRootOrSystemLocked() {
private void assertCallerIsOwnerOrRootOrSystem() {
final int callingUid = Binder.getCallingUid();
if (callingUid != Process.ROOT_UID && callingUid != mInstallerUid
&& callingUid != Process.SYSTEM_UID) {
@@ -1929,9 +1928,9 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub {
*/
private boolean markAsSealed(@NonNull IntentSender statusReceiver, boolean forTransfer) {
Objects.requireNonNull(statusReceiver);
assertCallerIsOwnerOrRoot();
synchronized (mLock) {
assertCallerIsOwnerOrRootLocked();
assertPreparedAndNotDestroyedLocked("commit of session " + sessionId);
assertNoWriteFileTransfersOpenLocked();
@@ -2004,9 +2003,11 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub {
"Session destroyed");
}
if (!isIncrementalInstallation()) {
// For non-incremental installs, client staging is fully done at this point
mClientProgress = 1f;
computeProgressLocked(true);
synchronized (mProgressLock) {
// For non-incremental installs, client staging is fully done at this point
mClientProgress = 1f;
computeProgressLocked(true);
}
}
// This ongoing commit should keep session active, even though client
@@ -2191,7 +2192,7 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub {
}
synchronized (mLock) {
assertCallerIsOwnerOrRootLocked();
assertCallerIsOwnerOrRoot();
assertPreparedAndNotSealedLocked("transfer");
try {
@@ -2376,8 +2377,10 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub {
PackageLite result = parseApkLite();
if (result != null) {
mPackageLite = result;
mInternalProgress = 0.5f;
computeProgressLocked(true);
synchronized (mProgressLock) {
mInternalProgress = 0.5f;
computeProgressLocked(true);
}
extractNativeLibraries(
mPackageLite, stageDir, params.abiOverride, mayInheritNativeLibs());
@@ -3554,7 +3557,7 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub {
int activeCount;
synchronized (mLock) {
if (checkCaller) {
assertCallerIsOwnerOrRootLocked();
assertCallerIsOwnerOrRoot();
}
activeCount = mActiveCount.decrementAndGet();
@@ -3593,7 +3596,7 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub {
private void abandonNonStaged() {
synchronized (mLock) {
assertNotChildLocked("abandonNonStaged");
assertCallerIsOwnerOrRootLocked();
assertCallerIsOwnerOrRoot();
if (mRelinquished) {
if (LOGD) Slog.d(TAG, "Ignoring abandon after commit relinquished control");
return;
@@ -3659,7 +3662,7 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub {
}
synchronized (mLock) {
assertCallerIsOwnerOrRootLocked();
assertCallerIsOwnerOrRoot();
assertPreparedAndNotSealedLocked("addFile");
if (!mFiles.add(new FileEntry(mFiles.size(),
@@ -3680,7 +3683,7 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub {
}
synchronized (mLock) {
assertCallerIsOwnerOrRootLocked();
assertCallerIsOwnerOrRoot();
assertPreparedAndNotSealedLocked("removeFile");
if (!mFiles.add(new FileEntry(mFiles.size(),
@@ -3893,7 +3896,7 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub {
new IPackageLoadingProgressCallback.Stub() {
@Override
public void onPackageLoadingProgressChanged(float progress) {
synchronized (mLock) {
synchronized (mProgressLock) {
mIncrementalProgress = progress;
computeProgressLocked(true);
}
@@ -3993,7 +3996,7 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub {
+ " as it is in an invalid state.");
}
synchronized (mLock) {
assertCallerIsOwnerOrRootLocked();
assertCallerIsOwnerOrRoot();
assertPreparedAndNotSealedLocked("addChildSessionId");
final int indexOfSession = mChildSessions.indexOfKey(childSessionId);
@@ -4012,7 +4015,7 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub {
@Override
public void removeChildSessionId(int sessionId) {
synchronized (mLock) {
assertCallerIsOwnerOrRootLocked();
assertCallerIsOwnerOrRoot();
assertPreparedAndNotSealedLocked("removeChildSessionId");
final int indexOfSession = mChildSessions.indexOfKey(sessionId);