diff --git a/src/Autofac/Core/Lifetime/LifetimeScope.cs b/src/Autofac/Core/Lifetime/LifetimeScope.cs index cc11ef3fe..028c922cc 100644 --- a/src/Autofac/Core/Lifetime/LifetimeScope.cs +++ b/src/Autofac/Core/Lifetime/LifetimeScope.cs @@ -589,9 +589,62 @@ private ComponentRegistryBuilder CreateScopeRestrictedRegistry(object tag, Actio configurationAction(builder); builder.UpdateRegistry(registryBuilder); + + CheckLocalRegistrationsDoNotBindToAncestorScope(tag, tracker.Registrations); + return registryBuilder; } + private void CheckLocalRegistrationsDoNotBindToAncestorScope(object newScopeTag, IEnumerable localRegistrations) + { + // Issue #1460: A locally-registered component whose MatchingScopeLifetime references an + // ancestor tag (e.g. InstancePerMatchingLifetimeScope("outer") registered in a child scope's + // configuration action) gets hoisted into that ancestor on every child scope creation, + // accumulating instances and causing a memory leak. Fail fast here rather than silently + // leaking. Only matching-lifetime registrations can trigger this, and creating configured + // child scopes is a hot path, so we avoid all work (no allocation, no parent-chain walk) + // unless and until we actually encounter a MatchingScopeLifetime registration. + HashSet? ancestorTags = null; + + foreach (var registration in localRegistrations) + { + if (registration.Lifetime is not MatchingScopeLifetime matchingLifetime) + { + continue; + } + + // Lazily build the set of strict ancestor tags on first need: the current scope and all + // its parents (but NOT the new child scope's own tag, which is a legal, leak-free target). + ancestorTags ??= BuildAncestorTagSet(); + + foreach (var matchedTag in matchingLifetime.TagsToMatch) + { + if (ancestorTags.Contains(matchedTag)) + { + throw new InvalidOperationException( + string.Format( + CultureInfo.CurrentCulture, + LifetimeScopeResources.MatchingScopeLifetimeAncestorTag, + matchedTag, + newScopeTag)); + } + } + } + } + + private HashSet BuildAncestorTagSet() + { + var ancestorTags = new HashSet(); + ISharingLifetimeScope? current = this; + while (current is not null) + { + ancestorTags.Add(current.Tag); + current = current.ParentLifetimeScope; + } + + return ancestorTags; + } + /// /// Gets a value indicating whether this or any of the parent disposables have been disposed. /// diff --git a/src/Autofac/Core/Lifetime/LifetimeScopeResources.resx b/src/Autofac/Core/Lifetime/LifetimeScopeResources.resx index 22b990e88..fa75c6312 100644 --- a/src/Autofac/Core/Lifetime/LifetimeScopeResources.resx +++ b/src/Autofac/Core/Lifetime/LifetimeScopeResources.resx @@ -123,6 +123,9 @@ The tag '{0}' has already been assigned to a parent lifetime scope. If you are using Owned<T> this indicates you may have a circular dependency chain. + + A component registered in a BeginLifetimeScope configuration action has a lifetime bound to tag '{0}', which belongs to a strict ancestor scope. Such a registration can only be resolved within the current child scope, but each child scope creation will hoist a new instance into the ancestor, causing a memory leak. Either register the component in the ancestor scope directly, or use a tag that matches the current scope ('{1}') or a future descendant scope. + Instances cannot be resolved and nested lifetimes cannot be created from this LifetimeScope as it (or one of its parent scopes) has already been disposed. diff --git a/test/Autofac.Specification.Test/Lifetime/InstancePerMatchingLifetimeScopeTests.cs b/test/Autofac.Specification.Test/Lifetime/InstancePerMatchingLifetimeScopeTests.cs index dbb8b6304..897d0fa7e 100644 --- a/test/Autofac.Specification.Test/Lifetime/InstancePerMatchingLifetimeScopeTests.cs +++ b/test/Autofac.Specification.Test/Lifetime/InstancePerMatchingLifetimeScopeTests.cs @@ -8,6 +8,107 @@ namespace Autofac.Specification.Test.Lifetime; public class InstancePerMatchingLifetimeScopeTests { + [Fact] + public void LocalRegistration_BindToAncestorTag() + { + // #1460: Reproduces the exact memory-leak scenario from the issue. + // A child scope is created with a local registration whose matching-scope + // tag refers to an ancestor (the outer scope). Each BeginLifetimeScope call + // would hoist a fresh instance into the ancestor indefinitely, so we now + // throw InvalidOperationException at scope-creation time instead. + var builder = new ContainerBuilder(); + var container = builder.Build(); + + using var outer = container.BeginLifetimeScope("outer"); + + Assert.Throws(() => + outer.BeginLifetimeScope("inner", b => + b.RegisterType().InstancePerMatchingLifetimeScope("outer"))); + } + + [Fact] + public void LocalRegistration_BindToScopeOwnTag() + { + // #1460: A scope-local registration whose MatchingScopeLifetime tag matches the + // NEW scope's own tag is perfectly legal: the component's lifetime exactly + // matches its reachability, so there is no leak. + var builder = new ContainerBuilder(); + var container = builder.Build(); + + // Must not throw. + using var scope = container.BeginLifetimeScope("x", b => + b.RegisterType().InstancePerMatchingLifetimeScope("x")); + + // The component should be resolvable within the scope. + var instance = scope.Resolve(); + Assert.NotNull(instance); + + // And it should be shared within the same scope (singleton-within-scope semantics). + Assert.Same(instance, scope.Resolve()); + } + + [Fact] + public void LocalRegistration_BindToDescendantTag() + { + // #1460: A scope-local registration whose MatchingScopeLifetime tag will only be + // matched by a future descendant scope must NOT throw at registration time — + // this is the documented, intended usage pattern. + var builder = new ContainerBuilder(); + var container = builder.Build(); + + // "child" does not yet exist in the scope chain → must not throw. + using var parent = container.BeginLifetimeScope("parent", b => + b.RegisterType().InstancePerMatchingLifetimeScope("child")); + + // Creating a named child scope with the matching tag should work and resolve correctly. + using var child = parent.BeginLifetimeScope("child"); + var instance = child.Resolve(); + Assert.NotNull(instance); + } + + [Fact] + public void LocalRegistration_BindToGrandchildTag() + { + // #1460: a descendant tag more than one level down is still a future descendant, + // not an ancestor, so it must not throw at scope-creation time. + var builder = new ContainerBuilder(); + var container = builder.Build(); + + using var child = container.BeginLifetimeScope("child", b => + b.RegisterType().InstancePerMatchingLifetimeScope("grandchild")); + + using var grandchild = child.BeginLifetimeScope("grandchild"); + Assert.NotNull(grandchild.Resolve()); + } + + [Fact] + public void LocalRegistration_BindToRootContainerTag() + { + // #1460: The container's root tag is also a strict ancestor; binding to it from a + // scope-local registration should likewise be rejected. + var builder = new ContainerBuilder(); + var container = builder.Build(); + + Assert.Throws(() => + container.BeginLifetimeScope("child", b => + b.RegisterType().InstancePerMatchingLifetimeScope(LifetimeScope.RootTag))); + } + + [Fact] + public void LocalRegistration_BindToMultipleTagsIncludingAncestor() + { + // #1460: When multiple tags are supplied to InstancePerMatchingLifetimeScope and at + // least one of them refers to an ancestor, it must still throw. + var builder = new ContainerBuilder(); + var container = builder.Build(); + + using var outer = container.BeginLifetimeScope("outer"); + + Assert.Throws(() => + outer.BeginLifetimeScope("inner", b => + b.RegisterType().InstancePerMatchingLifetimeScope("inner", "outer"))); + } + [Fact] public void ChildOfNamedScopeGetsSharedInstance() {