Fix open generic multi-service registrations overriding later defaults

[tillig]

Fixes #1465

A registration source that provides one component for several services applied that component to every one of those services immediately. Services that had not yet drained the source took it ahead of the higher-priority sources they still had to query, and since the first source implementation is the default, whichever service was resolved second got the shared component rather than its own overriding registration.

That made the result depend on resolution order, so two identical containers could disagree:

builder.RegisterGeneric(typeof(AB<>)).As(typeof(IA<>)).As(typeof(IB<>));
builder.RegisterGeneric(typeof(A<>)).As(typeof(IA<>));
builder.RegisterGeneric(typeof(B<>)).As(typeof(IB<>));

container.Resolve<IA<int>>(); // A<int>  - correct
container.Resolve<IB<int>>(); // AB<int> - expected B<int>

Closed types were unaffected, because they never go through the source-query path.

Proposed Changes

• Hold a source-provided implementation against the source that produced it when the target service has not reached that source yet, and apply it when the service drains that source, so source priority rather than resolution order decides the default.
• Keep the additional service's source queue intact instead of removing the already-run source. The held implementation is applied in place of re-querying, so the source is still queried once and a shared component stays shared.
• Replace AddRegistration's originatedFromDynamicSource flag with the originating IRegistrationSource. The flag was only ever true when a source produced the registration, so the source being non-null carries the same information and the two cannot drift apart.
• Remove ExcludeSource, InitializeOrSkipSource, and ServiceRegistrationInfo.SkipSource, which this replaces.
• Add regression tests covering both resolution orders, several type arguments, nested scopes, the non-generic control case, and the deduplication invariant.

Keeping the common case free

Only a component exposing more than one service can hold anything back, which is rare, so none of the new state lives on ServiceRegistrationInfo — that type is left exactly the size it was. The held implementations live in a lazily created dictionary on the tracker, so a container that never registers such a component pays a single null field read per source drained, and allocates nothing.

An earlier revision did put a field on ServiceRegistrationInfo. It measured a real cost, so it was reworked:

	base	field on info	tracker-side (this PR)
ChildScopeResolve.Resolve	8.19 us	8.48 us (+3.5%)	7.92 us (-3.3%)
ResolveNeverRegisteredFromChild	3.88 us	4.02 us (+3.6%)	3.86 us (-0.5%)
ResolveNeverRegisteredFromChild Gen0	2.2278	2.2583 (+1.4%)	2.2278 (identical)
ChildScopeResolve.Resolve allocated	30.71 KB	30.8 KB (+276 B)	30.73 KB (+20 B)

Gen0 and Gen1 counts are now identical to develop on every benchmark measured, and allocation is within 20 bytes everywhere. Being slightly ahead of develop is consistent with SkipSource having used a LINQ query plus a queue allocation that IsSourceQueued does not.

Measured with two worktrees running the same filter in interleaved rounds, since the harness only compares source against a released package. Single runs on this machine drift up to ~3% — warm steady-state resolves that this change cannot touch (NoDependencies, ResolveOpenGeneric) moved that much between runs — so only deltas reproducing across paired rounds are treated as real.

Notes

All of this is internal (IRegisteredServicesTracker and ServiceRegistrationInfo are internal, and InternalsVisibleTo is limited to Autofac.Test), so there is no public API change.

Four of the nine new tests fail on develop and pass with the fix; the other five assert the deduplication and no-override invariants and pass either way. 1387 tests pass on net8.0 and net10.0.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 78.39%. Comparing base (3084244) to head (7a5b929).

Additional details and impacted files

@@             Coverage Diff             @@
##           develop    #1499      +/-   ##
===========================================
+ Coverage    78.23%   78.39%   +0.15%     
===========================================
  Files          217      217              
  Lines         5928     5965      +37     
  Branches      1269     1281      +12     
===========================================
+ Hits          4638     4676      +38     
  Misses         753      753              
+ Partials       537      536       -1     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[tillig]

Review response

Thanks — four real problems, and chasing finding 1 turned up a fifth that the tests I added caught. Pushed in 31136ad.

Finding 1 — the cross-thread handoff

Confirmed and fixed, though not the suggested way. Taking Monitor.Enter(additionalInfo) while holding the resolved service's monitor is an AB-BA deadlock: two threads resolving IA<int> and IB<int> of one AB<T> each already hold the monitor the other needs. So the monitor is taken with Monitor.TryEnter — atomic decide-and-record when uncontended, and on contention the implementation is applied immediately, which is what happened before this ordering fix existed. A test asserts the no-deadlock property so a future change back to a blocking acquire fails loudly.

Two more gaps in the same handoff:

• The record was published before the registration's pipeline was built. A concurrent drain could apply it and then resolve through an unbuilt pipeline — InvalidOperationException: Component pipeline has not yet been built. This reproduced on net8.0. The pipeline is now built before recording; building is idempotent, so AddRegistration's later call does nothing.
• The held list was mutated in place, so a drain could see a half-built entry — the empty-list case in the finding. It now appends onto a new array, which also removes both locks.

On severity, one correction worth having. I wrote the concurrency tests the finding asked for, then ran them against develop. All three fail there. develop does not merely duplicate the component — it throws InvalidOperationException: Collection was modified; enumeration operation may not execute out of ServiceRegistrationInfo.SkipSource, the method this PR deletes, because it rebuilds a queue another thread is enumerating. So this PR is a net improvement to concurrent behaviour rather than a regression from it.

What it still does not do: guarantee that concurrent first resolution of two services of one multi-service component shares a single component or honours the override. That needs the source query itself memoised per (source, service), which is a larger change than this fix. It is pre-existing, no worse here, and worth its own issue — the only test kept is the one this PR actually satisfies, since the suite has no skipped tests.

Finding 2 — the cost once latched

Fair, and the framing was wrong. TryRemove takes the bucket lock on hit or miss. It is now preceded by a lock-free ContainsKey, so a miss — the common case once latched — no longer takes a lock. The field <remarks> and the drain comment now say the guard is only a field read until the first implementation is held.

You were also right that the earlier table proved nothing about the triggered path, because every benchmark in it registers single-service components. OpenGenericMultiServiceBenchmark covers the shape this PR exists for. Interleaved rounds against develop:

	develop	this PR
BuildAndResolveAllServices	17,320 ns	18,728 ns (+8.1%)
allocated	52.6 KB	53.4 KB (+1.5%)
ResolveAllServicesWarm	439 ns	466 ns, identical bytes and Gen0

So first resolution of a multi-service open generic container costs about 8% more, warm resolution is unchanged, and containers without that shape are unaffected. That is the price of the fix, now measured rather than asserted.

Finding 3 — the ! dereference

Fixed as suggested. TryApplyDeferredSourceImplementations, WasDeferred, and the new discard all take a non-nullable map parameter and are static, so the compiler enforces the guard the way GetEphemeralServiceInfo does.

Finding 4 — the stranded entry

Fixed by discarding rather than applying, keeping the behaviour you identified as correct. The check moved above the isolation continue only far enough to release the entry.

Verification

Release build clean at zero warnings; dotnet format clean; 889 + 504 tests pass on net8.0 and net10.0, with 10 consecutive full-suite runs to shake out the new concurrency test.

[tillig]

Finding 4 is now fixed and tested — 7a5b9290.

To correct what I said earlier: I had called this pre-existing, and that was wrong. _deferredSourceImplementations does not exist on develop, so the stranded entry is introduced here. What is pre-existing is the growth axis — one ServiceRegistrationInfo cached per distinct closed generic service, forever — and in this same scenario develop retains a comparable amount anyway, because it applies the component immediately, which makes IsRegistered true and stops the isolated-service removal from firing. So the size is about the same but the structure pinning it is new, and that is worth fixing rather than explaining away.

The entry is now released in the isolation branch. It is still not applied there, which is the behaviour you identified as correct.

Tested against the tracker directly instead of through a load context: a per-scope source exposing two services, the first queried normally so the implementation is held for the second, then the second queried as a ScopeIsolatedService. Confirmed the test fails without the fix (Expected: 0, Actual: 1) and passes with it, so it is not vacuous. A second test covers the ordinary non-isolated path, asserting the held implementation is applied and both services share the one component.

The assertion reads the private field the way Assertions.cs already reads _activatorData, rather than adding an internal member for tests. Releasing the entry has no effect on what resolves — a later query just builds a fresh service info — which is precisely why it needed state inspection and why it went unnoticed.

892 + 504 tests pass on net8.0 and net10.0, Release build clean, dotnet format clean, and every line this PR changes is covered with no partial branches.

[tillig]

Filed #1500 for the concurrency hole, with the measured rates and the reason a tracker-wide gate around source draining would not close it.

To restate the trade this PR is taking, since it is the one thing #1500 records against it: leaving the source in the other service's queue is what makes the ordering fix work, and it also makes the duplication case somewhat more likely under concurrent first resolution — 1.10% to 1.63% of completed attempts in a 4,000-attempt probe. In exchange, sequential resolution goes from 0/200 to 200/200 correct, override loss under the same race drops from 4.86% to 3.15%, and exceptions across 8,000 attempts drop from 2,071 to 1. Every failure mode #1500 describes predates this PR.

No code change from this; the PR is unchanged at 7a5b9290.