`cache.Clear()` doesn't seem to be clearing entire cache

[Kritner]

Given this test on v2.3.2, running .net 7 on an m1 mac:

    private record FakeCacheKey(int SomeProperty);
    private record FakeCacheValue
    {
        public int SomeProperty { get; set; }
    };

    [Theory]
    [InlineData(true)]
    [InlineData(false)]
    public void WhenClearingCache_ShouldActuallyClearCache(bool shouldClearTwice)
    {
        var cache = new ConcurrentTLru<FakeCacheKey, FakeCacheValue>(3, TimeSpan.FromMinutes(10));
        var keyOne = new FakeCacheKey(1);
        var keyTwo = new FakeCacheKey(2);
        var cacheValue = new FakeCacheValue();
        
        cache.AddOrUpdate(keyOne, cacheValue);
        cache.AddOrUpdate(keyTwo, cacheValue);
        
        cache.TryGet(keyOne, out var retrievedKeyValueOne);
        retrievedKeyValueOne.Should().BeSameAs(cacheValue);
        cache.TryGet(keyTwo, out var retrievedKeyTwoValue);
        retrievedKeyTwoValue.Should().BeSameAs(cacheValue);
        
        
        cache.Clear();
        if (shouldClearTwice)
            cache.Clear();

        cache.TryGet(keyOne, out retrievedKeyValueOne);
        retrievedKeyValueOne.Should().NotBeSameAs(cacheValue);
        cache.TryGet(keyOne, out retrievedKeyTwoValue);
        retrievedKeyTwoValue.Should().NotBeSameAs(cacheValue);
    }

The test is consistently failing except when the cache.Clear() is run a second time. Is this a race condition of some sort? Am i just misunderstanding what Clear() is supposed to be doing?

Stepping through the "double clear" test while watching the internal cache state:

[bitfaster]

That does look weird in the debugger - it should always have zero elements on line 187, I will take a look later today.

One thing that jumps out at me is that in your test at the end you are calling:

        cache.TryGet(keyOne, out retrievedKeyValueOne);
        retrievedKeyValueOne.Should().NotBeSameAs(cacheValue);
        cache.TryGet(keyOne, out retrievedKeyTwoValue);
        retrievedKeyTwoValue.Should().NotBeSameAs(cacheValue);

but not checking the return value of TryGet, which I would expect to be false indicating that the value did not exist. When TryGet returns false, the value in the out variable is not defined. Since you re-use the variable from the prior cache read it is possible that it simply contains the prior value that you already looked up.

So, I am curious if your test passes if it is like this:

        bool r = cache.TryGet(keyOne, out retrievedKeyValueOne);
        r.Should().BeFalse();

or this:

        cache.TryGet(keyOne, out var totallyNewVariable);
        totallyNewVariable.Should().NotBeSameAs(cacheValue);

Since version 2.3.1, the library is compiled with SkipLocalsInit which could potentially result in your out variable not being cleared, when it may have been on a prior version.

[Kritner]

good call out on the issues in my test - tried updating with your suggestions but still seeing similar behavior:

    [Theory]
    [InlineData(true)]
    [InlineData(false)]
    public void WhenClearingCache_ShouldActuallyClearCache(bool shouldClearTwice)
    {
        var cache = new ConcurrentTLru<FakeCacheKey, FakeCacheValue>(3, TimeSpan.FromMinutes(10));
        var keyOne = new FakeCacheKey(1);
        var keyTwo = new FakeCacheKey(2);
        var cacheValue = new FakeCacheValue();
        
        cache.AddOrUpdate(keyOne, cacheValue);
        cache.AddOrUpdate(keyTwo, cacheValue);
        
        cache.TryGet(keyOne, out var retrievedKeyValueOne);
        retrievedKeyValueOne.Should().BeSameAs(cacheValue);
        cache.TryGet(keyTwo, out var retrievedKeyTwoValue);
        retrievedKeyTwoValue.Should().BeSameAs(cacheValue);
                
        cache.Clear();
        if (shouldClearTwice)
            cache.Clear();

        cache.TryGet(keyOne, out var a);
        a.Should().NotBeSameAs(cacheValue);
        cache.TryGet(keyOne, out var b);
        b.Should().NotBeSameAs(cacheValue);
    }
    
    [Theory]
    [InlineData(true)]
    [InlineData(false)]
    public void WhenClearingCache_ShouldActuallyClearCache_2(bool shouldClearTwice)
    {
        var cache = new ConcurrentTLru<FakeCacheKey, FakeCacheValue>(3, TimeSpan.FromMinutes(10));
        var keyOne = new FakeCacheKey(1);
        var keyTwo = new FakeCacheKey(2);
        var cacheValue = new FakeCacheValue();
        
        cache.AddOrUpdate(keyOne, cacheValue);
        cache.AddOrUpdate(keyTwo, cacheValue);
        
        cache.TryGet(keyOne, out var retrievedKeyValueOne);
        retrievedKeyValueOne.Should().BeSameAs(cacheValue);
        cache.TryGet(keyTwo, out var retrievedKeyTwoValue);
        retrievedKeyTwoValue.Should().BeSameAs(cacheValue);
        
        cache.Clear();
        if (shouldClearTwice)
            cache.Clear();

        var a = cache.TryGet(keyOne, out _);
        a.Should().BeFalse();
        var b = cache.TryGet(keyOne, out _);
        a.Should().BeFalse();
    }

[bitfaster]

Thanks for trying that out. I was also able to repro - this is a bug.

I also verified that the out values are initialized to default explicitly when TryGet returns false, this is not skipped by SkipLocalsInit.

It's broken also for LRU, it repros with this case as well:

    [Fact]
    public void WhenClearingCache_ShouldActuallyClearCache()
    {
        var cache = new ConcurrentLru<int, int>(3);

        cache.AddOrUpdate(1, 1);
        cache.AddOrUpdate(2, 2);
        
        cache.TryGet(1, out _);
        cache.TryGet(2, out _);

        cache.Clear();

        cache.Count.Should().Be(0);
   }

Internally, the first value has propagated to the warm queue and is flagged as accessed. During Clear(), the queues are cycled, and in this case the warm value is cycled back into warm because it was touched. After the clear logic finishes running, this item has been pushed to the cold queue but not yet evicted.

[bitfaster]

Thanks for reporting this - I pushed a fix and verified that your tests now pass. Fix will be in the next release.

[Kritner]

Awesome, thanks!

[bitfaster]

Btw I found another bug in Clear - if any items have been deleted from the cache either by TryRemove or time-based expiry, items can be left in the cache after calling Clear. I have fixed it in #488, the fix will be in the next release.