Merge pull request #1285 from alistairjevans/develop

Ensure that IAsyncDisposable instances that have not been activated are correctly disposed when the scope is disposed.

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"
 }
 
 <#

global.json

@@ -1,6 +1,6 @@
 {
   "sdk": {
-    "version": "5.0.100",
+    "version": "5.0.400",
     "rollForward": "latestFeature"
   },
 

src/Autofac/Core/Activators/ProvidedInstance/ProvidedInstanceActivator.cs

@@ -2,8 +2,11 @@
 // 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;
+using System.Threading.Tasks;
 using Autofac.Core.Resolving;
 using Autofac.Core.Resolving.Pipeline;
 
@@ -64,23 +67,62 @@ private object GetInstance()
         /// </summary>
         public bool DisposeInstance { get; set; }
 
-        /// <summary>
-        /// Releases unmanaged and - optionally - managed resources.
-        /// </summary>
-        /// <param name="disposing"><c>true</c> to release both managed and unmanaged resources; <c>false</c> to release only unmanaged resources.</param>
+        /// <inheritdoc />
         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 asyncDisposable)
+                {
+                    Trace.TraceWarning(DisposerResources.TypeOnlyImplementsIAsyncDisposable, LimitType.FullName);
+
+                    // Type only implements IAsyncDisposable. We will need to do sync-over-async.
+                    // 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();
+                }
             }
 
             base.Dispose(disposing);
         }
 
+        /// <inheritdoc />
+        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)
         {

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.

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,16 @@ 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.
+                            // 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();
                         }
                     }
 

src/Autofac/Core/DisposerResources.resx

@@ -121,6 +121,6 @@
     <value>The Disposer object has already been Disposed, so no items can be added to it.</value>
   </data>
   <data name="TypeOnlyImplementsIAsyncDisposable" xml:space="preserve">
-    <value>A synchronous Dispose has been attempted, but the tracked object of type '{0}' only implements IAsyncDisposable.</value>
+    <value>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.</value>
   </data>
 </root>

src/Autofac/Core/IComponentRegistration.cs

@@ -12,7 +12,7 @@ namespace Autofac.Core
     /// <summary>
     /// Describes a logical component within the container.
     /// </summary>
-    public interface IComponentRegistration : IDisposable
+    public interface IComponentRegistration : IDisposable, IAsyncDisposable
     {
         /// <summary>
         /// Gets a unique identifier for this component (shared in all sub-contexts.)

src/Autofac/Core/IComponentRegistry.cs

@@ -12,7 +12,7 @@ namespace Autofac.Core
     /// <summary>
     /// Provides component registrations according to the services they provide.
     /// </summary>
-    public interface IComponentRegistry : IDisposable
+    public interface IComponentRegistry : IDisposable, IAsyncDisposable
     {
         /// <summary>
         /// Gets the set of properties used during component registration.

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());
         }
 
-        /// <summary>
-        /// Releases unmanaged and - optionally - managed resources.
-        /// </summary>
-        /// <param name="disposing"><c>true</c> to release both managed and unmanaged resources; <c>false</c> to release only unmanaged resources.</param>
+        /// <inheritdoc />
         protected override void Dispose(bool disposing)
         {
             if (disposing)
@@ -323,5 +321,30 @@ protected override void Dispose(bool disposing)
 
             base.Dispose(disposing);
         }
+
+        /// <inheritdoc />
+        protected override async ValueTask DisposeAsync(bool disposing)
+        {
+            if (disposing)
+            {
+                // 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)
+                    {
+                        await vt.ConfigureAwait(false);
+                    }
+                }
+                else
+                {
+                    Activator.Dispose();
+                }
+            }
+
+            // Do not call the base, otherwise the standard Dispose will fire.
+        }
     }
 }

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)
         /// <inheritdoc/>
         protected override void Dispose(bool disposing)
         {
-            _inner.Dispose();
+            if (disposing)
+            {
+                _inner.Dispose();
+            }
+        }
+
+        /// <inheritdoc/>
+        protected override ValueTask DisposeAsync(bool disposing)
+        {
+            if (disposing)
+            {
+                return _inner.DisposeAsync();
+            }
+
+            return default;
         }
     }
 }

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<IComponentRegistration> RegistrationsFor(Service service)
         public IEnumerable<ServiceRegistration> ServiceRegistrationsFor(Service service)
             => _registeredServicesTracker.ServiceRegistrationsFor(service);
 
-        /// <summary>
-        /// Releases unmanaged and - optionally - managed resources.
-        /// </summary>
-        /// <param name="disposing"><c>true</c> to release both managed and unmanaged resources; <c>false</c> to release only unmanaged resources.</param>
+        /// <inheritdoc />
         protected override void Dispose(bool disposing)
         {
             _registeredServicesTracker.Dispose();
 
             base.Dispose(disposing);
         }
+
+        /// <inheritdoc />
+        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.
+        }
     }
 }

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));
         }
 
-        /// <summary>
-        /// Releases unmanaged and - optionally - managed resources.
-        /// </summary>
-        /// <param name="disposing"><c>true</c> to release both managed and unmanaged resources; <c>false</c> to release only unmanaged resources.</param>
+        /// <inheritdoc />
         protected override void Dispose(bool disposing)
         {
             _registeredServicesTracker.Registered -= OnRegistered;
@@ -61,6 +58,16 @@ protected override void Dispose(bool disposing)
             base.Dispose(disposing);
         }
 
+        /// <inheritdoc />
+        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();
+        }
+
         /// <summary>
         /// Gets the set of properties used during component registration.
         /// </summary>