From c5e5cfb38fc5ec363b96e95917b77d86d577c03f Mon Sep 17 00:00:00 2001 From: Alex Meyer-Gleaves Date: Tue, 25 Jul 2017 22:09:59 +1000 Subject: [PATCH] Fixed #780: Disposal tracking of provided instances is now consistent in root and nested lifetime scopes. --- .../ComponentRegistrationLifetimeDecorator.cs | 24 +-- ...onentRegistrationLifetimeDecoratorTests.cs | 20 +++ test/Autofac.Test/IntegrationTests.cs | 163 ++++++++++++++++-- 3 files changed, 185 insertions(+), 22 deletions(-) create mode 100644 test/Autofac.Test/Core/Registration/ComponentRegistrationLifetimeDecoratorTests.cs diff --git a/src/Autofac/Core/Registration/ComponentRegistrationLifetimeDecorator.cs b/src/Autofac/Core/Registration/ComponentRegistrationLifetimeDecorator.cs index f1ccf583f..bd9b2cb50 100644 --- a/src/Autofac/Core/Registration/ComponentRegistrationLifetimeDecorator.cs +++ b/src/Autofac/Core/Registration/ComponentRegistrationLifetimeDecorator.cs @@ -40,11 +40,8 @@ internal class ComponentRegistrationLifetimeDecorator : Disposable, IComponentRe public ComponentRegistrationLifetimeDecorator(IComponentRegistration inner, IComponentLifetime lifetime) { - if (inner == null) throw new ArgumentNullException(nameof(inner)); - if (lifetime == null) throw new ArgumentNullException(nameof(lifetime)); - - _inner = inner; - Lifetime = lifetime; + _inner = inner ?? throw new ArgumentNullException(nameof(inner)); + Lifetime = lifetime ?? throw new ArgumentNullException(nameof(lifetime)); } public Guid Id => _inner.Id; @@ -65,8 +62,8 @@ public ComponentRegistrationLifetimeDecorator(IComponentRegistration inner, ICom public event EventHandler Preparing { - add { _inner.Preparing += value; } - remove { _inner.Preparing -= value; } + add => _inner.Preparing += value; + remove => _inner.Preparing -= value; } public void RaisePreparing(IComponentContext context, ref IEnumerable parameters) @@ -76,8 +73,8 @@ public void RaisePreparing(IComponentContext context, ref IEnumerable public event EventHandler> Activating { - add { _inner.Activating += value; } - remove { _inner.Activating -= value; } + add => _inner.Activating += value; + remove => _inner.Activating -= value; } public void RaiseActivating(IComponentContext context, IEnumerable parameters, ref object instance) @@ -87,13 +84,18 @@ public void RaiseActivating(IComponentContext context, IEnumerable pa public event EventHandler> Activated { - add { _inner.Activated += value; } - remove { _inner.Activated -= value; } + add => _inner.Activated += value; + remove => _inner.Activated -= value; } public void RaiseActivated(IComponentContext context, IEnumerable parameters, object instance) { _inner.RaiseActivated(context, parameters, instance); } + + protected override void Dispose(bool disposing) + { + _inner.Dispose(); + } } } diff --git a/test/Autofac.Test/Core/Registration/ComponentRegistrationLifetimeDecoratorTests.cs b/test/Autofac.Test/Core/Registration/ComponentRegistrationLifetimeDecoratorTests.cs new file mode 100644 index 000000000..2d92cc09a --- /dev/null +++ b/test/Autofac.Test/Core/Registration/ComponentRegistrationLifetimeDecoratorTests.cs @@ -0,0 +1,20 @@ +using Autofac.Core.Lifetime; +using Autofac.Core.Registration; +using Xunit; + +namespace Autofac.Test.Core.Registration +{ + public class ComponentRegistrationLifetimeDecoratorTests + { + [Fact] + public void DecoratorCallsDisposeOnInnerInstance() + { + var inner = Mocks.GetComponentRegistration(); + var decorator = new ComponentRegistrationLifetimeDecorator(inner, new CurrentScopeLifetime()); + + decorator.Dispose(); + + Assert.True(inner.IsDisposed); + } + } +} \ No newline at end of file diff --git a/test/Autofac.Test/IntegrationTests.cs b/test/Autofac.Test/IntegrationTests.cs index d5819e25f..b5662c716 100644 --- a/test/Autofac.Test/IntegrationTests.cs +++ b/test/Autofac.Test/IntegrationTests.cs @@ -1,7 +1,5 @@ -using System; -using System.Collections.Generic; +using System.Collections.Generic; using System.Linq; -using Autofac.Builder; using Autofac.Core; using Autofac.Test.Scenarios.Dependencies.Circularity; using Autofac.Test.Scenarios.Graph1; @@ -23,14 +21,14 @@ public void CanCorrectlyBuildGraph1() var target = builder.Build(); - E1 e = target.Resolve(); - A1 a = target.Resolve(); - B1 b = target.Resolve(); - IC1 c = target.Resolve(); - ID1 d = target.Resolve(); + var e = target.Resolve(); + var a = target.Resolve(); + var b = target.Resolve(); + var c = target.Resolve(); + var d = target.Resolve(); Assert.IsType(c); - CD1 cd = (CD1)c; + var cd = (CD1)c; Assert.Same(a, b.A); Assert.Same(a, cd.A); @@ -56,18 +54,21 @@ public void DetectsAndIdentifiesCircularDependencies() } [Fact] - public void UnresolvedProvidedInstances_DisposedWithLifetimeScope() + public void ResolvedProvidedInstances_DisposedWithLifetimeScope() { var builder = new ContainerBuilder(); var disposable = new DisposeTracker(); builder.RegisterInstance(disposable); var container = builder.Build(); + container.Resolve(); + container.Dispose(); + Assert.True(disposable.IsDisposed); } [Fact] - public void ResolvedProvidedInstances_OnlyDisposedOnce() + public void ResolvedProvidedInstances_DisposedWithLifetimeScope_OnlyDisposedOnce() { // Issue 383: Disposing a container should only dispose a provided instance one time. var builder = new ContainerBuilder(); @@ -77,6 +78,129 @@ public void ResolvedProvidedInstances_OnlyDisposedOnce() builder.RegisterInstance(disposable); var container = builder.Build(); container.Resolve(); + + container.Dispose(); + + Assert.Equal(1, count); + } + + [Fact] + public void ResolvedProvidedInstances_DisposedWithNestedLifetimeScope() + { + var builder = new ContainerBuilder(); + var disposable = new DisposeTracker(); + var container = builder.Build(); + var scope = container.BeginLifetimeScope(b => b.RegisterInstance(disposable)); + scope.Resolve(); + + scope.Dispose(); + + Assert.True(disposable.IsDisposed); + } + + [Fact] + public void ResolvedProvidedInstances_DisposedWithNestedLifetimeScope_OnlyDisposedOnce() + { + // Issue 383: Disposing a container should only dispose a provided instance one time. + var builder = new ContainerBuilder(); + var count = 0; + var disposable = new DisposeTracker(); + disposable.Disposing += (sender, e) => count++; + var container = builder.Build(); + var scope = container.BeginLifetimeScope(b => b.RegisterInstance(disposable)); + scope.Resolve(); + + scope.Dispose(); + Assert.Equal(1, count); + + container.Dispose(); + Assert.Equal(1, count); + } + + [Fact] + public void ResolvedProvidedInstances_NotOwnedByLifetimeScope_NeverDisposed() + { + var builder = new ContainerBuilder(); + var disposable = new DisposeTracker(); + builder.RegisterInstance(disposable).ExternallyOwned(); + var container = builder.Build(); + container.Resolve(); + + container.Dispose(); + + Assert.False(disposable.IsDisposed); + } + + [Fact] + public void ResolvedProvidedInstances_NotOwnedByNestedLifetimeScope_NeverDisposed() + { + var builder = new ContainerBuilder(); + var disposable = new DisposeTracker(); + var container = builder.Build(); + var scope = container.BeginLifetimeScope(b => b.RegisterInstance(disposable).ExternallyOwned()); + scope.Resolve(); + + scope.Dispose(); + Assert.False(disposable.IsDisposed); + + container.Dispose(); + Assert.False(disposable.IsDisposed); + } + + [Fact] + public void UnresolvedProvidedInstances_DisposedWithLifetimeScope() + { + var builder = new ContainerBuilder(); + var disposable = new DisposeTracker(); + builder.RegisterInstance(disposable); + var container = builder.Build(); + + container.Dispose(); + + Assert.True(disposable.IsDisposed); + } + + [Fact] + public void UnresolvedProvidedInstances_DisposedWithLifetimeScope_OnlyDisposedOnce() + { + var builder = new ContainerBuilder(); + var count = 0; + var disposable = new DisposeTracker(); + disposable.Disposing += (sender, e) => count++; + builder.RegisterInstance(disposable); + var container = builder.Build(); + + container.Dispose(); + + Assert.Equal(1, count); + } + + [Fact] + public void UnresolvedProvidedInstances_DisposedWithNestedLifetimeScope() + { + var builder = new ContainerBuilder(); + var disposable = new DisposeTracker(); + var container = builder.Build(); + var scope = container.BeginLifetimeScope(b => b.RegisterInstance(disposable)); + + scope.Dispose(); + + Assert.True(disposable.IsDisposed); + } + + [Fact] + public void UnresolvedProvidedInstances_DisposedWithNestedLifetimeScope_OnlyDisposedOnce() + { + var builder = new ContainerBuilder(); + var count = 0; + var disposable = new DisposeTracker(); + disposable.Disposing += (sender, e) => count++; + var container = builder.Build(); + var scope = container.BeginLifetimeScope(b => b.RegisterInstance(disposable)); + + scope.Dispose(); + Assert.Equal(1, count); + container.Dispose(); Assert.Equal(1, count); } @@ -88,6 +212,23 @@ public void UnresolvedProvidedInstances_NotOwnedByLifetimeScope_NeverDisposed() var disposable = new DisposeTracker(); builder.RegisterInstance(disposable).ExternallyOwned(); var container = builder.Build(); + + container.Dispose(); + + Assert.False(disposable.IsDisposed); + } + + [Fact] + public void UnresolvedProvidedInstances_NotOwnedByNestedLifetimeScope_NeverDisposed() + { + var builder = new ContainerBuilder(); + var disposable = new DisposeTracker(); + var container = builder.Build(); + var scope = container.BeginLifetimeScope(b => b.RegisterInstance(disposable).ExternallyOwned()); + + scope.Dispose(); + Assert.False(disposable.IsDisposed); + container.Dispose(); Assert.False(disposable.IsDisposed); }