Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 659dcc86 authored by Lee Shombert's avatar Lee Shombert
Browse files

Modify PropertyInvalidatedCache

Bug: 186778818

This makes two changes to PropertyInvalidatedCache.

1. disableLocal() now disables all current and future caches that use
   the same name (not the necessarily the same property) , in the
   local process.  Previously, disableLocal() only disabled a single
   cache instance, but the intent was always to disable all instances
   of the same cache in the process.

   disableInstance() is available with the old behavior.

2. A bypass() method has been added.  If bypass() returns true,
   query() will skip the cache and go straight to the binder call as
   though the cache had been disabled.  The default implementation
   always returns false.  Caches can override the implementation to
   avoid caching selected queries.

These changes specifically address the problem of caches that are
created dynamically and which should be disabled in the local
process.

A unit-test is added for PropertyInvalidatedCache.  This is not a
complete test because test processes are not allowed to set system
properties.  The unit-test will be improved in the future by modifying
PropertyInvalidatedCache to use an invalidation mechanism other than
system properties.

Manual test: boot a phone with a baseline build and with the build
under test and verified that the list of disabled caches is the same.
Use 'dumpsys cacheinfo' to get the cache status.

Test: atest
 * FrameworksServicesTests:UserManagerServiceCreateProfileTest

Change-Id: I9f604b872911290e4e3d8a58b3e28e328b2000a9
parent 3c0f63ab
Loading
Loading
Loading
Loading
+57 −5
Original line number Diff line number Diff line
@@ -34,6 +34,7 @@ import java.io.IOException;
import java.io.PrintWriter;
import java.util.ArrayList;
import java.util.HashMap;
import java.util.HashSet;
import java.util.LinkedHashMap;
import java.util.Map;
import java.util.Objects;
@@ -259,13 +260,20 @@ public abstract class PropertyInvalidatedCache<Query, Result> {
    @GuardedBy("sCorkLock")
    private static final HashMap<String, Integer> sCorks = new HashMap<>();

    /**
     * A map of cache keys that have been disabled in the local process.  When a key is
     * disabled locally, existing caches are disabled and the key is saved in this map.
     * Future cache instances that use the same key will be disabled in their constructor.
     */
    @GuardedBy("sCorkLock")
    private static final HashSet<String> sDisabledKeys = new HashSet<>();

    /**
     * Weakly references all cache objects in the current process, allowing us to iterate over
     * them all for purposes like issuing debug dumps and reacting to memory pressure.
     */
    @GuardedBy("sCorkLock")
    private static final WeakHashMap<PropertyInvalidatedCache, Void> sCaches =
            new WeakHashMap<>();
    private static final WeakHashMap<PropertyInvalidatedCache, Void> sCaches = new WeakHashMap<>();

    private final Object mLock = new Object();

@@ -348,6 +356,9 @@ public abstract class PropertyInvalidatedCache<Query, Result> {
            };
        synchronized (sCorkLock) {
            sCaches.put(this, null);
            if (sDisabledKeys.contains(mCacheName)) {
                disableInstance();
            }
        }
    }

