From 9d86367ffe0cca56c9f156dc28b3a6f016a49c05 Mon Sep 17 00:00:00 2001 From: Alistair Evans Date: Sat, 21 Aug 2021 10:33:16 +0100 Subject: [PATCH 01/11] Add tests to confirm #1284 --- .../AsyncDisposeProvidedInstanceTests.cs | 194 ++++++++++++++++++ .../Util/AsyncDisposeTracker.cs | 23 +++ 2 files changed, 217 insertions(+) create mode 100644 test/Autofac.Specification.Test/Lifetime/AsyncDisposeProvidedInstanceTests.cs create mode 100644 test/Autofac.Specification.Test/Util/AsyncDisposeTracker.cs diff --git a/test/Autofac.Specification.Test/Lifetime/AsyncDisposeProvidedInstanceTests.cs b/test/Autofac.Specification.Test/Lifetime/AsyncDisposeProvidedInstanceTests.cs new file mode 100644 index 000000000..45ad55d3f --- /dev/null +++ b/test/Autofac.Specification.Test/Lifetime/AsyncDisposeProvidedInstanceTests.cs @@ -0,0 +1,194 @@ +// Copyright (c) Autofac Project. All rights reserved. +// Licensed under the MIT License. See LICENSE in the project root for license information. + +using System; +using System.Runtime.CompilerServices; +using System.Threading.Tasks; +using Autofac.Specification.Test.Util; +using Xunit; + +namespace Autofac.Specification.Test.Lifetime +{ + public class AsyncDisposeProvidedInstanceTests + { + [Fact] + public async Task ResolvedProvidedInstances_DisposedWithLifetimeScope() + { + var builder = new ContainerBuilder(); + var disposable = new AsyncDisposeTracker(); + builder.RegisterInstance(disposable); + var container = builder.Build(); + container.Resolve(); + + await container.DisposeAsync(); + + Assert.True(disposable.IsDisposed); + } + + [Fact] + public async Task ResolvedProvidedInstances_DisposedWithLifetimeScope_OnlyDisposedOnce() + { + // Issue 383: Disposing a container should only dispose a provided instance one time. + var builder = new ContainerBuilder(); + var count = 0; + var disposable = new AsyncDisposeTracker(); + disposable.Disposing += (sender, e) => count++; + builder.RegisterInstance(disposable); + var container = builder.Build(); + container.Resolve(); + + await container.DisposeAsync(); + + Assert.Equal(1, count); + } + + [Fact] + public async Task ResolvedProvidedInstances_DisposedWithNestedLifetimeScope() + { + var builder = new ContainerBuilder(); + var disposable = new AsyncDisposeTracker(); + var container = builder.Build(); + var scope = container.BeginLifetimeScope(b => b.RegisterInstance(disposable)); + scope.Resolve(); + + await scope.DisposeAsync(); + + Assert.True(disposable.IsDisposed); + } + + [Fact] + public async Task 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 AsyncDisposeTracker(); + disposable.Disposing += (sender, e) => count++; + var container = builder.Build(); + var scope = container.BeginLifetimeScope(b => b.RegisterInstance(disposable)); + scope.Resolve(); + + await scope.DisposeAsync(); + Assert.Equal(1, count); + + await container.DisposeAsync(); + Assert.Equal(1, count); + } + + [Fact] + public async Task ResolvedProvidedInstances_NotOwnedByLifetimeScope_NeverDisposed() + { + var builder = new ContainerBuilder(); + var disposable = new AsyncDisposeTracker(); + builder.RegisterInstance(disposable).ExternallyOwned(); + var container = builder.Build(); + container.Resolve(); + + await container.DisposeAsync(); + + Assert.False(disposable.IsDisposed); + } + + [Fact] + public async Task ResolvedProvidedInstances_NotOwnedByNestedLifetimeScope_NeverDisposed() + { + var builder = new ContainerBuilder(); + var disposable = new AsyncDisposeTracker(); + var container = builder.Build(); + var scope = container.BeginLifetimeScope(b => b.RegisterInstance(disposable).ExternallyOwned()); + scope.Resolve(); + + await scope.DisposeAsync(); + Assert.False(disposable.IsDisposed); + + await container.DisposeAsync(); + Assert.False(disposable.IsDisposed); + } + + [Fact] + public async Task UnresolvedProvidedInstances_DisposedWithLifetimeScope() + { + var builder = new ContainerBuilder(); + var disposable = new AsyncDisposeTracker(); + builder.RegisterInstance(disposable); + var container = builder.Build(); + + await container.DisposeAsync(); + + Assert.True(disposable.IsDisposed); + } + + [Fact] + public async Task UnresolvedProvidedInstances_DisposedWithLifetimeScope_OnlyDisposedOnce() + { + var builder = new ContainerBuilder(); + var count = 0; + var disposable = new AsyncDisposeTracker(); + disposable.Disposing += (sender, e) => count++; + builder.RegisterInstance(disposable); + var container = builder.Build(); + + await container.DisposeAsync(); + + Assert.Equal(1, count); + } + + [Fact] + public async Task UnresolvedProvidedInstances_DisposedWithNestedLifetimeScope() + { + var builder = new ContainerBuilder(); + var disposable = new AsyncDisposeTracker(); + var container = builder.Build(); + var scope = container.BeginLifetimeScope(b => b.RegisterInstance(disposable)); + + await scope.DisposeAsync(); + + Assert.True(disposable.IsDisposed); + } + + [Fact] + public async Task UnresolvedProvidedInstances_DisposedWithNestedLifetimeScope_OnlyDisposedOnce() + { + var builder = new ContainerBuilder(); + var count = 0; + var disposable = new AsyncDisposeTracker(); + disposable.Disposing += (sender, e) => count++; + var container = builder.Build(); + var scope = container.BeginLifetimeScope(b => b.RegisterInstance(disposable)); + + await scope.DisposeAsync(); + Assert.Equal(1, count); + + await container.DisposeAsync(); + Assert.Equal(1, count); + } + + [Fact] + public async Task UnresolvedProvidedInstances_NotOwnedByLifetimeScope_NeverDisposed() + { + var builder = new ContainerBuilder(); + var disposable = new AsyncDisposeTracker(); + builder.RegisterInstance(disposable).ExternallyOwned(); + var container = builder.Build(); + + await container.DisposeAsync(); + + Assert.False(disposable.IsDisposed); + } + + [Fact] + public async Task UnresolvedProvidedInstances_NotOwnedByNestedLifetimeScope_NeverDisposed() + { + var builder = new ContainerBuilder(); + var disposable = new AsyncDisposeTracker(); + var container = builder.Build(); + var scope = container.BeginLifetimeScope(b => b.RegisterInstance(disposable).ExternallyOwned()); + + await scope.DisposeAsync(); + Assert.False(disposable.IsDisposed); + + await container.DisposeAsync(); + Assert.False(disposable.IsDisposed); + } + } +} diff --git a/test/Autofac.Specification.Test/Util/AsyncDisposeTracker.cs b/test/Autofac.Specification.Test/Util/AsyncDisposeTracker.cs new file mode 100644 index 000000000..575321c91 --- /dev/null +++ b/test/Autofac.Specification.Test/Util/AsyncDisposeTracker.cs @@ -0,0 +1,23 @@ +// Copyright (c) Autofac Project. All rights reserved. +// Licensed under the MIT License. See LICENSE in the project root for license information. + +using System; +using System.Threading.Tasks; + +namespace Autofac.Specification.Test.Util +{ + public class AsyncDisposeTracker : IAsyncDisposable + { + public event EventHandler Disposing; + + public bool IsDisposed { get; set; } + + public ValueTask DisposeAsync() + { + IsDisposed = true; + Disposing?.Invoke(this, EventArgs.Empty); + + return default; + } + } +} From b52aba2a8db12cf64e3f04928a8d4aa302a5ddc9 Mon Sep 17 00:00:00 2001 From: Alistair Evans Date: Sat, 21 Aug 2021 11:55:41 +0100 Subject: [PATCH 02/11] Fix #1284 - Make sure we dispose non-activated IAsyncDisposable instances when we dispose of scopes. --- .../ProvidedInstanceActivator.cs | 50 +++++++++++++++-- src/Autofac/Core/Container.cs | 4 +- src/Autofac/Core/IComponentRegistration.cs | 2 +- src/Autofac/Core/IComponentRegistry.cs | 2 +- src/Autofac/Core/IInstanceActivator.cs | 2 +- .../Registration/ComponentRegistration.cs | 23 ++++++-- .../ComponentRegistrationLifetimeDecorator.cs | 17 +++++- .../Core/Registration/ComponentRegistry.cs | 20 +++++-- .../Registration/ComponentRegistryBuilder.cs | 16 ++++-- .../DefaultRegisteredServicesTracker.cs | 23 ++++++-- .../ExternalComponentRegistration.cs | 3 + .../Registration/IComponentRegistryBuilder.cs | 2 +- .../IRegisteredServicesTracker.cs | 2 +- .../AsyncDisposeProvidedInstanceTests.cs | 56 +++++++++---------- ...eTracker.cs => AsyncOnlyDisposeTracker.cs} | 6 +- .../ProvidedInstanceActivatorTests.cs | 26 +++++++++ test/Autofac.Test/Mocks.cs | 8 +++ 17 files changed, 199 insertions(+), 63 deletions(-) rename test/Autofac.Specification.Test/Util/{AsyncDisposeTracker.cs => AsyncOnlyDisposeTracker.cs} (75%) diff --git a/src/Autofac/Core/Activators/ProvidedInstance/ProvidedInstanceActivator.cs b/src/Autofac/Core/Activators/ProvidedInstance/ProvidedInstanceActivator.cs index b8e41fd87..e5daac55c 100644 --- a/src/Autofac/Core/Activators/ProvidedInstance/ProvidedInstanceActivator.cs +++ b/src/Autofac/Core/Activators/ProvidedInstance/ProvidedInstanceActivator.cs @@ -4,6 +4,7 @@ using System; using System.Diagnostics.CodeAnalysis; using System.Globalization; +using System.Threading.Tasks; using Autofac.Core.Resolving; using Autofac.Core.Resolving.Pipeline; @@ -64,23 +65,60 @@ private object GetInstance() /// public bool DisposeInstance { get; set; } - /// - /// Releases unmanaged and - optionally - managed resources. - /// - /// true to release both managed and unmanaged resources; false to release only unmanaged resources. + /// protected override void Dispose(bool disposing) { // Only dispose of the instance here if it wasn't activated. If it was activated, // then either the owning lifetime scope will dispose of it automatically // (see InstanceLookup.Activate) or an OnRelease handler will take care of it. - if (disposing && DisposeInstance && _instance is IDisposable disposable && !_activated) + if (disposing && DisposeInstance && !_activated) { - disposable.Dispose(); + // If we are in synchronous dispose, and an object implements IDisposable, + // then use it. + if (_instance is IDisposable disposable) + { + disposable.Dispose(); + } + else if (_instance is IAsyncDisposable) + { + // Type only implements IAsyncDisposable, which is not valid if there + // is a synchronous dispose being done. + throw new InvalidOperationException(string.Format( + DisposerResources.Culture, + DisposerResources.TypeOnlyImplementsIAsyncDisposable, + _instance.GetType().FullName)); + } } base.Dispose(disposing); } + /// + protected override async ValueTask DisposeAsync(bool disposing) + { + if (disposing && DisposeInstance && !_activated) + { + // If the item implements IAsyncDisposable we will call its DisposeAsync Method. + if (_instance is IAsyncDisposable asyncDisposable) + { + var vt = asyncDisposable.DisposeAsync(); + + // Don't await if it's already completed (this is a slight gain in performance of using ValueTask). + if (!vt.IsCompletedSuccessfully) + { + await vt.ConfigureAwait(false); + } + } + else if (_instance is IDisposable disposable) + { + // Call the standard Dispose. + disposable.Dispose(); + } + } + + // Do not call the base, otherwise the standard Dispose will fire. + } + [SuppressMessage("CA2222", "CA2222", Justification = "False positive. GetType doesn't hide an inherited member.")] private static Type GetType(object instance) { diff --git a/src/Autofac/Core/Container.cs b/src/Autofac/Core/Container.cs index 005ffff47..205b79b6e 100644 --- a/src/Autofac/Core/Container.cs +++ b/src/Autofac/Core/Container.cs @@ -153,9 +153,7 @@ protected override async ValueTask DisposeAsync(bool disposing) { await _rootLifetimeScope.DisposeAsync().ConfigureAwait(false); - // Registries are not likely to have async tasks to dispose of, - // so we will leave it as a straight dispose. - ComponentRegistry.Dispose(); + await ComponentRegistry.DisposeAsync().ConfigureAwait(false); } // Do not call the base, otherwise the standard Dispose will fire. diff --git a/src/Autofac/Core/IComponentRegistration.cs b/src/Autofac/Core/IComponentRegistration.cs index fbff3dd94..1f3c5582a 100644 --- a/src/Autofac/Core/IComponentRegistration.cs +++ b/src/Autofac/Core/IComponentRegistration.cs @@ -12,7 +12,7 @@ namespace Autofac.Core /// /// Describes a logical component within the container. /// - public interface IComponentRegistration : IDisposable + public interface IComponentRegistration : IDisposable, IAsyncDisposable { /// /// Gets a unique identifier for this component (shared in all sub-contexts.) diff --git a/src/Autofac/Core/IComponentRegistry.cs b/src/Autofac/Core/IComponentRegistry.cs index 3615b49ae..514ef627d 100644 --- a/src/Autofac/Core/IComponentRegistry.cs +++ b/src/Autofac/Core/IComponentRegistry.cs @@ -12,7 +12,7 @@ namespace Autofac.Core /// /// Provides component registrations according to the services they provide. /// - public interface IComponentRegistry : IDisposable + public interface IComponentRegistry : IDisposable, IAsyncDisposable { /// /// Gets the set of properties used during component registration. diff --git a/src/Autofac/Core/IInstanceActivator.cs b/src/Autofac/Core/IInstanceActivator.cs index bc0aad73f..70d13e850 100644 --- a/src/Autofac/Core/IInstanceActivator.cs +++ b/src/Autofac/Core/IInstanceActivator.cs @@ -9,7 +9,7 @@ namespace Autofac.Core /// /// Activates component instances. /// - public interface IInstanceActivator : IDisposable + public interface IInstanceActivator : IDisposable, IAsyncDisposable { /// /// Allows an implementation to add middleware to a registration's resolve pipeline. diff --git a/src/Autofac/Core/Registration/ComponentRegistration.cs b/src/Autofac/Core/Registration/ComponentRegistration.cs index b68066760..8fb9a328e 100644 --- a/src/Autofac/Core/Registration/ComponentRegistration.cs +++ b/src/Autofac/Core/Registration/ComponentRegistration.cs @@ -6,6 +6,7 @@ using System.Diagnostics.CodeAnalysis; using System.Globalization; using System.Linq; +using System.Threading.Tasks; using Autofac.Core.Resolving.Middleware; using Autofac.Core.Resolving.Pipeline; using Autofac.Util; @@ -310,10 +311,7 @@ public override string ToString() _builtComponentPipeline is null ? ComponentRegistrationResources.PipelineNotBuilt : _builtComponentPipeline.ToString()); } - /// - /// Releases unmanaged and - optionally - managed resources. - /// - /// true to release both managed and unmanaged resources; false to release only unmanaged resources. + /// protected override void Dispose(bool disposing) { if (disposing) @@ -323,5 +321,22 @@ protected override void Dispose(bool disposing) base.Dispose(disposing); } + + /// + protected override async ValueTask DisposeAsync(bool disposing) + { + if (disposing) + { + var vt = Activator.DisposeAsync(); + + // Don't await if it's already completed (this is a slight gain in performance of using ValueTask). + if (!vt.IsCompletedSuccessfully) + { + await vt.ConfigureAwait(false); + } + } + + // Do not call the base, otherwise the standard Dispose will fire. + } } } diff --git a/src/Autofac/Core/Registration/ComponentRegistrationLifetimeDecorator.cs b/src/Autofac/Core/Registration/ComponentRegistrationLifetimeDecorator.cs index 26b4679d8..b9dc7ef96 100644 --- a/src/Autofac/Core/Registration/ComponentRegistrationLifetimeDecorator.cs +++ b/src/Autofac/Core/Registration/ComponentRegistrationLifetimeDecorator.cs @@ -4,6 +4,7 @@ using System; using System.Collections.Generic; using System.Diagnostics.CodeAnalysis; +using System.Threading.Tasks; using Autofac.Core.Resolving.Pipeline; using Autofac.Util; @@ -74,7 +75,21 @@ public void BuildResolvePipeline(IComponentRegistryServices registryServices) /// protected override void Dispose(bool disposing) { - _inner.Dispose(); + if (disposing) + { + _inner.Dispose(); + } + } + + /// + protected override ValueTask DisposeAsync(bool disposing) + { + if (disposing) + { + return _inner.DisposeAsync(); + } + + return default; } } } diff --git a/src/Autofac/Core/Registration/ComponentRegistry.cs b/src/Autofac/Core/Registration/ComponentRegistry.cs index 5d4ffa7e8..84f33bea0 100644 --- a/src/Autofac/Core/Registration/ComponentRegistry.cs +++ b/src/Autofac/Core/Registration/ComponentRegistry.cs @@ -3,6 +3,7 @@ using System.Collections.Generic; using System.Diagnostics.CodeAnalysis; +using System.Threading.Tasks; using Autofac.Core.Resolving.Pipeline; using Autofac.Util; @@ -101,15 +102,26 @@ public IEnumerable RegistrationsFor(Service service) public IEnumerable ServiceRegistrationsFor(Service service) => _registeredServicesTracker.ServiceRegistrationsFor(service); - /// - /// Releases unmanaged and - optionally - managed resources. - /// - /// true to release both managed and unmanaged resources; false to release only unmanaged resources. + /// protected override void Dispose(bool disposing) { _registeredServicesTracker.Dispose(); base.Dispose(disposing); } + + /// + protected override async ValueTask DisposeAsync(bool disposing) + { + var vt = _registeredServicesTracker.DisposeAsync(); + + // Don't await if it's already completed (this is a slight gain in performance of using ValueTask). + if (!vt.IsCompleted) + { + await vt.ConfigureAwait(false); + } + + // Do not call the base, otherwise the standard Dispose will fire. + } } } diff --git a/src/Autofac/Core/Registration/ComponentRegistryBuilder.cs b/src/Autofac/Core/Registration/ComponentRegistryBuilder.cs index c79e66348..45a4fbb93 100644 --- a/src/Autofac/Core/Registration/ComponentRegistryBuilder.cs +++ b/src/Autofac/Core/Registration/ComponentRegistryBuilder.cs @@ -4,7 +4,7 @@ using System; using System.Collections.Generic; using System.Runtime.CompilerServices; - +using System.Threading.Tasks; using Autofac.Builder; using Autofac.Core.Resolving.Pipeline; using Autofac.Util; @@ -48,10 +48,7 @@ private void OnRegistrationSourceAdded(object? sender, IRegistrationSource e) handler?.Invoke(this, new RegistrationSourceAddedEventArgs(this, e)); } - /// - /// Releases unmanaged and - optionally - managed resources. - /// - /// true to release both managed and unmanaged resources; false to release only unmanaged resources. + /// protected override void Dispose(bool disposing) { _registeredServicesTracker.Registered -= OnRegistered; @@ -61,6 +58,15 @@ protected override void Dispose(bool disposing) base.Dispose(disposing); } + /// + protected override ValueTask DisposeAsync(bool disposing) + { + _registeredServicesTracker.Registered -= OnRegistered; + _registeredServicesTracker.RegistrationSourceAdded -= OnRegistrationSourceAdded; + + return _registeredServicesTracker.DisposeAsync(); + } + /// /// Gets the set of properties used during component registration. /// diff --git a/src/Autofac/Core/Registration/DefaultRegisteredServicesTracker.cs b/src/Autofac/Core/Registration/DefaultRegisteredServicesTracker.cs index f0902cba7..24473d92d 100644 --- a/src/Autofac/Core/Registration/DefaultRegisteredServicesTracker.cs +++ b/src/Autofac/Core/Registration/DefaultRegisteredServicesTracker.cs @@ -8,6 +8,7 @@ using System.Linq; using System.Runtime.CompilerServices; using System.Threading; +using System.Threading.Tasks; using Autofac.Core.Resolving.Pipeline; using Autofac.Util; @@ -236,10 +237,7 @@ public void Complete() _trackerPopulationComplete = true; } - /// - /// Releases unmanaged and - optionally - managed resources. - /// - /// true to release both managed and unmanaged resources; false to release only unmanaged resources. + /// protected override void Dispose(bool disposing) { foreach (var registration in _registrations) @@ -262,6 +260,23 @@ protected override void Dispose(bool disposing) base.Dispose(disposing); } + /// + protected override async ValueTask DisposeAsync(bool disposing) + { + foreach (var registration in _registrations) + { + var vt = registration.DisposeAsync(); + + // Don't await if it's already completed (this is a slight gain in performance of using ValueTask). + if (!vt.IsCompletedSuccessfully) + { + await vt.ConfigureAwait(false); + } + } + + // Do not call the base, otherwise the standard Dispose will fire. + } + private ServiceRegistrationInfo GetInitializedServiceInfo(Service service) { var createdEphemeralSet = false; diff --git a/src/Autofac/Core/Registration/ExternalComponentRegistration.cs b/src/Autofac/Core/Registration/ExternalComponentRegistration.cs index 1842f411c..ec18c6f49 100644 --- a/src/Autofac/Core/Registration/ExternalComponentRegistration.cs +++ b/src/Autofac/Core/Registration/ExternalComponentRegistration.cs @@ -2,6 +2,7 @@ // Licensed under the MIT License. See LICENSE in the project root for license information. using System; +using System.Threading.Tasks; using Autofac.Core.Resolving.Pipeline; namespace Autofac.Core.Registration @@ -47,6 +48,8 @@ public void Dispose() { // Do not do anything here. } + + public ValueTask DisposeAsync() => default; } } } diff --git a/src/Autofac/Core/Registration/IComponentRegistryBuilder.cs b/src/Autofac/Core/Registration/IComponentRegistryBuilder.cs index 8f4d5a928..648d34759 100644 --- a/src/Autofac/Core/Registration/IComponentRegistryBuilder.cs +++ b/src/Autofac/Core/Registration/IComponentRegistryBuilder.cs @@ -10,7 +10,7 @@ namespace Autofac.Core.Registration /// /// Used to build a . /// - public interface IComponentRegistryBuilder : IDisposable + public interface IComponentRegistryBuilder : IDisposable, IAsyncDisposable { /// /// Create a new with all the component registrations that have been made. diff --git a/src/Autofac/Core/Registration/IRegisteredServicesTracker.cs b/src/Autofac/Core/Registration/IRegisteredServicesTracker.cs index 991db2719..141308b73 100644 --- a/src/Autofac/Core/Registration/IRegisteredServicesTracker.cs +++ b/src/Autofac/Core/Registration/IRegisteredServicesTracker.cs @@ -10,7 +10,7 @@ namespace Autofac.Core.Registration /// /// Keeps track of the status of registered services. /// - internal interface IRegisteredServicesTracker : IDisposable, IComponentRegistryServices + internal interface IRegisteredServicesTracker : IDisposable, IAsyncDisposable, IComponentRegistryServices { /// /// Adds a registration to the list of registered services. diff --git a/test/Autofac.Specification.Test/Lifetime/AsyncDisposeProvidedInstanceTests.cs b/test/Autofac.Specification.Test/Lifetime/AsyncDisposeProvidedInstanceTests.cs index 45ad55d3f..c288806b0 100644 --- a/test/Autofac.Specification.Test/Lifetime/AsyncDisposeProvidedInstanceTests.cs +++ b/test/Autofac.Specification.Test/Lifetime/AsyncDisposeProvidedInstanceTests.cs @@ -15,14 +15,14 @@ public class AsyncDisposeProvidedInstanceTests public async Task ResolvedProvidedInstances_DisposedWithLifetimeScope() { var builder = new ContainerBuilder(); - var disposable = new AsyncDisposeTracker(); + var disposable = new AsyncOnlyDisposeTracker(); builder.RegisterInstance(disposable); var container = builder.Build(); - container.Resolve(); + container.Resolve(); await container.DisposeAsync(); - Assert.True(disposable.IsDisposed); + Assert.True(disposable.IsAsyncDisposed); } [Fact] @@ -31,11 +31,11 @@ public async Task ResolvedProvidedInstances_DisposedWithLifetimeScope_OnlyDispos // Issue 383: Disposing a container should only dispose a provided instance one time. var builder = new ContainerBuilder(); var count = 0; - var disposable = new AsyncDisposeTracker(); + var disposable = new AsyncOnlyDisposeTracker(); disposable.Disposing += (sender, e) => count++; builder.RegisterInstance(disposable); var container = builder.Build(); - container.Resolve(); + container.Resolve(); await container.DisposeAsync(); @@ -46,14 +46,14 @@ public async Task ResolvedProvidedInstances_DisposedWithLifetimeScope_OnlyDispos public async Task ResolvedProvidedInstances_DisposedWithNestedLifetimeScope() { var builder = new ContainerBuilder(); - var disposable = new AsyncDisposeTracker(); + var disposable = new AsyncOnlyDisposeTracker(); var container = builder.Build(); var scope = container.BeginLifetimeScope(b => b.RegisterInstance(disposable)); - scope.Resolve(); + scope.Resolve(); await scope.DisposeAsync(); - Assert.True(disposable.IsDisposed); + Assert.True(disposable.IsAsyncDisposed); } [Fact] @@ -62,11 +62,11 @@ public async Task ResolvedProvidedInstances_DisposedWithNestedLifetimeScope_Only // Issue 383: Disposing a container should only dispose a provided instance one time. var builder = new ContainerBuilder(); var count = 0; - var disposable = new AsyncDisposeTracker(); + var disposable = new AsyncOnlyDisposeTracker(); disposable.Disposing += (sender, e) => count++; var container = builder.Build(); var scope = container.BeginLifetimeScope(b => b.RegisterInstance(disposable)); - scope.Resolve(); + scope.Resolve(); await scope.DisposeAsync(); Assert.Equal(1, count); @@ -79,43 +79,43 @@ public async Task ResolvedProvidedInstances_DisposedWithNestedLifetimeScope_Only public async Task ResolvedProvidedInstances_NotOwnedByLifetimeScope_NeverDisposed() { var builder = new ContainerBuilder(); - var disposable = new AsyncDisposeTracker(); + var disposable = new AsyncOnlyDisposeTracker(); builder.RegisterInstance(disposable).ExternallyOwned(); var container = builder.Build(); - container.Resolve(); + container.Resolve(); await container.DisposeAsync(); - Assert.False(disposable.IsDisposed); + Assert.False(disposable.IsAsyncDisposed); } [Fact] public async Task ResolvedProvidedInstances_NotOwnedByNestedLifetimeScope_NeverDisposed() { var builder = new ContainerBuilder(); - var disposable = new AsyncDisposeTracker(); + var disposable = new AsyncOnlyDisposeTracker(); var container = builder.Build(); var scope = container.BeginLifetimeScope(b => b.RegisterInstance(disposable).ExternallyOwned()); - scope.Resolve(); + scope.Resolve(); await scope.DisposeAsync(); - Assert.False(disposable.IsDisposed); + Assert.False(disposable.IsAsyncDisposed); await container.DisposeAsync(); - Assert.False(disposable.IsDisposed); + Assert.False(disposable.IsAsyncDisposed); } [Fact] public async Task UnresolvedProvidedInstances_DisposedWithLifetimeScope() { var builder = new ContainerBuilder(); - var disposable = new AsyncDisposeTracker(); + var disposable = new AsyncOnlyDisposeTracker(); builder.RegisterInstance(disposable); var container = builder.Build(); await container.DisposeAsync(); - Assert.True(disposable.IsDisposed); + Assert.True(disposable.IsAsyncDisposed); } [Fact] @@ -123,7 +123,7 @@ public async Task UnresolvedProvidedInstances_DisposedWithLifetimeScope_OnlyDisp { var builder = new ContainerBuilder(); var count = 0; - var disposable = new AsyncDisposeTracker(); + var disposable = new AsyncOnlyDisposeTracker(); disposable.Disposing += (sender, e) => count++; builder.RegisterInstance(disposable); var container = builder.Build(); @@ -137,13 +137,13 @@ public async Task UnresolvedProvidedInstances_DisposedWithLifetimeScope_OnlyDisp public async Task UnresolvedProvidedInstances_DisposedWithNestedLifetimeScope() { var builder = new ContainerBuilder(); - var disposable = new AsyncDisposeTracker(); + var disposable = new AsyncOnlyDisposeTracker(); var container = builder.Build(); var scope = container.BeginLifetimeScope(b => b.RegisterInstance(disposable)); await scope.DisposeAsync(); - Assert.True(disposable.IsDisposed); + Assert.True(disposable.IsAsyncDisposed); } [Fact] @@ -151,7 +151,7 @@ public async Task UnresolvedProvidedInstances_DisposedWithNestedLifetimeScope_On { var builder = new ContainerBuilder(); var count = 0; - var disposable = new AsyncDisposeTracker(); + var disposable = new AsyncOnlyDisposeTracker(); disposable.Disposing += (sender, e) => count++; var container = builder.Build(); var scope = container.BeginLifetimeScope(b => b.RegisterInstance(disposable)); @@ -167,28 +167,28 @@ public async Task UnresolvedProvidedInstances_DisposedWithNestedLifetimeScope_On public async Task UnresolvedProvidedInstances_NotOwnedByLifetimeScope_NeverDisposed() { var builder = new ContainerBuilder(); - var disposable = new AsyncDisposeTracker(); + var disposable = new AsyncOnlyDisposeTracker(); builder.RegisterInstance(disposable).ExternallyOwned(); var container = builder.Build(); await container.DisposeAsync(); - Assert.False(disposable.IsDisposed); + Assert.False(disposable.IsAsyncDisposed); } [Fact] public async Task UnresolvedProvidedInstances_NotOwnedByNestedLifetimeScope_NeverDisposed() { var builder = new ContainerBuilder(); - var disposable = new AsyncDisposeTracker(); + var disposable = new AsyncOnlyDisposeTracker(); var container = builder.Build(); var scope = container.BeginLifetimeScope(b => b.RegisterInstance(disposable).ExternallyOwned()); await scope.DisposeAsync(); - Assert.False(disposable.IsDisposed); + Assert.False(disposable.IsAsyncDisposed); await container.DisposeAsync(); - Assert.False(disposable.IsDisposed); + Assert.False(disposable.IsAsyncDisposed); } } } diff --git a/test/Autofac.Specification.Test/Util/AsyncDisposeTracker.cs b/test/Autofac.Specification.Test/Util/AsyncOnlyDisposeTracker.cs similarity index 75% rename from test/Autofac.Specification.Test/Util/AsyncDisposeTracker.cs rename to test/Autofac.Specification.Test/Util/AsyncOnlyDisposeTracker.cs index 575321c91..cc78e9185 100644 --- a/test/Autofac.Specification.Test/Util/AsyncDisposeTracker.cs +++ b/test/Autofac.Specification.Test/Util/AsyncOnlyDisposeTracker.cs @@ -6,15 +6,15 @@ namespace Autofac.Specification.Test.Util { - public class AsyncDisposeTracker : IAsyncDisposable + public class AsyncOnlyDisposeTracker : IAsyncDisposable { public event EventHandler Disposing; - public bool IsDisposed { get; set; } + public bool IsAsyncDisposed { get; set; } public ValueTask DisposeAsync() { - IsDisposed = true; + IsAsyncDisposed = true; Disposing?.Invoke(this, EventArgs.Empty); return default; diff --git a/test/Autofac.Test/Core/Activators/ProvidedInstance/ProvidedInstanceActivatorTests.cs b/test/Autofac.Test/Core/Activators/ProvidedInstance/ProvidedInstanceActivatorTests.cs index e95691514..b0037ef0d 100644 --- a/test/Autofac.Test/Core/Activators/ProvidedInstance/ProvidedInstanceActivatorTests.cs +++ b/test/Autofac.Test/Core/Activators/ProvidedInstance/ProvidedInstanceActivatorTests.cs @@ -2,7 +2,9 @@ // Licensed under the MIT License. See LICENSE in the project root for license information. using System; +using System.Threading.Tasks; using Autofac.Core.Activators.ProvidedInstance; +using Autofac.Test.Util; using Xunit; namespace Autofac.Test.Component.Activation @@ -48,5 +50,29 @@ public void ActivatingAProvidedInstanceTwice_RaisesException() Assert.Throws(() => invoker(container, Factory.NoParameters)); } + + [Fact] + public void SyncDisposeAsyncDisposable_RaisesException() + { + var asyncDisposable = new AsyncOnlyDisposeTracker(); + + ProvidedInstanceActivator target = new ProvidedInstanceActivator(asyncDisposable); + target.DisposeInstance = true; + + Assert.Throws(() => target.Dispose()); + } + + [Fact] + public async Task AsyncDisposeAsyncDisposable_DisposesAsExpected() + { + var asyncDisposable = new AsyncOnlyDisposeTracker(); + + ProvidedInstanceActivator target = new ProvidedInstanceActivator(asyncDisposable); + target.DisposeInstance = true; + + await target.DisposeAsync(); + + Assert.True(asyncDisposable.IsAsyncDisposed); + } } } diff --git a/test/Autofac.Test/Mocks.cs b/test/Autofac.Test/Mocks.cs index 42eef6b3b..00e9b0b56 100644 --- a/test/Autofac.Test/Mocks.cs +++ b/test/Autofac.Test/Mocks.cs @@ -4,6 +4,7 @@ using System; using System.Collections.Generic; using System.Reflection; +using System.Threading.Tasks; using Autofac.Core; using Autofac.Core.Activators.Reflection; using Autofac.Core.Registration; @@ -72,6 +73,13 @@ public void Dispose() IsDisposed = true; } + public ValueTask DisposeAsync() + { + IsDisposed = true; + + return default; + } + public bool IsDisposed { get; private set; } public Guid Id { get; } From 60c1d9a2b7f677c324f94efd1a4d10e325648b7b Mon Sep 17 00:00:00 2001 From: Alistair Evans Date: Sat, 21 Aug 2021 12:04:26 +0100 Subject: [PATCH 03/11] Avoid the need to modify IInstanceActivator. --- src/Autofac/Core/IInstanceActivator.cs | 2 +- .../Core/Registration/ComponentRegistration.cs | 16 ++++++++++++---- .../ExternalComponentRegistration.cs | 2 -- 3 files changed, 13 insertions(+), 7 deletions(-) diff --git a/src/Autofac/Core/IInstanceActivator.cs b/src/Autofac/Core/IInstanceActivator.cs index 70d13e850..bc0aad73f 100644 --- a/src/Autofac/Core/IInstanceActivator.cs +++ b/src/Autofac/Core/IInstanceActivator.cs @@ -9,7 +9,7 @@ namespace Autofac.Core /// /// Activates component instances. /// - public interface IInstanceActivator : IDisposable, IAsyncDisposable + public interface IInstanceActivator : IDisposable { /// /// Allows an implementation to add middleware to a registration's resolve pipeline. diff --git a/src/Autofac/Core/Registration/ComponentRegistration.cs b/src/Autofac/Core/Registration/ComponentRegistration.cs index 8fb9a328e..d4e846948 100644 --- a/src/Autofac/Core/Registration/ComponentRegistration.cs +++ b/src/Autofac/Core/Registration/ComponentRegistration.cs @@ -327,12 +327,20 @@ protected override async ValueTask DisposeAsync(bool disposing) { if (disposing) { - var vt = Activator.DisposeAsync(); + // Check the Activator type so we don't have to force Activators to implement IAsyncDisposable if they don't need it. + if (Activator is IAsyncDisposable asyncDispose) + { + var vt = asyncDispose.DisposeAsync(); - // Don't await if it's already completed (this is a slight gain in performance of using ValueTask). - if (!vt.IsCompletedSuccessfully) + // Don't await if it's already completed (this is a slight gain in performance of using ValueTask). + if (!vt.IsCompletedSuccessfully) + { + await vt.ConfigureAwait(false); + } + } + else { - await vt.ConfigureAwait(false); + Activator.Dispose(); } } diff --git a/src/Autofac/Core/Registration/ExternalComponentRegistration.cs b/src/Autofac/Core/Registration/ExternalComponentRegistration.cs index ec18c6f49..88b2ff89c 100644 --- a/src/Autofac/Core/Registration/ExternalComponentRegistration.cs +++ b/src/Autofac/Core/Registration/ExternalComponentRegistration.cs @@ -48,8 +48,6 @@ public void Dispose() { // Do not do anything here. } - - public ValueTask DisposeAsync() => default; } } } From 07ce2186cdc81cd747110d8bb3dde73453057423 Mon Sep 17 00:00:00 2001 From: Alistair Evans Date: Sat, 21 Aug 2021 12:08:31 +0100 Subject: [PATCH 04/11] Remove unused namespace --- src/Autofac/Core/Registration/ExternalComponentRegistration.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/Autofac/Core/Registration/ExternalComponentRegistration.cs b/src/Autofac/Core/Registration/ExternalComponentRegistration.cs index 88b2ff89c..1842f411c 100644 --- a/src/Autofac/Core/Registration/ExternalComponentRegistration.cs +++ b/src/Autofac/Core/Registration/ExternalComponentRegistration.cs @@ -2,7 +2,6 @@ // Licensed under the MIT License. See LICENSE in the project root for license information. using System; -using System.Threading.Tasks; using Autofac.Core.Resolving.Pipeline; namespace Autofac.Core.Registration From 1f2492e01d7b0b3c1f20e7dae4b92367c977127d Mon Sep 17 00:00:00 2001 From: Alistair Evans Date: Sat, 21 Aug 2021 13:50:09 +0100 Subject: [PATCH 05/11] Up test coverage --- .../AsyncDisposeProvidedInstanceTests.cs | 15 ++++++++++ .../Util/AsyncOnlyDisposeTracker.cs | 16 ++++++++-- .../ProvidedInstanceActivatorTests.cs | 13 +++++++++ .../Core/ComponentRegistrationTests.cs | 29 +++++++++++++++++++ 4 files changed, 70 insertions(+), 3 deletions(-) diff --git a/test/Autofac.Specification.Test/Lifetime/AsyncDisposeProvidedInstanceTests.cs b/test/Autofac.Specification.Test/Lifetime/AsyncDisposeProvidedInstanceTests.cs index c288806b0..091528a45 100644 --- a/test/Autofac.Specification.Test/Lifetime/AsyncDisposeProvidedInstanceTests.cs +++ b/test/Autofac.Specification.Test/Lifetime/AsyncDisposeProvidedInstanceTests.cs @@ -146,6 +146,21 @@ public async Task UnresolvedProvidedInstances_DisposedWithNestedLifetimeScope() Assert.True(disposable.IsAsyncDisposed); } + [Fact] + public async Task UnresolvedProvidedInstances_ActualAsyncDisposable_CorrectlyDisposed() + { + var builder = new ContainerBuilder(); + var disposable = new AsyncOnlyDisposeTracker(completeAsync: true); + var container = builder.Build(); + var scope = container.BeginLifetimeScope(b => b.RegisterInstance(disposable).ExternallyOwned()); + + await scope.DisposeAsync(); + Assert.False(disposable.IsAsyncDisposed); + + await container.DisposeAsync(); + Assert.False(disposable.IsAsyncDisposed); + } + [Fact] public async Task UnresolvedProvidedInstances_DisposedWithNestedLifetimeScope_OnlyDisposedOnce() { diff --git a/test/Autofac.Specification.Test/Util/AsyncOnlyDisposeTracker.cs b/test/Autofac.Specification.Test/Util/AsyncOnlyDisposeTracker.cs index cc78e9185..f84860925 100644 --- a/test/Autofac.Specification.Test/Util/AsyncOnlyDisposeTracker.cs +++ b/test/Autofac.Specification.Test/Util/AsyncOnlyDisposeTracker.cs @@ -8,16 +8,26 @@ namespace Autofac.Specification.Test.Util { public class AsyncOnlyDisposeTracker : IAsyncDisposable { + private readonly bool _completeAsync; + + public AsyncOnlyDisposeTracker(bool completeAsync = false) + { + _completeAsync = completeAsync; + } + public event EventHandler Disposing; public bool IsAsyncDisposed { get; set; } - public ValueTask DisposeAsync() + public async ValueTask DisposeAsync() { + if (_completeAsync) + { + await Task.Delay(1); + } + IsAsyncDisposed = true; Disposing?.Invoke(this, EventArgs.Empty); - - return default; } } } diff --git a/test/Autofac.Test/Core/Activators/ProvidedInstance/ProvidedInstanceActivatorTests.cs b/test/Autofac.Test/Core/Activators/ProvidedInstance/ProvidedInstanceActivatorTests.cs index b0037ef0d..7f9d74df0 100644 --- a/test/Autofac.Test/Core/Activators/ProvidedInstance/ProvidedInstanceActivatorTests.cs +++ b/test/Autofac.Test/Core/Activators/ProvidedInstance/ProvidedInstanceActivatorTests.cs @@ -74,5 +74,18 @@ public async Task AsyncDisposeAsyncDisposable_DisposesAsExpected() Assert.True(asyncDisposable.IsAsyncDisposed); } + + [Fact] + public async Task AsyncDisposeSyncDisposable_DisposesAsExpected() + { + var asyncDisposable = new DisposeTracker(); + + ProvidedInstanceActivator target = new ProvidedInstanceActivator(asyncDisposable); + target.DisposeInstance = true; + + await target.DisposeAsync(); + + Assert.True(asyncDisposable.IsDisposed); + } } } diff --git a/test/Autofac.Test/Core/ComponentRegistrationTests.cs b/test/Autofac.Test/Core/ComponentRegistrationTests.cs index 997ed2ed3..d0e949754 100644 --- a/test/Autofac.Test/Core/ComponentRegistrationTests.cs +++ b/test/Autofac.Test/Core/ComponentRegistrationTests.cs @@ -4,8 +4,10 @@ using System; using System.Collections.Generic; using System.Linq; +using System.Threading.Tasks; using Autofac.Builder; using Autofac.Core; +using Autofac.Test.Util; using Xunit; namespace Autofac.Test.Core @@ -32,6 +34,33 @@ public void ShouldHaveRegistrationOrderMetadataKey() Assert.Contains(MetadataKeys.RegistrationOrderMetadataKey, registration.Metadata.Keys); } + [Fact] + public async Task AsyncDisposeComponentRegistrationAsyncDisposesActivator() + { + var services = new Service[] { new TypedService(typeof(object)) }; + + var disposable = new AsyncDisposeTracker(); + + var activator = Factory.CreateProvidedInstanceActivator(disposable); + activator.DisposeInstance = true; + + var registration = Factory.CreateSingletonRegistration(services, activator); + + await registration.DisposeAsync(); + + Assert.True(disposable.IsAsyncDisposed); + } + + [Fact] + public async Task AsyncDisposeComponentRegistrationCanDisposeSyncActivator() + { + var services = new Service[] { new TypedService(typeof(object)) }; + + var registration = Factory.CreateSingletonRegistration(services, Factory.CreateReflectionActivator(typeof(object))); + + await registration.DisposeAsync(); + } + [Fact] public void ShouldHaveAscendingRegistrationOrderMetadataValue() { From cd76a583df205b4ddaf71327b899a1ff63c0245c Mon Sep 17 00:00:00 2001 From: Alistair Evans Date: Tue, 24 Aug 2021 18:12:46 +0100 Subject: [PATCH 06/11] Fix test and apply common comment --- src/Autofac/Core/Registration/ComponentRegistryBuilder.cs | 1 + .../Lifetime/AsyncDisposeProvidedInstanceTests.cs | 7 ++----- 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/src/Autofac/Core/Registration/ComponentRegistryBuilder.cs b/src/Autofac/Core/Registration/ComponentRegistryBuilder.cs index 45a4fbb93..57c290fce 100644 --- a/src/Autofac/Core/Registration/ComponentRegistryBuilder.cs +++ b/src/Autofac/Core/Registration/ComponentRegistryBuilder.cs @@ -64,6 +64,7 @@ protected override ValueTask DisposeAsync(bool disposing) _registeredServicesTracker.Registered -= OnRegistered; _registeredServicesTracker.RegistrationSourceAdded -= OnRegistrationSourceAdded; + // Do not call the base, otherwise the standard Dispose will fire. return _registeredServicesTracker.DisposeAsync(); } diff --git a/test/Autofac.Specification.Test/Lifetime/AsyncDisposeProvidedInstanceTests.cs b/test/Autofac.Specification.Test/Lifetime/AsyncDisposeProvidedInstanceTests.cs index 091528a45..a39a81a65 100644 --- a/test/Autofac.Specification.Test/Lifetime/AsyncDisposeProvidedInstanceTests.cs +++ b/test/Autofac.Specification.Test/Lifetime/AsyncDisposeProvidedInstanceTests.cs @@ -152,13 +152,10 @@ public async Task UnresolvedProvidedInstances_ActualAsyncDisposable_CorrectlyDis var builder = new ContainerBuilder(); var disposable = new AsyncOnlyDisposeTracker(completeAsync: true); var container = builder.Build(); - var scope = container.BeginLifetimeScope(b => b.RegisterInstance(disposable).ExternallyOwned()); + var scope = container.BeginLifetimeScope(b => b.RegisterInstance(disposable)); await scope.DisposeAsync(); - Assert.False(disposable.IsAsyncDisposed); - - await container.DisposeAsync(); - Assert.False(disposable.IsAsyncDisposed); + Assert.True(disposable.IsAsyncDisposed); } [Fact] From 3b6f6848396c2363dfcc07ece0b28c86a0ec1ba6 Mon Sep 17 00:00:00 2001 From: Alistair Evans Date: Wed, 25 Aug 2021 08:22:35 +0100 Subject: [PATCH 07/11] Change sync disposal of IAsyncDisposable-only objects to be allowed, but with a Trace warning. --- .../ProvidedInstance/ProvidedInstanceActivator.cs | 13 ++++++------- src/Autofac/Core/Disposer.cs | 13 ++++++------- src/Autofac/Core/DisposerResources.Designer.cs | 9 ++++----- src/Autofac/Core/DisposerResources.resx | 2 +- .../ProvidedInstanceActivatorTests.cs | 6 ++++-- test/Autofac.Test/Core/DisposerTests.cs | 9 ++++----- 6 files changed, 25 insertions(+), 27 deletions(-) diff --git a/src/Autofac/Core/Activators/ProvidedInstance/ProvidedInstanceActivator.cs b/src/Autofac/Core/Activators/ProvidedInstance/ProvidedInstanceActivator.cs index e5daac55c..30e60e681 100644 --- a/src/Autofac/Core/Activators/ProvidedInstance/ProvidedInstanceActivator.cs +++ b/src/Autofac/Core/Activators/ProvidedInstance/ProvidedInstanceActivator.cs @@ -2,6 +2,7 @@ // Licensed under the MIT License. See LICENSE in the project root for license information. using System; +using System.Diagnostics; using System.Diagnostics.CodeAnalysis; using System.Globalization; using System.Threading.Tasks; @@ -79,14 +80,12 @@ protected override void Dispose(bool disposing) { disposable.Dispose(); } - else if (_instance is IAsyncDisposable) + else if (_instance is IAsyncDisposable asyncDisposable) { - // Type only implements IAsyncDisposable, which is not valid if there - // is a synchronous dispose being done. - throw new InvalidOperationException(string.Format( - DisposerResources.Culture, - DisposerResources.TypeOnlyImplementsIAsyncDisposable, - _instance.GetType().FullName)); + Trace.TraceWarning(DisposerResources.TypeOnlyImplementsIAsyncDisposable, LimitType.FullName); + + // Type only implements IAsyncDisposable. We will need to do sync-over-async. + asyncDisposable.DisposeAsync().AsTask().GetAwaiter().GetResult(); } } diff --git a/src/Autofac/Core/Disposer.cs b/src/Autofac/Core/Disposer.cs index ef9c1d477..cecc5b45d 100644 --- a/src/Autofac/Core/Disposer.cs +++ b/src/Autofac/Core/Disposer.cs @@ -3,6 +3,7 @@ using System; using System.Collections.Generic; +using System.Diagnostics; using System.Threading; using System.Threading.Tasks; using Autofac.Util; @@ -45,14 +46,12 @@ protected override void Dispose(bool disposing) { disposable.Dispose(); } - else + else if (item is IAsyncDisposable asyncDisposable) { - // Type only implements IAsyncDisposable, which is not valid if there - // is a synchronous dispose being done. - throw new InvalidOperationException(string.Format( - DisposerResources.Culture, - DisposerResources.TypeOnlyImplementsIAsyncDisposable, - item.GetType().FullName)); + Trace.TraceWarning(DisposerResources.TypeOnlyImplementsIAsyncDisposable, item.GetType().FullName); + + // Type only implements IAsyncDisposable. We will need to do sync-over-async. + asyncDisposable.DisposeAsync().AsTask().GetAwaiter().GetResult(); } } diff --git a/src/Autofac/Core/DisposerResources.Designer.cs b/src/Autofac/Core/DisposerResources.Designer.cs index c6ed5fb32..db55ffc33 100644 --- a/src/Autofac/Core/DisposerResources.Designer.cs +++ b/src/Autofac/Core/DisposerResources.Designer.cs @@ -10,9 +10,8 @@ namespace Autofac.Core { using System; - using System.Reflection; - - + + /// /// A strongly-typed resource class, for looking up localized strings, etc. /// @@ -40,7 +39,7 @@ internal DisposerResources() { internal static global::System.Resources.ResourceManager ResourceManager { get { if (object.ReferenceEquals(resourceMan, null)) { - global::System.Resources.ResourceManager temp = new global::System.Resources.ResourceManager("Autofac.Core.DisposerResources", typeof(DisposerResources).GetTypeInfo().Assembly); + global::System.Resources.ResourceManager temp = new global::System.Resources.ResourceManager("Autofac.Core.DisposerResources", typeof(DisposerResources).Assembly); resourceMan = temp; } return resourceMan; @@ -71,7 +70,7 @@ internal static string CannotAddToDisposedDisposer { } /// - /// Looks up a localized string similar to A synchronous Dispose has been attempted, but the tracked object of type '{0}' only implements IAsyncDisposable.. + /// Looks up a localized string similar to AUTOFAC: A synchronous Dispose has been attempted, but the tracked object of type '{0}' only implements IAsyncDisposable. This will result in an efficient blocking dispose. Consider either implementing IDisposable on '{0}' or disposing of the scope/container with DisposeAsync.. /// internal static string TypeOnlyImplementsIAsyncDisposable { get { diff --git a/src/Autofac/Core/DisposerResources.resx b/src/Autofac/Core/DisposerResources.resx index 2ec5f5fbc..5af61c08b 100644 --- a/src/Autofac/Core/DisposerResources.resx +++ b/src/Autofac/Core/DisposerResources.resx @@ -121,6 +121,6 @@ The Disposer object has already been Disposed, so no items can be added to it. - A synchronous Dispose has been attempted, but the tracked object of type '{0}' only implements IAsyncDisposable. + AUTOFAC: A synchronous Dispose has been attempted, but the tracked object of type '{0}' only implements IAsyncDisposable. This will result in an efficient blocking dispose. Consider either implementing IDisposable on '{0}' or disposing of the scope/container with DisposeAsync. \ No newline at end of file diff --git a/test/Autofac.Test/Core/Activators/ProvidedInstance/ProvidedInstanceActivatorTests.cs b/test/Autofac.Test/Core/Activators/ProvidedInstance/ProvidedInstanceActivatorTests.cs index 7f9d74df0..c336b87d0 100644 --- a/test/Autofac.Test/Core/Activators/ProvidedInstance/ProvidedInstanceActivatorTests.cs +++ b/test/Autofac.Test/Core/Activators/ProvidedInstance/ProvidedInstanceActivatorTests.cs @@ -52,14 +52,16 @@ public void ActivatingAProvidedInstanceTwice_RaisesException() } [Fact] - public void SyncDisposeAsyncDisposable_RaisesException() + public void SyncDisposeAsyncDisposable_DisposesAsExpected() { var asyncDisposable = new AsyncOnlyDisposeTracker(); ProvidedInstanceActivator target = new ProvidedInstanceActivator(asyncDisposable); target.DisposeInstance = true; - Assert.Throws(() => target.Dispose()); + target.Dispose(); + + Assert.True(asyncDisposable.IsAsyncDisposed); } [Fact] diff --git a/test/Autofac.Test/Core/DisposerTests.cs b/test/Autofac.Test/Core/DisposerTests.cs index e6ceaba37..774bf1139 100644 --- a/test/Autofac.Test/Core/DisposerTests.cs +++ b/test/Autofac.Test/Core/DisposerTests.cs @@ -129,17 +129,16 @@ public async ValueTask CannotAddObjectsToDisposerAfterAsyncDispose() } [Fact] - public void SyncDisposalOnObjectWithNoIDisposableThrows() + public void SyncDisposalOnObjectWithNoIDisposableCanDispose() { var instance = new AsyncOnlyDisposeTracker(); var disposer = new Disposer(); disposer.AddInstanceForAsyncDisposal(instance); - Assert.Throws(() => - { - disposer.Dispose(); - }); + disposer.Dispose(); + + Assert.True(instance.IsAsyncDisposed); } [Fact] From e9b2cd5f5896f29288729ca51c43458a18cb03a8 Mon Sep 17 00:00:00 2001 From: Alistair Evans Date: Wed, 25 Aug 2021 15:22:10 +0100 Subject: [PATCH 08/11] Use latest .net5 SDK --- global.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/global.json b/global.json index 7849af13c..b43fcda95 100644 --- a/global.json +++ b/global.json @@ -1,6 +1,6 @@ { "sdk": { - "version": "5.0.100", + "version": "5.0.400", "rollForward": "latestFeature" }, From bcd176e603f4a0b554424c4763127c55faff818e Mon Sep 17 00:00:00 2001 From: Alistair Evans Date: Wed, 25 Aug 2021 17:32:19 +0100 Subject: [PATCH 09/11] Add path separator fix --- build/Autofac.Build.psm1 | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/build/Autofac.Build.psm1 b/build/Autofac.Build.psm1 index 6ececbc12..4118d73ab 100644 --- a/build/Autofac.Build.psm1 +++ b/build/Autofac.Build.psm1 @@ -92,11 +92,19 @@ function Add-Path { [string] $Path ) - $pathValues = $env:PATH.Split(";"); + + $pathSeparator = ":"; + + if ($IsWindows) { + $pathSeparator = ";"; + } + + $pathValues = $env:PATH.Split($pathSeparator); if ($pathValues -Contains $Path) { return; } - $env:PATH = "$Path;$env:PATH" + + $env:PATH = "${Path}${pathSeparator}$env:PATH" } <# From f2f0d04e1a37d7e477e32ad6c107c9a2236d9e80 Mon Sep 17 00:00:00 2001 From: Alistair Evans Date: Wed, 25 Aug 2021 17:41:15 +0100 Subject: [PATCH 10/11] Do sync-over-async dispose with ConfigureAwait(false) to avoid deadlocks --- .../Activators/ProvidedInstance/ProvidedInstanceActivator.cs | 2 +- src/Autofac/Core/Disposer.cs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Autofac/Core/Activators/ProvidedInstance/ProvidedInstanceActivator.cs b/src/Autofac/Core/Activators/ProvidedInstance/ProvidedInstanceActivator.cs index 30e60e681..fc495e9b2 100644 --- a/src/Autofac/Core/Activators/ProvidedInstance/ProvidedInstanceActivator.cs +++ b/src/Autofac/Core/Activators/ProvidedInstance/ProvidedInstanceActivator.cs @@ -85,7 +85,7 @@ protected override void Dispose(bool disposing) Trace.TraceWarning(DisposerResources.TypeOnlyImplementsIAsyncDisposable, LimitType.FullName); // Type only implements IAsyncDisposable. We will need to do sync-over-async. - asyncDisposable.DisposeAsync().AsTask().GetAwaiter().GetResult(); + asyncDisposable.DisposeAsync().AsTask().ConfigureAwait(false).GetAwaiter().GetResult(); } } diff --git a/src/Autofac/Core/Disposer.cs b/src/Autofac/Core/Disposer.cs index cecc5b45d..a23a9fcc1 100644 --- a/src/Autofac/Core/Disposer.cs +++ b/src/Autofac/Core/Disposer.cs @@ -51,7 +51,7 @@ protected override void Dispose(bool disposing) Trace.TraceWarning(DisposerResources.TypeOnlyImplementsIAsyncDisposable, item.GetType().FullName); // Type only implements IAsyncDisposable. We will need to do sync-over-async. - asyncDisposable.DisposeAsync().AsTask().GetAwaiter().GetResult(); + asyncDisposable.DisposeAsync().AsTask().ConfigureAwait(false).GetAwaiter().GetResult(); } } From e4a4a3d8543cda8db3b8e49769423a945eb68e59 Mon Sep 17 00:00:00 2001 From: Alistair Evans Date: Sat, 28 Aug 2021 10:14:48 +0100 Subject: [PATCH 11/11] Try bumping the whole dispose operation onto the threadpool. --- .../ProvidedInstance/ProvidedInstanceActivator.cs | 7 ++++++- src/Autofac/Core/Disposer.cs | 6 +++++- 2 files changed, 11 insertions(+), 2 deletions(-) diff --git a/src/Autofac/Core/Activators/ProvidedInstance/ProvidedInstanceActivator.cs b/src/Autofac/Core/Activators/ProvidedInstance/ProvidedInstanceActivator.cs index fc495e9b2..2a04394d8 100644 --- a/src/Autofac/Core/Activators/ProvidedInstance/ProvidedInstanceActivator.cs +++ b/src/Autofac/Core/Activators/ProvidedInstance/ProvidedInstanceActivator.cs @@ -5,6 +5,7 @@ using System.Diagnostics; using System.Diagnostics.CodeAnalysis; using System.Globalization; +using System.Threading; using System.Threading.Tasks; using Autofac.Core.Resolving; using Autofac.Core.Resolving.Pipeline; @@ -85,7 +86,11 @@ protected override void Dispose(bool disposing) Trace.TraceWarning(DisposerResources.TypeOnlyImplementsIAsyncDisposable, LimitType.FullName); // Type only implements IAsyncDisposable. We will need to do sync-over-async. - asyncDisposable.DisposeAsync().AsTask().ConfigureAwait(false).GetAwaiter().GetResult(); + // We want to ensure we lose all context here, because if we don't we can deadlock. + // So we push this disposal onto the threadpool. + Task.Run(async () => await asyncDisposable.DisposeAsync().ConfigureAwait(false)) + .ConfigureAwait(false) + .GetAwaiter().GetResult(); } } diff --git a/src/Autofac/Core/Disposer.cs b/src/Autofac/Core/Disposer.cs index a23a9fcc1..6edd7b146 100644 --- a/src/Autofac/Core/Disposer.cs +++ b/src/Autofac/Core/Disposer.cs @@ -51,7 +51,11 @@ protected override void Dispose(bool disposing) Trace.TraceWarning(DisposerResources.TypeOnlyImplementsIAsyncDisposable, item.GetType().FullName); // Type only implements IAsyncDisposable. We will need to do sync-over-async. - asyncDisposable.DisposeAsync().AsTask().ConfigureAwait(false).GetAwaiter().GetResult(); + // We want to ensure we lose all context here, because if we don't we can deadlock. + // So we push this disposal onto the threadpool. + Task.Run(async () => await asyncDisposable.DisposeAsync().ConfigureAwait(false)) + .ConfigureAwait(false) + .GetAwaiter().GetResult(); } }