Fix persistence, cache, encoding, DI and distribution bugs [patch]

CLAUDE.md

@@ -45,12 +45,14 @@ This is a .NET library (`ktsu.Essentials`) providing high-performance interfaces
 - `Essentials/IContinuousDistribution.cs` - `IDistribution<double>` plus `Pdf`/`LogPdf`
 - `Essentials/IDiscreteDistribution.cs` - `IDistribution<int>` plus `Pmf`/`LogPmf`, and a default `Quantile` that bisects the CDF. That default is an explicit reimplementation of `IDistribution<int>.Quantile`, which CA1033 misreads as an explicit implementation on a class; the suppression on it says why
 - `Shared/DeflateBound.cs` - zlib's `deflateBound` plus container overhead, linked into the Deflate, Gzip and ZLib providers as their `GetMaxCompressedLength`
-- `Shared/BoundedDecompression.cs` - The span overload of `TryDecompress` for Brotli, Deflate, Gzip and ZLib: reads the decompressing stream straight into the destination and fails once it is full and more output remains, so the destination bounds the work. See issue #122
+- `Shared/BoundedDecompression.cs` - The span overload of `TryDecompress` for Deflate, Gzip and ZLib: reads the decompressing stream straight into the destination and fails once it is full and more output remains, so the destination bounds the work. See issue #122. Brotli uses `BrotliDecoder` directly instead, which reports a truncated stream itself
+- `Shared/CompressedStreamEnd.cs` - Gzip's CRC-32 and length trailer and ZLib's Adler-32, checked against the output on every decompression path, because `GZipStream` and `ZLibStream` treat input that runs out as a clean end of stream. Raw Deflate has no trailer, so its truncation cannot be detected. See issue #80
+- `Shared/StreamChunks.cs` - Reads a stream a chunk at a time through a pooled buffer, linked into the FNV hash and the Xor, Caesar and BitRotate obfuscation providers, whose stream paths otherwise made a call per byte. See issue #91
 - `Shared/SpecialFunctions.cs` - Incomplete gamma, incomplete beta, error function, normal quantile and cancellation-free `ExpM1`/`Log1P`, linked into the Normal, LogNormal, Exponential, Binomial, Poisson and Geometric providers rather than placed in the interfaces-only package
 - `Shared/DistributionArguments.cs` - Parameter validation shared by every distribution provider, linked in the same way
 - `Shared/ProviderRegistration.cs` - DI registration for providers whose `Add…Provider` overloads take arguments (Xor, Caesar, BitRotate and every distribution): a call replaces an earlier registration of the same concrete type in place, and `AddEssentials()` keeps a registration made before it, whether as the concrete type, the interface or both, moved to its default's position, rather than overwriting it; a concrete-only one gains an interface registration forwarding to it. Linked into those providers and `Essentials.All`. See issues #132 and #156
