diff --git a/.agents/skills/foundatio/SKILL.md b/.agents/skills/foundatio/SKILL.md index 0984a3b96..b4e650bcd 100644 --- a/.agents/skills/foundatio/SKILL.md +++ b/.agents/skills/foundatio/SKILL.md @@ -265,6 +265,8 @@ Use `Foundatio.Xunit.v3` for test logging and DI integration. Two base classes: - **`TestWithLoggingBase`** -- lightweight, no DI container. `_logger` (`ILogger`) for logging; `Log` (`ILoggerFactory`) for passing to Foundatio services. - **`TestLoggerBase`** -- full DI via `TestLoggerFixture`. Override `ConfigureServices` to register services. `Log` (`ILogger`) for logging; `TestLogger` (`ILoggerFactory`) for passing to Foundatio services. +Use `Microsoft.Extensions.Time.Testing.FakeTimeProvider` for expiration and timer tests. Inject the same instance into collaborating caches and lock providers, and advance it explicitly. A real-time timeout may bound an awaited completion, but should not simulate cache expiration. Keep shared test-harness coverage when adding provider-specific fake-time tests. Test observable behavior rather than the number of clock reads. + ```csharp using Foundatio.Caching; using Foundatio.Xunit; @@ -306,7 +308,7 @@ public class OrderServiceTests : TestLoggerBase - **Lock returns null**: `TryAcquireAsync` returns `null` when the lock cannot be acquired -- always guard with `is not null` before doing work. `AcquireAsync` throws `LockAcquisitionTimeoutException` instead of returning null. - **Dispose streams and locks**: `ILock` is `IAsyncDisposable` -- use `await using`. Streams from `GetFileStreamAsync` are `IDisposable` -- use `using var`. -- **Cache TTL floor**: Expiration values below 5ms are treated as already-expired and the key is silently removed. If you compute TTL dynamically (e.g., `expiresAt - now`), guard against near-zero values. +- **Cache TTL floor**: Expiration values below 5ms are treated as already expired. Key writes remove the key; `ListAddAsync` removes only the supplied values. Guard dynamically computed TTLs against near-zero values. - **Cache `GetAsync` returns `CacheValue`**: Check `result.HasValue` before accessing `result.Value`. A missing key returns `HasValue = false`, not an exception. - **Cache stampede (thundering herd)**: The cache-aside pattern (`Get` -> miss -> load -> `Set`) is vulnerable to stampedes when a popular key expires and many callers regenerate simultaneously. Use `CacheLockProvider` to serialize regeneration: acquire a lock keyed on the cache key, double-check the cache after acquiring, and only then call the backing store. See the [Cache Stampede Protection](https://foundatio.readthedocs.io/guide/caching.html#cache-stampede-protection) docs for the full pattern. - **Queue auto-complete**: `QueueJobBase` auto-completes entries based on `JobResult` by default. Set `AutoComplete = false` only when you need manual `CompleteAsync()`/`AbandonAsync()` control. Manual `DequeueAsync` does NOT auto-complete. @@ -314,6 +316,9 @@ public class OrderServiceTests : TestLoggerBase - **Failure semantics depend on job type**: `JobResult` only has `IsSuccess` -- there is no separate "failed but don't retry" status. For **queue-processed jobs** (`QueueJobBase.ProcessQueueEntryAsync`, or setting `context.Result` in a `WorkItemJob` handler), a non-success result triggers `AbandonAsync`, which re-queues the entry and eventually dead-letters it after `Retries` is exhausted -- reserve `FailedWithMessage`/`FromException` for transient errors you want retried, and log + return `JobResult.Success`/`SuccessWithMessage(...)` for permanent errors to avoid a pointless retry loop. For **standalone/manual jobs** (`JobBase`, a one-off `RunAsync()`/`RunInConsoleAsync()` run, or scheduled/cron jobs via `Foundatio.Extensions.Hosting`), there is no built-in retry or dead letter queue -- a failed result just produces an error-level log, a non-zero exit code from `RunInConsoleAsync`, or a failed entry in the job run history. Returning `FailedWithMessage`/`FromException` there is correct even for permanent errors, since nothing inside Foundatio will retry it. - **JobWithLockBase vs manual locking**: Use `JobWithLockBase` when the entire run must be single-instance (leader election). Use manual `ILockProvider.AcquireAsync` inside `JobBase` for finer-grained locking within a job. - **JobContext.RenewLockAsync**: Call in long-running jobs (both `JobBase` and `QueueJobBase`) to prevent lock expiration mid-processing. +- **In-memory expired entries**: Writes treat an expired entry as missing before maintenance sweeps it (as Redis does): increments and `SetIfHigher`/`SetIfLower` start fresh, `ReplaceAsync`, `SetExpirationAsync`, `RemoveIfEqualAsync` and `ReplaceIfEqualAsync` miss. The write reclaims the expired entry and raises `ItemExpired` once. Successful live conditional removal and empty-list removal free memory without that event. Size-rejected updates preserve value, TTL, and tracked size. `ReplaceIfEqualAsync` also rejects a replacement that expires during preparation or a retry, preserving a still-live entry. +- **In-memory numeric TTLs**: `SetIfHigherAsync` and `SetIfLowerAsync` apply the requested expiration even when the numeric value is unchanged. A zero return value does not mean the TTL was preserved; `null` clears it. Size-rejected updates remain unchanged. +- **In-memory lists and cloning**: List writes preserve supported dictionary types and comparers; copying an unsupported `IDictionary` throws `NotSupportedException` without changing the entry. Batch large updates because cloning, custom sizing, and raw dictionary access can require whole-list copies. `Items` honors `CloneValues` without affecting LRU. Collection snapshots do not isolate shared mutable elements when cloning is disabled. See the [caching guide](https://foundatio.readthedocs.io/guide/caching.html#in-memory-update-behavior) for details. - **Register as singletons**: All infrastructure services (`ICacheClient`, `IMessageBus`, `IQueue`, `IFileStorage`, `ILockProvider`) maintain internal state and connections -- always register as singletons. - **CacheLockProvider + IMessageBus**: `IMessageBus` is optional but recommended. Without it, lock release falls back to polling. With it, locks are released instantly via pub/sub notification. - **In-memory for tests**: In-memory implementations follow the same interfaces as production providers, but some behaviors differ (delivery delay limits, ordering, deduplication, group ids). Swap via DI for fast, isolated unit tests, and check [provider behavioral gaps](https://foundatio.dev/guide/provider-behavioral-gaps) before relying on provider-specific behavior. @@ -331,7 +336,7 @@ public class OrderServiceTests : TestLoggerBase ### Serializers -`ITextSerializer` extends `ISerializer` for human-readable formats (JSON). `ISerializer` covers binary formats. Default is `SystemTextJsonSerializer` (included in core). +`ITextSerializer` extends `ISerializer` for human-readable formats (JSON). `ISerializer` covers binary and text serialization. Default is `SystemTextJsonSerializer` (included in core). | Package | Provides | | ------- | -------- | diff --git a/benchmarks/InMemoryListBenchmarks.cs b/benchmarks/InMemoryListBenchmarks.cs new file mode 100644 index 000000000..8b7f0bd7c --- /dev/null +++ b/benchmarks/InMemoryListBenchmarks.cs @@ -0,0 +1,107 @@ +using System; +using System.Collections.Generic; +using System.Linq; +using System.Threading.Tasks; +using BenchmarkDotNet.Attributes; +using Foundatio.Caching; + +namespace Foundatio.Benchmarks; + +[MemoryDiagnoser] +public class InMemoryListBenchmarks +{ + public enum SizingMode { None, Fixed, Custom, Dynamic } + + [Params(false, true)] + public bool CloneValues { get; set; } + + [Params(10, 1000)] + public int Count { get; set; } + + [Params(SizingMode.None, SizingMode.Fixed, SizingMode.Custom, SizingMode.Dynamic)] + public SizingMode Sizing { get; set; } + + private InMemoryCacheClient _cache = null!; + private int[] _initial = null!; + private int[] _batch = null!; + private readonly int[] _item = [-1]; + private readonly int[] _workers = [0, 1, 2, 3]; + + [GlobalSetup] + public async Task Setup() + { + _cache = new InMemoryCacheClient(o => + { + o.CloneValues(CloneValues).TimeProvider(new FixedTimeProvider()); + return Sizing switch + { + SizingMode.Fixed => o.WithFixedSizing(100000000, 100), + SizingMode.Custom => o.MaxMemorySize(100000000).SizeCalculator(_ => 100), + SizingMode.Dynamic => o.WithDynamicSizing(100000000), + _ => o + }; + }); + _initial = Enumerable.Range(0, Count).ToArray(); + _batch = Enumerable.Range(0, Count / 2).ToArray(); + await _cache.ListAddAsync("set", _initial); + } + + [GlobalCleanup] + public void Cleanup() => _cache.Dispose(); + + [Benchmark] + public async Task AddRemove() + { + await _cache.ListAddAsync("set", _item); + return await _cache.ListRemoveAsync("set", _item); + } + + [Benchmark] + public async Task BulkInsert() + { + await _cache.RemoveAsync("bulk"); + return await _cache.ListAddAsync("bulk", _initial); + } + + [Benchmark] + public async Task BulkRemoveRestore() + { + await _cache.ListRemoveAsync("set", _batch); + return await _cache.ListAddAsync("set", _batch); + } + + [Benchmark] + public Task ContendedWrites() => Parallel.ForEachAsync(_workers, async (worker, token) => + { + int[] item = [-worker - 1]; + await Task.Yield(); + await _cache.ListAddAsync("set", item); + await _cache.ListRemoveAsync("set", item); + }); + + [Benchmark] + public async Task FirstModification() + { + await _cache.RemoveAsync("cold"); + await _cache.ListAddAsync("cold", _initial); + return await _cache.ListAddAsync("cold", _item); + } + + [Benchmark] + public Task>> Read() => _cache.GetListAsync("set"); + + [Benchmark] + public Task RemoveAbsent() => _cache.ListRemoveAsync("set", _item); + + [Benchmark] + public async Task>> WriteThenRead() + { + await AddRemove(); + return await _cache.GetListAsync("set"); + } + + private sealed class FixedTimeProvider : TimeProvider + { + public override DateTimeOffset GetUtcNow() => new(2026, 1, 1, 0, 0, 0, TimeSpan.Zero); + } +} diff --git a/benchmarks/InMemoryWriteBenchmarks.cs b/benchmarks/InMemoryWriteBenchmarks.cs new file mode 100644 index 000000000..cc47c6b29 --- /dev/null +++ b/benchmarks/InMemoryWriteBenchmarks.cs @@ -0,0 +1,40 @@ +using System.Threading.Tasks; +using BenchmarkDotNet.Attributes; +using Foundatio.Caching; + +namespace Foundatio.Benchmarks; + +/// +/// Scalar writes on a key that holds a live value, which is the common case. Every write goes through +/// UpdateEntry, so these show what its wrapper costs. +/// +[MemoryDiagnoser] +public class InMemoryWriteBenchmarks +{ + private InMemoryCacheClient _cache = null!; + + [GlobalSetup] + public async Task Setup() + { + _cache = new InMemoryCacheClient(o => o.CloneValues(false)); + await _cache.SetAsync("key", 1L); + } + + [GlobalCleanup] + public void Cleanup() => _cache.Dispose(); + + [Benchmark] + public Task Set() => _cache.SetAsync("key", 1L); + + [Benchmark] + public Task Add_ExistingKey() => _cache.AddAsync("key", 1L); + + [Benchmark] + public Task Increment() => _cache.IncrementAsync("key", 1L); + + [Benchmark] + public Task SetIfHigher_NotHigher() => _cache.SetIfHigherAsync("key", 0L); + + [Benchmark] + public Task ReplaceIfEqual() => _cache.ReplaceIfEqualAsync("key", 1L, 1L); +} diff --git a/docs/guide/caching.md b/docs/guide/caching.md index 0fa5ec3b5..0ae7bfd5c 100644 --- a/docs/guide/caching.md +++ b/docs/guide/caching.md @@ -42,9 +42,9 @@ public interface ICacheClient : IDisposable Task SetAllExpirationAsync(IDictionary expirations); // List operations - Task ListAddAsync(string key, IEnumerable values, TimeSpan? expiresIn = null); - Task ListRemoveAsync(string key, IEnumerable values); - Task>> GetListAsync(string key, int? page = null, int pageSize = 100); + Task ListAddAsync(string key, IEnumerable values, TimeSpan? expiresIn = null) where T : notnull; + Task ListRemoveAsync(string key, IEnumerable values) where T : notnull; + Task>> GetListAsync(string key, int? page = null, int pageSize = 100) where T : notnull; } ``` @@ -54,9 +54,11 @@ Many cache methods accept an optional `expiresIn` parameter that controls the TT ### Quick Reference +The following describes key writes. Conditional operations and lists have additional rules described below. + | `expiresIn` Value | Behavior | |-------------------|----------| -| `null` | Entry will not expire. **Removes any existing TTL** on the key. | +| `null` | Entry will not expire. **Removes any existing TTL** when the write is applied. | | Positive `TimeSpan` ≥ 5ms | Entry expires after the specified duration from now. | | Greater than 0 and less than 5ms | **Treated as already expired.** Key is removed, operation returns failure value. See [Minimum Expiration](#minimum-expiration) below. | | Zero or negative | **Treated as already expired.** Key is removed, operation returns failure value. | @@ -64,7 +66,7 @@ Many cache methods accept an optional `expiresIn` parameter that controls the TT ### Minimum Expiration -Foundatio enforces a **minimum expiration of 5 milliseconds** (`CacheClientExtensions.MinimumExpiration`) on all cache operations. Any `expiresIn` value greater than zero but less than 5ms is treated as already-expired: the key is removed and the operation returns its failure value. +Foundatio uses a **minimum expiration of 5 milliseconds** (`CacheClientExtensions.MinimumExpiration`) for cache writes. A shorter `expiresIn` is treated as already expired: key writes remove the key and return their failure value; `ListAddAsync` removes only the supplied values and returns `0`. **Why 5ms?** @@ -74,7 +76,7 @@ External cache providers—most notably Redis—represent TTLs as integers. Stac ERR invalid expire time in 'setex' ``` -This truncation-to-zero can happen legitimately in production when computing `expiresAtUtc - DateTime.UtcNow` on a time very close to "now"—a common race condition in high-throughput systems. The 5ms floor provides a safe margin above the 1ms truncation boundary while remaining far below any real-world cache TTL. +This truncation-to-zero can happen legitimately in production when computing `expiresAtUtc - DateTime.UtcNow` on a time very close to "now"—a common race condition in high-throughput systems. The 5ms floor provides a margin above the 1ms truncation boundary. **Behavior summary:** @@ -93,7 +95,7 @@ await cache.SetAsync("key", value, TimeSpan.FromMilliseconds(100)); // 100ms > 5 The constant is accessible for consumers that need to validate TTLs before calling cache methods: ```csharp -using Foundatio.Extensions; +using Foundatio.Caching; if (myExpiration < CacheClientExtensions.MinimumExpiration) { @@ -103,42 +105,38 @@ if (myExpiration < CacheClientExtensions.MinimumExpiration) ### TTL Behavior by Method -Different methods handle the `expiresIn` parameter slightly differently. The table below shows exactly what happens for each method: +The table describes expiration when the operation is applied. For example, `AddAsync` with an existing live key or `ReplaceIfEqualAsync` with a mismatched expected value leaves that entry unchanged. Expirations below 5ms follow the removal behavior in the fourth column. -| Method | `null` expiresIn | Positive expiresIn | Zero/Negative | Return on Failure | -|--------|------------------|-------------------|---------------|-------------------| +| Method | `null` expiresIn | Positive expiresIn ≥ 5ms | Below 5ms | Return on Failure | +|--------|------------------|-------------------------|-----------|-------------------| | `SetAsync` | No TTL (removes existing) | Sets TTL | Removes key | `false` | | `AddAsync` | No TTL | Sets TTL | Removes key | `false` | -| `SetAllAsync` | No TTL (removes existing) | Sets TTL | Removes all keys | `0` | +| `SetAllAsync` | No TTL (removes existing) | Sets TTL | Removes all supplied keys | `0` | | `ReplaceAsync` | No TTL (removes existing) | Sets TTL | Removes key | `false` | | `ReplaceIfEqualAsync` | No TTL (removes existing) | Sets TTL | Removes key | `false` | | `IncrementAsync` | No TTL (removes existing) | Sets/updates TTL | Removes key | `0` | | `SetIfHigherAsync` | No TTL (removes existing)* | Sets TTL* | Removes key | `0` | | `SetIfLowerAsync` | No TTL (removes existing)* | Sets TTL* | Removes key | `0` | -| `ListAddAsync` | No TTL | Sets TTL | Removes key | `0` | +| `ListAddAsync` | Supplied values do not expire | Sets per-value TTL | Removes supplied values | `0` | -\* **Conditional operations**: `SetIfHigherAsync` and `SetIfLowerAsync` only update TTL when the condition is met. If the value is not higher/lower, the entire operation is a no-op (including expiration). +\* **In-memory numeric conditions**: `InMemoryCacheClient` applies the requested expiration even when `SetIfHigherAsync` or `SetIfLowerAsync` leaves the numeric value unchanged. A return value of `0` does not imply that the TTL was preserved. Passing `null` clears the TTL. Updates rejected by size validation leave the existing entry unchanged. See [Conditional Expiration Behavior](#conditional-expiration-behavior) for an example. ::: tip ListRemoveAsync -`ListRemoveAsync` does not accept an `expiresIn` parameter. It simply removes values from the list without modifying the key's expiration. +`ListRemoveAsync` does not accept an `expiresIn` parameter. In-memory list removal recomputes the key's expiration from the remaining values: any non-expiring value keeps the key non-expiring; otherwise the latest remaining expiration is used. Removing the last value removes the key. ::: ::: info Integer vs Floating-Point Increments -`IncrementAsync` supports both integer (`long`) and floating-point (`double`) amounts. Both overloads work correctly with expiration: +`IncrementAsync` supports both integer (`long`) and floating-point (`double`) amounts. Use the same numeric type consistently for a counter, especially when fractional values are possible: ```csharp // Integer increments -await cache.IncrementAsync("counter", 1L, TimeSpan.FromHours(1)); // long overload +await cache.IncrementAsync("counter", 1L, TimeSpan.FromHours(1)); await cache.IncrementAsync("counter", 5L, TimeSpan.FromHours(1)); -// Floating-point increments -await cache.IncrementAsync("score", 1.5, TimeSpan.FromHours(1)); // double overload -await cache.IncrementAsync("score", 2.25, TimeSpan.FromHours(1)); // Total: 3.75 - -// Mixed increments work correctly -await cache.IncrementAsync("mixed", 1, TimeSpan.FromHours(1)); // 1 -await cache.IncrementAsync("mixed", 1.5, TimeSpan.FromHours(1)); // 2.5 -await cache.IncrementAsync("mixed", 2, TimeSpan.FromHours(1)); // 4.5 +// Floating-point increments, including whole-number amounts +await cache.IncrementAsync("score", 1d, TimeSpan.FromHours(1)); +await cache.IncrementAsync("score", 1.5d, TimeSpan.FromHours(1)); +await cache.IncrementAsync("score", 2d, TimeSpan.FromHours(1)); // Total: 4.5 ``` For Redis implementations, integer amounts (including `2.0` where the fractional part is zero) use the more efficient `INCRBY` command, while fractional amounts use `INCRBYFLOAT`. @@ -149,7 +147,7 @@ For Redis implementations, integer amounts (including `2.0` where the fractional ```csharp // Basic Set Operations -// No expiration - item lives until explicitly removed +// No expiration - item lives until explicitly removed or evicted await cache.SetAsync("permanent-key", value); // null is default await cache.SetAsync("also-permanent", value, null); // explicit null @@ -229,7 +227,7 @@ On Azure Managed Redis (and many Redis deployments), the default eviction policy **Recommendations:** - Always set appropriate TTLs for cache entries when possible -- Use `TimeSpan.MaxValue` only when you explicitly need permanent storage +- Use `TimeSpan.MaxValue` only when you explicitly need non-expiring cache entries; it does not make cached data durable - Monitor your Redis memory usage and eviction metrics **Further Reading:** @@ -259,9 +257,35 @@ var result = await cache.GetAsync("key"); await cache.SetAsync("session", sessionData, TimeSpan.FromMinutes(30)); // With item limits (LRU eviction) -var limitedCache = new InMemoryCacheClient(o => o.MaxItems = 1000); +var limitedCache = new InMemoryCacheClient(o => o.MaxItems(1000)); ``` +#### In-memory update behavior + +Conditional writes and list updates publish replacement entries atomically. An oversized update rejected by size validation preserves the previous value, expiration, and tracked size. + +Every write treats an expired entry as missing, even before background maintenance removes it, matching Redis: `IncrementAsync`, `SetIfHigherAsync`, `SetIfLowerAsync` and `ListAddAsync` start fresh, `ReplaceAsync`, `SetExpirationAsync` and `ListRemoveAsync` find nothing to change, and `RemoveIfEqualAsync` and `ReplaceIfEqualAsync` return `false` rather than letting a stale owner remove or renew it. A write that finds an expired entry reclaims it and raises `ItemExpired` once, without touching a concurrent replacement. + +Successful removal of a live matching value, or removal of the last list value, frees tracked memory immediately and does not raise an expiration event. + +::: info Behavior changes from earlier versions +- `RemoveIfEqualAsync` removes a matching live value immediately and no longer raises `ItemExpired` for it. Earlier versions marked the entry expired and let maintenance remove it, which raised the event. +- `ListAddAsync` returns `0` and leaves the list unchanged when the merged list fails size validation. +- `Items` returns the cached values rather than the internal entry objects. +- `IncrementAsync` recalculates the entry's tracked size after each increment. +- `ReplaceAsync` on an expired key returns `false` and raises `ItemExpired` once while reclaiming it. +- `ReplaceIfEqualAsync` returns `false` if the replacement expires during preparation or a retry, preserving any still-live entry's value, expiration, and tracked size. +- `CurrentMemorySize` is approximate while `RemoveAllAsync()` or `Dispose` races concurrent writes. This is unchanged from earlier versions; it self-corrects on the next recount after compaction. +::: + +List updates preserve the collection structure of previously returned snapshots. They preserve the comparers of `Dictionary`, `SortedDictionary`, `SortedList`, `ConcurrentDictionary`, and, on .NET 9 or later, `OrderedDictionary`. Other `IDictionary` implementations throw `NotSupportedException` when an update requires copying them; the original entry remains unchanged. + +`Items` returns cached values without changing eviction order. Both `Items` and ordinary reads honor `CloneValues`. With cloning disabled, mutable payloads and list elements can remain shared: a collection snapshot is not a deep copy. Do not mutate shared objects concurrently with cache operations. + +::: tip Large list updates +Batch values into a single `ListAddAsync` or `ListRemoveAsync` call. Cloning, custom size calculation, and raw dictionary access can require copies proportional to the list size. Benchmark representative list sizes and read/write patterns before choosing these options; atomic updates do not make every workload allocation-free. +::: + ### HybridCacheClient Combines a local in-memory cache (L1) with a distributed cache (L2) for maximum performance. This implements the industry-standard L1/L2 caching architecture, ideal for read-heavy workloads where the same data is accessed frequently across multiple requests. @@ -334,7 +358,7 @@ using Foundatio.Redis.Cache; using StackExchange.Redis; var redis = await ConnectionMultiplexer.ConnectAsync("localhost:6379"); -var cache = new RedisCacheClient(o => o.ConnectionMultiplexer = redis); +var cache = new RedisCacheClient(o => o.ConnectionMultiplexer(redis)); await cache.SetAsync("user:123", user, TimeSpan.FromHours(1)); ``` @@ -415,15 +439,10 @@ public class MyService(IMemoryCacheClient localCache) { } ### Serialization and Cloning Overhead -Every cache operation has performance overhead from serialization, deserialization, and optional value cloning: +Distributed caches serialize values for storage and deserialize them on reads. `InMemoryCacheClient` stores objects directly and optionally deep-clones mutable payloads; it does not use a JSON round trip for cloning. -**Distributed Cache (Redis, Azure, etc.):** -- **Write**: Serialize object to bytes for storage -- **Read**: Deserialize bytes back to object - -**In-Memory Cache:** -- **With `CloneValues = true`** (default: `false`): Serialize and deserialize on every get/set to create independent copies -- **With `CloneValues = false`**: Direct reference storage (no overhead, but risk of mutation) +- **With `CloneValues = true`**: mutable payloads are copied when stored and returned, adding allocation and copying costs. +- **With `CloneValues = false`** (the default): payloads can be shared by reference. This avoids cloning costs, but callers must manage mutation safely. ### Value Cloning {#clonevalues} @@ -433,17 +452,17 @@ The `CloneValues` option controls whether cached values are cloned on read and w #### The Problem: Reference Sharing -Without cloning, the cache stores direct references to objects. If code mutates a cached object, **all future reads see the mutated value**: +Without cloning, the cache stores direct references to objects. Mutating a shared object changes the value seen by subsequent reads until the entry is replaced or removed: ```csharp var cache = new InMemoryCacheClient(); // CloneValues = false (default) -var user = new User { Name = "Alice", Balance = 100.0 }; +var user = new User { Name = "Alice", Balance = 100.0m }; await cache.SetAsync("user:1", user); // Get from cache and accidentally mutate var cached = (await cache.GetAsync("user:1")).Value; -cached.Balance = 0.0; // ⚠️ Mutates the cached object! +cached.Balance = 0.0m; // ⚠️ Mutates the cached object! // Later reads return the MUTATED value var again = (await cache.GetAsync("user:1")).Value; @@ -458,14 +477,14 @@ This is especially dangerous when: #### The Solution: Enable Cloning ```csharp -var cache = new InMemoryCacheClient(o => o.CloneValues = true); +var cache = new InMemoryCacheClient(o => o.CloneValues(true)); -var user = new User { Name = "Alice", Balance = 100.0 }; +var user = new User { Name = "Alice", Balance = 100.0m }; await cache.SetAsync("user:1", user); // Mutations are isolated to this copy var cached = (await cache.GetAsync("user:1")).Value; -cached.Balance = 0.0; // Only affects this instance +cached.Balance = 0.0m; // Only affects this instance // Fresh reads get original value var fresh = (await cache.GetAsync("user:1")).Value; @@ -474,20 +493,17 @@ Console.WriteLine(fresh.Balance); // 100.0 ✓ **How it works:** -When `CloneValues = true`, each `GetAsync` and `SetAsync` serializes and deserializes the value using the configured serializer (default: JSON). This creates independent copies that are isolated from mutation. +`InMemoryCacheClient` uses deep cloning for payloads that require it. `SetAsync` stores a copy and `GetAsync` returns a copy; `Items` also honors this setting. Changing the configured JSON serializer does not change this cloning mechanism. Primitive values and strings do not require a deep copy. #### Performance Trade-offs | Operation | `CloneValues = false` | `CloneValues = true` | |-----------|----------------------|---------------------| -| `SetAsync` | Store reference (zero overhead) | Serialize → Deserialize (overhead) | -| `GetAsync` | Return reference (zero overhead) | Serialize → Deserialize (overhead) | -| Memory | Single instance | Multiple copies | +| `SetAsync` | Store the payload without cloning | Deep-copy mutable payloads before storage | +| `GetAsync` | Return the stored payload without cloning | Deep-copy mutable payloads before returning them | +| Payload memory | May share one instance | Additional copies for stored and returned values | -**Benchmark example (10,000 operations):** -- Simple objects (< 1KB): ~2-5ms overhead per 10k ops -- Complex objects (> 10KB): ~50-100ms overhead per 10k ops -- Primitive types: Negligible difference +Cloning cost depends on the payload's object graph, collection sizes, and access pattern. Measure representative workloads instead of assuming a fixed per-operation overhead. List updates may also copy collection storage independently of payload cloning. #### When to Enable Cloning @@ -499,48 +515,46 @@ When `CloneValues = true`, each `GetAsync` and `SetAsync` serializes and deseria - Debugging unexplained cache corruption ❌ **Keep `CloneValues = false` when:** -- Caching immutable types (strings, primitives, records with `init`-only properties) -- You have strict control over mutation (internal APIs, single code path) +- Caching strings, primitives, or object graphs that are genuinely immutable +- You have strict control over mutation and synchronization - Performance is critical and you can guarantee immutability -- Using frozen/immutable collections (`ImmutableArray`, `FrozenDictionary`) +- Using immutable collections whose contained values are also immutable + +An `init`-only property or a record does not make referenced lists or other mutable objects immutable. Likewise, an immutable collection does not make its elements immutable. #### Best Practices -**Pattern 1: Use immutable types (no cloning needed):** +**Pattern 1: Use immutable values (no cloning needed):** ```csharp var cache = new InMemoryCacheClient(); // CloneValues = false +await cache.SetAsync("user:1", new UserDto(1, "Alice", 100.0m)); -// C# records are immutable by default +// This record contains only immutable values. public record UserDto(int Id, string Name, decimal Balance); - -await cache.SetAsync("user:1", new UserDto(1, "Alice", 100.0)); -// Safe: Cannot mutate records ``` **Pattern 2: Clone when mutability is unavoidable:** ```csharp -var cache = new InMemoryCacheClient(o => o.CloneValues = true); +var cache = new InMemoryCacheClient(o => o.CloneValues(true)); +await cache.SetAsync("user:1", userEntity); +// Mutations to userEntity or a returned copy do not change the stored copy. -// Mutable class public class UserEntity { public int Id { get; set; } - public string Name { get; set; } + public string Name { get; set; } = String.Empty; public decimal Balance { get; set; } } - -await cache.SetAsync("user:1", userEntity); -// Safe: Mutations are isolated ``` **Pattern 3: Mix both strategies:** ```csharp // Separate caches for different needs -var immutableCache = new InMemoryCacheClient(o => o.CloneValues = false); -var mutableCache = new InMemoryCacheClient(o => o.CloneValues = true); +var immutableCache = new InMemoryCacheClient(o => o.CloneValues(false)); +var mutableCache = new InMemoryCacheClient(o => o.CloneValues(true)); // Immutable config await immutableCache.SetAsync("config:theme", "dark"); @@ -606,7 +620,7 @@ public class ProductService return cached.Value; // Only one caller regenerates; others wait for the lock then re-check cache. - await using var lck = await _locker.AcquireAsync( + await using var lck = await _locker.TryAcquireAsync( $"cache-load:{cacheKey}", timeUntilExpires: TimeSpan.FromSeconds(30), cancellationToken: ct); @@ -614,7 +628,7 @@ public class ProductService if (lck is null) return null; // Could not acquire -- caller decides how to handle - // Double-check: another caller may have populated cache while we waited. + // Double-check: another caller may have populated the cache while we waited. cached = await _cache.GetAsync(cacheKey); if (cached.HasValue) return cached.Value; @@ -678,50 +692,47 @@ double diff = await cache.SetIfHigherAsync("max-users", 100); // Returns 100 // Value is higher - returns the delta diff = await cache.SetIfHigherAsync("max-users", 150); // Returns 50 (150 - 100) -// Value is NOT higher - returns 0 (no change) +// Value is NOT higher - returns 0 (numeric value is unchanged) diff = await cache.SetIfHigherAsync("max-users", 120); // Returns 0 -// To get the actual current value after the operation: -var currentMax = (await cache.GetAsync("max-users")).Value; // 150 +// Read the current value separately; another writer may have changed it in the meantime. +var currentMax = (await cache.GetAsync("max-users")).Value; ``` -::: warning Conditional Expiration Behavior -`SetIfHigherAsync` and `SetIfLowerAsync` only update the expiration **when the condition is met**. If the value is not higher/lower, the operation is a complete no-op—including the expiration. +#### Conditional Expiration Behavior + +For `InMemoryCacheClient`, `SetIfHigherAsync` and `SetIfLowerAsync` apply `expiresIn` even when the numeric condition is not met. This differs from a rejected `ReplaceIfEqualAsync`, which leaves the entry unchanged. Do not infer expiration behavior solely from a numeric return value of `0`. ```csharp -// Set with 1-hour TTL -await cache.SetIfHigherAsync("max-users", 100, TimeSpan.FromHours(1)); +using var cache = new InMemoryCacheClient(); -// Try to set lower value with 2-hour TTL -await cache.SetIfHigherAsync("max-users", 50, TimeSpan.FromHours(2)); -// TTL is STILL 1 hour! The condition failed, so nothing changed. +// Set with a 1-hour TTL +await cache.SetIfHigherAsync("max-users", 100L, TimeSpan.FromHours(1)); -// Set higher value with 2-hour TTL -await cache.SetIfHigherAsync("max-users", 200, TimeSpan.FromHours(2)); -// TTL is now 2 hours (condition was met) +// Numeric value stays 100, but the TTL is reset to 2 hours. +long difference = await cache.SetIfHigherAsync("max-users", 50L, TimeSpan.FromHours(2)); +// difference == 0 + +// Numeric value still stays 100; passing null removes the TTL. +await cache.SetIfHigherAsync("max-users", 50L, null); ``` -This is intentional—the semantic is "set IF higher/lower", so a failed condition means the entire operation is skipped. -::: +An in-memory update rejected by size validation does not apply its new expiration. Check the behavior of the cache provider you use rather than assuming every provider treats an unmet numeric condition identically. ### List Operations Foundatio lists support **per-value expiration**, where each item in the list can have its own independent TTL. This is different from standard cache keys where expiration applies to the entire key. +List values are unique according to the list's comparer. Adding an existing value updates that value's expiration rather than creating a duplicate. Do not use separately paged reads as a consistent snapshot while other callers are updating the list. + #### Why Per-Value Expiration? -Per-value expiration prevents unbounded list growth. Consider tracking recently deleted items: +A sliding TTL for the whole list can retain old values indefinitely when new values are continually added. Per-value expiration lets old values expire without extending their lifetime every time another value is added: ```csharp -// Without per-value expiration (sliding expiration problem): -// Adding ANY item resets the entire list's TTL, causing indefinite growth -await cache.ListAddAsync("deleted-items", [itemId], TimeSpan.FromDays(7)); -// After months: list has 100,000+ items because TTL keeps resetting! - -// With per-value expiration (Foundatio's approach): -// Each item expires independently after 7 days +// Each supplied value expires after 7 days. await cache.ListAddAsync("deleted-items", [itemId], TimeSpan.FromDays(7)); -// List stays bounded - old items expire even as new ones are added +// Adding a different item later does not refresh itemId's expiration. ``` **Real-world use cases:** @@ -741,10 +752,10 @@ await cache.ListAddAsync("user:123:recent-searches", new[] { "query2" }, TimeSpa // Items expire independently - query1 expires 1 hour after it was added, // query2 expires 1 hour after IT was added (not when query1 was added) -// Get paginated list (expired items are automatically filtered) +// Get the first page (pages are one-based; expired items are filtered). var searches = await cache.GetListAsync( "user:123:recent-searches", - page: 0, + page: 1, pageSize: 10 ); @@ -756,9 +767,11 @@ await cache.ListRemoveAsync("user:123:recent-searches", new[] { "query1" }); | `expiresIn` Value | Behavior | |-------------------|----------| -| `null` | Values will not expire. Key expiration is set to max of all item expirations. | -| Positive `TimeSpan` | Each value expires independently after this duration. | -| Zero or negative | The specified values are removed from the list (if present), returns 0. | +| `null` | Supplied values do not expire. In memory, any non-expiring value keeps the key non-expiring. | +| Positive `TimeSpan` ≥ 5ms | Each supplied value expires independently after this duration. | +| Below 5ms, including zero or negative | The supplied values are removed from the list if present; `ListAddAsync` returns `0`. | + +In-memory list updates recompute the key's expiration from the remaining values. If every remaining value expires, the key uses the latest expiration. An empty list is removed. Removing values does not refresh the expiration of values left in the list. ### Bulk Operations @@ -791,11 +804,11 @@ services.AddSingleton(); // With options services.AddSingleton(sp => - new InMemoryCacheClient(o => o.MaxItems = 1000)); + new InMemoryCacheClient(o => o.MaxItems(1000))); // Redis (production) services.AddSingleton(sp => - new RedisCacheClient(o => o.ConnectionMultiplexer = redis)); + new RedisCacheClient(o => o.ConnectionMultiplexer(redis))); ``` ### Hybrid with DI @@ -805,7 +818,7 @@ services.AddSingleton(sp => { var redis = sp.GetRequiredService(); return new HybridCacheClient( - new RedisCacheClient(o => o.ConnectionMultiplexer = redis), + new RedisCacheClient(o => o.ConnectionMultiplexer(redis)), sp.GetRequiredService() ); }); @@ -817,11 +830,47 @@ Use different caches for different purposes: ```csharp services.AddKeyedSingleton("session", - new InMemoryCacheClient(o => o.MaxItems = 10000)); + new InMemoryCacheClient(o => o.MaxItems(10000))); services.AddKeyedSingleton("geo", - new InMemoryCacheClient(o => o.MaxItems = 250)); + new InMemoryCacheClient(o => o.MaxItems(250))); ``` +## Testing Expiration + +Use `FakeTimeProvider` from the `Microsoft.Extensions.TimeProvider.Testing` package instead of sleeping or creating a custom clock. Inject the same provider into the cache and any collaborating lock provider or service that depends on time. + +```csharp +using System; +using System.Threading.Tasks; +using Foundatio.Caching; +using Microsoft.Extensions.Time.Testing; +using Xunit; + +public class CacheExpirationTests +{ + [Fact] + public async Task GetAsync_AfterExpiration_ReturnsNoValue() + { + // Arrange + var timeProvider = new FakeTimeProvider(); + using var cache = new InMemoryCacheClient(o => o.TimeProvider(timeProvider)); + await cache.SetAsync("key", "value", TimeSpan.FromMinutes(5)); + Assert.True((await cache.GetAsync("key")).HasValue); + timeProvider.Advance(TimeSpan.FromMinutes(5) + TimeSpan.FromMilliseconds(1)); + + // Act + var result = await cache.GetAsync("key"); + + // Assert + Assert.False(result.HasValue); + } +} +``` + +Advancing fake time drives timers registered with that provider, but does not wait for unrelated background work. Await the operation being tested, or await an `ItemExpired` notification when testing expiration events. A real-time `WaitAsync` timeout can bound that wait to prevent a hung test; it should not be used to simulate expiration. + +Keep provider integration tests alongside in-memory tests. A shared interface does not guarantee identical serialization, expiration, or concurrency behavior in every implementation. See Microsoft's [FakeTimeProvider testing guidance](https://learn.microsoft.com/en-us/dotnet/core/extensions/timeprovider-testing). + ## Best Practices ### 1. Use Meaningful Key Patterns diff --git a/samples/Foundatio.AppHost/Foundatio.AppHost.csproj b/samples/Foundatio.AppHost/Foundatio.AppHost.csproj index 9f616654a..7aafcd16c 100644 --- a/samples/Foundatio.AppHost/Foundatio.AppHost.csproj +++ b/samples/Foundatio.AppHost/Foundatio.AppHost.csproj @@ -11,6 +11,7 @@ + diff --git a/src/Foundatio.TestHarness/Caching/CacheClientTestsBase.cs b/src/Foundatio.TestHarness/Caching/CacheClientTestsBase.cs index 223e5671d..aa43c4071 100644 --- a/src/Foundatio.TestHarness/Caching/CacheClientTestsBase.cs +++ b/src/Foundatio.TestHarness/Caching/CacheClientTestsBase.cs @@ -55,6 +55,46 @@ public virtual async Task AddAsync_WhenKeyDoesNotExist_AddsValueAndReturnsTrue(s } } + public virtual async Task AddAsync_WithConcurrentAcquireAndRelease_OnlyOneOwnerAtATime() + { + // Arrange + var cache = GetCacheClient(); + if (cache is null) + return; + + using (cache) + { + await cache.RemoveAllAsync(); + + // Mirrors CacheLockProvider: AddAsync acquires and RemoveIfEqualAsync releases. An AddAsync that stores + // the key must report success, otherwise the key is left behind with no owner to release it. + const string cacheKey = "lock"; + int owners = 0; + int overlaps = 0; + int failedReleases = 0; + + // Act + await Parallel.ForEachAsync(Enumerable.Range(0, 1000), TestCancellationToken, async (i, _) => + { + string owner = i.ToString(); + if (!await cache.AddAsync(cacheKey, owner)) + return; + + if (Interlocked.Increment(ref owners) > 1) + Interlocked.Increment(ref overlaps); + + Interlocked.Decrement(ref owners); + if (!await cache.RemoveIfEqualAsync(cacheKey, owner)) + Interlocked.Increment(ref failedReleases); + }); + + // Assert + Assert.Equal(0, overlaps); + Assert.Equal(0, failedReleases); + Assert.False(await cache.ExistsAsync(cacheKey)); + } + } + public virtual async Task AddAsync_WithConcurrentRequests_OnlyOneSucceeds() { var cache = GetCacheClient(); @@ -698,6 +738,27 @@ public virtual async Task GetListAsync_WithExpiredItems_RemovesExpiredAndReturns } } + public virtual async Task IncrementAsync_WithConcurrentRequests_DoesNotLoseUpdates() + { + // Arrange + var cache = GetCacheClient(); + if (cache is null) + return; + + using (cache) + { + await cache.RemoveAllAsync(); + + const int increments = 1000; + // Act + await Parallel.ForEachAsync(Enumerable.Range(0, increments), TestCancellationToken, + async (_, _) => await cache.IncrementAsync("counter", 1)); + + // Assert + Assert.Equal(increments, (await cache.GetAsync("counter")).Value); + } + } + public virtual async Task IncrementAsync_WithExpiration_SetsExpirationCorrectly() { var cache = GetCacheClient(); @@ -928,6 +989,30 @@ public virtual async Task IncrementAsync_WithFloatingPointDecimals_PreservesDeci } } + public virtual async Task SetIfHigherAsync_WithConcurrentRequests_DifferencesSumToMaximum() + { + // Arrange + var cache = GetCacheClient(); + if (cache is null) + return; + + using (cache) + { + await cache.RemoveAllAsync(); + + // The first call returns its full value and each later increase returns the delta, so the + // differences of every successful update add up to the final maximum. + long total = 0; + // Act + await Parallel.ForEachAsync(Enumerable.Range(1, 1000), TestCancellationToken, + async (i, _) => Interlocked.Add(ref total, await cache.SetIfHigherAsync("set-if-higher", (long)i))); + + // Assert + Assert.Equal(1000, (await cache.GetAsync("set-if-higher")).Value); + Assert.Equal(1000, total); + } + } + public virtual async Task SetIfHigherAsync_WithFloatingPointDecimals_ComparesCorrectly() { var cache = GetCacheClient(); @@ -997,6 +1082,29 @@ public virtual async Task SetIfHigherAsync_WithFloatingPointDecimals_ComparesCor } } + public virtual async Task SetIfLowerAsync_WithConcurrentRequests_DifferencesSumToDecrease() + { + // Arrange + var cache = GetCacheClient(); + if (cache is null) + return; + + using (cache) + { + await cache.RemoveAllAsync(); + await cache.SetAsync("set-if-lower", 1001L); + + long total = 0; + // Act + await Parallel.ForEachAsync(Enumerable.Range(1, 1000), TestCancellationToken, + async (i, _) => Interlocked.Add(ref total, await cache.SetIfLowerAsync("set-if-lower", (long)i))); + + // Assert + Assert.Equal(1, (await cache.GetAsync("set-if-lower")).Value); + Assert.Equal(1000, total); + } + } + public virtual async Task SetIfLowerAsync_WithFloatingPointDecimals_ComparesCorrectly() { var cache = GetCacheClient(); @@ -1066,6 +1174,28 @@ public virtual async Task SetIfLowerAsync_WithFloatingPointDecimals_ComparesCorr } } + public virtual async Task ListAddAsync_WithConcurrentRequests_DoesNotLoseValues() + { + // Arrange + var cache = GetCacheClient(); + if (cache is null) + return; + + using (cache) + { + await cache.RemoveAllAsync(); + + const int values = 1000; + // Act + await Parallel.ForEachAsync(Enumerable.Range(0, values), TestCancellationToken, + async (i, _) => await cache.ListAddAsync("set", new[] { i })); + + var result = await cache.GetListAsync("set"); + // Assert + Assert.Equal(values, result.Value.Count); + } + } + public virtual async Task ListAddAsync_WithExpiration_SetsExpirationCorrectly() { var cache = GetCacheClient(); @@ -3150,6 +3280,27 @@ public virtual async Task SetIfHigherAsync_WithExpiration_SetsExpirationCorrectl } } + public virtual async Task SetIfHigherAsync_WithLowerValue_ReturnsZeroAndKeepsValue() + { + // Arrange + var cache = GetCacheClient(); + if (cache is null) + return; + + using (cache) + { + await cache.RemoveAllAsync(); + await cache.SetAsync("set-if-higher-lower", 10L); + + // Act + long difference = await cache.SetIfHigherAsync("set-if-higher-lower", 5L, TimeSpan.FromMinutes(5)); + + // Assert + Assert.Equal(0, difference); + Assert.Equal(10, (await cache.GetAsync("set-if-higher-lower")).Value); + } + } + public virtual async Task SetIfLowerAsync_WithDateTime_UpdatesWhenLower() { var cache = GetCacheClient(); diff --git a/src/Foundatio/Caching/InMemoryCacheClient.cs b/src/Foundatio/Caching/InMemoryCacheClient.cs index e1fcd63c0..6f00ef7c2 100644 --- a/src/Foundatio/Caching/InMemoryCacheClient.cs +++ b/src/Foundatio/Caching/InMemoryCacheClient.cs @@ -1,9 +1,11 @@ using System; using System.Collections.Concurrent; using System.Collections.Generic; +using System.Collections.Immutable; using System.Collections.ObjectModel; using System.Diagnostics.CodeAnalysis; using System.Linq; +using System.Numerics; using System.Reflection; using System.Text.RegularExpressions; using System.Threading; @@ -24,6 +26,7 @@ public class InMemoryCacheClient : IMemoryCacheClient, IHaveTimeProvider, IHaveL private readonly int? _maxItems; private readonly long? _maxMemorySize; private readonly bool _hasSizeCalculator; + private readonly bool _canShareListValues; private readonly bool _shouldTrackMemory; private Func? _sizeCalculator; private readonly long? _maxEntrySize; @@ -38,6 +41,8 @@ public class InMemoryCacheClient : IMemoryCacheClient, IHaveTimeProvider, IHaveL private readonly ILoggerFactory _loggerFactory; private readonly AsyncLock _lock = new(); private readonly CancellationTokenSource _disposedCancellationTokenSource = new(); + // Cached so maintenance racing Dispose never reads Token from a disposed source + private readonly CancellationToken _disposedCancellationToken; private bool _isDisposed; public InMemoryCacheClient() : this(o => o) @@ -46,6 +51,7 @@ public InMemoryCacheClient() : this(o => o) public InMemoryCacheClient(InMemoryCacheClientOptions? options = null) { + _disposedCancellationToken = _disposedCancellationTokenSource.Token; if (options is null) options = new InMemoryCacheClientOptions(); _shouldClone = options.CloneValues; @@ -70,6 +76,7 @@ public InMemoryCacheClient(InMemoryCacheClientOptions? options = null) _sizeCalculator = options.SizeCalculator; _hasSizeCalculator = _sizeCalculator is not null; + _canShareListValues = !_shouldClone && (!_hasSizeCalculator || _sizeCalculator?.Target is InMemoryCacheClientOptionsBuilder.FixedSizeCalculator); _shouldTrackMemory = _hasSizeCalculator && _maxMemorySize.HasValue; _memory = new ConcurrentDictionary(); @@ -137,19 +144,6 @@ private void UpdateMemorySize(long delta) delta, currentValue, newValue, _maxMemorySize); } - /// - /// Updates memory size when an entry is added or updated. - /// Calculates the size delta based on old and new entry sizes. - /// - private void UpdateMemorySizeForEntry(long newSize, long oldSize, bool wasNewEntry) - { - if (!_shouldTrackMemory) - return; - - long sizeDelta = wasNewEntry ? newSize : (newSize - oldSize); - UpdateMemorySize(sizeDelta); - } - /// /// Recalculates the current memory size by summing all non-expired cache entries. /// @@ -161,16 +155,10 @@ private void UpdateMemorySizeForEntry(long newSize, long oldSize, bool wasNewEnt /// on subsequent operations or recalculations. /// /// - /// Known limitation: If an entry's value is updated during iteration (via the Value setter - /// which resets the estimated size), the calculation could include a mix of old and newly-calculated - /// sizes. This is acceptable for approximate memory tracking. To reduce this window, a snapshot - /// of values is taken at the start of the iteration. - /// - /// - /// Expired entries are excluded from the calculation to ensure accurate memory reporting. + /// Expired entries that are still stored are counted, because removing them later subtracts their size again. /// /// - private long RecalculateMemorySize() + internal long RecalculateMemorySize() { if (!_shouldTrackMemory) return 0; @@ -179,11 +167,7 @@ private long RecalculateMemorySize() var entries = _memory.Values.ToArray(); long totalSize = 0; foreach (var entry in entries) - { - // Skip expired entries to ensure accurate memory reporting - if (!entry.IsExpired) - totalSize += entry.Size; - } + totalSize += entry.Size; Interlocked.Exchange(ref _currentMemorySize, totalSize); return totalSize; @@ -220,7 +204,7 @@ public void ResetStats() private void OnItemExpired(string key, bool sendNotification = true) { - if (ItemExpired is null) + if (!ItemExpired.HasHandlers) return; Task.Factory.StartNew(_ => @@ -249,7 +233,7 @@ public ICollection Keys } } - public ICollection> Items + public ICollection> Items { get { @@ -257,7 +241,7 @@ public ICollection> Items .Where(kvp => !kvp.Value.IsExpired) .OrderBy(kvp => kvp.Value.LastAccessTicks) .ThenBy(kvp => kvp.Value.InstanceNumber) - .Select(kvp => new KeyValuePair(kvp.Key, kvp.Value)) + .Select(kvp => new KeyValuePair(kvp.Key, kvp.Value.PeekValue())) .ToList(); } } @@ -267,16 +251,10 @@ public Task RemoveAsync(string key) ArgumentException.ThrowIfNullOrEmpty(key); _logger.LogTrace("RemoveAsync: Removing key: {Key}", key); - if (!_memory.TryRemove(key, out var entry)) - { - return Task.FromResult(false); - } - - // Key was found and removed. Update memory size. - UpdateMemorySize(-entry.Size); + var removed = UpdateStoredEntry(key, current => (null, current)); // Return false if the entry was expired (consistent with Redis behavior) - return Task.FromResult(!entry.IsExpired); + return Task.FromResult(removed is { IsExpired: false }); } public async Task RemoveIfEqualAsync(string key, T expected) @@ -285,21 +263,9 @@ public async Task RemoveIfEqualAsync(string key, T expected) _logger.LogTrace("RemoveIfEqualAsync Key: {Key} Expected: {Expected}", key, expected); - bool wasExpectedValue = false; - bool success = _memory.TryUpdate(key, (existingKey, existingEntry) => - { - var currentValue = existingEntry.GetValue(); - if (EqualityComparer.Default.Equals(currentValue, expected)) - { - _logger.LogTrace("RemoveIfEqualAsync Key: {Key} Updating ExpiresAt to DateTime.MinValue", existingKey); - existingEntry.ExpiresAt = DateTime.MinValue; - wasExpectedValue = true; - } - - return existingEntry; - }); - - success = success && wasExpectedValue; + bool success = UpdateLease(key, current => EqualityComparer.Default.Equals(current.GetValue(), expected) + ? (null, true) + : null, missResult: false); await StartMaintenanceAsync().AnyContext(); @@ -325,12 +291,8 @@ public Task RemoveAllAsync(IEnumerable? keys = null) ArgumentException.ThrowIfNullOrEmpty(key, nameof(keys)); _logger.LogTrace("RemoveAllAsync: Removing key: {Key}", key); - if (_memory.TryRemove(key, out var removedEntry)) - { + if (UpdateStoredEntry(key, current => (null, current is not null))) removed++; - if (removedEntry != null) - UpdateMemorySize(-removedEntry.Size); - } } return Task.FromResult(removed); @@ -369,42 +331,116 @@ internal long RemoveExpiredKey(string key, bool sendNotification = true) ArgumentException.ThrowIfNullOrEmpty(key); _logger.LogTrace("Removing expired key: {Key}", key); - if (_memory.TryRemove(key, out var removedEntry)) + if (!UpdateStoredEntry(key, current => (null, current is not null))) + return 0; + + OnItemExpired(key, sendNotification); + return 1; + } + + /// + /// Atomically moves the entry for to the state chosen by and + /// keeps the tracked memory size in step. An expired entry is treated as missing, the same as a read: the + /// callback receives null, and when the expired entry is replaced or removed, + /// is raised for it. + /// + /// The cache key. + /// + /// Receives the current live entry (null when the key is missing or expired) and returns the entry the + /// key should hold: the same instance to leave it unchanged, a new instance to add or replace it, or + /// null to remove it. It runs again with the latest entry when another writer changes the key first, + /// so it must not have side effects; only the result of the attempt that was published is returned. + /// + internal TResult UpdateEntry(string key, Func update) + { + bool reclaimedExpired = false; + var result = UpdateStoredEntry(key, stored => { - // Update memory size tracking - UpdateMemorySize(-removedEntry.Size); + // Checked once per attempt, so the decision and the event always agree + reclaimedExpired = stored is { IsExpired: true }; + return update(reclaimedExpired ? null : stored); + }); - OnItemExpired(key, sendNotification); - return 1; - } + if (reclaimedExpired) + OnItemExpired(key); - return 0; + return result; } /// - /// Used by the maintenance task to remove expired keys. + /// Like , but the callback sees the stored entry even when it has expired, and + /// no event is raised. Only maintenance, compaction and explicit removal need this view. /// - private long RemoveKeyIfExpired(string key, bool sendNotification = true) + private TResult UpdateStoredEntry(string key, Func update) { - ArgumentException.ThrowIfNullOrEmpty(key); + // Retries only when another writer changed the key first, so some writer always makes progress; Dispose clears + // the store, which lets in-flight updates finish on the next attempt. + while (true) + { + _memory.TryGetValue(key, out var current); + var (desired, result) = update(current); + if (ReferenceEquals(desired, current) || TryPublish(key, current, desired)) + return result; + } + } - if (_memory.TryGetValue(key, out var existingEntry) && existingEntry.IsExpired) + /// + /// Reclaims a lease whose entry expired while the caller was comparing it, raising + /// only when the reclaiming attempt is the one that was published. + /// + private TResult UpdateLease(string key, Func update, TResult missResult) + { + bool reclaimed = false; + var result = UpdateEntry(key, current => { - if (_memory.TryRemove(key, out var removedEntry)) - { - if (!removedEntry.IsExpired) - throw new Exception("Removed item was not expired"); + reclaimed = false; + if (current is null) + return (current, missResult); - // Update memory size tracking - UpdateMemorySize(-removedEntry.Size); + var decision = update(current); - _logger.LogDebug("Removing expired cache entry {Key}", key); - OnItemExpired(key, sendNotification); - return 1; + // A lease that expired while it was being compared no longer belongs to the caller: reclaim it + if (current.IsExpired) + { + reclaimed = true; + return (null, missResult); } - } - return 0; + return decision ?? (current, missResult); + }); + + if (reclaimed) + OnItemExpired(key); + + return result; + } + + /// + /// Publishes in place of with a compare-and-swap, adjusting + /// tracked memory by the size difference. Returns false, changing nothing, if another writer got there first. + /// + private bool TryPublish(string key, CacheEntry? current, CacheEntry? desired) + { + bool published = (current, desired) switch + { + (null, not null) => _memory.TryAdd(key, desired), + (not null, null) => _memory.TryRemove(new KeyValuePair(key, current)), + _ => _memory.TryUpdate(key, desired!, current!) + }; + + if (published) + UpdateMemorySize((desired?.Size ?? 0) - (current?.Size ?? 0)); + + return published; + } + + /// + /// Removes the entry for only if it is still the exact instance that was observed, + /// so an entry replaced concurrently is never removed by mistake. + /// + internal bool TryRemoveEntry(string key, CacheEntry observed) + { + return UpdateStoredEntry(key, current => ReferenceEquals(current, observed) ? (null, true) : (current, false)); } public Task> GetAsync(string key) @@ -454,23 +490,15 @@ public async Task>> GetAllAsync(IEnumerable public Task AddAsync(string key, T value, TimeSpan? expiresIn = null) { - ArgumentException.ThrowIfNullOrEmpty(key); - - if (expiresIn < CacheClientExtensions.MinimumExpiration) - { - RemoveExpiredKey(key); - return Task.FromResult(false); - } - - DateTime? expiresAt = expiresIn.HasValue ? _timeProvider.GetUtcNow().UtcDateTime.SafeAdd(expiresIn.Value) : null; - var entry = CreateEntry(value, expiresAt); - if (entry is null) - return Task.FromResult(false); // Entry exceeds limits - - return SetInternalAsync(key, entry, addOnly: true); + return WriteAsync(key, value, expiresIn, WriteMode.Add); } public Task SetAsync(string key, T value, TimeSpan? expiresIn = null) + { + return WriteAsync(key, value, expiresIn, WriteMode.Set); + } + + private Task WriteAsync(string key, T value, TimeSpan? expiresIn, WriteMode mode) { ArgumentException.ThrowIfNullOrEmpty(key); @@ -481,269 +509,87 @@ public Task SetAsync(string key, T value, TimeSpan? expiresIn = null) } DateTime? expiresAt = expiresIn.HasValue ? _timeProvider.GetUtcNow().UtcDateTime.SafeAdd(expiresIn.Value) : null; - - // Fast path: when no size calculator, create entry directly (matches main branch behavior) - if (!_hasSizeCalculator) - return SetInternalAsync(key, new CacheEntry(value, expiresAt, _timeProvider, _shouldClone, 0)); - - // Slow path: calculate size and check limits var entry = CreateEntry(value, expiresAt); if (entry is null) return Task.FromResult(false); // Entry exceeds limits - return SetInternalAsync(key, entry); + return SetInternalAsync(key, entry, mode); } - public async Task SetIfHigherAsync(string key, double value, TimeSpan? expiresIn = null) + private enum WriteMode { - ArgumentException.ThrowIfNullOrEmpty(key); - - if (expiresIn < CacheClientExtensions.MinimumExpiration) - { - RemoveExpiredKey(key); - return 0; - } - - DateTime? expiresAt = expiresIn.HasValue ? _timeProvider.GetUtcNow().UtcDateTime.SafeAdd(expiresIn.Value) : null; - var newEntry = CreateEntry(value, expiresAt); - if (newEntry is null) - return 0; - - Interlocked.Increment(ref _writes); - - double difference = value; - long oldSize = 0; - long newSize = 0; - bool wasNewEntry = false; - _memory.AddOrUpdate(key, _ => - { - wasNewEntry = true; - newSize = newEntry.Size; - return newEntry; - }, (_, existingEntry) => - { - oldSize = existingEntry.Size; - double? currentValue = null; - try - { - currentValue = existingEntry.GetValue(); - } - catch (Exception ex) - { - _logger.LogError(ex, "Unable to increment value, expected integer type: {Message}", ex.Message); - } - - if (currentValue.HasValue && currentValue.Value < value) - { - difference = value - currentValue.Value; - existingEntry.Value = value; - // Size remains unchanged for primitive numeric types (double = 8 bytes) - } - else - { - difference = 0; - } - - existingEntry.ExpiresAt = expiresAt; - - newSize = existingEntry.Size; - return existingEntry; - }); - - UpdateMemorySizeForEntry(newSize, oldSize, wasNewEntry); - - await StartMaintenanceAsync().AnyContext(); - - return difference; + /// Always write the value. + Set, + /// Write only when the key is missing or expired. + Add, + /// Write only when the key holds a live value. + Replace } - public async Task SetIfHigherAsync(string key, long value, TimeSpan? expiresIn = null) + public Task SetIfHigherAsync(string key, double value, TimeSpan? expiresIn = null) { - ArgumentException.ThrowIfNullOrEmpty(key); - - if (expiresIn < CacheClientExtensions.MinimumExpiration) - { - RemoveExpiredKey(key); - return 0; - } - - DateTime? expiresAt = expiresIn.HasValue ? _timeProvider.GetUtcNow().UtcDateTime.SafeAdd(expiresIn.Value) : null; - var newEntry = CreateEntry(value, expiresAt); - if (newEntry is null) - return 0; - - Interlocked.Increment(ref _writes); - - long difference = value; - long oldSize = 0; - long newSize = 0; - bool wasNewEntry = false; - _memory.AddOrUpdate(key, _ => - { - wasNewEntry = true; - newSize = newEntry.Size; - return newEntry; - }, (_, existingEntry) => - { - oldSize = existingEntry.Size; - long? currentValue = null; - try - { - currentValue = existingEntry.GetValue(); - } - catch (Exception ex) - { - _logger.LogError(ex, "Unable to increment value, expected integer type"); - } - - if (currentValue.HasValue && currentValue.Value < value) - { - difference = value - currentValue.Value; - existingEntry.Value = value; - // Size remains unchanged for primitive numeric types (long = 8 bytes) - } - else - { - difference = 0; - } - - existingEntry.ExpiresAt = expiresAt; - - newSize = existingEntry.Size; - return existingEntry; - }); + return SetIfAsync(key, value, expiresIn, higher: true); + } - UpdateMemorySizeForEntry(newSize, oldSize, wasNewEntry); + public Task SetIfHigherAsync(string key, long value, TimeSpan? expiresIn = null) + { + return SetIfAsync(key, value, expiresIn, higher: true); + } - await StartMaintenanceAsync().AnyContext(); + public Task SetIfLowerAsync(string key, double value, TimeSpan? expiresIn = null) + { + return SetIfAsync(key, value, expiresIn, higher: false); + } - return difference; + public Task SetIfLowerAsync(string key, long value, TimeSpan? expiresIn = null) + { + return SetIfAsync(key, value, expiresIn, higher: false); } - public async Task SetIfLowerAsync(string key, double value, TimeSpan? expiresIn = null) + private async Task SetIfAsync(string key, T value, TimeSpan? expiresIn, bool higher) where T : struct, INumber { ArgumentException.ThrowIfNullOrEmpty(key); if (expiresIn < CacheClientExtensions.MinimumExpiration) { RemoveExpiredKey(key); - return 0; + return T.Zero; } DateTime? expiresAt = expiresIn.HasValue ? _timeProvider.GetUtcNow().UtcDateTime.SafeAdd(expiresIn.Value) : null; var newEntry = CreateEntry(value, expiresAt); if (newEntry is null) - return 0; + return T.Zero; Interlocked.Increment(ref _writes); - double difference = value; - long oldSize = 0; - long newSize = 0; - bool wasNewEntry = false; - _memory.AddOrUpdate(key, _ => - { - wasNewEntry = true; - newSize = newEntry.Size; - return newEntry; - }, (_, existingEntry) => + T difference = UpdateEntry(key, current => { - oldSize = existingEntry.Size; - double? currentValue = null; - try - { - currentValue = existingEntry.GetValue(); - } - catch (Exception ex) - { - _logger.LogError(ex, "Unable to increment value, expected integer type"); - } + if (current is null) + return (newEntry, value); - if (currentValue.HasValue && currentValue.Value > value) - { - difference = currentValue.Value - value; - existingEntry.Value = value; - // Size remains unchanged for primitive numeric types (double = 8 bytes) - } - else - { - difference = 0; - } - - existingEntry.ExpiresAt = expiresAt; + if (GetNumericValue(current) is { } currentValue && (higher ? currentValue < value : currentValue > value)) + return (current.WithValue(value, expiresAt, newEntry.Size), higher ? value - currentValue : currentValue - value); - newSize = existingEntry.Size; - return existingEntry; + return (current.WithExpiration(expiresAt), T.Zero); }); - UpdateMemorySizeForEntry(newSize, oldSize, wasNewEntry); - await StartMaintenanceAsync().AnyContext(); return difference; } - public async Task SetIfLowerAsync(string key, long value, TimeSpan? expiresIn = null) + private T? GetNumericValue(CacheEntry entry) where T : struct { - ArgumentException.ThrowIfNullOrEmpty(key); - - if (expiresIn < CacheClientExtensions.MinimumExpiration) + try { - RemoveExpiredKey(key); - return 0; + return entry.GetValue(); } - - DateTime? expiresAt = expiresIn.HasValue ? _timeProvider.GetUtcNow().UtcDateTime.SafeAdd(expiresIn.Value) : null; - var newEntry = CreateEntry(value, expiresAt); - if (newEntry is null) - return 0; - - Interlocked.Increment(ref _writes); - - long difference = value; - long oldSize = 0; - long newSize = 0; - bool wasNewEntry = false; - _memory.AddOrUpdate(key, _ => - { - wasNewEntry = true; - newSize = newEntry.Size; - return newEntry; - }, (_, existingEntry) => + catch (Exception ex) { - oldSize = existingEntry.Size; - long? currentValue = null; - try - { - currentValue = existingEntry.GetValue(); - } - catch (Exception ex) - { - _logger.LogError(ex, "Unable to increment value, expected integer type"); - } - - if (currentValue.HasValue && currentValue.Value > value) - { - difference = currentValue.Value - value; - existingEntry.Value = value; - // Size remains unchanged for primitive numeric types (long = 8 bytes) - } - else - { - difference = 0; - } - - existingEntry.ExpiresAt = expiresAt; - - newSize = existingEntry.Size; - return existingEntry; - }); - - UpdateMemorySizeForEntry(newSize, oldSize, wasNewEntry); - - await StartMaintenanceAsync().AnyContext(); - - return difference; + _logger.LogError(ex, "Unable to read cached value as {ValueType}: {Message}", typeof(T).Name, ex.Message); + return null; + } } public async Task ListAddAsync(string key, IEnumerable values, TimeSpan? expiresIn = null) where T : notnull @@ -760,93 +606,82 @@ public async Task ListAddAsync(string key, IEnumerable values, TimeS var utcNow = _timeProvider.GetUtcNow().UtcDateTime; DateTime? expiresAt = expiresIn.HasValue ? utcNow.SafeAdd(expiresIn.Value) : null; - long oldSize = 0; - long newSize = 0; - bool wasNewEntry = false; - - if (values is string stringValue) - { - var items = new Dictionary(StringComparer.OrdinalIgnoreCase) - { - { stringValue, expiresAt } - }; - - var entry = CreateEntry(items, expiresAt); - if (entry is null) - return 0; - - Interlocked.Increment(ref _writes); - - _memory.AddOrUpdate(key, k => - { - wasNewEntry = true; - newSize = entry.Size; - return entry; - }, (existingKey, existingEntry) => - { - if (existingEntry.Value is not IDictionary dictionary) - throw new InvalidOperationException($"Unable to add value for key: {existingKey}. Cache value does not contain a dictionary"); + long added = values is string stringValue + ? ListAdd(key, new Dictionary(StringComparer.OrdinalIgnoreCase) { { stringValue, expiresAt } }, expiresAt) + : ListAdd(key, new HashSet(values.Where(v => v is not null)).ToDictionary(k => k, _ => expiresAt), expiresAt); - oldSize = existingEntry.Size; + if (added > 0) + await StartMaintenanceAsync().AnyContext(); - ExpireListValues(dictionary, existingKey); + return added; + } - dictionary[stringValue] = expiresAt; - existingEntry.Value = dictionary; - existingEntry.ExpiresAt = dictionary.Values.Contains(null) ? null : dictionary.Values.Max(); - existingEntry.Size = _hasSizeCalculator ? CalculateEntrySize(dictionary) : 0; + private long ListAdd(string key, Dictionary items, DateTime? expiresAt) where T : notnull + { + if (items.Count is 0) + return 0; - newSize = existingEntry.Size; - return existingEntry; - }); + var entry = CreateEntry(items, expiresAt); + if (entry is null) + return 0; - UpdateMemorySizeForEntry(newSize, oldSize, wasNewEntry); + Interlocked.Increment(ref _writes); - await StartMaintenanceAsync().AnyContext(); - return items.Count; - } - else + return UpdateEntry(key, current => { - var items = new HashSet(values.Where(v => v is not null)).ToDictionary(k => k, _ => expiresAt); - if (items.Count == 0) - return 0; - - var entry = CreateEntry(items, expiresAt); - if (entry is null) - return 0; - - Interlocked.Increment(ref _writes); + if (current is null) + return (_canShareListValues && items.Count > ListSnapshot.BlockSize + ? entry.WithValue(new ListSnapshot(items), expiresAt, entry.Size) + : entry, (long)items.Count); - _memory.AddOrUpdate(key, k => + if (current.StoredValue is ListSnapshot { Snapshot: null } stored) { - wasNewEntry = true; - newSize = entry.Size; - return entry; - }, (existingKey, existingEntry) => - { - if (existingEntry.Value is not IDictionary dictionary) - throw new InvalidOperationException($"Unable to add value for key: {existingKey}. Cache value does not contain a set"); - - oldSize = existingEntry.Size; - - ExpireListValues(dictionary, existingKey); + var list = stored.Update(items.Keys, expiresAt, _timeProvider.GetUtcNow().UtcDateTime, remove: false, out _); + return (current.WithValue(list, list.ExpiresAt, entry.Size), (long)items.Count); + } - foreach (var kvp in items) - dictionary[kvp.Key] = kvp.Value; + if (CopyListValues(current) is not { } dictionary) + throw new InvalidOperationException($"Unable to add value for key: {key}. Cache value does not contain a set"); - existingEntry.Value = dictionary; - existingEntry.ExpiresAt = dictionary.Values.Contains(null) ? null : dictionary.Values.Max(); - existingEntry.Size = _hasSizeCalculator ? CalculateEntrySize(dictionary) : 0; + // Merge from the new entry's stored values, which were cloned when CloneValues is enabled + ExpireListValues(dictionary, key); + foreach (var kvp in (Dictionary)entry.StoredValue!) + dictionary[kvp.Key] = kvp.Value; - newSize = existingEntry.Size; - return existingEntry; - }); + long size = _hasSizeCalculator ? CalculateEntrySize(dictionary) : 0; + return size < 0 + ? (current, 0L) + : (WithListValues(current, dictionary, size), (long)items.Count); + }); + } - UpdateMemorySizeForEntry(newSize, oldSize, wasNewEntry); + private CacheEntry WithListValues(CacheEntry current, IDictionary dictionary, long size) where T : notnull + { + object value = _canShareListValues && dictionary is Dictionary values && values.Count > ListSnapshot.BlockSize + ? new ListSnapshot(values) + : dictionary; + return current.WithValue(value, GetListExpiration(dictionary), size); + } - await StartMaintenanceAsync().AnyContext(); - return items.Count; - } + /// + /// Returns a private copy of an entry's list values that can be modified and published with + /// , or null if the entry does not hold a list. + /// + private static IDictionary? CopyListValues(CacheEntry entry) where T : notnull + { + object? value = entry.StoredValue is IListSnapshot list ? list.GetSnapshot() : entry.StoredValue; + return value switch + { + Dictionary dictionary => new Dictionary(dictionary, dictionary.Comparer), + SortedDictionary dictionary => new SortedDictionary(dictionary, dictionary.Comparer), + SortedList dictionary => new SortedList(dictionary, dictionary.Comparer), + ConcurrentDictionary dictionary => new ConcurrentDictionary(dictionary, dictionary.Comparer), +#if NET9_0_OR_GREATER + OrderedDictionary dictionary => new OrderedDictionary(dictionary, dictionary.Comparer), +#endif + IDictionary dictionary => throw new NotSupportedException($"List updates cannot preserve the comparer of {dictionary.GetType().FullName}."), + _ => null + }; } private int ExpireListValues(IDictionary dictionary, string existingKey) @@ -860,6 +695,11 @@ private int ExpireListValues(IDictionary dictionary, string exi return expiredValues; } + private static DateTime? GetListExpiration(IDictionary dictionary) + { + return dictionary.Values.Contains(null) ? null : dictionary.Values.Max(); + } + public Task ListRemoveAsync(string key, IEnumerable values) where T : notnull { ArgumentException.ThrowIfNullOrEmpty(key); @@ -867,102 +707,56 @@ public Task ListRemoveAsync(string key, IEnumerable values) where T Interlocked.Increment(ref _writes); - long removed = 0; - long oldSize = 0; - long newSize = 0; - if (values is string stringValue) - { - var items = new HashSet([stringValue]); - _memory.TryUpdate(key, (existingKey, existingEntry) => - { - if (existingEntry.Value is IDictionary { Count: > 0 } dictionary) - { - oldSize = existingEntry.Size; - int expired = ExpireListValues(dictionary, existingKey); - - foreach (string value in items) - { - if (dictionary.Remove(value)) - Interlocked.Increment(ref removed); - } - - if (expired > 0 || removed > 0) - { - existingEntry.Value = dictionary; - if (dictionary.Count > 0) - existingEntry.ExpiresAt = dictionary.Values.Contains(null) ? null : dictionary.Values.Max(); - else - existingEntry.ExpiresAt = DateTime.MinValue; - - existingEntry.Size = _hasSizeCalculator ? CalculateEntrySize(dictionary) : 0; - newSize = existingEntry.Size; - } - else - { - newSize = oldSize; - } - } + return Task.FromResult(ListRemove(key, new HashSet([stringValue]))); - if (removed > 0) - _logger.LogTrace("Removed value from set with cache key: {Key}", existingKey); + var items = new HashSet(values.Where(v => v is not null)); + if (items.Count is 0) + return Task.FromResult(0); - return existingEntry; - }); - - if (_shouldTrackMemory && oldSize != newSize) - UpdateMemorySize(newSize - oldSize); + return Task.FromResult(ListRemove(key, items)); + } - return Task.FromResult(removed); - } - else + private long ListRemove(string key, HashSet items) where T : notnull + { + long removed = UpdateEntry(key, current => { - var items = new HashSet(values.Where(v => v is not null)); - if (items.Count == 0) - return Task.FromResult(0); - - _memory.TryUpdate(key, (existingKey, existingEntry) => + if (current?.StoredValue is ListSnapshot { Snapshot: null } list) { - if (existingEntry.Value is IDictionary { Count: > 0 } dictionary) - { - oldSize = existingEntry.Size; - - int expired = ExpireListValues(dictionary, existingKey); - - foreach (var value in items) - { - if (dictionary.Remove(value)) - Interlocked.Increment(ref removed); - } - - if (expired > 0 || removed > 0) - { - existingEntry.Value = dictionary; - if (dictionary.Count > 0) - existingEntry.ExpiresAt = dictionary.Values.Contains(null) ? null : dictionary.Values.Max(); - else - existingEntry.ExpiresAt = DateTime.MinValue; - - existingEntry.Size = _hasSizeCalculator ? CalculateEntrySize(dictionary) : 0; - newSize = existingEntry.Size; - } - else - { - newSize = oldSize; - } - } - - if (removed > 0) - _logger.LogTrace("Removed value from set with cache key: {Key}", existingKey); + var replacement = list.Update(items, null, _timeProvider.GetUtcNow().UtcDateTime, remove: true, out long count); + return ReferenceEquals(list, replacement) ? (current, 0L) + : (replacement.Count == 0 ? null : current.WithValue(replacement, replacement.ExpiresAt, current.Size), count); + } - return existingEntry; - }); + object? value = current?.StoredValue is IListSnapshot snapshot ? snapshot.GetSnapshot() : current?.StoredValue; + if (current is null || value is not IDictionary { Count: > 0 } stored) + return (current, 0L); + + // A missing-value removal needs no private copy unless expired values also need pruning. + // Use the stored dictionary's comparer, which may differ from the input set's comparer. + var utcNow = _timeProvider.GetUtcNow().UtcDateTime; + if (!items.Any(stored.ContainsKey) && !stored.Any(kvp => kvp.Value < utcNow)) + return (current, 0L); + + var dictionary = CopyListValues(current)!; + int expired = ExpireListValues(dictionary, key); + long removedCount = items.Count(dictionary.Remove); + if (expired is 0 && removedCount is 0) + return (current, 0L); + + if (dictionary.Count is 0) + return (null, removedCount); + + long size = _hasSizeCalculator ? CalculateEntrySize(dictionary) : 0; + return size < 0 + ? (current, 0L) + : (WithListValues(current, dictionary, size), removedCount); + }); - if (_shouldTrackMemory && oldSize != newSize) - UpdateMemorySize(newSize - oldSize); + if (removed > 0) + _logger.LogTrace("Removed value from set with cache key: {Key}", key); - return Task.FromResult(removed); - } + return removed; } public async Task>> GetListAsync(string key, int? page = null, int pageSize = 100) where T : notnull @@ -972,6 +766,20 @@ public async Task>> GetListAsync(string key, int? p if (page is < 1) throw new ArgumentOutOfRangeException(nameof(page), "Page cannot be less than 1"); + if (_memory.TryGetValue(key, out var entry) && !entry.IsExpired && entry.StoredValue is ListSnapshot { Snapshot: null } list) + { + Interlocked.Increment(ref _hits); + entry.RecordAccess(); + var values = list.Read(_timeProvider.GetUtcNow().UtcDateTime); + if (values.Length == 0) + return new CacheValue>([], false); + + if (page.HasValue) + values = values.Skip((page.Value - 1) * pageSize).Take(pageSize).ToArray(); + + return new CacheValue>(values, true); + } + var dictionaryCacheValue = await GetAsync>(key); if (!dictionaryCacheValue.HasValue) return new CacheValue>([], false); @@ -990,7 +798,7 @@ public async Task>> GetListAsync(string key, int? p return new CacheValue>(pagedItems, true); } - private async Task SetInternalAsync(string key, CacheEntry entry, bool addOnly = false) + private async Task SetInternalAsync(string key, CacheEntry entry, WriteMode mode) { ArgumentException.ThrowIfNullOrEmpty(key); @@ -1002,56 +810,15 @@ private async Task SetInternalAsync(string key, CacheEntry entry, bool add Interlocked.Increment(ref _writes); - bool wasUpdated = true; - long oldSize = 0; - - if (addOnly) + bool wasUpdated = UpdateEntry(key, current => mode switch { - _memory.AddOrUpdate(key, entry, (existingKey, existingEntry) => - { - // NOTE: This update factory method will run multiple times if the key is already in the cache, especially during lock contention. - wasUpdated = false; - - // check to see if existing entry is expired - if (existingEntry.IsExpired) - { - _logger.LogTrace("Attempting to replacing expired cache key: {Key}", existingKey); - - oldSize = existingEntry.Size; - wasUpdated = true; - return entry; - } - - return existingEntry; - }); + WriteMode.Add when current is not null => (current, false), + WriteMode.Replace when current is null => (current, false), + _ => (entry, true) + }); - if (wasUpdated) - _logger.LogTrace("Added cache key: {Key}", key); - } - else if (_shouldTrackMemory) - { - // Capture only the size, not the whole entry - _memory.AddOrUpdate(key, entry, (_, existingEntry) => - { - oldSize = existingEntry.Size; - return entry; - }); + if (wasUpdated) _logger.LogTrace("Set cache key: {Key}", key); - } - else - { - // Fast path: no memory tracking needed - _memory.AddOrUpdate(key, entry, (_, _) => entry); - _logger.LogTrace("Set cache key: {Key}", key); - } - - // Update memory size tracking (size was pre-calculated and stored in entry.Size) - if (_shouldTrackMemory) - { - long sizeDelta = entry.Size - oldSize; - if (wasUpdated && sizeDelta != 0) - UpdateMemorySize(sizeDelta); - } // Check if compaction is needed AFTER memory size update await StartMaintenanceAsync(ShouldCompact).AnyContext(); @@ -1100,10 +867,11 @@ public Task ReplaceAsync(string key, T value, TimeSpan? expiresIn = nul { ArgumentException.ThrowIfNullOrEmpty(key); + // Skip sizing and cloning the value when there is nothing to replace; the write itself rechecks atomically if (!_memory.ContainsKey(key)) return Task.FromResult(false); - return SetAsync(key, value, expiresIn); + return WriteAsync(key, value, expiresIn, WriteMode.Replace); } public async Task ReplaceIfEqualAsync(string key, T value, T expected, TimeSpan? expiresIn = null) @@ -1121,31 +889,19 @@ public async Task ReplaceIfEqualAsync(string key, T value, T expected, Interlocked.Increment(ref _writes); DateTime? expiresAt = expiresIn.HasValue ? _timeProvider.GetUtcNow().UtcDateTime.SafeAdd(expiresIn.Value) : null; - bool wasExpectedValue = false; - long oldSize = 0; - long newSize = 0; - bool success = _memory.TryUpdate(key, (_, existingEntry) => - { - var currentValue = existingEntry.GetValue(); - if (EqualityComparer.Default.Equals(currentValue, expected)) - { - oldSize = existingEntry.Size; - existingEntry.Value = value; - existingEntry.Size = _hasSizeCalculator ? CalculateEntrySize(value) : 0; - wasExpectedValue = true; - newSize = existingEntry.Size; - - existingEntry.ExpiresAt = expiresAt; - } - - return existingEntry; - }); - success = success && wasExpectedValue; + // Build the replacement only once the expected value matches, so a mismatch never pays for (or throws + // from) sizing and cloning the new value. It is built at most once, even when the update retries. + CacheEntry? replacement = null; + bool success = UpdateLease(key, current => + { + if (!EqualityComparer.Default.Equals(current.GetValue(), expected)) + return null; - // Update memory size tracking if the value was replaced - if (_shouldTrackMemory && wasExpectedValue && oldSize != newSize) - UpdateMemorySize(newSize - oldSize); + // A lease that expires while it is being sized is reclaimed too, so it is never revived + replacement ??= CreateEntry(value, expiresAt); + return replacement is null || replacement.IsExpired ? null : (replacement, true); + }, missResult: false); await StartMaintenanceAsync().AnyContext(); @@ -1154,117 +910,46 @@ public async Task ReplaceIfEqualAsync(string key, T value, T expected, return success; } - public async Task IncrementAsync(string key, double amount, TimeSpan? expiresIn = null) + public Task IncrementAsync(string key, double amount, TimeSpan? expiresIn = null) { - ArgumentException.ThrowIfNullOrEmpty(key); - - if (expiresIn < CacheClientExtensions.MinimumExpiration) - { - RemoveExpiredKey(key); - return 0; - } - - DateTime? expiresAt = expiresIn.HasValue ? _timeProvider.GetUtcNow().UtcDateTime.SafeAdd(expiresIn.Value) : null; - var newEntry = CreateEntry(amount, expiresAt); - if (newEntry is null) - return 0; - - Interlocked.Increment(ref _writes); - - long oldSize = 0; - long newSize = 0; - bool wasNewEntry = false; - var result = _memory.AddOrUpdate(key, _ => - { - wasNewEntry = true; - newSize = newEntry.Size; - return newEntry; - }, (_, existingEntry) => - { - oldSize = existingEntry.Size; - double? currentValue = null; - try - { - currentValue = existingEntry.GetValue(); - } - catch (Exception ex) - { - _logger.LogError(ex, "Unable to increment value, expected integer type: {Message}", ex.Message); - } - - if (currentValue.HasValue) - existingEntry.Value = currentValue.Value + amount; - else - existingEntry.Value = amount; - - existingEntry.ExpiresAt = expiresAt; - - existingEntry.Size = _hasSizeCalculator ? CalculateEntrySize(existingEntry.Value) : 0; - newSize = existingEntry.Size; - return existingEntry; - }); - - UpdateMemorySizeForEntry(newSize, oldSize, wasNewEntry); - - await StartMaintenanceAsync().AnyContext(); + return IncrementInternalAsync(key, amount, expiresIn); + } - return result.GetValue(); + public Task IncrementAsync(string key, long amount, TimeSpan? expiresIn = null) + { + return IncrementInternalAsync(key, amount, expiresIn); } - public async Task IncrementAsync(string key, long amount, TimeSpan? expiresIn = null) + private async Task IncrementInternalAsync(string key, T amount, TimeSpan? expiresIn) where T : struct, INumber { ArgumentException.ThrowIfNullOrEmpty(key); if (expiresIn < CacheClientExtensions.MinimumExpiration) { RemoveExpiredKey(key); - return 0; + return T.Zero; } DateTime? expiresAt = expiresIn.HasValue ? _timeProvider.GetUtcNow().UtcDateTime.SafeAdd(expiresIn.Value) : null; var newEntry = CreateEntry(amount, expiresAt); if (newEntry is null) - return 0; + return T.Zero; Interlocked.Increment(ref _writes); - long oldSize = 0; - long newSize = 0; - bool wasNewEntry = false; - var result = _memory.AddOrUpdate(key, _ => - { - wasNewEntry = true; - newSize = newEntry.Size; - return newEntry; - }, (_, existingEntry) => + T result = UpdateEntry(key, current => { - oldSize = existingEntry.Size; - long? currentValue = null; - try - { - currentValue = existingEntry.GetValue(); - } - catch (Exception ex) - { - _logger.LogError(ex, "Unable to increment value, expected integer type: {Message}", ex.Message); - } - - if (currentValue.HasValue) - existingEntry.Value = currentValue.Value + amount; - else - existingEntry.Value = amount; - - existingEntry.ExpiresAt = expiresAt; + if (current is null) + return (newEntry, amount); - newSize = existingEntry.Size; - return existingEntry; + T newValue = (GetNumericValue(current) ?? T.Zero) + amount; + long size = _hasSizeCalculator ? CalculateEntrySize(newValue) : 0; + return size < 0 ? (current, T.Zero) : (current.WithValue(newValue, expiresAt, size), newValue); }); - UpdateMemorySizeForEntry(newSize, oldSize, wasNewEntry); - await StartMaintenanceAsync().AnyContext(); - return result.GetValue(); + return result; } public Task ExistsAsync(string key) @@ -1369,12 +1054,22 @@ public async Task SetExpirationAsync(string key, TimeSpan expiresIn) return; } - if (_memory.TryGetValue(key, out var existingEntry) && existingEntry.ExpiresAt != expiresAt) + if (TrySetExpiration(key, expiresAt)) + await StartMaintenanceAsync().AnyContext(); + } + + private bool TrySetExpiration(string key, DateTime? expiresAt) + { + bool changed = UpdateEntry(key, current => { + var updated = current?.WithExpiration(expiresAt); + return (updated, !ReferenceEquals(updated, current)); + }); + + if (changed) Interlocked.Increment(ref _writes); - existingEntry.ExpiresAt = expiresAt; - await StartMaintenanceAsync().AnyContext(); - } + + return changed; } public async Task SetAllExpirationAsync(IDictionary expirations) @@ -1391,17 +1086,10 @@ public async Task SetAllExpirationAsync(IDictionary expiratio { ArgumentException.ThrowIfNullOrEmpty(kvp.Key); - if (!_memory.TryGetValue(kvp.Key, out var existingEntry)) - continue; - if (kvp.Value is null) { - if (existingEntry.ExpiresAt is null) - continue; - - Interlocked.Increment(ref _writes); - existingEntry.ExpiresAt = null; - updated++; + if (TrySetExpiration(kvp.Key, null)) + updated++; } else { @@ -1416,10 +1104,8 @@ public async Task SetAllExpirationAsync(IDictionary expiratio { RemoveExpiredKey(kvp.Key); } - else if (existingEntry.ExpiresAt != expiresAt) + else if (TrySetExpiration(kvp.Key, expiresAt)) { - Interlocked.Increment(ref _writes); - existingEntry.ExpiresAt = expiresAt; updated++; } } @@ -1434,7 +1120,8 @@ public async Task SetAllExpirationAsync(IDictionary expiratio private async Task StartMaintenanceAsync(bool compactImmediately = false) { - _logger.LogTrace("StartMaintenanceAsync called with compactImmediately={CompactImmediately}", compactImmediately); + if (_logger.IsEnabled(LogLevel.Trace)) + _logger.LogTrace("StartMaintenanceAsync called with compactImmediately={CompactImmediately}", compactImmediately); if (_disposedCancellationTokenSource.IsCancellationRequested) return; @@ -1446,7 +1133,7 @@ private async Task StartMaintenanceAsync(bool compactImmediately = false) if (TimeSpan.FromMilliseconds(250) < utcNow - _lastMaintenance) { _lastMaintenance = utcNow; - _ = Task.Run(DoMaintenanceAsync, _disposedCancellationTokenSource.Token); + _ = Task.Run(DoMaintenanceAsync, _disposedCancellationToken); } } @@ -1463,7 +1150,7 @@ private async Task CompactAsync() _logger.LogTrace("CompactAsync: Compacting cache"); var expiredKeys = new List(); - using (await _lock.LockAsync(_disposedCancellationTokenSource.Token).AnyContext()) + using (await _lock.LockAsync(_disposedCancellationToken).AnyContext()) { int removalCount = 0; @@ -1488,7 +1175,8 @@ private async Task CompactAsync() if (maxRemovals > absoluteMaxRemovals) maxRemovals = absoluteMaxRemovals; - while (ShouldCompact && removalCount < maxRemovals) + int conflicts = 0; + while (ShouldCompact && removalCount < maxRemovals && conflicts < maxRemovals) { // Check if we still need compaction bool needsItemCompaction = _maxItems.HasValue && _memory.Count > _maxItems; @@ -1499,30 +1187,26 @@ private async Task CompactAsync() // For memory compaction, prefer size-aware eviction // For item compaction, prefer traditional LRU - string? keyToRemove = needsMemoryCompaction ? FindWorstSizeToUsageRatio() : FindLeastRecentlyUsed(); + var candidate = needsMemoryCompaction ? FindWorstSizeToUsageRatio() : FindLeastRecentlyUsed(); - if (keyToRemove == null) + if (candidate is not { } entryToRemove) break; _logger.LogDebug("Removing cache entry {Key} due to cache exceeding limit (Items: {ItemCount}/{MaxItems}, Memory: {MemorySize:N0}/{MaxMemorySize:N0})", - keyToRemove, _memory.Count, _maxItems, _currentMemorySize, _maxMemorySize); + entryToRemove.Key, _memory.Count, _maxItems, _currentMemorySize, _maxMemorySize); - if (_memory.TryRemove(keyToRemove, out var cacheEntry)) + if (!TryRemoveEntry(entryToRemove.Key, entryToRemove.Value)) { - // Update memory size tracking - if (cacheEntry != null) - UpdateMemorySize(-cacheEntry.Size); + // The entry changed since it was selected. Losing that race frees nothing, so it must not use up + // the removal budget; conflicts have their own cap so a pass under heavy contention still ends. + conflicts++; + continue; + } - if (cacheEntry is { IsExpired: true }) - expiredKeys.Add(keyToRemove); + if (entryToRemove.Value.IsExpired) + expiredKeys.Add(entryToRemove.Key); - removalCount++; - } - else - { - // Couldn't remove the item, break to prevent infinite loop - break; - } + removalCount++; } // Log if we hit maxRemovals but still need compaction @@ -1541,23 +1225,22 @@ private async Task CompactAsync() OnItemExpired(expiredKey); } - private string? FindLeastRecentlyUsed() + private KeyValuePair? FindLeastRecentlyUsed() { - (string? Key, long LastAccessTicks, long InstanceNumber) oldest = (null, Int64.MaxValue, 0); + KeyValuePair? oldest = null; foreach (var kvp in _memory) { - bool isExpired = kvp.Value.IsExpired; - if (isExpired || - kvp.Value.LastAccessTicks < oldest.LastAccessTicks || - (kvp.Value.LastAccessTicks == oldest.LastAccessTicks && kvp.Value.InstanceNumber < oldest.InstanceNumber)) - oldest = (kvp.Key, kvp.Value.LastAccessTicks, kvp.Value.InstanceNumber); + if (kvp.Value.IsExpired) + return kvp; - if (isExpired) - break; + if (oldest is not { } current || + kvp.Value.LastAccessTicks < current.Value.LastAccessTicks || + (kvp.Value.LastAccessTicks == current.Value.LastAccessTicks && kvp.Value.InstanceNumber < current.Value.InstanceNumber)) + oldest = kvp; } - return oldest.Key; + return oldest; } /// @@ -1593,10 +1276,10 @@ private async Task CompactAsync() /// to bound the cache size at a level where O(n) iteration remains acceptable. /// /// - /// The key of the entry to evict, or null if no suitable candidate found. - private string? FindWorstSizeToUsageRatio() + /// The entry to evict, or null if no suitable candidate found. + private KeyValuePair? FindWorstSizeToUsageRatio() { - string? candidateKey = null; + KeyValuePair? candidate = null; double worstRatio = Double.MinValue; // Start with minimum value so any score can win long currentTime = _timeProvider.GetUtcNow().Ticks; @@ -1606,7 +1289,7 @@ private async Task CompactAsync() { // Prioritize expired items first if (kvp.Value.IsExpired) - return kvp.Key; + return kvp; // Calculate a "waste score" based on size vs recent usage long size = kvp.Value.Size; @@ -1627,41 +1310,30 @@ private async Task CompactAsync() if (wasteScore > worstRatio) { worstRatio = wasteScore; - candidateKey = kvp.Key; + candidate = kvp; } } - _logger.LogTrace("FindWorstSizeToUsageRatio: Selected {Key} with score {Score}", candidateKey, worstRatio); - return candidateKey; + _logger.LogTrace("FindWorstSizeToUsageRatio: Selected {Key} with score {Score}", candidate?.Key, worstRatio); + return candidate; } - private async Task DoMaintenanceAsync() + internal async Task DoMaintenanceAsync() { _logger.LogTrace("DoMaintenance: Starting"); - // NOTE: We want to ensure we are comparing DateTimes to avoid using the systems timezone for implicit conversion between DateTime and DateTimeOffset. - var utcNow = _timeProvider.GetUtcNow().UtcDateTime.SafeAddMilliseconds(50); - - // Remove expired items and items that are infrequently accessed as they may be updated by add. - long lastAccessMaximumTicks = utcNow.SafeAddMilliseconds(-300).Ticks; - try { - foreach (var kvp in _memory.ToArray()) + // Sample UTC once per pass, using the same strict expiration boundary as reads. + // Concurrent enumeration is safe; remove only the observed entry, not a later replacement. + var utcNow = _timeProvider.GetUtcNow().UtcDateTime; + foreach (var kvp in _memory) { - bool lastAccessTimeIsInfrequent = kvp.Value.LastAccessTicks < lastAccessMaximumTicks; - if (!lastAccessTimeIsInfrequent) - continue; - - var expiresAt = kvp.Value.ExpiresAt; - if (!expiresAt.HasValue) + if (kvp.Value.ExpiresAt is not { } expiresAt || expiresAt >= utcNow || !TryRemoveEntry(kvp.Key, kvp.Value)) continue; - if (expiresAt < DateTime.MaxValue && expiresAt <= utcNow) - { - _logger.LogDebug("DoMaintenance: Removing expired key {Key}", kvp.Key); - RemoveKeyIfExpired(kvp.Key); - } + _logger.LogDebug("DoMaintenance: Removed expired key {Key}", kvp.Key); + OnItemExpired(kvp.Key); } } catch (Exception ex) @@ -1706,9 +1378,251 @@ public virtual void Dispose() return new CacheEntry(value, expiresAt, _timeProvider, _shouldClone, size); } - private sealed record CacheEntry + private interface IListSnapshot + { + object GetSnapshot(); + } + + // Updates copy one bounded block and the block directory, not the entire list. The immutable + // key index shares unchanged nodes. Raw dictionary reads retain their existing public shape. + private sealed class ListSnapshot : IListSnapshot where T : notnull + { + public const int BlockSize = 32; + private ImmutableDictionary? _indices; + private readonly Dictionary? _initialValues; + private readonly Item[][] _blocks; + private readonly ImmutableSortedDictionary _expirations; + private readonly ImmutableStack _freeIndices; + private readonly int _nextIndex; + private readonly int _permanentCount; + private Dictionary? _snapshot; + + private readonly record struct Item(T Key, DateTime? Expiration, bool Present); + + public ListSnapshot(Dictionary values) + { + _initialValues = values; + var expirations = ImmutableSortedDictionary.CreateBuilder(); + _blocks = new Item[(values.Count + BlockSize - 1) / BlockSize][]; + for (int i = 0; i < _blocks.Length; i++) + _blocks[i] = new Item[BlockSize]; + foreach (var pair in values) + { + _blocks[_nextIndex / BlockSize][_nextIndex % BlockSize] = new Item(pair.Key, pair.Value, true); + _nextIndex++; + if (pair.Value is { } expiration) + expirations[expiration] = expirations.GetValueOrDefault(expiration) + 1; + else + _permanentCount++; + } + _expirations = expirations.ToImmutable(); + _freeIndices = ImmutableStack.Empty; + } + + private ListSnapshot(ImmutableDictionary indices, Item[][] blocks, + ImmutableSortedDictionary expirations, ImmutableStack freeIndices, int nextIndex, int permanentCount) + { + _indices = indices; + _blocks = blocks; + _expirations = expirations; + _freeIndices = freeIndices; + _nextIndex = nextIndex; + _permanentCount = permanentCount; + } + + public int Count => _indices?.Count ?? _initialValues!.Count; + private ImmutableDictionary Indices + { + get + { + if (Volatile.Read(ref _indices) is { } existing) + return existing; + + var builder = ImmutableDictionary.CreateBuilder(_initialValues!.Comparer); + int index = 0; + foreach (var key in _initialValues.Keys) + builder.Add(key, index++); + var indices = builder.ToImmutable(); + return Interlocked.CompareExchange(ref _indices, indices, null) ?? indices; + } + } + + public Dictionary? Snapshot => Volatile.Read(ref _snapshot); + public DateTime? ExpiresAt => _permanentCount > 0 || _expirations.IsEmpty ? null : _expirations.Last().Key; + + public T[] Read(DateTime utcNow) + { + var result = new T[Count]; + int count = 0; + foreach (var block in _blocks) + foreach (var item in block) + if (item.Present && (item.Expiration is null || item.Expiration >= utcNow)) + result[count++] = item.Key; + if (count != result.Length) + Array.Resize(ref result, count); + return result; + } + + public object GetSnapshot() + { + if (Snapshot is { } snapshot) + return snapshot; + + if (_initialValues is not null) + return Interlocked.CompareExchange(ref _snapshot, _initialValues, null) ?? _initialValues; + + var dictionary = CopyValues(); + return Interlocked.CompareExchange(ref _snapshot, dictionary, null) ?? dictionary; + } + + private Dictionary CopyValues() + { + if (_initialValues is not null) + return new Dictionary(_initialValues, _initialValues.Comparer); + + var dictionary = new Dictionary(Count, Indices.KeyComparer); + foreach (var block in _blocks) + foreach (var item in block) + if (item.Present) + dictionary.Add(item.Key, item.Expiration); + return dictionary; + } + + public ListSnapshot Update(ICollection items, DateTime? expiration, DateTime utcNow, bool remove, out long removed) + { + removed = 0; + bool prune = !_expirations.IsEmpty && _expirations.First().Key < utcNow; + if (remove && !prune && !items.Any(key => _initialValues?.ContainsKey(key) ?? Indices.ContainsKey(key))) + return this; + + // When a batch could copy every block, one dictionary copy is cheaper than + // rebuilding many index paths. It also avoids indexing short-lived bulk lists. + if (items.Count >= Math.Max(1, Count / BlockSize)) + { + var dictionary = CopyValues(); + if (prune) + foreach (var key in dictionary.Where(pair => pair.Value < utcNow).Select(pair => pair.Key).ToArray()) + dictionary.Remove(key); + if (remove) + removed = items.Count(dictionary.Remove); + else + foreach (var key in items) + dictionary[key] = expiration; + return new ListSnapshot(dictionary); + } + + var indices = Indices.ToBuilder(); + int capacity = Math.Max(_nextIndex, Count + (remove ? 0 : items.Count)); + var blocks = new Item[(capacity + BlockSize - 1) / BlockSize][]; + Array.Copy(_blocks, blocks, _blocks.Length); + var copied = new bool[blocks.Length]; + ImmutableSortedDictionary.Builder? expirations = null; + var freeIndices = _freeIndices; + int nextIndex = _nextIndex; + int permanentCount = _permanentCount; + bool changed = false; + + if (prune) + foreach (var block in _blocks) + foreach (var item in block) + if (item.Present && item.Expiration < utcNow) + Remove(item.Key); + + foreach (var item in items) + { + if (remove) + { + if (Remove(item)) + removed++; + continue; + } + + T key = item; + bool exists = indices.TryGetValue(key, out int index); + if (exists) + { + var previous = blocks[index / BlockSize][index % BlockSize]; + if (previous.Expiration == expiration) + continue; + key = previous.Key; + AdjustExpiration(previous.Expiration, -1); + } + else if (!freeIndices.IsEmpty) + { + index = freeIndices.Peek(); + freeIndices = freeIndices.Pop(); + } + else + index = nextIndex++; + + CopyBlock(index); + blocks[index / BlockSize][index % BlockSize] = new Item(key, expiration, true); + indices[key] = index; + AdjustExpiration(expiration, 1); + } + + if (!changed) + return this; + + int blockCount = (nextIndex + BlockSize - 1) / BlockSize; + if (blocks.Length != blockCount) + Array.Resize(ref blocks, blockCount); + return new ListSnapshot(indices.ToImmutable(), blocks, expirations?.ToImmutable() ?? _expirations, freeIndices, nextIndex, permanentCount); + + bool Remove(T key) + { + if (!indices.TryGetValue(key, out int index)) + return false; + CopyBlock(index); + AdjustExpiration(blocks[index / BlockSize][index % BlockSize].Expiration, -1); + blocks[index / BlockSize][index % BlockSize] = default; + indices.Remove(key); + freeIndices = freeIndices.Push(index); + return true; + } + + void CopyBlock(int index) + { + int blockIndex = index / BlockSize; + if (copied[blockIndex]) + return; + blocks[blockIndex] = blockIndex < _blocks.Length ? (Item[])_blocks[blockIndex].Clone() : new Item[BlockSize]; + copied[blockIndex] = true; + changed = true; + } + + void AdjustExpiration(DateTime? value, int delta) + { + if (value is not { } time) + { + permanentCount += delta; + return; + } + expirations ??= _expirations.ToBuilder(); + int count = expirations.GetValueOrDefault(time) + delta; + if (count == 0) + expirations.Remove(time); + else + expirations[time] = count; + } + } + } + + /// + /// A cache entry's value, expiration and size never change after it is published to the cache dictionary; + /// changes publish a copy (, ). This lets conditional removals + /// compare by reference and guarantees the entry they checked is the entry they remove. Only access metadata + /// () is updated in place, and it does not participate in equality. + /// + /// + /// Keep this a class with reference equality. compares entries with + /// in its compare-and-swap operations, so making this a record or + /// implementing would let replace or remove an + /// entry it never observed. + /// + internal sealed class CacheEntry { - private object? _cacheValue; + private readonly object? _cacheValue; private static long _instanceCount; private readonly bool _shouldClone; private readonly TimeProvider _timeProvider; @@ -1730,16 +1644,28 @@ public CacheEntry(object? value, DateTime? expiresAt, TimeProvider timeProvider, Size = size; } - internal long InstanceNumber { get; private set; } - internal DateTime? ExpiresAt { get; set; } + private CacheEntry(CacheEntry source, object? cacheValue, DateTime? expiresAt, long size, long lastModifiedTicks) + { + _timeProvider = source._timeProvider; + _shouldClone = source._shouldClone; + _cacheValue = cacheValue; + LastAccessTicks = source.LastAccessTicks; + LastModifiedTicks = lastModifiedTicks; + ExpiresAt = expiresAt; + InstanceNumber = source.InstanceNumber; + Size = size; + } + + internal long InstanceNumber { get; } + internal DateTime? ExpiresAt { get; } internal bool IsExpired => ExpiresAt.HasValue && ExpiresAt < _timeProvider.GetUtcNow().UtcDateTime; internal long LastAccessTicks { get; private set; } - internal long LastModifiedTicks { get; private set; } + internal long LastModifiedTicks { get; } /// /// The size of this cache entry in bytes. Set at construction time. /// - internal long Size { get; set; } + internal long Size { get; } #if DEBUG internal long UsageCount => _usageCount; @@ -1749,19 +1675,53 @@ internal object? Value { get { - LastAccessTicks = _timeProvider.GetUtcNow().Ticks; + RecordAccess(); + return PeekValue(); + } + } + + internal void RecordAccess() + { + LastAccessTicks = _timeProvider.GetUtcNow().Ticks; #if DEBUG - Interlocked.Increment(ref _usageCount); + Interlocked.Increment(ref _usageCount); #endif - return _shouldClone ? _cacheValue.DeepClone() : _cacheValue; - } - set - { - _cacheValue = _shouldClone ? value.DeepClone() : value; - var utcNow = _timeProvider.GetUtcNow(); - LastAccessTicks = utcNow.Ticks; - LastModifiedTicks = utcNow.Ticks; - } + } + + /// + /// Returns the value without recording an access, so diagnostic reads do not affect eviction order. + /// + internal object? PeekValue() + { + object? value = _cacheValue is IListSnapshot list ? list.GetSnapshot() : _cacheValue; + return _shouldClone ? value.DeepClone() : value; + } + + /// + /// The stored payload without cloning. It is shared with every reader and must never be mutated. + /// + internal object? StoredValue => _cacheValue; + + /// + /// Returns a copy of this entry with a new expiration, or this entry when the expiration is unchanged. + /// The value is shared, not cloned again. + /// + internal CacheEntry WithExpiration(DateTime? expiresAt) + { + if (expiresAt == ExpiresAt) + return this; + + return new CacheEntry(this, _cacheValue, expiresAt, Size, LastModifiedTicks); + } + + /// + /// Returns a copy of this entry with a new value, size and expiration, marked as modified now. + /// The value is stored as-is, so it must be an instance nothing else references. + /// + internal CacheEntry WithValue(object? value, DateTime? expiresAt, long size) + { + long utcNowTicks = _timeProvider.GetUtcNow().Ticks; + return new CacheEntry(this, value, expiresAt, size, utcNowTicks) { LastAccessTicks = utcNowTicks }; } [return: MaybeNull] @@ -1872,4 +1832,3 @@ public class ItemExpiredEventArgs : EventArgs public required string Key { get; set; } public bool SendNotification { get; set; } } - diff --git a/src/Foundatio/Caching/InMemoryCacheClientOptions.cs b/src/Foundatio/Caching/InMemoryCacheClientOptions.cs index 68cd5afb0..a7b6ad688 100644 --- a/src/Foundatio/Caching/InMemoryCacheClientOptions.cs +++ b/src/Foundatio/Caching/InMemoryCacheClientOptions.cs @@ -101,7 +101,7 @@ public InMemoryCacheClientOptionsBuilder SizeCalculator(Func sizeC public InMemoryCacheClientOptionsBuilder WithFixedSizing(long maxMemorySize, long averageEntrySize) { Target.MaxMemorySize = maxMemorySize; - Target.SizeCalculator = _ => averageEntrySize; + Target.SizeCalculator = new FixedSizeCalculator(averageEntrySize).Calculate; return this; } @@ -171,4 +171,9 @@ public InMemoryCacheClientOptionsBuilder ShouldThrowOnSerializationError(bool sh Target.ShouldThrowOnSerializationError = shouldThrow; return this; } + + internal sealed class FixedSizeCalculator(long size) + { + public long Calculate(object value) => size; + } } diff --git a/tests/Foundatio.Tests/Caching/InMemoryCacheClientCompactionTests.cs b/tests/Foundatio.Tests/Caching/InMemoryCacheClientCompactionTests.cs new file mode 100644 index 000000000..6f1ec4bc6 --- /dev/null +++ b/tests/Foundatio.Tests/Caching/InMemoryCacheClientCompactionTests.cs @@ -0,0 +1,121 @@ +using System; +using System.Threading.Tasks; +using Foundatio.Caching; +using Foundatio.Xunit; +using Microsoft.Extensions.Logging; +using Microsoft.Extensions.Time.Testing; +using Xunit; + +namespace Foundatio.Tests.Caching; + +public class InMemoryCacheClientCompactionTests : TestWithLoggingBase +{ + public InMemoryCacheClientCompactionTests(ITestOutputHelper output) : base(output) + { + } + + [Theory] + [InlineData(false)] + [InlineData(true)] + public async Task DoMaintenanceAsync_WithMoreEntriesThanPassBudget_CompactsAcrossBoundedPasses(bool useMemoryLimit) + { + // Arrange + const int entryCount = 1101; + const int maximumRemovalsPerPass = 1000; + const long entrySize = 50; + var timeProvider = new FakeTimeProvider(DateTimeOffset.UtcNow); + using var cache = new InMemoryCacheClient(o => + { + o.TimeProvider(timeProvider).LoggerFactory(Log); + return useMemoryLimit ? o.WithFixedSizing(entrySize, entrySize) : o.MaxItems(1); + }); + + // Seed directly so normal writes do not compact the cache before the pass under test. + for (int i = 0; i < entryCount; i++) + { + var entry = new InMemoryCacheClient.CacheEntry("value", null, timeProvider, shouldClone: false, size: entrySize); + cache.UpdateEntry(i.ToString(), _ => (entry, true)); + } + + // Act + await cache.DoMaintenanceAsync().WaitAsync(TimeSpan.FromSeconds(20), TestContext.Current.CancellationToken); + int countAfterFirstPass = cache.Count; + long sizeAfterFirstPass = cache.CurrentMemorySize; + await cache.DoMaintenanceAsync().WaitAsync(TimeSpan.FromSeconds(20), TestContext.Current.CancellationToken); + + // Assert + Assert.Equal(entryCount - maximumRemovalsPerPass, countAfterFirstPass); + Assert.Equal(useMemoryLimit ? countAfterFirstPass * entrySize : 0, sizeAfterFirstPass); + Assert.Equal(1, cache.Count); + Assert.Equal(useMemoryLimit ? entrySize : 0, cache.CurrentMemorySize); + Assert.Equal("value", Assert.Single(cache.Items).Value); + Assert.DoesNotContain(Log.LogEntries, entry => entry.LogLevel >= LogLevel.Error); + } + + [Fact] + public async Task DoMaintenanceAsync_WhenEvictionCandidateIsReplaced_DoesNotSpendRemovalBudget() + { + // Arrange: 10 entries over the limit, so the pass's budget is 20 removals. The oldest entry is replaced + // after compaction selects it, 15 times. Losing those races frees nothing, so the pass must still make the + // 10 removals it needs instead of stopping after 20 attempts. + const int maxItems = 100; + const int replacementsDuringSelection = 15; + var timeProvider = new FakeTimeProvider(DateTimeOffset.UtcNow); + using var cache = new InMemoryCacheClient(o => o.TimeProvider(timeProvider).MaxItems(maxItems).LoggerFactory(Log)); + int replacements = 0; + var oldestClock = new ReadHookTimeProvider(timeProvider); + oldestClock.OnRead = () => + { + // Keep the replacement's age and instance number (WithExpiration), so it stays the next candidate + if (replacements < replacementsDuringSelection) + cache.UpdateEntry("oldest", current => current is null ? (current, false) : (current.WithExpiration(current.ExpiresAt!.Value.AddTicks(1)), true)); + replacements++; + }; + var oldest = new InMemoryCacheClient.CacheEntry("value", DateTime.MaxValue.AddDays(-1), oldestClock, shouldClone: false); + cache.UpdateEntry("oldest", _ => (oldest, true)); + for (int i = 1; i < maxItems + 10; i++) + { + var entry = new InMemoryCacheClient.CacheEntry("value", null, timeProvider, shouldClone: false); + cache.UpdateEntry(i.ToString(), _ => (entry, true)); + } + + // Act + oldestClock.Armed = true; + await cache.DoMaintenanceAsync().WaitAsync(TimeSpan.FromSeconds(20), TestContext.Current.CancellationToken); + oldestClock.Armed = false; + + // Assert + Assert.True(replacements > replacementsDuringSelection, $"Expected {replacementsDuringSelection} lost races, saw {replacements}"); + Assert.Equal(maxItems, cache.Count); + } + + /// + /// A clock for one entry that runs when the entry checks its expiration, which compaction + /// does while choosing what to evict. That lets a test replace the entry between selection and removal. + /// + private sealed class ReadHookTimeProvider(TimeProvider inner) : TimeProvider + { + private bool _inHook; + + public bool Armed { get; set; } + public Action? OnRead { get; set; } + + public override DateTimeOffset GetUtcNow() + { + if (Armed && !_inHook && OnRead is { } onRead) + { + _inHook = true; + try + { + onRead(); + } + finally + { + _inHook = false; + } + } + + return inner.GetUtcNow(); + } + } +} diff --git a/tests/Foundatio.Tests/Caching/InMemoryCacheClientTests.cs b/tests/Foundatio.Tests/Caching/InMemoryCacheClientTests.cs index de8146f13..ac12520dd 100644 --- a/tests/Foundatio.Tests/Caching/InMemoryCacheClientTests.cs +++ b/tests/Foundatio.Tests/Caching/InMemoryCacheClientTests.cs @@ -1,5 +1,9 @@ using System; +using System.Collections.Concurrent; +using System.Collections.Generic; +using System.Collections.ObjectModel; using System.Linq; +using System.Threading; using System.Threading.Tasks; using Foundatio.Caching; using Microsoft.Extensions.Logging; @@ -19,6 +23,12 @@ protected override ICacheClient GetCacheClient(bool shouldThrowOnSerializationEr return new InMemoryCacheClient(o => o.LoggerFactory(Log).CloneValues(true).ShouldThrowOnSerializationError(shouldThrowOnSerializationError)); } + [Fact] + public override Task AddAsync_WithConcurrentAcquireAndRelease_OnlyOneOwnerAtATime() + { + return base.AddAsync_WithConcurrentAcquireAndRelease_OnlyOneOwnerAtATime(); + } + [Fact] public override Task AddAsync_WithConcurrentRequests_OnlyOneSucceeds() { @@ -43,6 +53,71 @@ public override Task CacheOperations_WithRepeatedSetAndGet_MeasuresThroughput() return base.CacheOperations_WithRepeatedSetAndGet_MeasuresThroughput(); } + [Theory] + [InlineData(false, false)] + [InlineData(false, true)] + [InlineData(true, false)] + [InlineData(true, true)] + public async Task ConditionalMutation_WithLiveIncompatibleValue_PreservesValueAndFaultsTaskAsync(bool replace, bool throwingEquality) + { + // Arrange + using var cache = new InMemoryCacheClient(o => o.LoggerFactory(Log).CloneValues(false)); + var value = new ComparedValue("owner", () => throw new InvalidOperationException("Comparison failed")); + object stored = throwingEquality ? value : new object(); + await cache.SetAsync("key", stored); + Task? result = null; + + // Act + var synchronousError = Record.Exception(() => + { + result = replace + ? cache.ReplaceIfEqualAsync("key", value, value) + : cache.RemoveIfEqualAsync("key", value); + }); + + // Assert + Assert.Null(synchronousError); + Assert.NotNull(result); + if (throwingEquality) + await Assert.ThrowsAsync(() => result); + else + await Assert.ThrowsAsync(() => result); + Assert.Same(stored, (await cache.GetAsync("key")).Value); + } + + [Theory] + [InlineData(false, false, null)] + [InlineData(false, false, 2)] + [InlineData(false, true, null)] + [InlineData(false, true, 2)] + [InlineData(true, false, null)] + [InlineData(true, false, 2)] + [InlineData(true, true, null)] + [InlineData(true, true, 2)] + public async Task ConditionalNumericUpdate_WithUnchangedValue_UpdatesExpiration(bool lower, bool useDouble, int? expirationMinutes) + { + // Arrange + var timeProvider = new FakeTimeProvider(DateTimeOffset.UtcNow); + using var cache = new InMemoryCacheClient(o => o.TimeProvider(timeProvider).WithFixedSizing(10_000, 50).LoggerFactory(Log)); + await cache.SetAsync("key", 10L, TimeSpan.FromMinutes(1)); + TimeSpan? expiresIn = expirationMinutes.HasValue ? TimeSpan.FromMinutes(expirationMinutes.Value) : null; + + // Act + double difference = (lower, useDouble) switch + { + (true, true) => await cache.SetIfLowerAsync("key", 15d, expiresIn), + (true, false) => await cache.SetIfLowerAsync("key", 15L, expiresIn), + (false, true) => await cache.SetIfHigherAsync("key", 5d, expiresIn), + _ => await cache.SetIfHigherAsync("key", 5L, expiresIn) + }; + + // Assert + Assert.Equal(0, difference); + Assert.Equal(10L, (await cache.GetAsync("key")).Value); + Assert.Equal(expiresIn, await cache.GetExpirationAsync("key")); + Assert.Equal(50, cache.CurrentMemorySize); + } + [Fact] public override Task ExistsAsync_WithVariousKeys_ReturnsCorrectExistenceStatus() { @@ -177,6 +252,12 @@ public override Task GetUnixTimeSecondsAsync_WithUtcDateTime_ReturnsCorrectly() return base.GetUnixTimeSecondsAsync_WithUtcDateTime_ReturnsCorrectly(); } + [Fact] + public override Task IncrementAsync_WithConcurrentRequests_DoesNotLoseUpdates() + { + return base.IncrementAsync_WithConcurrentRequests_DoesNotLoseUpdates(); + } + [Fact] public override Task IncrementAsync_WithExpiration_SetsExpirationCorrectly() { @@ -195,6 +276,35 @@ public override Task IncrementAsync_WithKey_IncrementsCorrectly() return base.IncrementAsync_WithKey_IncrementsCorrectly(); } + [Theory] + [InlineData(false, false)] + [InlineData(false, true)] + [InlineData(true, false)] + [InlineData(true, true)] + public async Task IncrementAsync_WithRejectedResult_PreservesEntry(bool useDouble, bool shouldThrow) + { + // Arrange + var timeProvider = new FakeTimeProvider(DateTimeOffset.UtcNow); + using var cache = new InMemoryCacheClient(o => o.TimeProvider(timeProvider).LoggerFactory(Log).MaxMemorySize(1000) + .SizeCalculator(value => Convert.ToInt64(value)).MaxEntrySize(10) + .ShouldThrowOnMaxEntrySizeExceeded(shouldThrow).LoggerFactory(Log)); + await cache.SetAsync("key", 9L, TimeSpan.FromMinutes(1)); + + // Act + async Task IncrementAsync() => useDouble + ? await cache.IncrementAsync("key", 2d, TimeSpan.FromMinutes(5)) + : await cache.IncrementAsync("key", 2L, TimeSpan.FromMinutes(5)); + if (shouldThrow) + await Assert.ThrowsAsync(IncrementAsync); + else + Assert.Equal(0, await IncrementAsync()); + + // Assert + Assert.Equal(9, (await cache.GetAsync("key")).Value); + Assert.Equal(TimeSpan.FromMinutes(1), await cache.GetExpirationAsync("key")); + Assert.Equal(9, cache.CurrentMemorySize); + } + [Fact] public override Task IncrementAsync_WithScopedCache_WorksWithinScope() { @@ -207,30 +317,408 @@ public override Task IncrementAsync_WithFloatingPointDecimals_PreservesDecimalPr return base.IncrementAsync_WithFloatingPointDecimals_PreservesDecimalPrecision(); } + [Fact] + public async Task Items_WhenRead_DoesNotChangeEvictionOrder() + { + // Arrange + var timeProvider = new FakeTimeProvider(DateTimeOffset.UtcNow); + using var cache = new InMemoryCacheClient(o => o.MaxItems(2).TimeProvider(timeProvider).LoggerFactory(Log)); + await cache.SetAsync("first", "1"); + timeProvider.Advance(TimeSpan.FromMilliseconds(100)); + await cache.SetAsync("second", "2"); + timeProvider.Advance(TimeSpan.FromMilliseconds(100)); + + // Act + Assert.Equal(2, cache.Items.Count); + timeProvider.Advance(TimeSpan.FromMilliseconds(100)); + await cache.SetAsync("third", "3"); + + // Assert + Assert.False(await cache.ExistsAsync("first")); + Assert.True(await cache.ExistsAsync("second")); + Assert.True(await cache.ExistsAsync("third")); + } + + [Fact] + public async Task Items_WithCachedValue_ReturnsValueNotInternalEntry() + { + // Arrange + using var cache = new InMemoryCacheClient(o => o.LoggerFactory(Log)); + await cache.SetAsync("key", "value"); + + // Act + var item = Assert.Single(cache.Items); + + // Assert + Assert.Equal("key", item.Key); + Assert.Equal("value", item.Value); + } + + [Theory] + [InlineData(false)] + [InlineData(true)] + public async Task Items_WithMutableValue_HonorsCloneValues(bool cloneValues) + { + // Arrange + using var cache = new InMemoryCacheClient(o => o.CloneValues(cloneValues).LoggerFactory(Log)); + var original = new SimpleModel { Data1 = "original" }; + await cache.SetAsync("key", original); + + // Act + var item = Assert.Single(cache.Items); + var exposed = Assert.IsType(item.Value); + exposed.Data1 = "changed"; + + // Assert + Assert.Equal("key", item.Key); + Assert.Equal(cloneValues ? "original" : "changed", (await cache.GetAsync("key")).Value!.Data1); + if (cloneValues) + Assert.NotSame(original, exposed); + else + Assert.Same(original, exposed); + } + + [Fact] + public async Task Items_WithNullValue_ReturnsCachedNull() + { + // Arrange + using var cache = new InMemoryCacheClient(o => o.LoggerFactory(Log)); + await cache.SetAsync("key", null); + + // Act + var item = Assert.Single(cache.Items); + + // Assert + Assert.Equal("key", item.Key); + Assert.Null(item.Value); + } + + [Fact] + public override Task SetIfHigherAsync_WithConcurrentRequests_DifferencesSumToMaximum() + { + return base.SetIfHigherAsync_WithConcurrentRequests_DifferencesSumToMaximum(); + } + [Fact] public override Task SetIfHigherAsync_WithFloatingPointDecimals_ComparesCorrectly() { return base.SetIfHigherAsync_WithFloatingPointDecimals_ComparesCorrectly(); } + [Fact] + public override Task SetIfLowerAsync_WithConcurrentRequests_DifferencesSumToDecrease() + { + return base.SetIfLowerAsync_WithConcurrentRequests_DifferencesSumToDecrease(); + } + [Fact] public override Task SetIfLowerAsync_WithFloatingPointDecimals_ComparesCorrectly() { return base.SetIfLowerAsync_WithFloatingPointDecimals_ComparesCorrectly(); } + [Theory] + [InlineData(false)] + [InlineData(true)] + public async Task ListAddAsync_WithActualLargeListUpdates_DoesNotCopyEntireList(bool fixedSizing) + { + // Arrange + using var cache = new InMemoryCacheClient(o => + { + o.CloneValues(false).TimeProvider(new FakeTimeProvider(DateTimeOffset.UtcNow)).LoggerFactory(Log); + return fixedSizing ? o.WithFixedSizing(1000000, 64) : o; + }); + await cache.ListAddAsync("small", Enumerable.Range(0, 10)); + await cache.ListAddAsync("large", Enumerable.Range(0, 1000)); + int[] item = [-1]; + await UpdateAsync("small"); + await UpdateAsync("large"); + + // Act + long smallAllocation = await MeasureAsync("small"); + long largeAllocation = await MeasureAsync("large"); + + // Assert + Assert.True(largeAllocation <= smallAllocation + 8192, + $"Small updates should not copy every element: small={smallAllocation}, large={largeAllocation} bytes."); + Assert.Equal(Enumerable.Range(0, 1000), (await cache.GetListAsync("large")).Value); + Assert.Equal(fixedSizing ? 128 : 0, cache.CurrentMemorySize); + + async Task UpdateAsync(string key) + { + await cache.ListAddAsync(key, item); + await cache.ListRemoveAsync(key, item); + } + + async Task MeasureAsync(string key) + { + long before = GC.GetAllocatedBytesForCurrentThread(); + await UpdateAsync(key); + return GC.GetAllocatedBytesForCurrentThread() - before; + } + } + + [Fact] + public async Task ListAddAsync_WithBatchCrossingBlocks_PreservesValuesAndSnapshots() + { + // Arrange + using var cache = new InMemoryCacheClient(o => o.LoggerFactory(Log).CloneValues(false)); + await cache.ListAddAsync("set", Enumerable.Range(0, 64)); + var previous = (await cache.GetListAsync("set")).Value!; + + // Act + await cache.ListAddAsync("set", Enumerable.Range(32, 1000)); + long removed = await cache.ListRemoveAsync("set", Enumerable.Range(16, 1000)); + await cache.ListAddAsync("set", Enumerable.Range(2000, 1000)); + + // Assert + Assert.Equal(1000, removed); + Assert.Equal(Enumerable.Range(0, 64), previous); + var expected = Enumerable.Range(0, 16).Concat(Enumerable.Range(1016, 16)).Concat(Enumerable.Range(2000, 1000)); + var current = (await cache.GetListAsync("set")).Value!; + Assert.Equal(expected, current.OrderBy(value => value)); + Assert.Equal(current.Skip(100).Take(100), (await cache.GetListAsync("set", 2)).Value); + } + + [Fact] + public async Task ListAddAsync_WithCloneValues_StoresCopiesOfAddedValues() + { + // Arrange + using var cache = new InMemoryCacheClient(o => o.CloneValues(true).LoggerFactory(Log)); + await cache.ListAddAsync("set", new[] { new SimpleModel { Data1 = "first" } }); + var added = new SimpleModel { Data1 = "second" }; + + // Act + await cache.ListAddAsync("set", new[] { added }); + + // Assert + var stored = Assert.IsAssignableFrom>(GetEntry(cache, "set").StoredValue); + Assert.Equal(2, stored.Count); + Assert.DoesNotContain(stored.Keys, k => ReferenceEquals(k, added)); + } + + [Theory] + [InlineData(false)] + [InlineData(true)] + public async Task ListAddAsync_WithConcurrentLargeListUpdates_PreservesUnrelatedValues(bool cloneValues) + { + // Arrange + using var cache = new InMemoryCacheClient(o => o.LoggerFactory(Log).CloneValues(cloneValues)); + var original = Enumerable.Range(0, 1000).ToArray(); + await cache.ListAddAsync("set", original); + + // Act + await Parallel.ForEachAsync(Enumerable.Range(0, 4), TestCancellationToken, async (worker, token) => + { + int[] item = [-worker - 1]; + for (int iteration = 0; iteration < 100; iteration++) + { + await Task.Yield(); + await cache.ListAddAsync("set", item); + await cache.ListRemoveAsync("set", item); + } + }); + + // Assert + var remaining = (await cache.GetListAsync("set")).Value!; + Assert.Equal(original, remaining.OrderBy(value => value)); + } + + [Fact] + public override Task ListAddAsync_WithConcurrentRequests_DoesNotLoseValues() + { + return base.ListAddAsync_WithConcurrentRequests_DoesNotLoseValues(); + } + + [Theory] + [InlineData(false, 0)] + [InlineData(false, 1)] + [InlineData(false, 2)] + [InlineData(false, 3)] + [InlineData(false, 4)] + [InlineData(true, 0)] + [InlineData(true, 1)] + [InlineData(true, 2)] + [InlineData(true, 3, Skip = "FastCloner 3.5.2 can overflow cloning ConcurrentDictionary internal cycles: https://github.com/lofcz/FastCloner/pull/58")] + [InlineData(true, 4)] + public async Task ListAddAsync_WithCustomComparer_PreservesComparerAcrossUpdates(bool cloneValues, int dictionaryType) + { + // Arrange + using var cache = new InMemoryCacheClient(o => o.LoggerFactory(Log).CloneValues(cloneValues)); + IDictionary values = dictionaryType switch + { + 0 => new Dictionary(StringComparer.OrdinalIgnoreCase), + 1 => new SortedDictionary(StringComparer.OrdinalIgnoreCase), + 2 => new SortedList(StringComparer.OrdinalIgnoreCase), + 3 => new ConcurrentDictionary(StringComparer.OrdinalIgnoreCase), + _ => new OrderedDictionary(StringComparer.OrdinalIgnoreCase) + }; + values["first"] = null; + await cache.SetAsync("set", values); + + // Act + await cache.ListAddAsync("set", new[] { "FIRST", "second" }); + long removed = await cache.ListRemoveAsync("set", new[] { "SECOND" }); + + // Assert + Assert.Equal(1, removed); + var stored = (await cache.GetAsync>("set")).Value!; + Assert.IsType(values.GetType(), stored); + Assert.Single(stored); + Assert.True(stored.ContainsKey("FIRST")); + } + + [Fact] + public async Task ListAddAsync_WithExistingClonedValues_DoesNotCloneThemAgain() + { + // Arrange + using var cache = new InMemoryCacheClient(o => o.LoggerFactory(Log).CloneValues(true)); + await cache.ListAddAsync("set", new[] { new SimpleModel { Data1 = "first" } }); + var before = Assert.IsType>(GetEntry(cache, "set").StoredValue); + var original = Assert.Single(before.Keys); + + // Act + await cache.ListAddAsync("set", new[] { new SimpleModel { Data1 = "second" } }); + + // Assert + var after = Assert.IsType>(GetEntry(cache, "set").StoredValue); + Assert.NotSame(before, after); + Assert.Contains(after.Keys, value => ReferenceEquals(original, value)); + Assert.Single(before); + } + + [Fact] + public async Task ListAddAsync_WithExistingList_DoesNotMutatePreviouslyReadValue() + { + // Arrange + using var cache = new InMemoryCacheClient(o => o.LoggerFactory(Log)); + await cache.ListAddAsync("set", new[] { "a" }); + var previouslyRead = (await cache.GetAsync>("set")).Value; + + // Act + await cache.ListAddAsync("set", new[] { "b" }); + await cache.ListRemoveAsync("set", new[] { "a" }); + + // Assert + Assert.Equal(["a"], previouslyRead!.Keys); + var current = await cache.GetListAsync("set"); + Assert.Equal(["b"], current.Value); + } + [Fact] public override Task ListAddAsync_WithExpiration_SetsExpirationCorrectly() { return base.ListAddAsync_WithExpiration_SetsExpirationCorrectly(); } + [Fact] + public async Task ListAddAsync_WithExposedDictionary_PreservesDirectMutationsAndPriorSnapshot() + { + // Arrange + using var cache = new InMemoryCacheClient(o => o.LoggerFactory(Log).CloneValues(false)); + await cache.ListAddAsync("set", Enumerable.Range(0, 64)); + await cache.ListAddAsync("set", new[] { 64 }); + var exposed = (await cache.GetAsync>("set")).Value!; + exposed[100] = null; + + // Act + await cache.ListAddAsync("set", new[] { 101 }); + await cache.ListRemoveAsync("set", new[] { 0 }); + + // Assert + var current = (await cache.GetListAsync("set")).Value!; + Assert.Contains(100, current); + Assert.Contains(101, current); + Assert.DoesNotContain(0, current); + Assert.Contains(0, exposed.Keys); + Assert.DoesNotContain(101, exposed.Keys); + } + [Fact] public override Task ListAddAsync_WithInvalidArguments_ThrowsException() { return base.ListAddAsync_WithInvalidArguments_ThrowsException(); } + [Fact] + public async Task ListAddAsync_WithOverriddenFixedSizer_ValidatesReplacement() + { + // Arrange + using var cache = new InMemoryCacheClient(o => o.LoggerFactory(Log).WithFixedSizing(1000, 1) + .SizeCalculator(value => ((Dictionary)value).Count).MaxEntrySize(64)); + await cache.ListAddAsync("set", Enumerable.Range(0, 64)); + + // Act + long added = await cache.ListAddAsync("set", new[] { 64 }); + + // Assert + Assert.Equal(0, added); + Assert.Equal(64, cache.CurrentMemorySize); + Assert.Equal(Enumerable.Range(0, 64), (await cache.GetListAsync("set")).Value); + } + + [Theory] + [InlineData(false, false)] + [InlineData(false, true)] + [InlineData(true, false)] + [InlineData(true, true)] + public async Task ListAddAsync_WithRejectedUpdate_PreservesEntry(bool remove, bool shouldThrow) + { + // Arrange + var timeProvider = new FakeTimeProvider(DateTimeOffset.UtcNow); + bool reject = false; + using var cache = new InMemoryCacheClient(o => o.TimeProvider(timeProvider).LoggerFactory(Log).MaxMemorySize(1000) + .SizeCalculator(value => reject && ((IDictionary)value).Count == (remove ? 1 : 3) ? 20 : 10) + .MaxEntrySize(15).ShouldThrowOnMaxEntrySizeExceeded(shouldThrow).LoggerFactory(Log)); + await cache.ListAddAsync("key", new[] { "a", "b" }, TimeSpan.FromMinutes(1)); + reject = true; + + // Act + async Task UpdateAsync() => remove + ? await cache.ListRemoveAsync("key", new[] { "a" }) + : await cache.ListAddAsync("key", new[] { "c" }, TimeSpan.FromMinutes(5)); + if (shouldThrow) + await Assert.ThrowsAsync(UpdateAsync); + else + Assert.Equal(0, await UpdateAsync()); + + // Assert + Assert.Equal(new[] { "a", "b" }, (await cache.GetListAsync("key")).Value); + Assert.Equal(TimeSpan.FromMinutes(1), await cache.GetExpirationAsync("key")); + Assert.Equal(10, cache.CurrentMemorySize); + } + + [Theory] + [InlineData(false)] + [InlineData(true)] + public async Task ListAddAsync_WithRepeatedMixedBatches_MatchesSetModel(bool fixedSizing) + { + // Arrange + using var cache = new InMemoryCacheClient(o => fixedSizing ? o.LoggerFactory(Log).WithFixedSizing(1000000, 64) : o.LoggerFactory(Log)); + var expected = new HashSet(Enumerable.Range(0, 1000)); + await cache.ListAddAsync("set", expected); + var random = new Random(570); + int[] batchSizes = [1, 5, 31, 33, 128]; + + for (int iteration = 0; iteration < 200; iteration++) + { + var batch = Enumerable.Range(0, batchSizes[iteration % batchSizes.Length]).Select(_ => random.Next(-1024, 2048)).ToArray(); + + bool add = iteration % 2 == 0; + int expectedCount = add ? batch.Distinct().Count() : batch.Count(expected.Remove); + if (add) + expected.UnionWith(batch); + + // Act + long affected = add ? await cache.ListAddAsync("set", batch) : await cache.ListRemoveAsync("set", batch); + + // Assert + Assert.Equal(expectedCount, affected); + Assert.Equal(expected.OrderBy(value => value), (await cache.GetListAsync("set")).Value!.OrderBy(value => value)); + Assert.Equal(fixedSizing ? 64 : 0, cache.CurrentMemorySize); + } + } + [Fact] public override Task ListAddAsync_WithSingleString_StoresAsStringNotCharArray() { @@ -243,6 +731,55 @@ public override Task ListAddAsync_WithVariousInputs_HandlesCorrectly() return base.ListAddAsync_WithVariousInputs_HandlesCorrectly(); } + [Theory] + [InlineData(false)] + [InlineData(true)] + public async Task ListMutation_WithUnsupportedDictionary_RejectsWithoutChangingValueAsync(bool remove) + { + // Arrange + var timeProvider = new FakeTimeProvider(DateTimeOffset.UtcNow); + using var cache = new InMemoryCacheClient(o => o.LoggerFactory(Log).TimeProvider(timeProvider).CloneValues(false).WithFixedSizing(10_000, 50)); + var original = new ReadOnlyDictionary(new Dictionary(StringComparer.OrdinalIgnoreCase) + { + ["first"] = null + }); + await cache.SetAsync("set", original, TimeSpan.FromMinutes(1)); + + // Act + var error = await Record.ExceptionAsync(async () => + { + if (remove) + await cache.ListRemoveAsync("set", new[] { "FIRST" }); + else + await cache.ListAddAsync("set", new[] { "FIRST", "second" }); + }); + + // Assert + Assert.IsType(error); + Assert.Same(original, (await cache.GetAsync>("set")).Value); + Assert.Single(original); + Assert.True(original.ContainsKey("FIRST")); + Assert.Equal(TimeSpan.FromMinutes(1), await cache.GetExpirationAsync("set")); + Assert.Equal(50, cache.CurrentMemorySize); + } + + [Fact] + public async Task ListRemoveAsync_WhenLastValueRemoved_RemovesKeyAndReleasesMemory() + { + // Arrange + using var cache = new InMemoryCacheClient(o => o.WithFixedSizing(10_000, 50).LoggerFactory(Log)); + await cache.ListAddAsync("set", new[] { "a" }); + Assert.Equal(50, cache.CurrentMemorySize); + + // Act + long removed = await cache.ListRemoveAsync("set", new[] { "a" }); + + // Assert + Assert.Equal(1, removed); + Assert.Equal(0, cache.CurrentMemorySize); + Assert.False(await cache.ExistsAsync("set")); + } + [Fact] public override Task ListRemoveAsync_WithInvalidInputs_ThrowsAppropriateException() { @@ -250,9 +787,76 @@ public override Task ListRemoveAsync_WithInvalidInputs_ThrowsAppropriateExceptio } [Fact] - public override Task ListRemoveAsync_WithValidValues_RemovesKeyWhenEmpty() + public async Task ListRemoveAsync_WithMissingValue_DoesNotAllocateInProportionToListSize() { - return base.ListRemoveAsync_WithValidValues_RemovesKeyWhenEmpty(); + // Arrange + using var cache = new InMemoryCacheClient(o => o.LoggerFactory(Log).CloneValues(false)); + await cache.ListAddAsync("small", Enumerable.Range(0, 10)); + await cache.ListAddAsync("large", Enumerable.Range(0, 1000)); + int[] absent = [-1]; + await cache.ListRemoveAsync("small", absent); + await cache.ListRemoveAsync("large", absent); + + // Act + long smallAllocation = await MeasureRemovalAsync("small"); + long largeAllocation = await MeasureRemovalAsync("large"); + + // Assert + Assert.True(largeAllocation <= smallAllocation + 1024, + $"Absent removal should not copy the list: small={smallAllocation}, large={largeAllocation} bytes."); + + async Task MeasureRemovalAsync(string key) + { + long before = GC.GetAllocatedBytesForCurrentThread(); + long result = await cache.ListRemoveAsync(key, absent); + long allocated = GC.GetAllocatedBytesForCurrentThread() - before; + Assert.Equal(0, result); + return allocated; + } + } + + [Fact] + public async Task ListRemoveAsync_WithMissingValue_PrunesExpiredValues() + { + // Arrange + var timeProvider = new FakeTimeProvider(DateTimeOffset.UtcNow); + using var cache = new InMemoryCacheClient(o => o.TimeProvider(timeProvider).LoggerFactory(Log).MaxMemorySize(1000) + .SizeCalculator(value => ((IDictionary)value).Count)); + await cache.ListAddAsync("set", new[] { "permanent" }); + await cache.ListAddAsync("set", new[] { "expired" }, TimeSpan.FromMilliseconds(5)); + timeProvider.Advance(TimeSpan.FromMilliseconds(6)); + + // Act + long removed = await cache.ListRemoveAsync("set", new[] { "absent" }); + + // Assert + Assert.Equal(0, removed); + Assert.Equal(1, cache.CurrentMemorySize); + Assert.Equal(new[] { "permanent" }, (await cache.GetListAsync("set")).Value); + Assert.Null(await cache.GetExpirationAsync("set")); + } + + [Theory] + [InlineData(1)] + [InlineData(1000)] + public async Task ListRemoveAsync_WithMixedExpirations_RecalculatesLifetimeAfterPruningAndRemoval(int expiringCount) + { + // Arrange + var timeProvider = new FakeTimeProvider(DateTimeOffset.UtcNow); + using var cache = new InMemoryCacheClient(o => o.LoggerFactory(Log).CloneValues(false).TimeProvider(timeProvider)); + await cache.ListAddAsync("set", new[] { "permanent" }); + await cache.ListAddAsync("set", Enumerable.Range(0, expiringCount).Select(index => $"first-{index}"), TimeSpan.FromSeconds(10)); + await cache.ListAddAsync("set", new[] { "last" }, TimeSpan.FromSeconds(20)); + + // Act + await cache.ListRemoveAsync("set", new[] { "permanent" }); + timeProvider.Advance(TimeSpan.FromSeconds(11)); + long removed = await cache.ListRemoveAsync("set", new[] { "first-0", "absent" }); + + // Assert + Assert.Equal(0, removed); + Assert.Equal(new[] { "last" }, (await cache.GetListAsync("set")).Value); + Assert.Equal(TimeSpan.FromSeconds(9), await cache.GetExpirationAsync("set")); } [Fact] @@ -261,6 +865,12 @@ public override Task ListRemoveAsync_WithMultipleItems_RemovesCorrectly() return base.ListRemoveAsync_WithMultipleItems_RemovesCorrectly(); } + [Fact] + public override Task ListRemoveAsync_WithValidValues_RemovesKeyWhenEmpty() + { + return base.ListRemoveAsync_WithValidValues_RemovesKeyWhenEmpty(); + } + [Fact] public override Task ListRemoveAsync_WithValues_RemovesCorrectly() { @@ -405,6 +1015,190 @@ public override Task RemoveByPrefixAsync_WithWildcardPattern_TreatsAsLiteral(str return base.RemoveByPrefixAsync_WithWildcardPattern_TreatsAsLiteral(pattern); } + [Fact] + public async Task RecalculateMemorySize_WithExpiredStoredEntry_KeepsSizeUntilRemoved() + { + // Arrange + var timeProvider = new FakeTimeProvider(DateTimeOffset.UtcNow); + using var cache = new InMemoryCacheClient(o => o.TimeProvider(timeProvider).WithFixedSizing(10_000, 50).LoggerFactory(Log)); + Publish(cache, "live", CreateEntry("live", timeProvider)); + Publish(cache, "expired", CreateEntry("expired", timeProvider, TimeSpan.FromMinutes(1))); + timeProvider.Advance(TimeSpan.FromMinutes(2)); + + // Act + long recounted = cache.RecalculateMemorySize(); + await cache.RemoveAsync("expired"); + + // Assert + Assert.Equal(100, recounted); + Assert.Equal(50, cache.CurrentMemorySize); + } + + [Theory] + [InlineData(false)] + [InlineData(true)] + public async Task RemoveIfEqualAsync_WhenEntryExpiresDuringComparison_ReturnsFalse(bool replace) + { + // Arrange + // A lease that expires while its value is being compared must not be renewed or released by the stale owner + var timeProvider = new FakeTimeProvider(DateTimeOffset.UtcNow); + using var cache = new InMemoryCacheClient(o => o.TimeProvider(timeProvider).CloneValues(false).LoggerFactory(Log)); + var owner = new ComparedValue("owner", () => timeProvider.Advance(TimeSpan.FromMinutes(2))); + Publish(cache, "lease", CreateEntry(owner, timeProvider, TimeSpan.FromMinutes(1))); + + // Act + bool changed = replace + ? await cache.ReplaceIfEqualAsync("lease", new ComparedValue("owner"), owner, TimeSpan.FromMinutes(10)) + : await cache.RemoveIfEqualAsync("lease", owner); + + // Assert + Assert.False(changed); + Assert.False(await cache.ExistsAsync("lease")); + } + + [Theory] + [InlineData(false, false, false)] + [InlineData(false, true, false)] + [InlineData(true, false, false)] + [InlineData(true, true, false)] + [InlineData(false, false, true)] + [InlineData(false, true, true)] + [InlineData(true, false, true)] + [InlineData(true, true, true)] + public async Task RemoveIfEqualAsync_WhenEntryExpires_ReclaimsMemoryAndNotifies(bool expiresDuringComparison, bool matches, bool recentMaintenance) + { + // Arrange + // Keep UTC within the initial 250ms maintenance window to exercise throttled cleanup. + var start = recentMaintenance + ? new DateTimeOffset(DateTime.MinValue, TimeSpan.Zero).AddMilliseconds(100) + : DateTimeOffset.UtcNow; + var timeProvider = new FakeTimeProvider(start); + using var cache = new InMemoryCacheClient(o => o.TimeProvider(timeProvider).WithFixedSizing(10_000, 50).LoggerFactory(Log)); + var expired = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + using var subscription = cache.ItemExpired.AddSyncHandler((_, args) => expired.TrySetResult(args.Key)); + var duration = recentMaintenance ? TimeSpan.FromMilliseconds(5) : TimeSpan.FromMinutes(1); + var owner = new ComparedValue("owner", expiresDuringComparison ? () => timeProvider.Advance(duration.Add(TimeSpan.FromMilliseconds(1))) : null); + Publish(cache, "lease", CreateEntry(owner, timeProvider, duration)); + if (!expiresDuringComparison) + timeProvider.Advance(duration.Add(TimeSpan.FromMilliseconds(1))); + Assert.Equal(50, cache.CurrentMemorySize); + + // Act + bool removed = await cache.RemoveIfEqualAsync("lease", new ComparedValue(matches ? "owner" : "other")); + + // Assert + Assert.False(removed); + Assert.Equal("lease", await expired.Task.WaitAsync(TimeSpan.FromSeconds(5), TestCancellationToken)); + Assert.Null(cache.UpdateEntry("lease", current => (current, current))); + Assert.Equal(0, cache.CurrentMemorySize); + } + + [Theory] + [InlineData(false, false)] + [InlineData(false, true)] + [InlineData(true, false)] + [InlineData(true, true)] + public async Task RemoveIfEqualAsync_WhenOwnerChangesDuringComparison_KeepsNewOwner(bool replace, bool expiresDuringComparison) + { + // Arrange + var timeProvider = new FakeTimeProvider(DateTimeOffset.UtcNow); + using var cache = new InMemoryCacheClient(o => o.TimeProvider(timeProvider).CloneValues(false).LoggerFactory(Log)); + InMemoryCacheClient.CacheEntry? newOwner = null; + var owner = new ComparedValue("owner", () => + { + if (expiresDuringComparison) + timeProvider.Advance(TimeSpan.FromMinutes(2)); + newOwner = CreateEntry(new ComparedValue("new-owner"), timeProvider, TimeSpan.FromMinutes(1)); + Publish(cache, "lease", newOwner); + }); + Publish(cache, "lease", CreateEntry(owner, timeProvider, TimeSpan.FromMinutes(1))); + + // Act + bool changed = replace + ? await cache.ReplaceIfEqualAsync("lease", new ComparedValue("owner"), owner, TimeSpan.FromMinutes(10)) + : await cache.RemoveIfEqualAsync("lease", owner); + + // Assert + Assert.False(changed); + Assert.NotNull(newOwner); + Assert.Same(newOwner, GetEntry(cache, "lease")); + } + + [Fact] + public async Task RemoveIfEqualAsync_WhenValueComparisonThrows_PreservesEntryAndReturnsFaultedTask() + { + // Arrange + var timeProvider = new FakeTimeProvider(DateTimeOffset.UtcNow); + using var cache = new InMemoryCacheClient(o => o.LoggerFactory(Log).TimeProvider(timeProvider).WithFixedSizing(10_000, 50)); + var owner = new ComparedValue("owner", () => throw new InvalidOperationException("Comparison failed")); + var entry = CreateEntry(owner, timeProvider); + Publish(cache, "lease", entry); + Task? removal = null; + + // Act + var synchronousError = Record.Exception(() => { removal = cache.RemoveIfEqualAsync("lease", owner); }); + + // Assert + Assert.Null(synchronousError); + Assert.NotNull(removal); + var error = await Assert.ThrowsAsync(() => removal); + Assert.Equal("Comparison failed", error.Message); + Assert.Same(entry, GetEntry(cache, "lease")); + Assert.Equal(50, cache.CurrentMemorySize); + } + + [Theory] + [InlineData(false)] + [InlineData(true)] + public async Task RemoveIfEqualAsync_WithExpiredIncompatibleValue_ReturnsFalseWithoutComparisonAsync(bool throwingEquality) + { + // Arrange + var timeProvider = new FakeTimeProvider(DateTimeOffset.UtcNow); + using var cache = new InMemoryCacheClient(o => o.TimeProvider(timeProvider).CloneValues(false).LoggerFactory(Log)); + int comparisons = 0; + var value = new ComparedValue("owner", () => + { + comparisons++; + throw new InvalidOperationException("Comparison failed"); + }); + Publish(cache, "key", CreateEntry(throwingEquality ? value : new object(), timeProvider, TimeSpan.FromMinutes(1))); + timeProvider.Advance(TimeSpan.FromMinutes(2)); + + // Act + bool changed = await cache.RemoveIfEqualAsync("key", value); + + // Assert + Assert.False(changed); + Assert.Equal(0, comparisons); + Assert.False(await cache.ExistsAsync("key")); + } + + [Theory] + [InlineData(false)] + [InlineData(true)] + public async Task RemoveIfEqualAsync_WithExplicitDeletion_DoesNotLeaveExpirationTombstone(bool removeList) + { + // Arrange + var timeProvider = new FakeTimeProvider(DateTimeOffset.UtcNow); + using var cache = new InMemoryCacheClient(o => o.LoggerFactory(Log).TimeProvider(timeProvider)); + if (removeList) + await cache.ListAddAsync("key", new[] { "value" }, TimeSpan.FromMinutes(1)); + else + await cache.SetAsync("key", "value", TimeSpan.FromMinutes(1)); + + // Act + if (removeList) + Assert.Equal(1, await cache.ListRemoveAsync("key", new[] { "value" })); + else + Assert.True(await cache.RemoveIfEqualAsync("key", "value")); + + // Assert + Assert.Null(cache.UpdateEntry("key", current => (current, current))); + timeProvider.Advance(TimeSpan.FromMinutes(2)); + await cache.DoMaintenanceAsync(); + Assert.Equal(0, cache.Count); + } + [Fact] public override Task RemoveIfEqualAsync_WithInvalidKey_ThrowsArgumentException() { @@ -453,10 +1247,133 @@ public override Task ReplaceAsync_WithExpiration_SetsExpirationCorrectly() return base.ReplaceAsync_WithExpiration_SetsExpirationCorrectly(); } - [Fact] - public override Task ReplaceIfEqualAsync_WithExpiration_SetsExpirationCorrectly() - { - return base.ReplaceIfEqualAsync_WithExpiration_SetsExpirationCorrectly(); + [Fact] + public async Task ReplaceIfEqualAsync_WhenEntryExpiresDuringSizing_ReturnsFalseWithoutRevivingAsync() + { + // Arrange + var timeProvider = new FakeTimeProvider(DateTimeOffset.UtcNow); + int sizingCalls = 0; + using var cache = new InMemoryCacheClient(o => o.TimeProvider(timeProvider).LoggerFactory(Log).CloneValues(false).SizeCalculator(_ => + { + sizingCalls++; + timeProvider.Advance(TimeSpan.FromMinutes(2)); + return 1; + })); + Publish(cache, "lease", CreateEntry("owner", timeProvider, TimeSpan.FromMinutes(1))); + + // Act + bool replaced = await cache.ReplaceIfEqualAsync("lease", "replacement", "owner", TimeSpan.FromMinutes(10)); + + // Assert + Assert.False(replaced); + Assert.Equal(1, sizingCalls); + Assert.False(await cache.ExistsAsync("lease")); + } + + [Theory] + [InlineData("comparison")] + [InlineData("sizing")] + [InlineData("retry")] + public async Task ReplaceIfEqualAsync_WhenReplacementExpiresBeforePublication_PreservesLiveEntry(string expirationStage) + { + // Arrange + var timeProvider = new FakeTimeProvider(DateTimeOffset.UtcNow); + int sizingCalls = 0; + using var cache = new InMemoryCacheClient(o => o.TimeProvider(timeProvider).LoggerFactory(Log).CloneValues(false) + .MaxMemorySize(10_000).SizeCalculator(_ => + { + sizingCalls++; + if (expirationStage == "sizing") + timeProvider.Advance(TimeSpan.FromMinutes(2)); + return 100; + })); + var expected = new ComparedValue("owner"); + var original = CreateEntry(new ComparedValue("owner", () => + { + if (expirationStage == "comparison") + timeProvider.Advance(TimeSpan.FromMinutes(2)); + else if (expirationStage == "retry") + Publish(cache, "lease", CreateEntry(new ComparedValue("owner", () => timeProvider.Advance(TimeSpan.FromMinutes(2))), + timeProvider, TimeSpan.FromMinutes(10))); + }), timeProvider, TimeSpan.FromMinutes(10)); + Publish(cache, "lease", original); + + // Act + bool replaced = await cache.ReplaceIfEqualAsync("lease", new ComparedValue("replacement"), expected, TimeSpan.FromMinutes(1)); + + // Assert + Assert.False(replaced); + Assert.Equal(1, sizingCalls); + Assert.Equal("owner", (await cache.GetAsync("lease")).Value.Id); + Assert.Equal(TimeSpan.FromMinutes(8), await cache.GetExpirationAsync("lease")); + Assert.Equal(50, cache.CurrentMemorySize); + } + + [Fact] + public async Task ReplaceIfEqualAsync_WithCloneValues_IsolatesReplacementFromCaller() + { + // Arrange + using var cache = new InMemoryCacheClient(o => o.CloneValues(true).LoggerFactory(Log)); + await cache.SetAsync("key", 1); + var replacement = new List { 1 }; + + // Act + bool replaced = await cache.ReplaceIfEqualAsync("key", replacement, 1); + replacement.Add(2); + + // Assert + Assert.True(replaced); + var cached = await cache.GetAsync>("key"); + Assert.Equal([1], cached.Value); + } + + [Fact] + public override Task ReplaceIfEqualAsync_WithExpiration_SetsExpirationCorrectly() + { + return base.ReplaceIfEqualAsync_WithExpiration_SetsExpirationCorrectly(); + } + + [Fact] + public async Task ReplaceIfEqualAsync_WithExpiredEntry_ReturnsFalseAndDoesNotRevive() + { + // Arrange + var timeProvider = new FakeTimeProvider(DateTimeOffset.UtcNow); + using var cache = new InMemoryCacheClient(o => o.TimeProvider(timeProvider).CloneValues(false).LoggerFactory(Log)); + Publish(cache, "lease", CreateEntry("owner", timeProvider, TimeSpan.FromMinutes(1))); + timeProvider.Advance(TimeSpan.FromMinutes(2)); + + // Act + bool replaced = await cache.ReplaceIfEqualAsync("lease", "owner", "owner", TimeSpan.FromMinutes(10)); + + // Assert + Assert.False(replaced); + Assert.False(await cache.ExistsAsync("lease")); + } + + [Theory] + [InlineData(false)] + [InlineData(true)] + public async Task ReplaceIfEqualAsync_WithExpiredIncompatibleValue_ReturnsFalseWithoutComparisonAsync(bool throwingEquality) + { + // Arrange + var timeProvider = new FakeTimeProvider(DateTimeOffset.UtcNow); + using var cache = new InMemoryCacheClient(o => o.TimeProvider(timeProvider).CloneValues(false).LoggerFactory(Log)); + int comparisons = 0; + var value = new ComparedValue("owner", () => + { + comparisons++; + throw new InvalidOperationException("Comparison failed"); + }); + Publish(cache, "key", CreateEntry(throwingEquality ? value : new object(), timeProvider, TimeSpan.FromMinutes(1))); + timeProvider.Advance(TimeSpan.FromMinutes(2)); + + // Act + bool changed = await cache.ReplaceIfEqualAsync("key", value, value); + + // Assert + Assert.False(changed); + Assert.Equal(0, comparisons); + Assert.False(await cache.ExistsAsync("key")); } [Fact] @@ -477,6 +1394,25 @@ public override Task ReplaceIfEqualAsync_WithMismatchedOldValue_ReturnsFalseAndD return base.ReplaceIfEqualAsync_WithMismatchedOldValue_ReturnsFalseAndDoesNotReplace(); } + [Fact] + public async Task ReplaceIfEqualAsync_WithMismatchedOversizedValue_ReturnsFalseWithoutThrowing() + { + // Arrange + using var cache = new InMemoryCacheClient(o => o + .WithDynamicSizing(10000, Log) + .MaxEntrySize(50) + .ShouldThrowOnMaxEntrySizeExceeded() + .LoggerFactory(Log)); + await cache.SetAsync("key", "small"); + + // Act + bool replaced = await cache.ReplaceIfEqualAsync("key", new string('x', 100), "other"); + + // Assert + Assert.False(replaced); + Assert.Equal("small", (await cache.GetAsync("key")).Value); + } + [Fact] public override Task ReplaceIfEqualAsync_WithNonExistentKey_ReturnsFalseAndDoesNotCreateKey() { @@ -687,6 +1623,47 @@ public async Task SetAsync_WithMaxItems_EnforcesLimit() } } + [Fact] + public async Task DoMaintenanceAsync_WhenExpiredEntryReplaced_KeepsLiveEntry() + { + // Arrange + var timeProvider = new FakeTimeProvider(DateTimeOffset.UtcNow); + using var cache = new InMemoryCacheClient(o => o.TimeProvider(timeProvider).WithFixedSizing(10_000, 50).LoggerFactory(Log)); + await cache.SetAsync("key", "expired", TimeSpan.FromMilliseconds(100)); + timeProvider.Advance(TimeSpan.FromSeconds(1)); + await cache.SetAsync("key", "fresh", TimeSpan.FromMinutes(5)); + + // Act + await cache.DoMaintenanceAsync(); + + // Assert + var result = await cache.GetAsync("key"); + Assert.True(result.HasValue); + Assert.Equal("fresh", result.Value); + Assert.Equal(50, cache.CurrentMemorySize); + Assert.DoesNotContain(Log.LogEntries, l => l.LogLevel is LogLevel.Error); + } + + [Fact] + public async Task DoMaintenanceAsync_WithExpiredEntry_RemovesEntryAndNotifies() + { + // Arrange + var timeProvider = new FakeTimeProvider(DateTimeOffset.UtcNow); + using var cache = new InMemoryCacheClient(o => o.TimeProvider(timeProvider).WithFixedSizing(10_000, 50).LoggerFactory(Log)); + var expired = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + using var subscription = cache.ItemExpired.AddSyncHandler((_, args) => expired.TrySetResult(args.Key)); + await cache.SetAsync("key", "value", TimeSpan.FromMilliseconds(100)); + timeProvider.Advance(TimeSpan.FromSeconds(1)); + + // Act + await cache.DoMaintenanceAsync(); + + // Assert + Assert.Equal("key", await expired.Task.WaitAsync(TimeSpan.FromSeconds(5), TestCancellationToken)); + Assert.Equal(0, cache.Count); + Assert.Equal(0, cache.CurrentMemorySize); + } + [Fact] public async Task DoMaintenanceAsync_WithMaxTimeSpanExpiration_ShouldNotThrowException() { @@ -714,6 +1691,142 @@ public async Task DoMaintenanceAsync_WithMaxTimeSpanExpiration_ShouldNotThrowExc } } + [Fact] + public async Task DoMaintenanceAsync_WithMaxTimeSpanExpiration_KeepsEntryAfterTimeAdvances() + { + // Arrange + var timeProvider = new FakeTimeProvider(DateTimeOffset.UtcNow); + using var cache = new InMemoryCacheClient(o => o.CloneValues(true).TimeProvider(timeProvider).LoggerFactory(Log)); + const string key = "cached-with-max-expiration"; + await cache.SetAsync(key, "value", TimeSpan.MaxValue); + timeProvider.Advance(TimeSpan.FromDays(365)); + + // Act + await cache.DoMaintenanceAsync(); + + // Assert + var result = await cache.GetAsync(key); + Assert.True(result.HasValue); + Assert.Equal("value", result.Value); + Assert.Null(await cache.GetExpirationAsync(key)); + Assert.DoesNotContain(Log.LogEntries, entry => entry.LogLevel is LogLevel.Error); + } + + [Fact] + public async Task DoMaintenanceAsync_WithPositiveTimezoneOffset_ShouldNotThrowOnDateTimeMinValue() + { + // This test reproduces an issue where RemoveIfEqualAsync sets ExpiresAt to DateTime.MinValue, + // and DoMaintenanceAsync then compares it with a DateTimeOffset. When the system's local timezone + // has a positive offset (like Beirut UTC+2/+3), converting DateTime.MinValue to DateTimeOffset + // throws ArgumentOutOfRangeException because the resulting UTC time would be before year 0001. + // + // The bug is in InMemoryCacheClient.cs when checking the expiration: + // if (expiresAt < DateTime.MaxValue && expiresAt <= utcNow) + // + // When expiresAt is DateTime.MinValue (Kind=Unspecified) and utcNow is a DateTimeOffset, + // the implicit conversion of DateTime.MinValue to DateTimeOffset uses the system's local + // timezone offset. If that offset is positive, the conversion fails. + + var timeProvider = new FakeTimeProvider(DateTimeOffset.UtcNow); + var cache = new InMemoryCacheClient(o => o.CloneValues(true).TimeProvider(timeProvider).LoggerFactory(Log)); + using (cache) + { + await cache.RemoveAllAsync(); + + // Set up a cache entry and then remove it via RemoveIfEqualAsync + // This sets ExpiresAt to DateTime.MinValue on the entry + await cache.SetAsync("test-key", "test-value", TimeSpan.FromMinutes(1)); + await cache.RemoveIfEqualAsync("test-key", "test-value"); + + + // Advance time to ensure the entry is considered "infrequently accessed" + // (LastAccessTicks must be < utcNow - 300ms for maintenance to process it) + timeProvider.Advance(TimeSpan.FromSeconds(1)); + + // Trigger another cache operation to start maintenance + // This causes DoMaintenanceAsync to iterate over entries including the one + // with ExpiresAt = DateTime.MinValue + await cache.SetAsync("trigger", "value"); + + // Wait a moment to allow maintenance to complete + await Task.Delay(100, TestCancellationToken); + + // Check for the error log that indicates the bug + Assert.Null(Log.LogEntries.SingleOrDefault(l => l.LogLevel == LogLevel.Error)); + } + } + + [Fact] + public async Task DoMaintenanceAsync_WithMinimumDateTimeExpiration_RemovesEntryWithoutError() + { + // Arrange + var timeProvider = new FakeTimeProvider(DateTimeOffset.UtcNow); + using var cache = new InMemoryCacheClient(o => o.TimeProvider(timeProvider).WithFixedSizing(10_000, 50).LoggerFactory(Log)); + // An unspecified minimum date must be compared as UTC, not converted using the local timezone. + var entry = new InMemoryCacheClient.CacheEntry("value", DateTime.MinValue, timeProvider, shouldClone: false, size: 50); + Publish(cache, "key", entry); + + // Act + await cache.DoMaintenanceAsync(); + + // Assert + Assert.Null(cache.UpdateEntry("key", current => (current, current))); + Assert.Equal(0, cache.CurrentMemorySize); + Assert.DoesNotContain(Log.LogEntries, logEntry => logEntry.LogLevel is LogLevel.Error); + } + + [Fact] + public async Task DoMaintenanceAsync_WithMixedExpirations_RemovesOnlyExpiredEntries() + { + // Arrange + var timeProvider = new FakeTimeProvider(DateTimeOffset.UtcNow); + using var cache = new InMemoryCacheClient(o => o.TimeProvider(timeProvider).WithFixedSizing(10_000, 50).LoggerFactory(Log)); + var boundary = CreateEntry("boundary", timeProvider, TimeSpan.FromMinutes(2)); + var future = CreateEntry("future", timeProvider, TimeSpan.FromMinutes(3)); + var permanent = CreateEntry("permanent", timeProvider); + // Publish directly so a background maintenance task cannot clean up before the explicit pass. + Publish(cache, "expired", CreateEntry("expired", timeProvider, TimeSpan.FromMinutes(1))); + Publish(cache, "boundary", boundary); + Publish(cache, "future", future); + Publish(cache, "permanent", permanent); + timeProvider.Advance(TimeSpan.FromMinutes(2)); + + // Act + await cache.DoMaintenanceAsync(); + + // Assert + Assert.Null(cache.UpdateEntry("expired", current => (current, current))); + Assert.Same(boundary, GetEntry(cache, "boundary")); + Assert.Same(future, GetEntry(cache, "future")); + Assert.Same(permanent, GetEntry(cache, "permanent")); + Assert.Equal(TimeSpan.Zero, await cache.GetExpirationAsync("boundary")); + Assert.Equal(TimeSpan.FromMinutes(1), await cache.GetExpirationAsync("future")); + Assert.Null(await cache.GetExpirationAsync("permanent")); + Assert.Equal(150, cache.CurrentMemorySize); + Assert.DoesNotContain(Log.LogEntries, entry => entry.LogLevel is LogLevel.Error); + } + + [Theory] + [InlineData(-1, true)] + [InlineData(0, false)] + [InlineData(25, false)] + public async Task DoMaintenanceAsync_WithRecentlyAccessedEntry_UsesActualExpiration(int expiresInMilliseconds, bool removed) + { + // Arrange + var timeProvider = new FakeTimeProvider(DateTimeOffset.UtcNow); + using var cache = new InMemoryCacheClient(o => o.LoggerFactory(Log).TimeProvider(timeProvider).WithFixedSizing(10_000, 50)); + var entry = CreateEntry("value", timeProvider, TimeSpan.FromMilliseconds(expiresInMilliseconds)); + Publish(cache, "key", entry); + + // Act + await cache.DoMaintenanceAsync(); + + // Assert + var remaining = cache.UpdateEntry("key", current => (current, current)); + Assert.Equal(removed, remaining is null); + Assert.Equal(removed ? 0 : 50, cache.CurrentMemorySize); + } + [Fact] public async Task SetAsync_WithMaxMemorySizeLimit_EvictsWhenOverLimit() { @@ -810,6 +1923,43 @@ public async Task SetAsync_WithBothLimits_RespectsMemoryAndItemLimits() } } + [Fact] + public async Task MemorySize_WithRemoveIfEqualOperation_FreesMemoryImmediately() + { + // Arrange + using var cache = new InMemoryCacheClient(o => o.WithFixedSizing(10_000, 50).LoggerFactory(Log)); + await cache.SetAsync("key", "value"); + Assert.Equal(50, cache.CurrentMemorySize); + + // Act + bool removed = await cache.RemoveIfEqualAsync("key", "value"); + + // Assert + Assert.True(removed); + Assert.Equal(0, cache.CurrentMemorySize); + Assert.False(await cache.ExistsAsync("key")); + } + + [Fact] + public async Task MemorySize_WithSetExpirationOperation_PreservesValueAndMemorySize() + { + // Arrange + var timeProvider = new FakeTimeProvider(DateTimeOffset.UtcNow); + using var cache = new InMemoryCacheClient(o => o.TimeProvider(timeProvider).WithFixedSizing(10_000, 50).LoggerFactory(Log)); + await cache.SetAsync("key", "value", TimeSpan.FromMinutes(1)); + await cache.SetAsync("persistent", "value", TimeSpan.FromMinutes(1)); + + // Act + await cache.SetExpirationAsync("key", TimeSpan.FromMinutes(5)); + await cache.SetAllExpirationAsync(new Dictionary { ["persistent"] = null }); + + // Assert + Assert.Equal("value", (await cache.GetAsync("key")).Value); + Assert.Equal(TimeSpan.FromMinutes(5), await cache.GetExpirationAsync("key")); + Assert.Null(await cache.GetExpirationAsync("persistent")); + Assert.Equal(100, cache.CurrentMemorySize); + } + [Fact] public async Task MemorySize_WithoutMaxMemorySize_DoesNotTrack() { @@ -1306,6 +2456,12 @@ public async Task SetIfHigherAsync_Double_WithMaxMemorySize_TracksMemoryCorrectl } } + [Fact] + public override Task SetIfHigherAsync_WithLowerValue_ReturnsZeroAndKeepsValue() + { + return base.SetIfHigherAsync_WithLowerValue_ReturnsZeroAndKeepsValue(); + } + [Fact] public async Task SetIfHigherAsync_WithMaxMemorySize_TracksMemoryCorrectly() { @@ -1346,6 +2502,33 @@ public async Task SetIfHigherAsync_WithMaxMemorySize_TracksMemoryCorrectly() } } + [Theory] + [InlineData(false, false)] + [InlineData(false, true)] + [InlineData(true, false)] + [InlineData(true, true)] + public async Task SetIfHigherAsync_WithNumericConversion_TracksReplacementSize(bool lower, bool useDouble) + { + // Arrange + using var cache = new InMemoryCacheClient(o => o.MaxMemorySize(1000) + .SizeCalculator(value => value is string ? 100 : 8).LoggerFactory(Log)); + await cache.SetAsync("key", "10"); + + // Act + double difference = (lower, useDouble) switch + { + (true, true) => await cache.SetIfLowerAsync("key", 5d), + (true, false) => await cache.SetIfLowerAsync("key", 5L), + (false, true) => await cache.SetIfHigherAsync("key", 15d), + _ => await cache.SetIfHigherAsync("key", 15L) + }; + + // Assert + Assert.Equal(5, difference); + Assert.Equal(lower ? 5 : 15, (await cache.GetAsync("key")).Value); + Assert.Equal(8, cache.CurrentMemorySize); + } + [Fact] public async Task SetIfLowerAsync_Double_WithMaxMemorySize_TracksMemoryCorrectly() { @@ -1464,6 +2647,31 @@ public async Task ReplaceIfEqualAsync_WithMaxMemorySize_TracksMemoryCorrectly() } } + [Theory] + [InlineData(false)] + [InlineData(true)] + public async Task ReplaceIfEqualAsync_WithRejectedReplacement_PreservesEntry(bool shouldThrow) + { + // Arrange + var timeProvider = new FakeTimeProvider(DateTimeOffset.UtcNow); + using var cache = new InMemoryCacheClient(o => o.TimeProvider(timeProvider).LoggerFactory(Log).MaxMemorySize(1000) + .SizeCalculator(value => ((string)value).Length).MaxEntrySize(5) + .ShouldThrowOnMaxEntrySizeExceeded(shouldThrow).LoggerFactory(Log)); + await cache.SetAsync("key", "old", TimeSpan.FromMinutes(1)); + + // Act + if (shouldThrow) + await Assert.ThrowsAsync(async () => + await cache.ReplaceIfEqualAsync("key", "oversized", "old", TimeSpan.FromMinutes(5))); + else + Assert.False(await cache.ReplaceIfEqualAsync("key", "oversized", "old", TimeSpan.FromMinutes(5))); + + // Assert + Assert.Equal("old", (await cache.GetAsync("key")).Value); + Assert.Equal(TimeSpan.FromMinutes(1), await cache.GetExpirationAsync("key")); + Assert.Equal(3, cache.CurrentMemorySize); + } + [Fact] public async Task SetAsync_WithOversizedEntry_LogsWarningAndReturnsFalse() { @@ -1567,48 +2775,112 @@ public async Task SetAsync_WithEntryUnderLimit_CachesSuccessfully() } } - [Fact] - public async Task DoMaintenanceAsync_WithPositiveTimezoneOffset_ShouldNotThrowOnDateTimeMinValue() - { - // This test reproduces an issue where RemoveIfEqualAsync sets ExpiresAt to DateTime.MinValue, - // and DoMaintenanceAsync then compares it with a DateTimeOffset. When the system's local timezone - // has a positive offset (like Beirut UTC+2/+3), converting DateTime.MinValue to DateTimeOffset - // throws ArgumentOutOfRangeException because the resulting UTC time would be before year 0001. - // - // The bug is in InMemoryCacheClient.cs when checking the expiration: - // if (expiresAt < DateTime.MaxValue && expiresAt <= utcNow) - // - // When expiresAt is DateTime.MinValue (Kind=Unspecified) and utcNow is a DateTimeOffset, - // the implicit conversion of DateTime.MinValue to DateTimeOffset uses the system's local - // timezone offset. If that offset is positive, the conversion fails. + [Theory] + [InlineData("Add", true)] + [InlineData("Increment", 10L)] + [InlineData("Replace", false)] + [InlineData("ReplaceIfEqual", false)] + [InlineData("Set", true)] + [InlineData("SetExpiration", null)] + [InlineData("SetIfHigher", 10L)] + [InlineData("SetIfLower", 10L)] + public async Task WriteAsync_WithExpiredUnsweptEntry_TreatsKeyAsMissing(string operation, object? expectedResult) + { + // Arrange: an expired entry that maintenance has not removed yet must behave as if the key were absent + var timeProvider = new FakeTimeProvider(DateTimeOffset.UtcNow); + using var cache = new InMemoryCacheClient(o => o.TimeProvider(timeProvider).LoggerFactory(Log)); + var expired = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + using var subscription = cache.ItemExpired.AddSyncHandler((_, args) => expired.TrySetResult(args.Key)); + Publish(cache, "key", CreateEntry(operation is "SetIfLower" ? 1L : 100L, timeProvider, TimeSpan.FromMinutes(1))); + timeProvider.Advance(TimeSpan.FromMinutes(2)); + + // Act + object? result = operation switch + { + "Add" => await cache.AddAsync("key", 10L), + "Increment" => await cache.IncrementAsync("key", 10L), + "Replace" => await cache.ReplaceAsync("key", 10L), + "ReplaceIfEqual" => await cache.ReplaceIfEqualAsync("key", 10L, 100L), + "Set" => await cache.SetAsync("key", 10L), + "SetExpiration" => await SetExpirationAsync(), + "SetIfHigher" => await cache.SetIfHigherAsync("key", 10L), + "SetIfLower" => await cache.SetIfLowerAsync("key", 10L), + _ => throw new ArgumentOutOfRangeException(nameof(operation), operation, null) + }; - var timeProvider = new FakeTimeProvider(); - var cache = new InMemoryCacheClient(o => o.CloneValues(true).TimeProvider(timeProvider).LoggerFactory(Log)); - using (cache) + // Assert + Assert.Equal(expectedResult, result); + var value = await cache.GetAsync("key"); + bool keyWritten = expectedResult is true or long; + Assert.Equal(keyWritten, value.HasValue); + if (keyWritten) + Assert.Equal(10L, value.Value); + Assert.Equal("key", await expired.Task.WaitAsync(TimeSpan.FromSeconds(5), TestCancellationToken)); + + async Task SetExpirationAsync() { - await cache.RemoveAllAsync(); + await cache.SetExpirationAsync("key", TimeSpan.FromMinutes(10)); + return null; + } + } - // Set up a cache entry and then remove it via RemoveIfEqualAsync - // This sets ExpiresAt to DateTime.MinValue on the entry - await cache.SetAsync("test-key", "test-value", TimeSpan.FromMinutes(1)); - await cache.RemoveIfEqualAsync("test-key", "test-value"); + [Theory] + [InlineData(false)] + [InlineData(true)] + public async Task ListAddAsync_WithExpiredUnsweptList_StartsNewList(bool remove) + { + // Arrange: the list's values never expire, but the key itself expired through SetExpirationAsync + var timeProvider = new FakeTimeProvider(DateTimeOffset.UtcNow); + using var cache = new InMemoryCacheClient(o => o.TimeProvider(timeProvider).LoggerFactory(Log)); + Publish(cache, "set", CreateEntry(new Dictionary { ["old"] = null }, timeProvider, TimeSpan.FromMinutes(1))); + timeProvider.Advance(TimeSpan.FromMinutes(2)); + // Act + long changed = remove + ? await cache.ListRemoveAsync("set", ["old"]) + : await cache.ListAddAsync("set", ["new"]); - // Advance time to ensure the entry is considered "infrequently accessed" - // (LastAccessTicks must be < utcNow - 300ms for maintenance to process it) - timeProvider.Advance(TimeSpan.FromSeconds(1)); + // Assert + Assert.Equal(remove ? 0 : 1, changed); + var list = await cache.GetListAsync("set"); + Assert.Equal(remove ? [] : ["new"], list.Value ?? []); + } - // Trigger another cache operation to start maintenance - // This causes DoMaintenanceAsync to iterate over entries including the one - // with ExpiresAt = DateTime.MinValue - await cache.SetAsync("trigger", "value"); + private static InMemoryCacheClient.CacheEntry CreateEntry(object value, TimeProvider timeProvider, TimeSpan? expiresIn = null) + { + DateTime? expiresAt = expiresIn.HasValue ? timeProvider.GetUtcNow().UtcDateTime.Add(expiresIn.Value) : null; + return new InMemoryCacheClient.CacheEntry(value, expiresAt, timeProvider, shouldClone: false, size: 50); + } - // Wait a moment to allow maintenance to complete - await Task.Delay(100, TestCancellationToken); + private static InMemoryCacheClient.CacheEntry GetEntry(InMemoryCacheClient cache, string key) + { + return cache.UpdateEntry(key, current => (current, current)) ?? throw new InvalidOperationException($"Missing cache key: {key}"); + } - // Check for the error log that indicates the bug - Assert.Null(Log.LogEntries.SingleOrDefault(l => l.LogLevel == LogLevel.Error)); + private static void Publish(InMemoryCacheClient cache, string key, InMemoryCacheClient.CacheEntry entry) + { + cache.UpdateEntry(key, _ => (entry, true)); + } + + /// + /// A value whose first equality check runs , so a test can change the + /// cache or the clock in the middle of a conditional operation's comparison, on the same thread. + /// + private sealed class ComparedValue(string id, Action? duringFirstComparison = null) : IEquatable + { + private Action? _duringFirstComparison = duringFirstComparison; + + public string Id { get; } = id; + + public bool Equals(ComparedValue? other) + { + Interlocked.Exchange(ref _duringFirstComparison, null)?.Invoke(); + return other is not null && String.Equals(Id, other.Id, StringComparison.Ordinal); } + + public override bool Equals(object? obj) => obj is ComparedValue other && Equals(other); + + public override int GetHashCode() => StringComparer.Ordinal.GetHashCode(Id); } [Theory] @@ -1664,6 +2936,160 @@ public override Task SetExpirationAsync_OnNonExistentKey_DoesNotThrow() return base.SetExpirationAsync_OnNonExistentKey_DoesNotThrow(); } + [Fact] + public async Task SetExpirationAsync_WithClonedPayload_ReusesStoredValueAndAccessOrder() + { + // Arrange + var timeProvider = new FakeTimeProvider(DateTimeOffset.UtcNow); + using var cache = new InMemoryCacheClient(o => o.LoggerFactory(Log).CloneValues(true).TimeProvider(timeProvider)); + await cache.SetAsync("key", new SimpleModel { Data1 = "original" }); + var before = GetEntry(cache, "key"); + timeProvider.Advance(TimeSpan.FromMilliseconds(1)); + + // Act + await cache.SetExpirationAsync("key", TimeSpan.FromMinutes(1)); + + // Assert + var after = GetEntry(cache, "key"); + Assert.NotSame(before, after); + Assert.Same(before.StoredValue, after.StoredValue); + Assert.Equal(before.LastAccessTicks, after.LastAccessTicks); + Assert.Equal(before.InstanceNumber, after.InstanceNumber); + var firstRead = (await cache.GetAsync("key")).Value!; + firstRead.Data1 = "changed"; + Assert.Equal("original", (await cache.GetAsync("key")).Value!.Data1); + } + + [Fact] + public void TryRemoveEntry_WhenObservedEntryWasReplaced_KeepsLiveEntry() + { + // Arrange + // Reproduces the production race deterministically: maintenance observes an expired entry, another + // writer replaces it, then maintenance tries to remove the entry it observed. + var timeProvider = new FakeTimeProvider(DateTimeOffset.UtcNow); + using var cache = new InMemoryCacheClient(o => o.TimeProvider(timeProvider).WithFixedSizing(10_000, 50).LoggerFactory(Log)); + var observed = CreateEntry("expired", timeProvider, TimeSpan.FromMilliseconds(100)); + Publish(cache, "key", observed); + timeProvider.Advance(TimeSpan.FromSeconds(1)); + Assert.True(observed.IsExpired); + var fresh = CreateEntry("fresh", timeProvider, TimeSpan.FromMinutes(5)); + Publish(cache, "key", fresh); + + // Act + bool removed = cache.TryRemoveEntry("key", observed); + + // Assert + Assert.False(removed); + Assert.Same(fresh, GetEntry(cache, "key")); + Assert.Equal(50, cache.CurrentMemorySize); + } + + [Fact] + public void UpdateEntry_WhenKeyAddedDuringAdd_RetriesAgainstAddedEntry() + { + // Arrange + var timeProvider = new FakeTimeProvider(DateTimeOffset.UtcNow); + using var cache = new InMemoryCacheClient(o => o.TimeProvider(timeProvider).WithFixedSizing(10_000, 50).LoggerFactory(Log)); + var competing = CreateEntry("competing", timeProvider); + int attempts = 0; + + // Act: another writer adds the key after this add read it as missing + bool added = cache.UpdateEntry("key", current => + { + if (++attempts is 1) + Publish(cache, "key", competing); + + return current is null ? (CreateEntry("mine", timeProvider), true) : (current, false); + }); + + // Assert + Assert.Equal(2, attempts); + Assert.False(added); + Assert.Same(competing, GetEntry(cache, "key")); + Assert.Equal(50, cache.CurrentMemorySize); + } + + [Fact] + public void UpdateEntry_WhenKeyChangesDuringRemove_KeepsNewEntry() + { + // Arrange + var timeProvider = new FakeTimeProvider(DateTimeOffset.UtcNow); + using var cache = new InMemoryCacheClient(o => o.TimeProvider(timeProvider).WithFixedSizing(10_000, 50).LoggerFactory(Log)); + Publish(cache, "key", CreateEntry("expected", timeProvider)); + var replacement = CreateEntry("replaced", timeProvider); + int attempts = 0; + + // Act: another writer replaces the value after this conditional remove matched it + bool removed = cache.UpdateEntry("key", current => + { + if (++attempts is 1) + Publish(cache, "key", replacement); + + return current?.StoredValue is "expected" ? (null, true) : (current, false); + }); + + // Assert + Assert.Equal(2, attempts); + Assert.False(removed); + Assert.Same(replacement, GetEntry(cache, "key")); + Assert.Equal(50, cache.CurrentMemorySize); + } + + [Fact] + public void UpdateEntry_WhenKeyChangesDuringUpdate_ReturnsOnlyPublishedResult() + { + // Arrange + var timeProvider = new FakeTimeProvider(DateTimeOffset.UtcNow); + using var cache = new InMemoryCacheClient(o => o.TimeProvider(timeProvider).WithFixedSizing(10_000, 50).LoggerFactory(Log)); + Publish(cache, "key", CreateEntry(1L, timeProvider)); + int attempts = 0; + + // Act: another writer changes the value after this increment read it + long result = cache.UpdateEntry("key", current => + { + if (++attempts is 1) + Publish(cache, "key", CreateEntry(10L, timeProvider)); + + long newValue = (long)current!.StoredValue! + 1; + return (current.WithValue(newValue, null, current.Size), newValue); + }); + + // Assert + Assert.Equal(2, attempts); + Assert.Equal(11, result); + Assert.Equal(11L, GetEntry(cache, "key").StoredValue); + Assert.Equal(50, cache.CurrentMemorySize); + } + + [Fact] + public void UpdateEntry_WhenKeyRemovedDuringUpdate_AddsInsteadOfUpdating() + { + // Arrange + // The key disappears between the read and the publish, so the retry must take the add path and report + // it. Reporting the discarded update's result is how AddAsync used to leave lock keys with no owner. + var timeProvider = new FakeTimeProvider(DateTimeOffset.UtcNow); + using var cache = new InMemoryCacheClient(o => o.TimeProvider(timeProvider).WithFixedSizing(10_000, 50).LoggerFactory(Log)); + Publish(cache, "key", CreateEntry("old", timeProvider)); + int attempts = 0; + + // Act + string outcome = cache.UpdateEntry("key", current => + { + if (++attempts is 1) + cache.UpdateEntry("key", _ => (null, true)); + + return current is null + ? (CreateEntry("added", timeProvider), "added") + : (current.WithValue("updated", null, current.Size), "updated"); + }); + + // Assert + Assert.Equal(2, attempts); + Assert.Equal("added", outcome); + Assert.Equal("added", GetEntry(cache, "key").StoredValue); + Assert.Equal(50, cache.CurrentMemorySize); + } + private record MyData(string Name, int Value); } diff --git a/tests/Foundatio.Tests/Caching/InMemoryHybridAwareCacheClientTests.cs b/tests/Foundatio.Tests/Caching/InMemoryHybridAwareCacheClientTests.cs index e86b97e8c..2c6e53576 100644 --- a/tests/Foundatio.Tests/Caching/InMemoryHybridAwareCacheClientTests.cs +++ b/tests/Foundatio.Tests/Caching/InMemoryHybridAwareCacheClientTests.cs @@ -25,6 +25,12 @@ protected override HybridCacheClient GetDistributedHybridCacheClient(bool should return new InMemoryHybridCacheClient(cache, _messageBus, Log, shouldThrowOnSerializationError); } + [Fact] + public override Task AddAsync_WithConcurrentAcquireAndRelease_OnlyOneOwnerAtATime() + { + return base.AddAsync_WithConcurrentAcquireAndRelease_OnlyOneOwnerAtATime(); + } + [Fact] public override Task AddAsync_WithConcurrentRequests_OnlyOneSucceeds() { @@ -198,6 +204,12 @@ public override Task GetUnixTimeSecondsAsync_WithUtcDateTime_ReturnsCorrectly() return base.GetUnixTimeSecondsAsync_WithUtcDateTime_ReturnsCorrectly(); } + [Fact] + public override Task IncrementAsync_WithConcurrentRequests_DoesNotLoseUpdates() + { + return base.IncrementAsync_WithConcurrentRequests_DoesNotLoseUpdates(); + } + [Fact] public override Task IncrementAsync_WithExpiration_SetsExpirationCorrectly() { @@ -228,6 +240,12 @@ public override Task IncrementAsync_WithMultipleInstances_InvalidatesOtherClient return base.IncrementAsync_WithMultipleInstances_InvalidatesOtherClientLocalCache(); } + [Fact] + public override Task ListAddAsync_WithConcurrentRequests_DoesNotLoseValues() + { + return base.ListAddAsync_WithConcurrentRequests_DoesNotLoseValues(); + } + [Fact] public override Task ListAddAsync_WithExpiration_SetsExpirationCorrectly() { @@ -638,6 +656,12 @@ public override Task SetExpirationAsync_WithExpiration_SetsExpirationCorrectly() return base.SetExpirationAsync_WithExpiration_SetsExpirationCorrectly(); } + [Fact] + public override Task SetIfHigherAsync_WithConcurrentRequests_DifferencesSumToMaximum() + { + return base.SetIfHigherAsync_WithConcurrentRequests_DifferencesSumToMaximum(); + } + [Fact] public override Task SetIfHigherAsync_WithDateTime_UpdatesWhenHigher() { @@ -656,6 +680,18 @@ public override Task SetIfHigherAsync_WithExpiration_SetsExpirationCorrectly() return base.SetIfHigherAsync_WithExpiration_SetsExpirationCorrectly(); } + [Fact] + public override Task SetIfHigherAsync_WithLowerValue_ReturnsZeroAndKeepsValue() + { + return base.SetIfHigherAsync_WithLowerValue_ReturnsZeroAndKeepsValue(); + } + + [Fact] + public override Task SetIfLowerAsync_WithConcurrentRequests_DifferencesSumToDecrease() + { + return base.SetIfLowerAsync_WithConcurrentRequests_DifferencesSumToDecrease(); + } + [Fact] public override Task SetIfLowerAsync_WithDateTime_UpdatesWhenLower() { diff --git a/tests/Foundatio.Tests/Caching/InMemoryHybridCacheClientTests.cs b/tests/Foundatio.Tests/Caching/InMemoryHybridCacheClientTests.cs index 993cf0f61..e27f0f938 100644 --- a/tests/Foundatio.Tests/Caching/InMemoryHybridCacheClientTests.cs +++ b/tests/Foundatio.Tests/Caching/InMemoryHybridCacheClientTests.cs @@ -21,6 +21,12 @@ protected override HybridCacheClient GetDistributedHybridCacheClient(bool should return new InMemoryHybridCacheClient(_distributedCache, _messageBus, Log, shouldThrowOnSerializationError); } + [Fact] + public override Task AddAsync_WithConcurrentAcquireAndRelease_OnlyOneOwnerAtATime() + { + return base.AddAsync_WithConcurrentAcquireAndRelease_OnlyOneOwnerAtATime(); + } + [Fact] public override Task AddAsync_WithConcurrentRequests_OnlyOneSucceeds() { @@ -194,6 +200,12 @@ public override Task GetUnixTimeSecondsAsync_WithUtcDateTime_ReturnsCorrectly() return base.GetUnixTimeSecondsAsync_WithUtcDateTime_ReturnsCorrectly(); } + [Fact(Skip = "Skip because HybridCacheClient can cache a stale local value under concurrent writes: https://github.com/FoundatioFx/Foundatio/issues/572")] + public override Task IncrementAsync_WithConcurrentRequests_DoesNotLoseUpdates() + { + return base.IncrementAsync_WithConcurrentRequests_DoesNotLoseUpdates(); + } + [Fact] public override Task IncrementAsync_WithExpiration_SetsExpirationCorrectly() { @@ -224,6 +236,12 @@ public override Task IncrementAsync_WithMultipleInstances_InvalidatesOtherClient return base.IncrementAsync_WithMultipleInstances_InvalidatesOtherClientLocalCache(); } + [Fact] + public override Task ListAddAsync_WithConcurrentRequests_DoesNotLoseValues() + { + return base.ListAddAsync_WithConcurrentRequests_DoesNotLoseValues(); + } + [Fact] public override Task ListAddAsync_WithExpiration_SetsExpirationCorrectly() { @@ -664,6 +682,12 @@ public override Task SetExpirationAsync_WithExpiration_SetsExpirationCorrectly() return base.SetExpirationAsync_WithExpiration_SetsExpirationCorrectly(); } + [Fact(Skip = "Skip because HybridCacheClient can cache a stale local value under concurrent writes: https://github.com/FoundatioFx/Foundatio/issues/572")] + public override Task SetIfHigherAsync_WithConcurrentRequests_DifferencesSumToMaximum() + { + return base.SetIfHigherAsync_WithConcurrentRequests_DifferencesSumToMaximum(); + } + [Fact] public override Task SetIfHigherAsync_WithDateTime_UpdatesWhenHigher() { @@ -682,6 +706,18 @@ public override Task SetIfHigherAsync_WithExpiration_SetsExpirationCorrectly() return base.SetIfHigherAsync_WithExpiration_SetsExpirationCorrectly(); } + [Fact] + public override Task SetIfHigherAsync_WithLowerValue_ReturnsZeroAndKeepsValue() + { + return base.SetIfHigherAsync_WithLowerValue_ReturnsZeroAndKeepsValue(); + } + + [Fact(Skip = "Skip because HybridCacheClient can cache a stale local value under concurrent writes: https://github.com/FoundatioFx/Foundatio/issues/572")] + public override Task SetIfLowerAsync_WithConcurrentRequests_DifferencesSumToDecrease() + { + return base.SetIfLowerAsync_WithConcurrentRequests_DifferencesSumToDecrease(); + } + [Fact] public override Task SetIfLowerAsync_WithDateTime_UpdatesWhenLower() { diff --git a/tests/Foundatio.Tests/Caching/ScopedInMemoryCacheClientTests.cs b/tests/Foundatio.Tests/Caching/ScopedInMemoryCacheClientTests.cs index 407eeb139..755a08bad 100644 --- a/tests/Foundatio.Tests/Caching/ScopedInMemoryCacheClientTests.cs +++ b/tests/Foundatio.Tests/Caching/ScopedInMemoryCacheClientTests.cs @@ -15,6 +15,12 @@ protected override ICacheClient GetCacheClient(bool shouldThrowOnSerializationEr return new ScopedCacheClient(new InMemoryCacheClient(o => o.LoggerFactory(Log).CloneValues(true).ShouldThrowOnSerializationError(shouldThrowOnSerializationError)), "scoped"); } + [Fact] + public override Task AddAsync_WithConcurrentAcquireAndRelease_OnlyOneOwnerAtATime() + { + return base.AddAsync_WithConcurrentAcquireAndRelease_OnlyOneOwnerAtATime(); + } + [Fact] public override Task AddAsync_WithConcurrentRequests_OnlyOneSucceeds() { @@ -164,6 +170,12 @@ public override Task GetUnixTimeSecondsAsync_WithUtcDateTime_ReturnsCorrectly() return base.GetUnixTimeSecondsAsync_WithUtcDateTime_ReturnsCorrectly(); } + [Fact] + public override Task IncrementAsync_WithConcurrentRequests_DoesNotLoseUpdates() + { + return base.IncrementAsync_WithConcurrentRequests_DoesNotLoseUpdates(); + } + [Fact] public override Task IncrementAsync_WithExpiration_SetsExpirationCorrectly() { @@ -188,6 +200,12 @@ public override Task IncrementAsync_WithScopedCache_WorksWithinScope() return base.IncrementAsync_WithScopedCache_WorksWithinScope(); } + [Fact] + public override Task ListAddAsync_WithConcurrentRequests_DoesNotLoseValues() + { + return base.ListAddAsync_WithConcurrentRequests_DoesNotLoseValues(); + } + [Fact] public override Task ListAddAsync_WithExpiration_SetsExpirationCorrectly() { @@ -556,6 +574,12 @@ public override Task SetExpirationAsync_WithExpiration_SetsExpirationCorrectly() return base.SetExpirationAsync_WithExpiration_SetsExpirationCorrectly(); } + [Fact] + public override Task SetIfHigherAsync_WithConcurrentRequests_DifferencesSumToMaximum() + { + return base.SetIfHigherAsync_WithConcurrentRequests_DifferencesSumToMaximum(); + } + [Fact] public override Task SetIfHigherAsync_WithDateTime_UpdatesWhenHigher() { @@ -574,6 +598,18 @@ public override Task SetIfHigherAsync_WithExpiration_SetsExpirationCorrectly() return base.SetIfHigherAsync_WithExpiration_SetsExpirationCorrectly(); } + [Fact] + public override Task SetIfHigherAsync_WithLowerValue_ReturnsZeroAndKeepsValue() + { + return base.SetIfHigherAsync_WithLowerValue_ReturnsZeroAndKeepsValue(); + } + + [Fact] + public override Task SetIfLowerAsync_WithConcurrentRequests_DifferencesSumToDecrease() + { + return base.SetIfLowerAsync_WithConcurrentRequests_DifferencesSumToDecrease(); + } + [Fact] public override Task SetIfLowerAsync_WithDateTime_UpdatesWhenLower() { diff --git a/tests/Foundatio.Tests/Caching/ScopedInMemoryHybridCacheClientTests.cs b/tests/Foundatio.Tests/Caching/ScopedInMemoryHybridCacheClientTests.cs index 8a05941f8..00abfc0aa 100644 --- a/tests/Foundatio.Tests/Caching/ScopedInMemoryHybridCacheClientTests.cs +++ b/tests/Foundatio.Tests/Caching/ScopedInMemoryHybridCacheClientTests.cs @@ -18,6 +18,12 @@ protected override HybridCacheClient GetDistributedHybridCacheClient(bool should return new InMemoryHybridCacheClient(_distributedCache, _messageBus, Log, shouldThrowOnSerializationError); } + [Fact] + public override Task AddAsync_WithConcurrentAcquireAndRelease_OnlyOneOwnerAtATime() + { + return base.AddAsync_WithConcurrentAcquireAndRelease_OnlyOneOwnerAtATime(); + } + [Fact] public override Task AddAsync_WithConcurrentRequests_OnlyOneSucceeds() { @@ -185,6 +191,12 @@ public override Task GetUnixTimeSecondsAsync_WithUtcDateTime_ReturnsCorrectly() return base.GetUnixTimeSecondsAsync_WithUtcDateTime_ReturnsCorrectly(); } + [Fact(Skip = "Skip because HybridCacheClient can cache a stale local value under concurrent writes: https://github.com/FoundatioFx/Foundatio/issues/572")] + public override Task IncrementAsync_WithConcurrentRequests_DoesNotLoseUpdates() + { + return base.IncrementAsync_WithConcurrentRequests_DoesNotLoseUpdates(); + } + [Fact] public override Task IncrementAsync_WithExpiration_SetsExpirationCorrectly() { @@ -215,6 +227,12 @@ public override Task IncrementAsync_WithMultipleInstances_InvalidatesOtherClient return base.IncrementAsync_WithMultipleInstances_InvalidatesOtherClientLocalCache(); } + [Fact] + public override Task ListAddAsync_WithConcurrentRequests_DoesNotLoseValues() + { + return base.ListAddAsync_WithConcurrentRequests_DoesNotLoseValues(); + } + [Fact] public override Task ListAddAsync_WithExpiration_SetsExpirationCorrectly() { @@ -625,6 +643,12 @@ public override Task SetExpirationAsync_WithExpiration_SetsExpirationCorrectly() return base.SetExpirationAsync_WithExpiration_SetsExpirationCorrectly(); } + [Fact(Skip = "Skip because HybridCacheClient can cache a stale local value under concurrent writes: https://github.com/FoundatioFx/Foundatio/issues/572")] + public override Task SetIfHigherAsync_WithConcurrentRequests_DifferencesSumToMaximum() + { + return base.SetIfHigherAsync_WithConcurrentRequests_DifferencesSumToMaximum(); + } + [Fact] public override Task SetIfHigherAsync_WithDateTime_UpdatesWhenHigher() { @@ -643,6 +667,18 @@ public override Task SetIfHigherAsync_WithExpiration_SetsExpirationCorrectly() return base.SetIfHigherAsync_WithExpiration_SetsExpirationCorrectly(); } + [Fact] + public override Task SetIfHigherAsync_WithLowerValue_ReturnsZeroAndKeepsValue() + { + return base.SetIfHigherAsync_WithLowerValue_ReturnsZeroAndKeepsValue(); + } + + [Fact(Skip = "Skip because HybridCacheClient can cache a stale local value under concurrent writes: https://github.com/FoundatioFx/Foundatio/issues/572")] + public override Task SetIfLowerAsync_WithConcurrentRequests_DifferencesSumToDecrease() + { + return base.SetIfLowerAsync_WithConcurrentRequests_DifferencesSumToDecrease(); + } + [Fact] public override Task SetIfLowerAsync_WithDateTime_UpdatesWhenLower() { diff --git a/tests/Foundatio.Tests/Locks/InMemoryLockTests.cs b/tests/Foundatio.Tests/Locks/InMemoryLockTests.cs index de9c2d073..a10e8b152 100644 --- a/tests/Foundatio.Tests/Locks/InMemoryLockTests.cs +++ b/tests/Foundatio.Tests/Locks/InMemoryLockTests.cs @@ -1,8 +1,10 @@ using System; +using System.Threading; using System.Threading.Tasks; using Foundatio.Caching; using Foundatio.Lock; using Foundatio.Messaging; +using Microsoft.Extensions.Time.Testing; using Xunit; namespace Foundatio.Tests.Locks; @@ -142,6 +144,50 @@ public override Task ReleaseAsync_WithForceRelease_ReleasesLockWithoutLockId() return base.ReleaseAsync_WithForceRelease_ReleasesLockWithoutLockId(); } + [Fact] + public async Task TryAcquireAsync_WithExhaustedPeriod_WaitsForNextPeriod() + { + // Arrange + const int allowedLocks = 25; + var period = TimeSpan.FromSeconds(2); + // A fixed start aligned to the throttling period, so the boundary below is deterministic + var timeProvider = new FakeTimeProvider(new DateTimeOffset(2026, 1, 1, 0, 0, 0, TimeSpan.Zero)); + using var cache = new InMemoryCacheClient(o => o.TimeProvider(timeProvider).LoggerFactory(Log)); + var locker = new ThrottlingLockProvider(cache, allowedLocks, period, timeProvider, null, Log); + for (int i = 0; i < allowedLocks; i++) + { + await using var hit = await locker.TryAcquireAsync("resource", cancellationToken: TestCancellationToken); + Assert.NotNull(hit); + } + using var cancellation = CancellationTokenSource.CreateLinkedTokenSource(TestCancellationToken); + + // Act + var waiting = locker.TryAcquireAsync("resource", cancellationToken: cancellation.Token); + try + { + bool waitsForQuota = !waiting.IsCompleted; + timeProvider.Advance(period - TimeSpan.FromMilliseconds(1)); + bool waitsBeforeBoundary = !waiting.IsCompleted; + bool exhaustedBeforeBoundary = await locker.IsLockedAsync("resource"); + timeProvider.Advance(TimeSpan.FromMilliseconds(1)); + bool exhaustedAtBoundary = await locker.IsLockedAsync("resource"); + // The provider wakes one millisecond after the boundary to avoid early system-timer wakeups. + timeProvider.Advance(TimeSpan.FromMilliseconds(1)); + await using var nextPeriodHit = await waiting.WaitAsync(TimeSpan.FromSeconds(5), TestCancellationToken); + + // Assert + Assert.True(waitsForQuota); + Assert.True(waitsBeforeBoundary); + Assert.True(exhaustedBeforeBoundary); + Assert.False(exhaustedAtBoundary); + Assert.NotNull(nextPeriodHit); + } + finally + { + await cancellation.CancelAsync(); + } + } + [Fact] public override Task TryUsingAsync_WithSuccessfulAction_ExecutesAndReleasesLock() {