ConcurrentLfu.Clear() polluted by removed items (#401)

* fix * cleanup ---------
bitfaster · Oct 2, 2023 · 822e160 · 822e160
1 parent 1e7741f
commit 822e160
Show file tree

Hide file tree

Showing 2 changed files with 30 additions and 6 deletions.
diff --git a/BitFaster.Caching.UnitTests/Lfu/ConcurrentLfuTests.cs b/BitFaster.Caching.UnitTests/Lfu/ConcurrentLfuTests.cs
@@ -676,18 +676,37 @@ public void WhenItemDoesNotExistTryUpdateIsFalse()
 
         [Fact]
         public void WhenClearedCacheIsEmpty()
-        {
+        {
             cache.GetOrAdd(1, k => k);
-            cache.GetOrAdd(2, k => k);
-            cache.DoMaintenance();
-
+            cache.GetOrAdd(2, k => k);
+
             cache.Clear();
-            cache.DoMaintenance();
 
             cache.Count.Should().Be(0);
             cache.TryGet(1, out var _).Should().BeFalse();
         }
 
+        [Fact]
+        public void WhenBackgroundMaintenanceRepeatedReadThenClearResultsInEmpty()
+        {
+            cache = new ConcurrentLfu<int, int>(1, 40, new BackgroundThreadScheduler(), EqualityComparer<int>.Default);
+
+            var overflow = 0;
+            for (var a = 0; a < 200; a++)
+            {
+                for (var i = 0; i < 40; i++)
+                {
+                    cache.GetOrAdd(i, k => k);
+                }
+
+                cache.Clear();
+                overflow += cache.Count;
+            }
+
+            // there should be no iteration of the loop where count != 0
+            overflow.Should().Be(0);
+        }
+
         [Fact]
         public void TrimRemovesNItems()
         {

diff --git a/BitFaster.Caching/Lfu/ConcurrentLfu.cs b/BitFaster.Caching/Lfu/ConcurrentLfu.cs
@@ -426,7 +426,12 @@ private static void TakeCandidatesInLruOrder(LfuNodeList<K, V> lru, List<LfuNode
 
             while (candidates.Count < itemCount && curr != null)
             {
-                candidates.Add(curr);
+                // LRUs can contain items that are already removed, skip those 
+                if (!curr.WasRemoved)
+                { 
+                    candidates.Add(curr); 
+                }
+
                 curr = curr.Next;
             }
         }