BrighterCommand / BrighterCommand/Brighter

ServiceProviderLifetimeScope caches a faulted resolution forever (Lazy ExecutionAndPublication)

Open
#4,260 0 comments 0 reactions 1 assignee Claimed by @iancooper View on GitHub
.NET 2 - In Progress V10.X
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

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.