-- `Shared/PersistenceDefaults.cs` - The `IPersistenceProvider` default `RetrieveOrCreateAsync` body, linked into the FileSystem and Temp providers, which declare that member themselves and so hide the default
-- `Shared/AtomicFile.cs` - The FileSystem and Temp providers' write and read paths: each store stages its data in a uniquely named `<guid>.tmp` beside the target and publishes it with an overwriting move, retrying briefly while Windows holds the target open, so concurrent writers can't collide and a reader never sees the key missing; reads retry only the Windows sharing violation a replace in progress raises. Linked into both providers like `PersistenceDefaults.cs`
+- `Shared/FileRecordStore.cs` - Everything the FileSystem and Temp persistence providers do, which differ only in where their directory is; linked into both, and ConfigHome and DataHome delegate to FileSystem. Cancellation passes through as itself, every other failure is a `PersistenceProviderException`. A record that is there but deserializes to null throws rather than reading as missing, so `RetrieveOrCreateAsync` never writes a default over a file it could not parse (#82). `RetrieveOrCreateAsync` publishes its new instance only if nothing has appeared meanwhile (#144); `ClearAsync` also removes staging files older than a minute, which an interrupted write left behind (#145)
+- `Shared/AtomicFile.cs` - The file operations under it: each store stages its data in a uniquely named `<guid>.tmp` beside the target and publishes it with an overwriting move, so concurrent writers can't collide and a reader never sees the key missing; reads and deletes retry the sharing violation a reader or a replace in progress raises (#143). `TryCreateAsync` opens the target with `FileMode.CreateNew` rather than staging and moving, because a non-overwriting `File.Move` is a check then a rename on Unix and loses a record published in between. Off Windows, .NET emulates `FileShare.None` with an advisory lock whose refusal carries a raw `EWOULDBLOCK` (11 on Linux, 35 on macOS) as its `HResult`, which counts as a sharing violation there
 - `Essentials/ISerializationProvider.cs` - Object serialization/deserialization interface
 - `Shared/Utf8Bom.cs` - Strips a leading UTF-8 byte order mark, linked into the four serialization providers so each byte `Deserialize` overload reads `File.ReadAllBytes` output from Notepad or Visual Studio the same way its text overloads do
 - `Essentials/ISerializationOptions.cs` - Configurable serialization options (naming, inclusion, boxing policies)
@@ -154,7 +156,10 @@ the line, including inside a `///` comment, which is how the doc comment broke w
 
 Rolling happens before an entry that would push a non-empty file past `MaxFileSizeBytes`, so no file exceeds
 the limit unless a single entry does, and such an entry is written whole to a fresh file. `MaxFileCount`
-counts the current file, so 1 means a full file is deleted and started again.
+counts the current file, so 1 means a full file is deleted and started again. A roll is all or nothing: the active file and every rolled one move under unique `.rolling`
+staging names or up a slot, each move recorded and undone in reverse if a later one is refused, and the
+oldest is deleted only after the roll is complete. A refused roll (a viewer holding the file without
+`FileShare.Delete` on Windows) appends the entry past the limit rather than dropping it. See issue #120.
 
 ### New packages and the validation baseline
 

Essentials.All/ServiceCollectionExtensions.cs

@@ -132,7 +132,7 @@ public static IServiceCollection AddCompressionProviders(this IServiceCollection
 #endif
 
 		// Registered last so that it is the provider an ICompressionProvider resolves to.
-		return services.AddGzipCompressionProvider();
+		return services.AddDefaultOrKeep<ICompressionProvider, GzipCompressionProvider>(static s => s.AddGzipCompressionProvider());
 	}
 
 	/// <summary>
@@ -184,7 +184,7 @@ public static IServiceCollection AddRandomProviders(this IServiceCollection serv
 			.AddXoshiroRandomProvider()
 			.AddPcgRandomProvider()
 			// Registered last so that it is the provider an IRandomProvider resolves to.
-			.AddCryptoRandomProvider();
+			.AddDefaultOrKeep<IRandomProvider, CryptoRandomProvider>(static s => s.AddCryptoRandomProvider());
 	}
 
 	/// <summary>
@@ -199,7 +199,7 @@ public static IServiceCollection AddEncodingProviders(this IServiceCollection se
 		return services
 			.AddHexEncodingProvider()
 			// Registered last so that it is the provider an IEncodingProvider resolves to.
-			.AddBase64EncodingProvider();
+			.AddDefaultOrKeep<IEncodingProvider, Base64EncodingProvider>(static s => s.AddBase64EncodingProvider());
 	}
 
 	/// <summary>
@@ -239,7 +239,7 @@ public static IServiceCollection AddHashProviders(this IServiceCollection servic
 			.AddXxHash3HashProvider()
 			.AddXxHash128HashProvider()
 			// Registered last so that it is the provider an IHashProvider resolves to.
-			.AddSHA256HashProvider();
+			.AddDefaultOrKeep<IHashProvider, SHA256HashProvider>(static s => s.AddSHA256HashProvider());
 	}
 
 	/// <summary>
@@ -255,7 +255,7 @@ public static IServiceCollection AddKeyedHashProviders(this IServiceCollection s
 			.AddHmacSha384KeyedHashProvider()
 			.AddHmacSha512KeyedHashProvider()
 			// Registered last so that it is the provider an IKeyedHashProvider resolves to.
-			.AddHmacSha256KeyedHashProvider();
+			.AddDefaultOrKeep<IKeyedHashProvider, HmacSha256KeyedHashProvider>(static s => s.AddHmacSha256KeyedHashProvider());
 	}
 
 	/// <summary>
@@ -296,7 +296,7 @@ public static IServiceCollection AddSerializationProviders(this IServiceCollecti
 			.AddTomlSerializationProvider()
 			// Registered last so that it is the provider an ISerializationProvider resolves to, which
 			// also makes it the format the file-backed persistence providers write.
-			.AddJsonSerializationProvider();
+			.AddDefaultOrKeep<ISerializationProvider, JsonSerializationProvider>(static s => s.AddJsonSerializationProvider());
 	}
 
 	/// <summary>

Essentials.CacheProviders.InMemory/InMemoryCacheProvider.cs

@@ -6,15 +6,63 @@ namespace ktsu.Essentials.CacheProviders.InMemory;
 using System;
 using System.Collections.Concurrent;
 using System.Collections.Generic;
+using System.Diagnostics;
+using System.Linq;
+using System.Threading;
 
 /// <summary>
 /// An in-memory cache provider that stores key-value pairs with optional expiration support.
 /// </summary>
+/// <remarks>
+/// <para>
+/// A time-to-live is measured on a monotonic clock, not the wall clock, so a clock correction, a manual change
+/// or a virtual machine resuming can neither keep an entry past its lifetime nor expire one early.
+/// </para>
+/// <para>
+/// Expired entries are swept away as entries are set, so a cache of short-lived keys that are seldom read again
+/// does not grow without bound. A sweep visits every entry, and runs once a second at most or once as many
+/// entries have been set as the cache holds, whichever comes first, so its cost is spread across those sets.
+/// </para>
+/// </remarks>
 /// <typeparam name="TKey">The type of the cache key.</typeparam>
 /// <typeparam name="TValue">The type of the cached value.</typeparam>
 public class InMemoryCacheProvider<TKey, TValue> : ICacheProvider<TKey, TValue> where TKey : notnull
 {
+	/// <summary>The fewest sets between two sweeps, so a small cache is not swept on every set.</summary>
+	private const int SweepMinimumSets = 1024;
+
+	/// <summary>The expiration of an entry that never expires.</summary>
+	private const long Never = long.MaxValue;
+
 	private readonly ConcurrentDictionary<TKey, CacheEntry> cache = new();
+	private readonly Func<long> clock;
+	private readonly long sweepInterval;
+	private long lastSweep;
+	private int setsSinceSweep;
+	private int sweeping;
+
+	/// <summary>
+	/// Initializes a new instance of the <see cref="InMemoryCacheProvider{TKey, TValue}"/> class.
+	/// </summary>
+	public InMemoryCacheProvider()
+		: this(Stopwatch.GetTimestamp)
+	{
+	}
+
+	/// <summary>
+	/// Initializes a new instance over a clock other than <see cref="Stopwatch.GetTimestamp"/>, so tests can move
+	/// time on rather than sleep.
+	/// </summary>
+	/// <param name="clock">Returns the current time in <see cref="Stopwatch"/> ticks, never going backwards.</param>
+	internal InMemoryCacheProvider(Func<long> clock)
+	{
+		this.clock = clock;
+		sweepInterval = Stopwatch.Frequency;
+		lastSweep = clock();
+	}
+
+	/// <summary>Gets how many entries the cache holds, expired or not.</summary>
+	internal int Count => cache.Count;
 
 	/// <summary>
 	/// Tries to get a cached value by key.
@@ -26,7 +74,7 @@ public bool TryGet(TKey key, out TValue? value)
 	{
 		if (cache.TryGetValue(key, out CacheEntry? entry))
 		{
-			if (entry.Expiration is null || entry.Expiration > DateTime.UtcNow)
+			if (!entry.IsExpired(clock()))
 			{
 				value = entry.Value;
 				return true;
@@ -47,8 +95,50 @@ public bool TryGet(TKey key, out TValue? value)
 	/// <param name="key">The cache key.</param>
 	/// <param name="value">The value to cache.</param>
 	/// <param name="expiration">The optional time-to-live for the cached entry. If null, the entry does not expire.</param>
-	public void Set(TKey key, TValue value, TimeSpan? expiration = null) =>
-		cache[key] = new CacheEntry(value, ComputeExpiration(expiration));
+	public void Set(TKey key, TValue value, TimeSpan? expiration = null)
+	{
+		long now = clock();
+		cache[key] = new CacheEntry(value, ComputeExpiration(expiration, now));
+		SweepIfDue(now);
+	}
+
+	/// <summary>
+	/// Gets the value cached under <paramref name="key"/>, or adds and returns the one <paramref name="factory"/> makes.
+	/// </summary>
+	/// <remarks>
+	/// Adds only if nothing live is cached by the time the factory returns: a value set while the factory ran is
+	/// kept and returned, and callers racing on one missing key all get the one value left cached. The factory
+	/// may therefore run and have its result discarded, as with <see cref="ConcurrentDictionary{TKey, TValue}"/>.
+	/// </remarks>
+	/// <param name="key">The cache key.</param>
+	/// <param name="factory">Makes the value to add when none is cached.</param>
+	/// <param name="expiration">The optional time-to-live for an added entry. If null, the entry does not expire.</param>
+	/// <returns>The cached value.</returns>
+	public TValue GetOrAdd(TKey key, Func<TKey, TValue> factory, TimeSpan? expiration = null)
+	{
+		Ensure.NotNull(factory);
+
+		while (true)
+		{
+			if (cache.TryGetValue(key, out CacheEntry? observed) && observed is { } existing && !existing.IsExpired(clock()))
+			{
+				return existing.Value;
+			}
+
+			TValue value = factory(key);
+			long now = clock();
+			CacheEntry created = new(value, ComputeExpiration(expiration, now));
+
+			// Replace only what was seen to be missing or expired; anything else means another writer got there
+			// first, and the loop returns its value.
+			bool stored = observed is null ? cache.TryAdd(key, created) : cache.TryUpdate(key, created, observed);
+			if (stored)
+			{
+				SweepIfDue(now);
+				return value;
+			}
+		}
+	}
 
 	/// <summary>
 	/// Removes a cached value by key.
@@ -59,32 +149,57 @@ public void Set(TKey key, TValue value, TimeSpan? expiration = null) =>
 	/// reports false, matching <see cref="TryGet"/>, which treats it as absent.
 	/// </returns>
 	public bool Remove(TKey key) =>
-		cache.TryRemove(key, out CacheEntry? entry) && (entry.Expiration is null || entry.Expiration > DateTime.UtcNow);
+		cache.TryRemove(key, out CacheEntry? entry) && !entry.IsExpired(clock());
 
 	/// <summary>
 	/// Clears all entries from the cache.
 	/// </summary>
 	public void Clear() => cache.Clear();
 
 	/// <summary>
-	/// Converts a time-to-live into an absolute expiration time, saturating rather than overflowing.
-	/// A time-to-live that reaches past <see cref="DateTime.MaxValue"/> never expires, and one that
-	/// reaches before <see cref="DateTime.MinValue"/> has already expired.
+	/// Converts a time-to-live into an expiration on the clock, saturating rather than overflowing. A time-to-live
+	/// too long to represent never expires, and one reaching before the clock's start has already expired.
 	/// </summary>
-	private static DateTime? ComputeExpiration(TimeSpan? expiration)
+	private static long ComputeExpiration(TimeSpan? expiration, long now)
 	{
 		if (expiration is null)
 		{
-			return null;
+			return Never;
 		}
 
-		DateTime now = DateTime.UtcNow;
-		if (expiration.Value > DateTime.MaxValue - now)
+		// In double, which holds every TimeSpan's tick count to within a part in 2^53 and cannot overflow here.
+		double ticks = expiration.Value.Ticks * ((double)Stopwatch.Frequency / TimeSpan.TicksPerSecond);
+		double at = now + ticks;
+		if (at >= Never)
 		{
-			return null;
+			return Never;
 		}
 
-		return expiration.Value < DateTime.MinValue - now ? DateTime.MinValue : now + expiration.Value;
+		return at <= long.MinValue ? long.MinValue : (long)at;
+	}
+
+	private void SweepIfDue(long now)
+	{
+		int sets = Interlocked.Increment(ref setsSinceSweep);
+		bool due = sets >= Math.Max(SweepMinimumSets, cache.Count) || now - Interlocked.Read(ref lastSweep) >= sweepInterval;
+		if (!due || Interlocked.CompareExchange(ref sweeping, 1, 0) != 0)
+		{
+			return;
+		}
+
+		try
+		{
+			Interlocked.Exchange(ref setsSinceSweep, 0);
+			Interlocked.Exchange(ref lastSweep, now);
+			foreach (KeyValuePair<TKey, CacheEntry> pair in cache.Where(pair => pair.Value.IsExpired(now)))
+			{
+				RemoveEntry(pair.Key, pair.Value);
+			}
+		}
+		finally
+		{
+			Interlocked.Exchange(ref sweeping, 0);
+		}
 	}
 
 	private void RemoveEntry(TKey key, CacheEntry entry)
@@ -97,9 +212,10 @@ private void RemoveEntry(TKey key, CacheEntry entry)
 #endif
 	}
 
-	private sealed class CacheEntry(TValue value, DateTime? expiration)
+	private sealed class CacheEntry(TValue value, long expiration)
 	{
 		public TValue Value { get; } = value;
-		public DateTime? Expiration { get; } = expiration;
+
+		public bool IsExpired(long now) => expiration <= now;
 	}
 }