diff --git a/services/core/java/com/android/server/pm/Computer.java b/services/core/java/com/android/server/pm/Computer.java index db48a1f630992..eb635500580a9 100644 --- a/services/core/java/com/android/server/pm/Computer.java +++ b/services/core/java/com/android/server/pm/Computer.java @@ -94,12 +94,13 @@ import java.util.Set; @VisibleForTesting(visibility = VisibleForTesting.Visibility.PRIVATE) public interface Computer extends PackageDataSnapshot { + int getVersion(); + /** * Administrative statistics: record that the snapshot has been used. Every call * to use() increments the usage counter. */ - default void use() { - } + Computer use(); /** * Fetch the snapshot usage counter. * @return The number of times this snapshot was used. diff --git a/services/core/java/com/android/server/pm/ComputerEngine.java b/services/core/java/com/android/server/pm/ComputerEngine.java index bf9f4fa8a2114..f3c41af05da37 100644 --- a/services/core/java/com/android/server/pm/ComputerEngine.java +++ b/services/core/java/com/android/server/pm/ComputerEngine.java @@ -367,6 +367,8 @@ public class ComputerEngine implements Computer { return (v1 > v2) ? -1 : ((v1 < v2) ? 1 : 0); }; + private final int mVersion; + // The administrative use counter. private int mUsed = 0; @@ -424,7 +426,8 @@ public class ComputerEngine implements Computer { return mLocalAndroidApplication; } - ComputerEngine(PackageManagerService.Snapshot args) { + ComputerEngine(PackageManagerService.Snapshot args, int version) { + mVersion = version; mSettings = new Settings(args.settings); mIsolatedOwners = args.isolatedOwners; mPackages = args.packages; @@ -464,11 +467,17 @@ public class ComputerEngine implements Computer { mService = args.service; } + @Override + public int getVersion() { + return mVersion; + } + /** * Record that the snapshot was used. */ - public final void use() { + public final Computer use() { mUsed++; + return this; } /** diff --git a/services/core/java/com/android/server/pm/ComputerLocked.java b/services/core/java/com/android/server/pm/ComputerLocked.java index af196d51331f1..37070dbe7131f 100644 --- a/services/core/java/com/android/server/pm/ComputerLocked.java +++ b/services/core/java/com/android/server/pm/ComputerLocked.java @@ -30,7 +30,7 @@ import com.android.internal.annotations.VisibleForTesting; public final class ComputerLocked extends ComputerEngine { ComputerLocked(PackageManagerService.Snapshot args) { - super(args); + super(args, -1); } protected ComponentName resolveComponentName() { diff --git a/services/core/java/com/android/server/pm/PackageManagerService.java b/services/core/java/com/android/server/pm/PackageManagerService.java index 2cef35fcf0f60..96f00414c494e 100644 --- a/services/core/java/com/android/server/pm/PackageManagerService.java +++ b/services/core/java/com/android/server/pm/PackageManagerService.java @@ -271,8 +271,8 @@ import java.util.Set; import java.util.concurrent.Executor; import java.util.concurrent.Future; import java.util.concurrent.TimeUnit; -import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.atomic.AtomicInteger; +import java.util.concurrent.atomic.AtomicReference; import java.util.function.Consumer; /** @@ -1038,22 +1038,15 @@ public class PackageManagerService implements PackageSender, TestUtilityService // times during the PackageManagerService constructor but it should not be modified thereafter. private ComputerLocked mLiveComputer; - // A lock-free cache for frequently called functions. - private volatile Computer mSnapshotComputer; + private static final AtomicReference sSnapshot = new AtomicReference<>(); - // If true, the snapshot is invalid (stale). The attribute is static since it may be - // set from outside classes. The attribute may be set to true anywhere, although it - // should only be set true while holding mLock. However, the attribute id guaranteed - // to be set false only while mLock and mSnapshotLock are both held. - private static final AtomicBoolean sSnapshotInvalid = new AtomicBoolean(true); - - static final ThreadLocal sThreadComputer = - ThreadLocal.withInitial(ThreadComputer::new); + // If this differs from Computer#getVersion, the snapshot is invalid (stale). + private static final AtomicInteger sSnapshotPendingVersion = new AtomicInteger(1); /** - * This lock is used to make reads from {@link #sSnapshotInvalid} and - * {@link #mSnapshotComputer} atomic inside {@code snapshotComputer()}. This lock is - * not meant to be used outside that method. This lock must be taken before + * This lock is used to make reads from {@link #sSnapshotPendingVersion} and + * {@link #sSnapshot} atomic inside {@code snapshotComputer()} when the versions mismatch. + * This lock is not meant to be used outside that method. This lock must be taken before * {@link #mLock} is taken. */ private final Object mSnapshotLock = new Object(); @@ -1077,48 +1070,53 @@ public class PackageManagerService implements PackageSender, TestUtilityService // yet invalidated the snapshot. Always give the thread the live computer. return mLiveComputer; } - synchronized (mSnapshotLock) { - // This synchronization block serializes access to the snapshot computer and - // to the code that samples mSnapshotInvalid. - Computer c = mSnapshotComputer; - if (sSnapshotInvalid.getAndSet(false) || (c == null)) { - // The snapshot is invalid if it is marked as invalid or if it is null. If it - // is null, then it is currently being rebuilt by rebuildSnapshot(). - synchronized (mLock) { - // Rebuild the snapshot if it is invalid. Note that the snapshot might be - // invalidated as it is rebuilt. However, the snapshot is still - // self-consistent (the lock is being held) and is current as of the time - // this function is entered. - rebuildSnapshot(); - // Guaranteed to be non-null. mSnapshotComputer is only be set to null - // temporarily in rebuildSnapshot(), which is guarded by mLock(). Since - // the mLock is held in this block and since rebuildSnapshot() is - // complete, the attribute can not now be null. - c = mSnapshotComputer; - } + var oldSnapshot = sSnapshot.get(); + var pendingVersion = sSnapshotPendingVersion.get(); + + if (oldSnapshot != null && oldSnapshot.getVersion() == pendingVersion) { + return oldSnapshot.use(); + } + + synchronized (mSnapshotLock) { + // Re-capture pending version in case a new invalidation occurred since last check + var rebuildSnapshot = sSnapshot.get(); + var rebuildVersion = sSnapshotPendingVersion.get(); + + // Check the versions again while the lock is held, in case the rebuild time caused + // multiple threads to wait on the snapshot lock. When the first thread finishes + // a rebuild, the snapshot is now valid and the other waiting threads can use it + // without kicking off their own rebuilds. + if (rebuildSnapshot != null && rebuildSnapshot.getVersion() == rebuildVersion) { + return rebuildSnapshot.use(); + } + + synchronized (mLock) { + // Fetch version one last time to ensure that the rebuilt snapshot matches + // the latest invalidation, which could have come in between entering the + // SnapshotLock and mLock sync blocks. + rebuildVersion = sSnapshotPendingVersion.get(); + + // Build the snapshot for this version + var newSnapshot = rebuildSnapshot(rebuildSnapshot, rebuildVersion); + sSnapshot.set(newSnapshot); + return newSnapshot.use(); } - c.use(); - return c; } } - /** - * Rebuild the cached computer. mSnapshotComputer is temporarily set to null to block other - * threads from using the invalid computer until it is rebuilt. - */ @GuardedBy({ "mLock", "mSnapshotLock"}) - private void rebuildSnapshot() { - final long now = SystemClock.currentTimeMicro(); - final int hits = mSnapshotComputer == null ? -1 : mSnapshotComputer.getUsed(); - mSnapshotComputer = null; - final Snapshot args = new Snapshot(Snapshot.SNAPPED); - mSnapshotComputer = new ComputerEngine(args); - final long done = SystemClock.currentTimeMicro(); + private Computer rebuildSnapshot(@Nullable Computer oldSnapshot, int newVersion) { + var now = SystemClock.currentTimeMicro(); + var hits = oldSnapshot == null ? -1 : oldSnapshot.getUsed(); + var args = new Snapshot(Snapshot.SNAPPED); + var newSnapshot = new ComputerEngine(args, newVersion); + var done = SystemClock.currentTimeMicro(); if (mSnapshotStatistics != null) { mSnapshotStatistics.rebuild(now, done, hits); } + return newSnapshot; } /** @@ -1138,7 +1136,7 @@ public class PackageManagerService implements PackageSender, TestUtilityService if (TRACE_SNAPSHOTS) { Log.i(TAG, "snapshot: onChange(" + what + ")"); } - sSnapshotInvalid.set(true); + sSnapshotPendingVersion.incrementAndGet(); } /** @@ -1665,7 +1663,6 @@ public class PackageManagerService implements PackageSender, TestUtilityService mRequiredSdkSandboxPackage = testParams.requiredSdkSandboxPackage; mLiveComputer = createLiveComputer(); - mSnapshotComputer = null; mSnapshotStatistics = null; mPackages.putAll(testParams.packages); @@ -1855,9 +1852,8 @@ public class PackageManagerService implements PackageSender, TestUtilityService // cached computer is the same as the live computer until the end of the // constructor, at which time the invalidation method updates it. mSnapshotStatistics = new SnapshotStatistics(); - sSnapshotInvalid.set(true); + sSnapshotPendingVersion.incrementAndGet(); mLiveComputer = createLiveComputer(); - mSnapshotComputer = null; registerObservers(true); } @@ -5388,10 +5384,8 @@ public class PackageManagerService implements PackageSender, TestUtilityService @Override public void setApplicationCategoryHint(String packageName, int categoryHint, String callerPackageName) { - final PackageStateMutator.InitialState initialState = recordInitialState(); - - final FunctionalUtils.ThrowingFunction - implementation = computer -> { + final FunctionalUtils.ThrowingBiFunction implementation = (initialState, computer) -> { if (computer.getInstantAppPackageName(Binder.getCallingUid()) != null) { throw new SecurityException( "Instant applications don't have access to this method"); @@ -5419,12 +5413,13 @@ public class PackageManagerService implements PackageSender, TestUtilityService } }; - PackageStateMutator.Result result = implementation.apply(snapshotComputer()); + PackageStateMutator.Result result = + implementation.apply(recordInitialState(), snapshotComputer()); if (result != null && result.isStateChanged() && !result.isSpecificPackageNull()) { // TODO: Specific return value of what state changed? // The installer on record might have changed, retry with lock synchronized (mPackageStateWriteLock) { - result = implementation.apply(snapshotComputer()); + result = implementation.apply(recordInitialState(), snapshotComputer()); } } @@ -7156,9 +7151,19 @@ public class PackageManagerService implements PackageSender, TestUtilityService public PackageStateMutator.Result commitPackageStateMutation( @Nullable PackageStateMutator.InitialState initialState, @NonNull String packageName, @NonNull Consumer consumer) { + PackageStateMutator.Result result = null; + if (Thread.holdsLock(mPackageStateWriteLock)) { + // If the thread is already holding the lock, this is likely a retry based on a prior + // failure, and re-calculating whether a state change occurred can be skipped. + result = PackageStateMutator.Result.SUCCESS; + } synchronized (mPackageStateWriteLock) { - final PackageStateMutator.Result result = mPackageStateMutator.generateResult( - initialState, mChangedPackagesTracker.getSequenceNumber()); + if (result == null) { + // If the thread wasn't previously holding, this is a first-try commit and so a + // state change may have happened. + result = mPackageStateMutator.generateResult( + initialState, mChangedPackagesTracker.getSequenceNumber()); + } if (result != PackageStateMutator.Result.SUCCESS) { return result; } diff --git a/services/core/java/com/android/server/pm/ThreadComputer.java b/services/core/java/com/android/server/pm/ThreadComputer.java deleted file mode 100644 index f603e635f75db..0000000000000 --- a/services/core/java/com/android/server/pm/ThreadComputer.java +++ /dev/null @@ -1,52 +0,0 @@ -/* - * Copyright (C) 2021 The Android Open Source Project - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package com.android.server.pm; - -/** - * This class records the Computer being used by a thread and the Computer's reference - * count. There is a thread-local copy of this class. - */ -public final class ThreadComputer implements AutoCloseable { - Computer mComputer = null; - int mRefCount = 0; - - void acquire(Computer c) { - if (mRefCount != 0 && mComputer != c) { - throw new RuntimeException("computer mismatch, count = " + mRefCount); - } - mComputer = c; - mRefCount++; - } - - void acquire() { - if (mRefCount == 0 || mComputer == null) { - throw new RuntimeException("computer acquire on empty ref count"); - } - mRefCount++; - } - - void release() { - if (--mRefCount == 0) { - mComputer = null; - } - } - - @Override - public void close() { - release(); - } -}