WithExpireAfterWrite unexpected behavior

[GitRowin]

The following Java code using Google Guava works as expected:

LoadingCache<String, AtomicInteger> cache = CacheBuilder.newBuilder()
        .expireAfterWrite(1, TimeUnit.SECONDS)
        .build(new CacheLoader<>() {
            @Override
            public AtomicInteger load(@NotNull String key) {
                return new AtomicInteger();
            }
        });
        
System.out.println(cache.get("count").incrementAndGet());
System.out.println(cache.get("count").incrementAndGet());
System.out.println(cache.get("count").incrementAndGet());
Thread.sleep(2000);
System.out.println(cache.get("count").incrementAndGet());

Output:

1
2
3
1

On the other hand, the following Go code using Mango Cache does not work as expected:

var loadingCache = cache.NewLoadingCache(func(k cache.Key) (cache.Value, error) {
	return new(Counter), nil
}, cache.WithExpireAfterWrite(time.Second))

value, _ := loadingCache.Get("count")
fmt.Println(value.(*Counter).Add())

value, _ = loadingCache.Get("count")
fmt.Println(value.(*Counter).Add())

value, _ = loadingCache.Get("count")
fmt.Println(value.(*Counter).Add())

time.Sleep(time.Second * 2)

value, _ = loadingCache.Get("count")
fmt.Println(value.(*Counter).Add())

time.Sleep(time.Millisecond)

value, _ = loadingCache.Get("count")
fmt.Println(value.(*Counter).Add())

Output:

1
2
3
4
1

As you can see, the fourth number being printed is 4 instead of 1. This probably happens because expireEntries is called in postReadCleanup, which is called after reading the value. Is this intended behavior? If so, is there a workaround?

[nqv]

@GitRowin due to performance concern, expireEntries is not called for every read request and it returns old/expired entry instead of block loading new value. If your application has low request rate, you can reduce this threshold

	// Number of cache access operations that will trigger clean up.
	drainThreshold = 64

[nqv]

Another way is changing https://github.com/goburrow/cache/blob/f6da914dd6e3546dffa8802919dbca80cd33abe3/local.go#L180

from refreshAsync(en) to refresh(en)

I can change that as default behavior instead, however it will block the whole cache.

[GitRowin]

Changing drainThreshold won't fix the problem because expireEntries is called in postReadCleanup. Instead of calling refreshAsync in Get, why don't you just call refresh? No need to call expireEntries. You could also make this toggleable through a setting or parameter. Get already blocks on initial load and it should be the same way on expiration in my opinion.

EDIT: You were 3 minutes faster than me :)

[nqv]

Yeah, that's a good point. I will change that later.

[nqv]

FYI, it's doing that way because sync.Map doesn't support closure to store value like in Java, so concurrent calls to Get("x") will end up multiple loading calls with same value, instead of just locking one segment in the cache and load once. I'll try to fix that too