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

Fix deadlock on CompatConfig.mChanges

Browse Source 
Call out to PackageManager before locking mChanges.

Bug: 187514238
Test: atest FrameworksServicesTests:CompatConfigTest
Merged-In: I72b23fb4651c6efece0afb53b5dc4ca8df63dddb
Change-Id: I72b23fb4651c6efece0afb53b5dc4ca8df63dddb
This commit is contained in:
tomnatan
2021-05-18 11:57:14 +00:00
parent 8df7b875d4
commit da0913d1aa
2 changed files with 42 additions and 38 deletions
Download Patch File Download Diff File Expand all files Collapse all files

32
services/core/java/com/android/server/compat/CompatChange.java
View File

@@ -26,9 +26,7 @@ import android.compat.annotation.ChangeId;
import android.compat.annotation.Disabled;
import android.compat.annotation.EnabledSince;
import android.compat.annotation.Overridable;
import android.content.Context;
import android.content.pm.ApplicationInfo;
import android.content.pm.PackageManager;
import com.android.internal.compat.AndroidBuildClassifier;
import com.android.internal.compat.CompatibilityChangeInfo;
@@ -161,15 +159,17 @@ public final class CompatChange extends CompatibilityChangeInfo {
*
* @param packageName Package name to tentatively enable the change for.
* @param override The package override to be set
* @param allowedState Whether the override is allowed.
* @param versionCode The version code of the package.
*/
void addPackageOverride(String packageName, PackageOverride override,
OverrideAllowedState allowedState, Context context) {
OverrideAllowedState allowedState, @Nullable Long versionCode) {
if (getLoggingOnly()) {
throw new IllegalArgumentException(
"Can't add overrides for a logging only change " + toString());
}
mRawOverrides.put(packageName, override);
recheckOverride(packageName, allowedState, context);
recheckOverride(packageName, allowedState, versionCode);
}
/**
@@ -179,32 +179,24 @@ public final class CompatChange extends CompatibilityChangeInfo {
* overrides, check if they need to be demoted to deferred.</p>
*
* @param packageName Package name to apply deferred overrides for.
* @param allowed Whether the override is allowed.
* @param allowedState Whether the override is allowed.
* @param versionCode The version code of the package.
*
* @return {@code true} if the recheck yielded a result that requires invalidating caches
* (a deferred override was consolidated or a regular override was removed).
*/
boolean recheckOverride(String packageName, OverrideAllowedState allowedState,
Context context) {
@Nullable Long versionCode) {
boolean allowed = (allowedState.state == OverrideAllowedState.ALLOWED);
Long version = null;
try {
ApplicationInfo applicationInfo = context.getPackageManager().getApplicationInfo(
packageName, 0);
version = applicationInfo.longVersionCode;
} catch (PackageManager.NameNotFoundException e) {
// Do nothing
}
// If the app is not installed or no longer has raw overrides, evaluate to false
if (version == null || !hasRawOverride(packageName) || !allowed) {
if (versionCode == null || !hasRawOverride(packageName) || !allowed) {
removePackageOverrideInternal(packageName);
return false;
}
// Evaluate the override based on its version
int overrideValue = mRawOverrides.get(packageName).evaluate(version);
int overrideValue = mRawOverrides.get(packageName).evaluate(versionCode);
switch (overrideValue) {
case VALUE_UNDEFINED:
removePackageOverrideInternal(packageName);
@@ -229,11 +221,13 @@ public final class CompatChange extends CompatibilityChangeInfo {
* <p>Note, this method is not thread safe so callers must ensure thread safety.
*
* @param pname Package name to reset to defaults for.
* @param allowedState Whether the override is allowed.
* @param versionCode The version code of the package.
*/
boolean removePackageOverride(String pname, OverrideAllowedState allowedState,
Context context) {
@Nullable Long versionCode) {
if (mRawOverrides.remove(pname) != null) {
recheckOverride(pname, allowedState, context);
recheckOverride(pname, allowedState, versionCode);
return true;
}
return false;

48
services/core/java/com/android/server/compat/CompatConfig.java
View File

@@ -16,11 +16,13 @@
package com.android.server.compat;
import android.annotation.Nullable;
import android.app.compat.ChangeIdStateCache;
import android.app.compat.PackageOverride;
import android.compat.Compatibility.ChangeConfig;
import android.content.Context;
import android.content.pm.ApplicationInfo;
import android.content.pm.PackageManager;
import android.os.Environment;
import android.text.TextUtils;
import android.util.LongArray;
@@ -51,7 +53,6 @@ import java.io.FileInputStream;
import java.io.IOException;
import java.io.InputStream;
import java.io.PrintWriter;
import java.util.ArrayList;
import java.util.HashSet;
import java.util.List;
import java.util.Set;
@@ -249,6 +250,7 @@ final class CompatConfig {
OverrideAllowedState allowedState =
mOverrideValidator.getOverrideAllowedState(changeId, packageName);
allowedState.enforce(changeId, packageName);
Long versionCode = getVersionCodeOrNull(packageName);
synchronized (mChanges) {
CompatChange c = mChanges.get(changeId);
if (c == null) {
@@ -256,7 +258,7 @@ final class CompatConfig {
c = new CompatChange(changeId);
addChange(c);
}
c.addPackageOverride(packageName, overrides, allowedState, mContext);
c.addPackageOverride(packageName, overrides, allowedState, versionCode);
invalidateCache();
}
return alreadyKnown;
@@ -336,6 +338,7 @@ final class CompatConfig {
* It does not invalidate the cache nor save the overrides.
*/
private boolean removeOverrideUnsafe(long changeId, String packageName) {
Long versionCode = getVersionCodeOrNull(packageName);
synchronized (mChanges) {
CompatChange c = mChanges.get(changeId);
if (c != null) {
@@ -343,7 +346,7 @@ final class CompatConfig {
mOverrideValidator.getOverrideAllowedState(changeId, packageName);
if (c.hasPackageOverride(packageName)) {
allowedState.enforce(changeId, packageName);
c.removePackageOverride(packageName, allowedState, mContext);
c.removePackageOverride(packageName, allowedState, versionCode);
invalidateCache();
return true;
}
@@ -653,26 +656,33 @@ final class CompatConfig {
* Rechecks all the existing overrides for a package.
*/
void recheckOverrides(String packageName) {
// Local cache of compat changes. Holding a lock on mChanges for the whole duration of the
// method will cause a deadlock.
List<CompatChange> changes;
Long versionCode = getVersionCodeOrNull(packageName);
synchronized (mChanges) {
changes = new ArrayList<>(mChanges.size());
boolean shouldInvalidateCache = false;
for (int idx = 0; idx < mChanges.size(); ++idx) {
changes.add(mChanges.valueAt(idx));
CompatChange c = mChanges.valueAt(idx);
if (!c.hasPackageOverride(packageName)) {
continue;
}
OverrideAllowedState allowedState =
mOverrideValidator.getOverrideAllowedStateForRecheck(c.getId(),
packageName);
shouldInvalidateCache |= c.recheckOverride(packageName, allowedState, versionCode);
}
if (shouldInvalidateCache) {
invalidateCache();
}
}
boolean shouldInvalidateCache = false;
for (CompatChange c: changes) {
if (!c.hasPackageOverride(packageName)) {
continue;
}
OverrideAllowedState allowedState =
mOverrideValidator.getOverrideAllowedStateForRecheck(c.getId(), packageName);
shouldInvalidateCache |= c.recheckOverride(packageName, allowedState, mContext);
}
if (shouldInvalidateCache) {
invalidateCache();
}
@Nullable
private Long getVersionCodeOrNull(String packageName) {
try {
ApplicationInfo applicationInfo = mContext.getPackageManager().getApplicationInfo(
packageName, 0);
return applicationInfo.longVersionCode;
} catch (PackageManager.NameNotFoundException e) {
return null;
}
}