diff --git a/services/core/java/com/android/server/pm/InstantAppRegistry.java b/services/core/java/com/android/server/pm/InstantAppRegistry.java index 7bf704299373e..bf323e7d4ff2e 100644 --- a/services/core/java/com/android/server/pm/InstantAppRegistry.java +++ b/services/core/java/com/android/server/pm/InstantAppRegistry.java @@ -57,6 +57,7 @@ import com.android.server.pm.parsing.PackageInfoUtils; import com.android.server.pm.parsing.pkg.AndroidPackage; import com.android.server.pm.permission.PermissionManagerServiceInternal; import com.android.server.utils.Snappable; +import com.android.server.utils.SnapshotCache; import com.android.server.utils.Watchable; import com.android.server.utils.WatchableImpl; import com.android.server.utils.Watched; @@ -146,7 +147,7 @@ class InstantAppRegistry implements Watchable, Snappable { /** * The cached snapshot */ - private volatile InstantAppRegistry mSnapshot = null; + private final SnapshotCache mSnapshot; /** * Watchable machinery @@ -162,7 +163,6 @@ class InstantAppRegistry implements Watchable, Snappable { return mWatchable.isRegisteredObserver(observer); } public void dispatchChange(@Nullable Watchable what) { - mSnapshot = null; mWatchable.dispatchChange(what); } /** @@ -180,6 +180,16 @@ class InstantAppRegistry implements Watchable, Snappable { } }; + private SnapshotCache makeCache() { + return new SnapshotCache(this, this) { + @Override + public InstantAppRegistry createSnapshot() { + InstantAppRegistry s = new InstantAppRegistry(mSource); + s.mWatchable.seal(); + return s; + }}; + } + public InstantAppRegistry(PackageManagerService service, PermissionManagerServiceInternal permissionManager) { mService = service; @@ -194,6 +204,8 @@ class InstantAppRegistry implements Watchable, Snappable { mInstantGrants.registerObserver(mObserver); mInstalledInstantAppUids.registerObserver(mObserver); Watchable.verifyWatchedAttributes(this, mObserver); + + mSnapshot = makeCache(); } /** @@ -211,20 +223,15 @@ class InstantAppRegistry implements Watchable, Snappable { mInstalledInstantAppUids = new WatchedSparseArray( r.mInstalledInstantAppUids); - // Do not register any observers. This is a clone + // Do not register any observers. This is a snapshot. + mSnapshot = null; } /** * Return a snapshot: the value is the cached snapshot if available. */ public InstantAppRegistry snapshot() { - InstantAppRegistry s = mSnapshot; - if (s == null) { - s = new InstantAppRegistry(this); - s.mWatchable.seal(); - mSnapshot = s; - } - return s; + return mSnapshot.snapshot(); } @GuardedBy("mService.mLock") diff --git a/services/core/java/com/android/server/pm/Settings.java b/services/core/java/com/android/server/pm/Settings.java index 4823c29b96f95..24f3930706233 100644 --- a/services/core/java/com/android/server/pm/Settings.java +++ b/services/core/java/com/android/server/pm/Settings.java @@ -117,6 +117,7 @@ import com.android.server.pm.verify.domain.DomainVerificationLegacySettings; import com.android.server.pm.verify.domain.DomainVerificationManagerInternal; import com.android.server.pm.verify.domain.DomainVerificationPersistence; import com.android.server.utils.Snappable; +import com.android.server.utils.SnapshotCache; import com.android.server.utils.TimingsTraceAndSlog; import com.android.server.utils.Watchable; import com.android.server.utils.WatchableImpl; @@ -164,11 +165,6 @@ import java.util.UUID; public final class Settings implements Watchable, Snappable { private static final String TAG = "PackageSettings"; - /** - * Cached snapshot - */ - private volatile Settings mSnapshot = null; - /** * Watchable machinery */ @@ -212,7 +208,6 @@ public final class Settings implements Watchable, Snappable { * @param what The {@link Watchable} that generated the event. */ public void dispatchChange(@Nullable Watchable what) { - mSnapshot = null; mWatchable.dispatchChange(what); } /** @@ -523,6 +518,19 @@ public final class Settings implements Watchable, Snappable { } }; + private final SnapshotCache mSnapshot; + + // Create a snapshot cache + private SnapshotCache makeCache() { + return new SnapshotCache(this, this) { + @Override + public Settings createSnapshot() { + Settings s = new Settings(mSource); + s.mWatchable.seal(); + return s; + }}; + } + @VisibleForTesting(visibility = VisibleForTesting.Visibility.PRIVATE) public Settings(Map pkgSettings) { mLock = new PackageManagerTracedLock(); @@ -557,6 +565,8 @@ public final class Settings implements Watchable, Snappable { mDefaultBrowserApp.registerObserver(mObserver); Watchable.verifyWatchedAttributes(this, mObserver); + + mSnapshot = makeCache(); } Settings(File dataDir, RuntimePermissionsPersistence runtimePermissionsPersistence, @@ -608,6 +618,8 @@ public final class Settings implements Watchable, Snappable { mDefaultBrowserApp.registerObserver(mObserver); Watchable.verifyWatchedAttributes(this, mObserver); + + mSnapshot = makeCache(); } /** @@ -661,22 +673,15 @@ public final class Settings implements Watchable, Snappable { mPermissions = r.mPermissions; mPermissionDataProvider = r.mPermissionDataProvider; - // Do not register any Watchables + // Do not register any Watchables and do not create a snapshot cache. + mSnapshot = null; } /** - * Return a snapshot. If the cached snapshot is null, build a new one. The logic in - * the function ensures that this function returns a valid snapshot even if a race - * condition causes the cached snapshot to be cleared asynchronously to this method. + * Return a snapshot. */ public Settings snapshot() { - Settings s = mSnapshot; - if (s == null) { - s = new Settings(this); - s.mWatchable.seal(); - mSnapshot = s; - } - return s; + return mSnapshot.snapshot(); } private void invalidatePackageCache() { diff --git a/services/core/java/com/android/server/utils/SnapshotCache.java b/services/core/java/com/android/server/utils/SnapshotCache.java new file mode 100644 index 0000000000000..f0fb8b25b402a --- /dev/null +++ b/services/core/java/com/android/server/utils/SnapshotCache.java @@ -0,0 +1,96 @@ +/* + * 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.utils; + +import android.annotation.NonNull; +import android.annotation.Nullable; + +/** + * A class that caches snapshots. Instances are instantiated on a {@link Watchable}; when the + * {@link Watchable} reports a change, the cache is cleared. The snapshot() method fetches the + * cache if it is valid, or rebuilds the cache if it has been cleared. + * + * The class is abstract; clients must implement the createSnapshot() method. + * + * @param The type returned by the snapshot() method. + */ +public abstract class SnapshotCache extends Watcher{ + + /** + * Global snapshot cache enable flag. Set to false for testing or debugging. + */ + private static final boolean ENABLED = true; + + // The source object from which snapshots are created. This may be null if createSnapshot() + // does not require it. + protected final T mSource; + + // The cached snapshot + private T mSnapshot = null; + + // True if the snapshot is sealed and may not be modified. + private boolean mSealed = false; + + /** + * Create a cache with a source object for rebuilding snapshots and a + * {@link Watchable} that notifies when the cache is invalid. + * @param source Source data for rebuilding snapshots. + * @param watchable The object that notifies when the cache is invalid. + */ + public SnapshotCache(@Nullable T source, @NonNull Watchable watchable) { + mSource = source; + watchable.registerObserver(this); + } + + /** + * Notify the object that the source object has changed. If the local object is sealed then + * IllegalStateException is thrown. Otherwise, the cache is cleared. + */ + public void onChange(@Nullable Watchable what) { + if (mSealed) { + throw new IllegalStateException("attempt to change a sealed object"); + } + mSnapshot = null; + } + + /** + * Seal the cache. Attempts to modify the cache will generate an exception. + */ + public void seal() { + mSealed = true; + } + + /** + * Return a snapshot. This uses the cache if it is non-null. Otherwise it creates a + * new snapshot and saves it in the cache. + * @return A snapshot as returned by createSnapshot() and possibly cached. + */ + public T snapshot() { + T s = mSnapshot; + if (s == null || !ENABLED) { + s = createSnapshot(); + mSnapshot = s; + } + return s; + } + + /** + * Create a single, uncached snapshot. Clients must implement this per local rules. + * @return A snapshot + */ + public abstract T createSnapshot(); +} diff --git a/services/tests/servicestests/src/com/android/server/utils/WatcherTest.java b/services/tests/servicestests/src/com/android/server/utils/WatcherTest.java index 57e95d7145930..f255c67cf47d3 100644 --- a/services/tests/servicestests/src/com/android/server/utils/WatcherTest.java +++ b/services/tests/servicestests/src/com/android/server/utils/WatcherTest.java @@ -896,4 +896,25 @@ public class WatcherTest { leafD.tick(); tester.verify(3, "tick leafD"); } + + @Test + public void testSnapshotCache() { + final String name = "SnapshotCache"; + WatchableTester tester; + + Leaf leafA = new Leaf(); + SnapshotCache cache = new SnapshotCache<>(leafA, leafA) { + @Override + public Leaf createSnapshot() { + return mSource.snapshot(); + }}; + + Leaf s1 = cache.snapshot(); + assertTrue(s1 == cache.snapshot()); + leafA.tick(); + Leaf s2 = cache.snapshot(); + assertTrue(s1 != s2); + assertTrue(leafA.get() == s1.get() + 1); + assertTrue(leafA.get() == s2.get()); + } }