Fix race in LRU concurrent update and TryRemove(kvp) (#616)

Author: bitfaster

BitFaster.Caching.UnitTests/Lru/ConcurrentLruSoakTests.cs

@@ -237,6 +237,31 @@ await Threaded.Run(4, () =>
             cache.Metrics.Value.Evicted.Should().Be(0);
         }
 
+        [Fact]
+        public async Task WhenConcurrentUpdateAndRemoveKvp()
+        {
+            TaskCompletionSource<int> tcs = new TaskCompletionSource<int> ();
+
+            var removal = Task.Run(() =>
+            {
+                while (!tcs.Task.IsCompleted)
+                {
+                    lru.TryRemove(new KeyValuePair<int, string>(5, "x"));
+                }
+            });
+
+            for (var i = 0; i < 100_000; i++)
+            {
+                lru.AddOrUpdate(5, "a");
+                lru.TryGet(5, out _).Should().BeTrue("key 'a' should not be deleted");
+                lru.AddOrUpdate(5, "x");
+            }
+
+            tcs.SetResult(int.MaxValue);
+
+            await removal;
+        }
+
         [Theory]
         [Repeat(10)]
         public async Task WhenConcurrentGetAndClearCacheEndsInConsistentState(int iteration)

BitFaster.Caching/Lru/ConcurrentLruCore.cs

@@ -308,18 +308,21 @@ public bool TryRemove(KeyValuePair<K, V> item)
         {
             if (this.dictionary.TryGetValue(item.Key, out var existing))
             {
-                if (EqualityComparer<V>.Default.Equals(existing.Value, item.Value))
+                lock (existing)
                 {
-                    var kvp = new KeyValuePair<K, I>(item.Key, existing);
+                    if (EqualityComparer<V>.Default.Equals(existing.Value, item.Value))
+                    {
+                        var kvp = new KeyValuePair<K, I>(item.Key, existing);
 #if NET6_0_OR_GREATER
                     if (this.dictionary.TryRemove(kvp))
 #else
-                    // https://devblogs.microsoft.com/pfxteam/little-known-gems-atomic-conditional-removals-from-concurrentdictionary/
-                    if (((ICollection<KeyValuePair<K, I>>)this.dictionary).Remove(kvp))
+                        // https://devblogs.microsoft.com/pfxteam/little-known-gems-atomic-conditional-removals-from-concurrentdictionary/
+                        if (((ICollection<KeyValuePair<K, I>>)this.dictionary).Remove(kvp))
 #endif
-                    {
-                        OnRemove(item.Key, kvp.Value, ItemRemovedReason.Removed);
-                        return true;
+                        {
+                            OnRemove(item.Key, kvp.Value, ItemRemovedReason.Removed);
+                            return true;
+                        }
                     }
                 }