Fix unbound scheduling of trim work items

This condition should almost never be hit except if .Cache(limit) is ran in a loop, then it is possible that a state is achieved where the amount of scheduled trim operations will be equal to the amount of .Cache() calls, which can be a lot. To be honest, CacheManager, QuickList and CacheStaticHolder need to be re-organized into something that looks less offensive, doesn't use circular static dependencies and lends itself into easier testability but I CBA to do it now.
neon-sunset · Jul 2, 2022 · b18e403 · b18e403
1 parent 6cb6dcc
commit b18e403
Show file tree

Hide file tree

Showing 2 changed files with 32 additions and 4 deletions.
diff --git a/src/FastCache.Cached/CacheManager.cs b/src/FastCache.Cached/CacheManager.cs
@@ -51,8 +51,19 @@ public static class CacheManager
             ThrowHelpers.ArgumentOutOfRange(percentage, nameof(percentage));
         }
 
-        static void ExecuteTrim(uint trimCount)
+        if (CacheStaticHolder<K, V>.QuickList.InProgress)
         {
+            // Bail out early if the items are being removed via quick list.
+            return false;
+        }
+
+        static void ExecuteTrim(uint trimCount, bool takeLock)
+        {
+            if (takeLock && !CacheStaticHolder<K, V>.QuickList.TryLock())
+            {
+                return;
+            }
+
             var removedFromQuickList = CacheStaticHolder<K, V>.QuickList.Trim(trimCount);
             if (removedFromQuickList >= trimCount)
             {
@@ -69,19 +80,24 @@ static void ExecuteTrim(uint trimCount)
                 store.TryRemove(enumerator.Current.Key, out _);
                 removed++;
             }
+
+            if (takeLock)
+            {
+                CacheStaticHolder<K, V>.QuickList.Release();
+            }
         }
 
         var trimCount = Math.Max(1, (uint)(CacheStaticHolder<K, V>.Store.Count * (percentage / 100.0)));
         if (trimCount <= Constants.InlineTrimCountThreshold)
         {
-            ExecuteTrim(trimCount);
+            ExecuteTrim(trimCount, takeLock: false);
             return true;
         }
 
 #if NETSTANDARD2_1_OR_GREATER || NETCOREAPP3_1_OR_GREATER
-        ThreadPool.QueueUserWorkItem(ExecuteTrim, trimCount, preferLocal: false);
+        ThreadPool.QueueUserWorkItem(static count => ExecuteTrim(count, takeLock: true), trimCount, preferLocal: false);
 #elif NETSTANDARD2_0
-        ThreadPool.QueueUserWorkItem(static count => ExecuteTrim((uint)count), trimCount);
+        ThreadPool.QueueUserWorkItem(static count => ExecuteTrim((uint)count, takeLock: true), trimCount);
 #endif
         return false;
     }

diff --git a/src/FastCache.Cached/EvictionQuickList.cs b/src/FastCache.Cached/EvictionQuickList.cs
@@ -17,6 +17,8 @@ internal sealed class EvictionQuickList<K, V> where K : notnull
 
     public uint FreeSpace => (uint)_active.Length - AtomicCount;
 
+    public bool InProgress => _evictionLock.CurrentCount == 0;
+
     public EvictionQuickList()
     {
         _active = ArrayPool<(K, long)>.Shared.Rent(Constants.QuickListMinLength);
@@ -270,6 +272,16 @@ internal void PullFromCacheStore()
         }
     }
 
+    internal bool TryLock()
+    {
+        return _evictionLock.Wait(0);
+    }
+
+    internal void Release()
+    {
+        _evictionLock.Release();
+    }
+
     private static int CalculateResize(long totalCount)
     {
         return Math.Max(