guava cache asMap() can raise AssertError or StackOverflowError when there is massive cache loading failed with exception

[stxmic]

Guava Version

33.2.1 and old versions use asMap() and ComputingValueReference

Description

the bug is specifically caused since a failure loading attempt results in ComputingValueReference keeps reference to previous loading attempt, repeating this behavior builds reference chain. When the specific entry is expired and to be evicted via preWriteCleanup(), the underlying copyEntry() call would evaluate the reference chain, if there is number of references in the chain exceeds a threshold, evaluating it would lead to the StackOverflowError, if there is other cache keys share the same bucket in the segment, and ahead of this key in the ReferenceEntry linked list, the accessQueue & the linked list will be out of sync, that access queue points to a new entry while the reference linked list failed to be updated as the result of the StackOverflow error. Future access to the segment would all fail with "AssertError" as out of sync.

Suggestion workaround: move away from asMap(), considering deprecating the support as the value reference chain is defected in design.

Example

`public void reproduceStackOverFlow() throws InterruptedException {
        long expireAfterAccess = 10000L;
        Cache<Integer, String> cache = CacheBuilder.newBuilder()
                .expireAfterAccess(Duration.ofMillis(expireAfterAccess))
                .build();

        int key = 100;
        int maxChainLength = 65536; //tested with JVM 2MB stack size, this number may go higher with larger stack size settings
        for (int i = 0; i < maxChainLength; i++) {
            try {
                cache.asMap().computeIfAbsent(key, (k) -> {
                    throw new RuntimeException(); //simulate loading error, can happen with no value for a specific key or a service call fail
                });
            } catch (Exception e) {
                //ok, eat it
            }
        }

        //let the entry expire
        Thread.sleep(expireAfterAccess);

        try {
            cache.asMap().computeIfAbsent(key, (k) -> "foobar");
            Assertions.fail();
        } catch (Error error) {
            error.printStackTrace();
            if (!(error instanceof StackOverflowError)) {
                Assertions.fail();
            }
        }
    }`

Expected Behavior

cache can be managed

Actual Behavior

failed with StackOverflowError

Packages

com.google.common.cache

Platforms

No response

Checklist

• I agree to follow the code of conduct.

• I can reproduce the bug with the latest version of Guava available.

[ben-manes]

Sounds like maybe this line needs to be wrapped in a try-catch that calls removeLoadingValue if an exception is thrown. It currently retains the failed. I saw something like this in #5438. I would generally avoid the compute methods in Guava's cache since it was not designed for that and made their addition fairly problematic. You might consider using Caffeine instead.

guava/guava/src/com/google/common/cache/LocalCache.java

Line 2267 in 176cf74

newValue = computingValueReference.compute(key, function);