Repository navigation
AddCache is not safe to call concurrently — static registration dictionary races #55
Description
CacheRegistrationExtensions merges each AddCache call into a process-wide static dictionary without synchronization, so two hosts built concurrently in one process can throw.
private static readonly Dictionary<Type, CacheTypeOptions> _configuredPersistTypes = new();
private static void AppendPreviousRegistrations(CacheOptions o)
{
var previouslyRegisteredTypes = _configuredPersistTypes.ToArray(); // sizes its array from Count
foreach (var item in o.GetRegistered())
{
_configuredPersistTypes.TryAdd(item.Key, item.Value); // grows it from another thread
}
...
}ToArray() on a Dictionary allocates from Count and then copies. If another thread is inside TryAdd on the same static instance, the copy overruns the array it just sized:
System.ArgumentException : Destination array is not long enough to copy all the items in the collection.
at System.Collections.Generic.Dictionary`2.CopyTo(KeyValuePair`2[] array, Int32 index)
at System.Linq.Enumerable.ICollectionToArray[TSource](ICollection`1 collection)
at Tharga.Cache.CacheRegistrationExtensions.AppendPreviousRegistrations(CacheOptions o)
at Tharga.Cache.CacheRegistrationExtensions.AddCache(IServiceCollection serviceCollection, Action`1 options)
How we hit it
An xUnit integration-test assembly where each test constructs its own WebApplicationFactory<Program>. xUnit runs test classes in parallel, so several hosts build at once, each calling AddCache (for us transitively, via AddQuilt4NetApplicationInsightsClientRemote). It appeared only after the assembly grew past a handful of concurrent boots, which made it read as a flaky test rather than as a race — the failure names an array copy, and nothing in the failing test mentions caching.
Worked around with [assembly: CollectionBehavior(DisableTestParallelization = true)].
Why it seems worth fixing rather than documenting
AddCache is a registration API, and registration is normally safe to call from anywhere. Nothing in the signature suggests process-wide shared state, so a consumer has no reason to serialize it. Any host that builds two service providers concurrently — parallel tests, a multi-tenant host spinning up isolated containers, a WebApplicationFactory used from more than one fixture — can hit it, and it will present as an unrelated intermittent failure.
Possible directions
- A
lockaround the read-and-merge inAppendPreviousRegistrations, which is the smallest fix. ConcurrentDictionary— note this alone is not sufficient:ToArray()on aConcurrentDictionaryis atomic, so it would fix this particular throw, but the read-then-merge would still interleave.- Scoping the accumulated registrations to the
IServiceCollectionrather than to the process, which removes the shared state entirely. This looks like the real fix, though it presumably changes behaviour for hosts that deliberately rely on cross-call accumulation.
Observed on Tharga.Cache 1.0.0, .NET 10.
Activity
poxet commentedon Aug 16, 2026
Fixed in #60.
Took the third of your three suggested directions — scoping the accumulated registrations to the
IServiceCollection— and your instinct that it was the real fix was right, for a reason beyond the race itself. The dictionary was process-wide state behind a per-container API, so one host's type registrations leaked into an unrelated host's merged options. The test suite was already working around that:AddCacheIdempotencyTestscalled an internalResetRegistrations()from its constructor andDisposepurely to stop one test's registrations reaching the next.It turned out no new plumbing was needed.
AddCachealready replacesIOptions<CacheOptions>on every call, so the previous call's options are still in the collection as anImplementationInstanceat the point the merge runs:No lock, no shared state, no cross-host leak.
_configuredPersistTypesandResetRegistrationsare deleted, and the idempotency tests now pass without the reset hooks.On your note that it read as a flaky test rather than a race — that was worth acting on. A new
AddCacheConcurrencyTestsbuilds 64 hosts in parallel and asserts both that nothing throws and that no registration crosses between collections. Against the old code all three tests failed, reproducing your exactArgumentException: Destination array is not long enoughatDictionary.CopyTo←Enumerable.ToArray, plus anInvalidOperationExceptionabout concurrent mutation.You can delete
[assembly: CollectionBehavior(DisableTestParallelization = true)]once you are on the release carrying this.One deliberate semantic change worth knowing about, since you called out that scoping might alter behaviour for hosts relying on cross-call accumulation: within a single service collection, a third-or-later
AddCachecall that does not mention a type now inherits the most recent value for it rather than the first-ever one. Two-call behaviour is unchanged.Merge pull request #60 from Tharga/fix/issue-55-56-registration-race-…