diff --git a/src/Autofac/Core/Resolving/Middleware/DisposalTrackingMiddleware.cs b/src/Autofac/Core/Resolving/Middleware/DisposalTrackingMiddleware.cs index 69b69d02b..3a90bc07c 100644 --- a/src/Autofac/Core/Resolving/Middleware/DisposalTrackingMiddleware.cs +++ b/src/Autofac/Core/Resolving/Middleware/DisposalTrackingMiddleware.cs @@ -28,7 +28,13 @@ public void Execute(ResolveRequestContext context, Action { next(context); - if (context.Registration.Ownership == InstanceOwnership.OwnedByLifetimeScope) + // When a decorator is being resolved, context.DecoratorTarget is the underlying component + // registration. If the underlying component is ExternallyOwned, the decorator should also + // not be tracked for disposal - the caller opted out of lifetime management for the entire + // decorated chain. See https://github.com/autofac/Autofac/issues/1402. + var effectiveOwnership = context.DecoratorTarget?.Ownership ?? context.Registration.Ownership; + + if (effectiveOwnership == InstanceOwnership.OwnedByLifetimeScope) { // The fact this adds instances for disposal agnostic of the activator is // important. The ProvidedInstanceActivator will NOT dispose of the provided diff --git a/test/Autofac.Specification.Test/Features/DecoratorTests.cs b/test/Autofac.Specification.Test/Features/DecoratorTests.cs index cc9560aec..1b852a897 100644 --- a/test/Autofac.Specification.Test/Features/DecoratorTests.cs +++ b/test/Autofac.Specification.Test/Features/DecoratorTests.cs @@ -675,6 +675,144 @@ public void DecoratorAndDecoratedBothDisposedWhenSingleInstance() Assert.Equal(1, decorated.DisposeCallCount); } + [Fact] + public void DecoratorNotDisposedWhenDecoratedServiceIsExternallyOwned() + { + // #1402: A decorator wrapping an ExternallyOwned service should itself + // not be disposed by the lifetime scope. + var builder = new ContainerBuilder(); + builder.RegisterType() + .As() + .ExternallyOwned(); + builder.RegisterDecorator(); + var container = builder.Build(); + + DisposableDecorator decorator; + DisposableImplementor decorated; + + using (var scope = container.BeginLifetimeScope()) + { + var instance = scope.Resolve(); + decorator = (DisposableDecorator)instance; + decorated = (DisposableImplementor)instance.Decorated; + } + + Assert.Equal(0, decorator.DisposeCallCount); + Assert.Equal(0, decorated.DisposeCallCount); + } + + [Fact] + public void StackedDecoratorsNotDisposedWhenDecoratedServiceIsExternallyOwned() + { + // #1402: When several decorators are stacked over an ExternallyOwned + // service, the ownership of the underlying component governs the entire + // chain - none of the decorators should be disposed by the scope. + var builder = new ContainerBuilder(); + builder.RegisterType() + .As() + .ExternallyOwned(); + builder.RegisterDecorator(); + builder.RegisterDecorator(); + var container = builder.Build(); + + DisposableDecoratorB outer; + DisposableDecorator inner; + DisposableImplementor decorated; + + using (var scope = container.BeginLifetimeScope()) + { + var instance = scope.Resolve(); + outer = (DisposableDecoratorB)instance; + inner = (DisposableDecorator)outer.Decorated; + decorated = (DisposableImplementor)inner.Decorated; + } + + Assert.Equal(0, outer.DisposeCallCount); + Assert.Equal(0, inner.DisposeCallCount); + Assert.Equal(0, decorated.DisposeCallCount); + } + + [Fact] + public void StackedDecoratorsAllDisposedWhenDecoratedServiceOwnedByLifetimeScope() + { + // #1402 regression guard: stacked decorators over a normally-owned + // service must continue to be disposed along with the decorated instance. + var builder = new ContainerBuilder(); + builder.RegisterType() + .As(); + builder.RegisterDecorator(); + builder.RegisterDecorator(); + var container = builder.Build(); + + DisposableDecoratorB outer; + DisposableDecorator inner; + DisposableImplementor decorated; + + using (var scope = container.BeginLifetimeScope()) + { + var instance = scope.Resolve(); + outer = (DisposableDecoratorB)instance; + inner = (DisposableDecorator)outer.Decorated; + decorated = (DisposableImplementor)inner.Decorated; + } + + Assert.Equal(1, outer.DisposeCallCount); + Assert.Equal(1, inner.DisposeCallCount); + Assert.Equal(1, decorated.DisposeCallCount); + } + + [Fact] + public void GenericDecoratorNotDisposedWhenDecoratedServiceIsExternallyOwned() + { + // #1402: The open-generic decorator path also runs through the decorator + // middleware, so an ExternallyOwned decorated component must prevent the + // generic decorator from being disposed by the scope. + var builder = new ContainerBuilder(); + builder.RegisterGeneric(typeof(DisposableGenericComponent<>)) + .As(typeof(IDisposableGenericService<>)) + .ExternallyOwned(); + builder.RegisterGenericDecorator(typeof(DisposableGenericDecorator<>), typeof(IDisposableGenericService<>)); + var container = builder.Build(); + + DisposableGenericDecorator decorator; + DisposableGenericComponent decorated; + + using (var scope = container.BeginLifetimeScope()) + { + var instance = scope.Resolve>(); + decorator = (DisposableGenericDecorator)instance; + decorated = (DisposableGenericComponent)decorator.Decorated; + } + + Assert.Equal(0, decorator.DisposeCallCount); + Assert.Equal(0, decorated.DisposeCallCount); + } + + [Fact] + public void GenericDecoratorAndDecoratedBothDisposedWhenOwnedByLifetimeScope() + { + // #1402 regression guard: open-generic decorator over a normally-owned + // service must continue to be disposed along with the decorated instance. + var builder = new ContainerBuilder(); + builder.RegisterGeneric(typeof(DisposableGenericComponent<>)) + .As(typeof(IDisposableGenericService<>)); + builder.RegisterGenericDecorator(typeof(DisposableGenericDecorator<>), typeof(IDisposableGenericService<>)); + var container = builder.Build(); + + DisposableGenericDecorator decorator; + DisposableGenericComponent decorated; + + using (var scope = container.BeginLifetimeScope()) + { + var instance = scope.Resolve>(); + decorator = (DisposableGenericDecorator)instance; + decorated = (DisposableGenericComponent)decorator.Decorated; + } + + Assert.Equal(1, decorator.DisposeCallCount); + Assert.Equal(1, decorated.DisposeCallCount); + } + [Fact] public void DecoratorAppliedOnlyOnceToComponentWithExternalRegistrySource() { @@ -1413,6 +1551,25 @@ public void Dispose() } } + // ReSharper disable once ClassNeverInstantiated.Local + private sealed class DisposableDecoratorB : Decorator, IDisposable + { + public DisposableDecoratorB(IDecoratedService decorated) + : base(decorated) + { + } + + public int DisposeCallCount + { + get; private set; + } + + public void Dispose() + { + DisposeCallCount++; + } + } + // ReSharper disable once ClassNeverInstantiated.Local private sealed class DisposableImplementor : IDecoratedService, IDisposable { @@ -1533,6 +1690,48 @@ public IGenericService Decorated } } + private interface IDisposableGenericService + { + } + + // ReSharper disable once ClassNeverInstantiated.Local + private sealed class DisposableGenericComponent : IDisposableGenericService, IDisposable + { + public int DisposeCallCount + { + get; private set; + } + + public void Dispose() + { + DisposeCallCount++; + } + } + + // ReSharper disable once ClassNeverInstantiated.Local + private sealed class DisposableGenericDecorator : IDisposableGenericService, IDisposable + { + public DisposableGenericDecorator(IDisposableGenericService decorated) + { + Decorated = decorated; + } + + public IDisposableGenericService Decorated + { + get; + } + + public int DisposeCallCount + { + get; private set; + } + + public void Dispose() + { + DisposeCallCount++; + } + } + private class ConditionalShouldDecorate { public bool ShouldDecorate