BrighterCommand / BrighterCommand/Brighter
ServiceProviderLifetimeScope caches a faulted resolution forever (Lazy ExecutionAndPublication)
- Dominant language
- C#
- Stars
- 2.5k
- Forks
- 296
- Avg merge
- 1d 11h
- Merged PRs (30d)
- 21
Description
### Where
`src/Paramore.Brighter.Extensions.DependencyInjection/ServiceProviderLifetimeScope.cs`
- `GetOrCreateSingleton` — `_singletonInstances.GetOrAdd(type, _ => new Lazy(() => _serviceProvider.GetService(type)))` (`:154-155`)
- `GetOrCreateScoped` — `_scopedInstances.GetOrAdd(type, _ => new Lazy(() => { ... scope.ServiceProvider.GetService(type) ... }))` (`:167-176`)
### Problem
Both memoize a resolution with a `Lazy` constructed **without** a `LazyThreadSafetyMode`, i.e. the default `ExecutionAndPublication`, which **caches an exception thrown by the value factory permanently**. If `IServiceProvider.GetService(type)` throws — a DI misconfiguration, a mapper/transform/handler constructor that throws, or a transient fault during resolution — that exception is cached against the type for the remaining life of the lifetime scope. Every later resolution of the same type on that scope rethrows the **stale** exception, even after the underlying cause is fixed and a fresh resolution would now succeed.
### Scope / provenance
Pre-existing; predates #4254. The #4254 review (finding 5) surfaced it because that PR adds an intentional `throw Disposed()` inside the scoped value factory. **That specific trigger is benign** — it only fires once `_scope` has already been claimed by `Dispose`, so the scope is being torn down and throwing `ObjectDisposedException` is correct; the object is dead regardless, so caching the throw changes nothing. The lasting concern is the `GetService`-faulting case described above, which is independent of #4254.
### Options (needs a decision on semantics)
- `LazyThreadSafetyMode.PublicationOnly` — does not cache a faulted factory; each caller retries until one succeeds. Trade-off: the factory can run more than once concurrently (fine for `GetService`, which is idempotent).
- Catch a faulted `Lazy` and evict it from the dictionary so the next resolution retries.
- Deliberately keep fail-fast caching and document it.
Retry-until-success (`PublicationOnly` or evict-on-fault) is the likely intent for a resolution cache, but it is a behaviour choice worth deciding explicitly.
Contributor guide
Assessment
This issue has not been assessed yet.