Concurrency problems caused by using guava cache's get() and getIfPresent() methods under multithreading

[ZME7777777]

Guava Version

30.1

Description

Concurrency problems caused by using guava cache's get() and getIfPresent() methods under multithreading

Example

`import com.google.common.cache.Cache;
import com.google.common.cache.CacheBuilder;

import java.util.HashMap;
import java.util.Map;
import java.util.concurrent.CountDownLatch;
import java.util.concurrent.ExecutionException;

public class Main {

    final Cache<String, Map<Integer, Integer>> cache = CacheBuilder.newBuilder().initialCapacity(100).build();

    public static void main(String[] args) throws InterruptedException {
        CountDownLatch startLatch = new CountDownLatch(1);
        int count = 10;
        Main main2 = new Main();
        CountDownLatch doneLatch = new CountDownLatch(count);
        for (int i = 0; i < count; i++) {
            Thread thread = new Thread(() -> {
                try {
                    startLatch.await();
                    main2.test();
                } catch (InterruptedException | ExecutionException e) {
                    throw new RuntimeException(e);
                } finally {
                    doneLatch.countDown();
                }
            });
            thread.setName("T" + (i + 1));
            thread.start();
        }
        startLatch.countDown();
        doneLatch.await();
    }

    public void test() throws ExecutionException {
        Map<Integer, Integer> target = cache.get("test", HashMap::new);
        int i = target.size();
        for (; i < 5; i++) {
            target.put(i, i);
        }
        System.out.println(target + ":" + cache.getIfPresent("test"));
    }
    
}`

result:
{0=0, 4=4, 1=1, 2=2, 3=3}:null

{0=0, 4=4, 1=1, 2=2, 3=3}:null

{0=0, 4=4, 1=1, 2=2, 3=3}:null

{0=0, 4=4, 1=1, 2=2, 3=3}:null

{0=0, 4=4, 1=1, 2=2, 3=3}:null

{0=0, 4=4, 1=1, 2=2, 3=3}:null

{0=0, 4=4, 1=1, 2=2, 3=3}:null

{0=0, 4=4, 1=1, 2=2, 3=3}:null

{0=0, 4=4, 1=1, 2=2, 3=3}:null

{0=0, 4=4, 1=1, 2=2, 3=3}:{0=0, 4=4, 1=1, 2=2, 3=3}

Expected Behavior

import com.github.benmanes.caffeine.cache.Cache;
import com.github.benmanes.caffeine.cache.Caffeine;

import java.util.Collections;
import java.util.HashMap;
import java.util.Map;
import java.util.concurrent.CountDownLatch;
import java.util.concurrent.ExecutionException;
import java.util.function.Function;

public class Main1 {

final Cache<String, Map<Integer, Integer>> cache = Caffeine.newBuilder().build();

public static void main(String[] args) throws InterruptedException {
    CountDownLatch startLatch = new CountDownLatch(1);
    int count = 10;
    Main1 main2 = new Main1();
    CountDownLatch doneLatch = new CountDownLatch(count);
    for (int i = 0; i < count; i++) {
        Thread thread = new Thread(() -> {
            try {
                startLatch.await();
                main2.test();
            } catch (InterruptedException | ExecutionException e) {
                throw new RuntimeException(e);
            } finally {
                doneLatch.countDown();
            }
        });
        thread.setName("T" + (i + 1));
        thread.start();
    }
    startLatch.countDown();
    doneLatch.await();
}

public void test() throws ExecutionException {
    Map<Integer, Integer> target = cache.get("test", s -> new HashMap<>());
    int i = target.size();
    for (; i < 5; i++) {
        target.put(i, i);
    }
    System.out.println(target + ":" + cache.getIfPresent("test"));
}

}

result:
{0=0, 1=1, 2=2, 3=3, 4=4}:{0=0, 1=1, 2=2, 3=3, 4=4}
{0=0, 1=1, 2=2, 3=3, 4=4}:{0=0, 1=1, 2=2, 3=3, 4=4}
{0=0, 1=1, 2=2, 3=3, 4=4}:{0=0, 1=1, 2=2, 3=3, 4=4}
{0=0, 1=1, 2=2, 3=3, 4=4}:{0=0, 1=1, 2=2, 3=3, 4=4}
{0=0, 1=1, 2=2, 3=3, 4=4}:{0=0, 1=1, 2=2, 3=3, 4=4}
{0=0, 1=1, 2=2, 3=3, 4=4}:{0=0, 1=1, 2=2, 3=3, 4=4}
{0=0, 1=1, 2=2, 3=3, 4=4}:{0=0, 1=1, 2=2, 3=3, 4=4}
{0=0, 1=1, 2=2, 3=3, 4=4}:{0=0, 1=1, 2=2, 3=3, 4=4}
{0=0, 1=1, 2=2, 3=3, 4=4}:{0=0, 1=1, 2=2, 3=3, 4=4}
{0=0, 1=1, 2=2, 3=3, 4=4}:{0=0, 1=1, 2=2, 3=3, 4=4}

Actual Behavior

getIfPresent cannot be immediately visible

Packages

com.google.common.cache

Platforms

Java 8

Checklist

• I agree to follow the code of conduct.

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

[ZME7777777]

The result of calling getIfPresent is null because the count in Segment is 0

[ZME7777777]

I wrote the wrong version, I used 31.0.1-jre

[ZME7777777]

Still exists in the latest version, Is it a mistake in my use?

[ben-manes]

This was also reported in #2131 (comment)

Guava’s cache does not offer linearizability guarantees, per #1881 (comment). This means such behavior is allowed because there are no promises otherwise. As a cache of transient data, a miss should not negatively impact usages. A stricter promise is outside of the original scope of the class.

As you noted, caffeine does attempt to provide a stronger guarantee. This is not absolute and it documents some known gaps (primarily bulk operations). Others have preferred layering onto guava the requirements they need.