Skip to content

Honor ExternallyOwned ownership for decorators (#1402)#1486

Merged
tillig merged 2 commits into
developfrom
feature/issue-1402
Jun 15, 2026
Merged

Honor ExternallyOwned ownership for decorators (#1402)#1486
tillig merged 2 commits into
developfrom
feature/issue-1402

Conversation

@tillig

@tillig tillig commented Jun 15, 2026

Copy link
Copy Markdown
Member

Summary

Fixes #1402. When a decorated service was registered as ExternallyOwned, the decorator wrapping it was still created with the default OwnedByLifetimeScope ownership. The lifetime scope therefore tracked the decorator for disposal and disposed it (and, through it, the decorated instance) — violating the user's ExternallyOwned opt-out and causing double-disposal.

Fix

DisposalTrackingMiddleware.Execute now uses the effective ownership when deciding whether to track an instance for disposal: context.DecoratorTarget?.Ownership ?? context.Registration.Ownership. For non-decorator resolutions DecoratorTarget is null and behavior is unchanged; for decorator resolutions the underlying decorated component's ownership governs, so an ExternallyOwned decorated service no longer has its decorator disposed by the scope.

This was chosen over propagating ownership onto the decorator registration at build time, because the decorator registration is created before the decorated registration is known. The middleware approach is non-invasive and covers all decorator styles (generic, non-generic, lambda, open-generic) as well as async disposal, since they all route through DisposalTrackingMiddleware.

Tests

Adds DecoratorNotDisposedWhenDecoratedServiceIsExternallyOwned: an ExternallyOwned disposable service decorated by a disposable decorator, asserting neither is disposed when the scope is disposed. The existing decorator disposal tests (InstancePerDependency, InstancePerLifetimeScope, InstancePerMatchingLifetimeScope, SingleInstance) serve as the regression guard for normal-ownership behavior.

No public API changes. Zero warnings/errors; full suites pass on net8.0 and net10.0.

…1402)

When a decorated service is registered as ExternallyOwned, the decorator
wrapping it was still tracked for disposal by the lifetime scope because
the dynamically-created decorator registration defaults to OwnedByLifetimeScope.

In DisposalTrackingMiddleware, when the context has a DecoratorTarget, that
target IS the underlying component registration. If that underlying registration
is ExternallyOwned, the decorator should inherit that intent and also not be
tracked for disposal — the caller has opted out of lifetime management for
the entire decorated chain.
@codecov

codecov Bot commented Jun 15, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 77.76%. Comparing base (f8d4095) to head (44e06bc).
⚠️ Report is 3 commits behind head on develop.

Additional details and impacted files
@@             Coverage Diff             @@
##           develop    #1486      +/-   ##
===========================================
+ Coverage    77.69%   77.76%   +0.07%     
===========================================
  Files          217      217              
  Lines         5829     5834       +5     
  Branches      1253     1254       +1     
===========================================
+ Hits          4529     4537       +8     
+ Misses         764      760       -4     
- Partials       536      537       +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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Decorators does not follow ExternallyOwned - problem with lifetime scope on decorator with IDisposable implementation

1 participant