@@ -371,6 +382,14 @@ public abstract class PropertyInvalidatedCache<Query, Result> {
     */
    protected abstract Result recompute(Query query);

    /**
     * Return true if the query should bypass the cache.  The default behavior is to
     * always use the cache but the method can be overridden for a specific class.
     */
    protected boolean bypass(Query query) {
        return false;
    }

    /**
     * Determines if a pair of responses are considered equal. Used to determine whether
     * a cache is inadvertently returning stale results when VERIFY is set to true.
@@ -414,13 +433,37 @@ public abstract class PropertyInvalidatedCache<Query, Result> {
    /**
     * Disable the use of this cache in this process.
     */
    public final void disableLocal() {
    public final void disableInstance() {
        synchronized (mLock) {
            mDisabled = true;
            clear();
        }
    }

    /**
     * Disable the local use of all caches with the same name.  All currently registered caches
     * using the key will be disabled now, and all future cache instances that use the key will be
     * disabled in their constructor.
     */
    public static final void disableLocal(@NonNull String name) {
        synchronized (sCorkLock) {
            sDisabledKeys.add(name);
            for (PropertyInvalidatedCache cache : sCaches.keySet()) {
                if (name.equals(cache.mCacheName)) {
                    cache.disableInstance();
                }
            }
        }
    }

    /**
     * Disable this cache in the current process, and all other caches that use the same
     * property.
     */
    public final void disableLocal() {
        disableLocal(mCacheName);
    }

    /**
     * Return whether the cache is disabled in this process.
     */
@@ -435,8 +478,8 @@ public abstract class PropertyInvalidatedCache<Query, Result> {
        // Let access to mDisabled race: it's atomic anyway.
        long currentNonce = (!isDisabledLocal()) ? getCurrentNonce() : NONCE_DISABLED;
        for (;;) {
            if (currentNonce == NONCE_DISABLED || currentNonce == NONCE_UNSET ||
                currentNonce == NONCE_CORKED) {
            if (currentNonce == NONCE_DISABLED || currentNonce == NONCE_UNSET
                    || currentNonce == NONCE_CORKED || bypass(query)) {
                if (!mDisabled) {
                    // Do not bother collecting statistics if the cache is
                    // locally disabled.
@@ -874,6 +917,15 @@ public abstract class PropertyInvalidatedCache<Query, Result> {
        sEnabled = false;
    }

    /**
     * Report the disabled status of this cache instance.  The return value does not
     * reflect status of the property key.
     */
    @VisibleForTesting(visibility = VisibleForTesting.Visibility.PACKAGE)
    public boolean getDisabledState() {
        return isDisabledLocal();
    }

    /**
     * Returns a list of caches alive at the current time.
     */
+117 −0
Original line number Diff line number Diff line
/*
 * 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 android.app;

import static org.junit.Assert.assertEquals;

import androidx.test.filters.SmallTest;

import org.junit.Test;

/**
 * Test for verifying the behavior of {@link PropertyInvalidatedCache}.  This test does
 * not use any actual binder calls - it is entirely self-contained.
 * <p>
 * Build/Install/Run:
 *  atest FrameworksCoreTests:PropertyInvalidatedCacheTests
 */
@SmallTest
public class PropertyInvalidatedCacheTests {

    static final String CACHE_PROPERTY = "cache_key.cache_test_a";

    // This class is a proxy for binder calls.  It contains a counter that increments
    // every time the class is queried.
    private static class ServerProxy {
        // The number of times this class was queried.
        private int mCount = 0;

        // A single query.  The key behavior is that the query count is incremented.
        boolean query(int x) {
            mCount++;
            return value(x);
        }

        // Return the expected value of an input, without incrementing the query count.
        boolean value(int x) {
            return x % 3 == 0;
        }

        // Verify the count.
        void verify(int x) {
            assertEquals(x, mCount);
        }
    }

    @Test
    public void testDisableCache1() {

        // A stand-in for the binder.  The test verifies that calls are passed through to
        // this class properly.
        ServerProxy tester = new ServerProxy();

        // Three caches, all using the same system property but one uses a different name.
        PropertyInvalidatedCache<Integer, Boolean> cache1 =
                new PropertyInvalidatedCache<>(4, CACHE_PROPERTY) {
                    @Override
                    protected Boolean recompute(Integer x) {
                        return tester.query(x);
                    }
                };
        PropertyInvalidatedCache<Integer, Boolean> cache2 =
                new PropertyInvalidatedCache<>(4, CACHE_PROPERTY) {
                    @Override
                    protected Boolean recompute(Integer x) {
                        return tester.query(x);
                    }
                };
        PropertyInvalidatedCache<Integer, Boolean> cache3 =
                new PropertyInvalidatedCache<>(4, CACHE_PROPERTY, "cache3") {
                    @Override
                    protected Boolean recompute(Integer x) {
                        return tester.query(x);
                    }
                };

        // Caches are enabled upon creation.
        assertEquals(false, cache1.getDisabledState());
        assertEquals(false, cache2.getDisabledState());
        assertEquals(false, cache3.getDisabledState());

        // Disable the cache1 instance.  Only cache1 is disabled
        cache1.disableInstance();
        assertEquals(true, cache1.getDisabledState());
        assertEquals(false, cache2.getDisabledState());
        assertEquals(false, cache3.getDisabledState());

        // Disable cache1.  This will disable cache1 and cache2 because they share the
        // same name.  cache3 has a different name and will not be disabled.
        cache1.disableLocal();
        assertEquals(true, cache1.getDisabledState());
        assertEquals(true, cache2.getDisabledState());
        assertEquals(false, cache3.getDisabledState());

        // Create a new cache1.  Verify that the new instance is disabled.
        cache1 = new PropertyInvalidatedCache<>(4, CACHE_PROPERTY) {
                    @Override
                    protected Boolean recompute(Integer x) {
                        return tester.query(x);
                    }
                };
        assertEquals(true, cache1.getDisabledState());
    }
}