Merge "Reduce lock contention on setting write." into tm-dev

This commit is contained in:
Alex Buynytskyy
2022-04-22 20:08:00 +00:00
committed by Android (Google) Code Review
3 changed files with 120 additions and 61 deletions

View File

@@ -1430,7 +1430,7 @@ public class PackageManagerService implements PackageSender, TestUtilityService
(i, pm) -> new Settings(Environment.getDataDirectory(), (i, pm) -> new Settings(Environment.getDataDirectory(),
RuntimePermissionsPersistence.createInstance(), RuntimePermissionsPersistence.createInstance(),
i.getPermissionManagerServiceInternal(), i.getPermissionManagerServiceInternal(),
domainVerificationService, lock), domainVerificationService, backgroundHandler, lock),
(i, pm) -> AppsFilterImpl.create(i, (i, pm) -> AppsFilterImpl.create(i,
i.getLocalService(PackageManagerInternal.class)), i.getLocalService(PackageManagerInternal.class)),
(i, pm) -> (PlatformCompat) ServiceManager.getService("platform_compat"), (i, pm) -> (PlatformCompat) ServiceManager.getService("platform_compat"),

View File

@@ -360,6 +360,8 @@ public final class Settings implements Watchable, Snappable {
private static final String ATTR_VALUE = "value"; private static final String ATTR_VALUE = "value";
private static final String ATTR_FIRST_INSTALL_TIME = "first-install-time"; private static final String ATTR_FIRST_INSTALL_TIME = "first-install-time";
private final Handler mHandler;
private final PackageManagerTracedLock mLock; private final PackageManagerTracedLock mLock;
@Watched(manual = true) @Watched(manual = true)
@@ -583,6 +585,8 @@ public final class Settings implements Watchable, Snappable {
"Settings.mInstallerPackages"); "Settings.mInstallerPackages");
mKeySetManagerService = new KeySetManagerService(mPackages); mKeySetManagerService = new KeySetManagerService(mPackages);
// Test-only handler working on background thread.
mHandler = new Handler(BackgroundThread.getHandler().getLooper());
mLock = new PackageManagerTracedLock(); mLock = new PackageManagerTracedLock();
mPackages.putAll(pkgSettings); mPackages.putAll(pkgSettings);
mAppIds = new AppIdSettingMap(); mAppIds = new AppIdSettingMap();
@@ -607,6 +611,7 @@ public final class Settings implements Watchable, Snappable {
Settings(File dataDir, RuntimePermissionsPersistence runtimePermissionsPersistence, Settings(File dataDir, RuntimePermissionsPersistence runtimePermissionsPersistence,
LegacyPermissionDataProvider permissionDataProvider, LegacyPermissionDataProvider permissionDataProvider,
@NonNull DomainVerificationManagerInternal domainVerificationManager, @NonNull DomainVerificationManagerInternal domainVerificationManager,
@NonNull Handler handler,
@NonNull PackageManagerTracedLock lock) { @NonNull PackageManagerTracedLock lock) {
mPackages = new WatchedArrayMap<>(); mPackages = new WatchedArrayMap<>();
mPackagesSnapshot = mPackagesSnapshot =
@@ -620,6 +625,7 @@ public final class Settings implements Watchable, Snappable {
"Settings.mInstallerPackages"); "Settings.mInstallerPackages");
mKeySetManagerService = new KeySetManagerService(mPackages); mKeySetManagerService = new KeySetManagerService(mPackages);
mHandler = handler;
mLock = lock; mLock = lock;
mAppIds = new AppIdSettingMap(); mAppIds = new AppIdSettingMap();
mPermissions = new LegacyPermissionSettings(lock); mPermissions = new LegacyPermissionSettings(lock);
@@ -627,10 +633,8 @@ public final class Settings implements Watchable, Snappable {
runtimePermissionsPersistence, new Consumer<Integer>() { runtimePermissionsPersistence, new Consumer<Integer>() {
@Override @Override
public void accept(Integer userId) { public void accept(Integer userId) {
synchronized (mLock) { mRuntimePermissionsPersistence.writeStateForUser(userId,
mRuntimePermissionsPersistence.writeStateForUserSync(userId, mPermissionDataProvider, mPackages, mSharedUsers, mHandler, mLock);
mPermissionDataProvider, mPackages, mSharedUsers);
}
} }
}); });
mPermissionDataProvider = permissionDataProvider; mPermissionDataProvider = permissionDataProvider;
@@ -679,6 +683,7 @@ public final class Settings implements Watchable, Snappable {
// needed by the read-only methods. Note especially that the lock // needed by the read-only methods. Note especially that the lock
// is not required because this clone is meant to support lock-free // is not required because this clone is meant to support lock-free
// read-only methods. // read-only methods.
mHandler = null;
mLock = null; mLock = null;
mRuntimePermissionsPersistence = r.mRuntimePermissionsPersistence; mRuntimePermissionsPersistence = r.mRuntimePermissionsPersistence;
mSettingsFilename = null; mSettingsFilename = null;
@@ -5286,8 +5291,8 @@ public final class Settings implements Watchable, Snappable {
public void writePermissionStateForUserLPr(int userId, boolean sync) { public void writePermissionStateForUserLPr(int userId, boolean sync) {
if (sync) { if (sync) {
mRuntimePermissionsPersistence.writeStateForUserSync(userId, mPermissionDataProvider, mRuntimePermissionsPersistence.writeStateForUser(userId, mPermissionDataProvider,
mPackages, mSharedUsers); mPackages, mSharedUsers, /*handler=*/null, mLock);
} else { } else {
mRuntimePermissionsPersistence.writeStateForUserAsync(userId); mRuntimePermissionsPersistence.writeStateForUserAsync(userId);
} }
@@ -5373,9 +5378,14 @@ public final class Settings implements Watchable, Snappable {
private String mExtendedFingerprint; private String mExtendedFingerprint;
@GuardedBy("mPersistenceLock")
private final RuntimePermissionsPersistence mPersistence; private final RuntimePermissionsPersistence mPersistence;
private final Object mPersistenceLock = new Object();
private final Handler mHandler = new MyHandler(); // Low-priority handlers running on SystemBg thread.
private final Handler mAsyncHandler = new MyHandler();
private final Handler mPersistenceHandler = new Handler(
BackgroundThread.getHandler().getLooper());
private final Object mLock = new Object(); private final Object mLock = new Object();
@@ -5398,6 +5408,11 @@ public final class Settings implements Watchable, Snappable {
// The mapping keys are user ids. // The mapping keys are user ids.
private final SparseBooleanArray mPermissionUpgradeNeeded = new SparseBooleanArray(); private final SparseBooleanArray mPermissionUpgradeNeeded = new SparseBooleanArray();
@GuardedBy("mLock")
// Staging area for states prepared to be written.
private final SparseArray<RuntimePermissionsState> mPendingStatesToWrite =
new SparseArray<>();
// This is a hack to allow this class to invoke a write using Settings's data structures, // This is a hack to allow this class to invoke a write using Settings's data structures,
// to facilitate moving to a finer scoped lock without a significant refactor. // to facilitate moving to a finer scoped lock without a significant refactor.
private final Consumer<Integer> mInvokeWriteUserStateAsyncCallback; private final Consumer<Integer> mInvokeWriteUserStateAsyncCallback;
@@ -5462,7 +5477,7 @@ public final class Settings implements Watchable, Snappable {
final long currentTimeMillis = SystemClock.uptimeMillis(); final long currentTimeMillis = SystemClock.uptimeMillis();
if (mWriteScheduled.get(userId)) { if (mWriteScheduled.get(userId)) {
mHandler.removeMessages(userId); mAsyncHandler.removeMessages(userId);
// If enough time passed, write without holding off anymore. // If enough time passed, write without holding off anymore.
final long lastNotWrittenMutationTimeMillis = mLastNotWrittenMutationTimesMillis final long lastNotWrittenMutationTimeMillis = mLastNotWrittenMutationTimesMillis
@@ -5471,7 +5486,7 @@ public final class Settings implements Watchable, Snappable {
- lastNotWrittenMutationTimeMillis; - lastNotWrittenMutationTimeMillis;
if (timeSinceLastNotWrittenMutationMillis if (timeSinceLastNotWrittenMutationMillis
>= MAX_WRITE_PERMISSIONS_DELAY_MILLIS) { >= MAX_WRITE_PERMISSIONS_DELAY_MILLIS) {
mHandler.obtainMessage(userId).sendToTarget(); mAsyncHandler.obtainMessage(userId).sendToTarget();
return; return;
} }
@@ -5481,31 +5496,37 @@ public final class Settings implements Watchable, Snappable {
final long writeDelayMillis = Math.min(WRITE_PERMISSIONS_DELAY_MILLIS, final long writeDelayMillis = Math.min(WRITE_PERMISSIONS_DELAY_MILLIS,
maxDelayMillis); maxDelayMillis);
Message message = mHandler.obtainMessage(userId); Message message = mAsyncHandler.obtainMessage(userId);
mHandler.sendMessageDelayed(message, writeDelayMillis); mAsyncHandler.sendMessageDelayed(message, writeDelayMillis);
} else { } else {
mLastNotWrittenMutationTimesMillis.put(userId, currentTimeMillis); mLastNotWrittenMutationTimesMillis.put(userId, currentTimeMillis);
Message message = mHandler.obtainMessage(userId); Message message = mAsyncHandler.obtainMessage(userId);
mHandler.sendMessageDelayed(message, WRITE_PERMISSIONS_DELAY_MILLIS); mAsyncHandler.sendMessageDelayed(message, WRITE_PERMISSIONS_DELAY_MILLIS);
mWriteScheduled.put(userId, true); mWriteScheduled.put(userId, true);
} }
} }
} }
public void writeStateForUserSync(int userId, @NonNull LegacyPermissionDataProvider public void writeStateForUser(int userId, @NonNull LegacyPermissionDataProvider
legacyPermissionDataProvider, legacyPermissionDataProvider,
@NonNull WatchedArrayMap<String, ? extends PackageStateInternal> packageStates, @NonNull WatchedArrayMap<String, ? extends PackageStateInternal> packageStates,
@NonNull WatchedArrayMap<String, SharedUserSetting> sharedUsers) { @NonNull WatchedArrayMap<String, SharedUserSetting> sharedUsers,
@Nullable Handler pmHandler, @NonNull Object pmLock) {
final int version;
final String fingerprint;
synchronized (mLock) { synchronized (mLock) {
mHandler.removeMessages(userId); mAsyncHandler.removeMessages(userId);
mWriteScheduled.delete(userId); mWriteScheduled.delete(userId);
version = mVersions.get(userId, INITIAL_VERSION);
fingerprint = mFingerprints.get(userId);
}
Runnable writer = () -> {
final RuntimePermissionsState runtimePermissions;
synchronized (pmLock) {
legacyPermissionDataProvider.writeLegacyPermissionStateTEMP(); legacyPermissionDataProvider.writeLegacyPermissionStateTEMP();
int version = mVersions.get(userId, INITIAL_VERSION);
String fingerprint = mFingerprints.get(userId);
Map<String, List<RuntimePermissionsState.PermissionState>> packagePermissions = Map<String, List<RuntimePermissionsState.PermissionState>> packagePermissions =
new ArrayMap<>(); new ArrayMap<>();
int packagesSize = packageStates.size(); int packagesSize = packageStates.size();
@@ -5516,17 +5537,19 @@ public final class Settings implements Watchable, Snappable {
List<RuntimePermissionsState.PermissionState> permissions = List<RuntimePermissionsState.PermissionState> permissions =
getPermissionsFromPermissionsState( getPermissionsFromPermissionsState(
packageState.getLegacyPermissionState(), userId); packageState.getLegacyPermissionState(), userId);
if (permissions.isEmpty() && !packageState.isInstallPermissionsFixed()) { if (permissions.isEmpty()
// Storing an empty state means the package is known to the system and && !packageState.isInstallPermissionsFixed()) {
// its install permissions have been granted and fixed. If this is not // Storing an empty state means the package is known to the
// the case, we should not store anything. // system and its install permissions have been granted and fixed.
// If this is not the case, we should not store anything.
continue; continue;
} }
packagePermissions.put(packageName, permissions); packagePermissions.put(packageName, permissions);
} }
} }
Map<String, List<RuntimePermissionsState.PermissionState>> sharedUserPermissions = Map<String, List<RuntimePermissionsState.PermissionState>>
sharedUserPermissions =
new ArrayMap<>(); new ArrayMap<>();
final int sharedUsersSize = sharedUsers.size(); final int sharedUsersSize = sharedUsers.size();
for (int i = 0; i < sharedUsersSize; i++) { for (int i = 0; i < sharedUsersSize; i++) {
@@ -5538,12 +5561,47 @@ public final class Settings implements Watchable, Snappable {
sharedUserPermissions.put(sharedUserName, permissions); sharedUserPermissions.put(sharedUserName, permissions);
} }
RuntimePermissionsState runtimePermissions = new RuntimePermissionsState(version, runtimePermissions = new RuntimePermissionsState(version,
fingerprint, packagePermissions, sharedUserPermissions); fingerprint, packagePermissions, sharedUserPermissions);
}
synchronized (mLock) {
mPendingStatesToWrite.put(userId, runtimePermissions);
}
if (pmHandler != null) {
// Async version.
mPersistenceHandler.post(() -> writePendingStates());
} else {
// Sync version.
writePendingStates();
}
};
if (pmHandler != null) {
// Async version, use pmHandler.
pmHandler.post(writer);
} else {
// Sync version, use caller's thread.
writer.run();
}
}
private void writePendingStates() {
while (true) {
final RuntimePermissionsState runtimePermissions;
final int userId;
synchronized (mLock) {
if (mPendingStatesToWrite.size() == 0) {
break;
}
userId = mPendingStatesToWrite.keyAt(0);
runtimePermissions = mPendingStatesToWrite.valueAt(0);
mPendingStatesToWrite.removeAt(0);
}
synchronized (mPersistenceLock) {
mPersistence.writeForUser(runtimePermissions, UserHandle.of(userId)); mPersistence.writeForUser(runtimePermissions, UserHandle.of(userId));
} }
} }
}
@NonNull @NonNull
private List<RuntimePermissionsState.PermissionState> getPermissionsFromPermissionsState( private List<RuntimePermissionsState.PermissionState> getPermissionsFromPermissionsState(
@@ -5563,7 +5621,7 @@ public final class Settings implements Watchable, Snappable {
private void onUserRemoved(int userId) { private void onUserRemoved(int userId) {
synchronized (mLock) { synchronized (mLock) {
// Make sure we do not // Make sure we do not
mHandler.removeMessages(userId); mAsyncHandler.removeMessages(userId);
mPermissionUpgradeNeeded.delete(userId); mPermissionUpgradeNeeded.delete(userId);
mVersions.delete(userId); mVersions.delete(userId);
@@ -5572,7 +5630,7 @@ public final class Settings implements Watchable, Snappable {
} }
public void deleteUserRuntimePermissionsFile(int userId) { public void deleteUserRuntimePermissionsFile(int userId) {
synchronized (mLock) { synchronized (mPersistenceLock) {
mPersistence.deleteForUser(UserHandle.of(userId)); mPersistence.deleteForUser(UserHandle.of(userId));
} }
} }
@@ -5581,16 +5639,17 @@ public final class Settings implements Watchable, Snappable {
@NonNull WatchedArrayMap<String, PackageSetting> packageSettings, @NonNull WatchedArrayMap<String, PackageSetting> packageSettings,
@NonNull WatchedArrayMap<String, SharedUserSetting> sharedUsers, @NonNull WatchedArrayMap<String, SharedUserSetting> sharedUsers,
@NonNull File userRuntimePermissionsFile) { @NonNull File userRuntimePermissionsFile) {
synchronized (mLock) { final RuntimePermissionsState runtimePermissions;
RuntimePermissionsState runtimePermissions = mPersistence.readForUser(UserHandle.of( synchronized (mPersistenceLock) {
userId)); runtimePermissions = mPersistence.readForUser(UserHandle.of(userId));
}
if (runtimePermissions == null) { if (runtimePermissions == null) {
readLegacyStateForUserSync(userId, userRuntimePermissionsFile, packageSettings, readLegacyStateForUserSync(userId, userRuntimePermissionsFile, packageSettings,
sharedUsers); sharedUsers);
writeStateForUserAsync(userId); writeStateForUserAsync(userId);
return; return;
} }
synchronized (mLock) {
// If the runtime permissions file exists but the version is not set this is // If the runtime permissions file exists but the version is not set this is
// an upgrade from P->Q. Hence mark it with the special UPGRADE_VERSION. // an upgrade from P->Q. Hence mark it with the special UPGRADE_VERSION.
int version = runtimePermissions.getVersion(); int version = runtimePermissions.getVersion();

View File

@@ -1516,7 +1516,7 @@ public class PackageManagerSettingsTests {
private Settings makeSettings() { private Settings makeSettings() {
return new Settings(InstrumentationRegistry.getContext().getFilesDir(), return new Settings(InstrumentationRegistry.getContext().getFilesDir(),
mRuntimePermissionsPersistence, mPermissionDataProvider, mRuntimePermissionsPersistence, mPermissionDataProvider,
mDomainVerificationManager, new PackageManagerTracedLock()); mDomainVerificationManager, null, new PackageManagerTracedLock());
} }
private void verifyKeySetMetaData(Settings settings) private void verifyKeySetMetaData(Settings settings)