diff --git a/BitFaster.Caching.UnitTests/Lru/ConcurrentLruSoakTests.cs b/BitFaster.Caching.UnitTests/Lru/ConcurrentLruSoakTests.cs index fc473381..ff90bba6 100644 --- a/BitFaster.Caching.UnitTests/Lru/ConcurrentLruSoakTests.cs +++ b/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 tcs = new TaskCompletionSource (); + + var removal = Task.Run(() => + { + while (!tcs.Task.IsCompleted) + { + lru.TryRemove(new KeyValuePair(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) diff --git a/BitFaster.Caching/Lru/ConcurrentLruCore.cs b/BitFaster.Caching/Lru/ConcurrentLruCore.cs index adbca475..937d6747 100644 --- a/BitFaster.Caching/Lru/ConcurrentLruCore.cs +++ b/BitFaster.Caching/Lru/ConcurrentLruCore.cs @@ -308,18 +308,21 @@ public bool TryRemove(KeyValuePair item) { if (this.dictionary.TryGetValue(item.Key, out var existing)) { - if (EqualityComparer.Default.Equals(existing.Value, item.Value)) + lock (existing) { - var kvp = new KeyValuePair(item.Key, existing); + if (EqualityComparer.Default.Equals(existing.Value, item.Value)) + { + var kvp = new KeyValuePair(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>)this.dictionary).Remove(kvp)) + // https://devblogs.microsoft.com/pfxteam/little-known-gems-atomic-conditional-removals-from-concurrentdictionary/ + if (((ICollection>)this.dictionary).Remove(kvp)) #endif - { - OnRemove(item.Key, kvp.Value, ItemRemovedReason.Removed); - return true; + { + OnRemove(item.Key, kvp.Value, ItemRemovedReason.Removed); + return true; + } } }