A call to Verify() (either implicitly or explicitly) will not cause a…

…n exception when IAsyncDisposable components have been registered. They are either added to an existing (ambient) scope for later disposal, or (in case there is no scope) their disposal will be ignored. Fixes #873.

src/SimpleInjector.Tests.Unit/ContainerFactory.cs

@@ -1,4 +1,6 @@
-namespace SimpleInjector.Tests.Unit
+using SimpleInjector.Lifestyles;
+
+namespace SimpleInjector.Tests.Unit
 {
     internal static class ContainerFactory
     {

src/SimpleInjector.Tests.Unit/ScopeTests.cs

@@ -225,7 +225,7 @@ public async Task DisposeScopeAsync_WithSyncAndAsyncDisposableScopedInstance_Dis
         }
 
         [TestMethod]
-        public async Task DisposeScopeAsync_WithSyncAndAsyncDisposableScopedInstanceDeleteRegistration_DisposesThatInstanceOnlyAsynchronously()
+        public async Task DisposeScopeAsync_WithSyncAndAsyncDisposableScopedInstanceDelegateRegistration_DisposesThatInstanceOnlyAsynchronously()
         {
             // Arrange
             var container = ContainerFactory.New();
@@ -248,7 +248,7 @@ public async Task DisposeScopeAsync_WithSyncAndAsyncDisposableScopedInstanceDele
         }
 
         [TestMethod]
-        public async Task DisposeScopeAsync_WithAsyncDisposableScopedDeleteRegistration_DisposesThatInstance()
+        public async Task DisposeScopeAsync_WithAsyncDisposableScopedDelegateRegistration_DisposesThatInstance()
         {
             // Arrange
             var container = ContainerFactory.New();
@@ -267,6 +267,29 @@ public async Task DisposeScopeAsync_WithAsyncDisposableScopedDeleteRegistration_
             Assert.IsTrue(plugin.AsyncDisposed);
         }
 
+        [TestMethod]
+        public void Dispose_WithSyncAsyncDisposableScopedRegistration_DisposesThatInstance()
+        {
+            // Arrange
+            var container = ContainerFactory.New();
+            container.Options.DefaultScopedLifestyle = new AsyncScopedLifestyle();
+
+            container.Register<SyncAsyncDisposable>(Lifestyle.Scoped);
+
+            var scope = AsyncScopedLifestyle.BeginScope(container);
+
+            var plugin = container.GetInstance<SyncAsyncDisposable>();
+
+            // Act
+            // Because SyncAsyncDisposable implements IDisposable, the sync dispose call should succeed.
+            scope.Dispose();
+
+            // Assert
+            Assert.IsTrue(plugin.SyncDisposed);
+            Assert.IsFalse(plugin.AsyncDisposed,
+                "DisposeAsync was called, but this involves blocking which could cause a deadlock. A no-no.");
+        }
+
         [TestMethod]
         public async Task DisposeScopeAsync_RegisterForIDisposableOnMixedDisposable_DisposesThatInstanceOnlyAsynchronously()
         {

src/SimpleInjector.Tests.Unit/VerifyTests.cs

@@ -5,8 +5,9 @@ namespace SimpleInjector.Tests.Unit
     using System.Collections.Generic;
     using System.Linq;
     using System.Threading;
+    using System.Threading.Tasks;
     using Microsoft.VisualStudio.TestTools.UnitTesting;
-    using SimpleInjector.Advanced;
+    using SimpleInjector.Lifestyles;
 
     [TestClass]
     public class VerifyTests
@@ -733,6 +734,65 @@ public void VerifyAndDiagnose_WithDiagnosticMessage_Succeeds()
             container.Verify(VerificationOption.VerifyAndDiagnose);
         }
 
+        [TestMethod]
+        public void Verify_WithAsyncDisposableScopedRegistration_Succeeds()
+        {
+            // Arrange
+            var container = ContainerFactory.New();
+            container.Options.DefaultScopedLifestyle = new AsyncScopedLifestyle();
+
+            container.Register<AsyncDisposablePlugin>(Lifestyle.Scoped);
+
+            // Act
+            container.Verify();
+        }
+
+        [TestMethod]
+        public async Task AutoVerification_WithAsyncDisposableScopedRegistration_Succeeds()
+        {
+            // Arrange
+            bool disposed = false;
+            bool created = false;
+            var container = new Container();
+            container.Options.EnableAutoVerification = true;
+            container.Options.DefaultScopedLifestyle = new AsyncScopedLifestyle();
+
+            container.Register<PluginImpl>();
+            container.Register(() =>
+                {
+                    created = true;
+                    return new AsyncDisposablePlugin { Disposing = () => disposed = true };
+                },
+                Lifestyle.Scoped);
+
+            var ambientScope = AsyncScopedLifestyle.BeginScope(container);
+
+            // Triggers auto verification on AsyncDisposablePlugin
+            container.GetInstance<PluginImpl>();
+
+            // Assert
+            Assert.IsTrue(created, "Auto verification didn't seem to go off.");
+            Assert.IsFalse(disposed, "Auto verification should have used ambient scope, not its own.");
+
+            // Act
+            await ambientScope.DisposeScopeAsync();
+
+            // Assert
+            Assert.IsTrue(disposed,
+                "Component should have tracked in ambient scope and disposed when calling DisposeScopeAsync.");
+        }
+
+        public class AsyncDisposablePlugin : IAsyncDisposable
+        {
+            public Action Disposing;
+
+            public ValueTask DisposeAsync()
+            {
+                this.Disposing?.Invoke();
+                return default;
+            }
+        }
+
         public class PluginWithBooleanDependency : IPlugin
         {
             public PluginWithBooleanDependency(bool isInUserContext)

src/SimpleInjector/Container.Verification.cs

@@ -110,9 +110,17 @@ private void VerifyInternal(bool suppressLifestyleMismatchVerification)
                 this.IsVerifying = true;
 
                 this.LockContainer();
+
                 bool original = this.Options.SuppressLifestyleMismatchVerification;
 
-                this.VerificationScope = new ContainerVerificationScope(this);
+                var scope = this.Options.DefaultScopedLifestyle?.GetCurrentScope(this);
+
+                // If there is a current active scope, prefer using that scope for verification. This prevents
+                // this method from having to manage a verification scope, which means calling Dispose() which
+                // is problematic when there are IAsyncDisposable registrations. Downside of using the active
+                // scope is that those components could be kept alive much longer (depending on when the
+                // active scope is disposed of).
+                this.VerificationScope = scope ?? new ContainerVerificationScope(this);
 
                 try
                 {
@@ -131,10 +139,26 @@ private void VerifyInternal(bool suppressLifestyleMismatchVerification)
                 finally
                 {
                     this.Options.SuppressLifestyleMismatchVerification = original;
-                    this.IsVerifying = false;
+
                     var scopeToDispose = this.VerificationScope;
                     this.VerificationScope = null;
-                    scopeToDispose.Dispose();
+
+                    try
+                    {
+                        // Only when Verify created the scope itself, should it dispose it. This will prevent
+                        // us from having to call Dispose, which will throw when there are IAsyncDisposable
+                        // registrations.
+                        if (scopeToDispose is ContainerVerificationScope)
+                        {
+                            scopeToDispose.Dispose();
+                        }
+                    }
+                    finally
+                    {
+                        // Must be reset after calling Dispose(), because Scope.Dispose() ignores
+                        // IAsyncDisposables during verification.
+                        this.IsVerifying = false;
+                    }
                 }
             }
         }

src/SimpleInjector/Lifestyles/FlowingScopedLifestyle.cs

@@ -23,7 +23,7 @@ public FlowingScopedLifestyle() : base("Scoped")
             // Notify the container that we're using the thread-resolve scope.
             container.UseCurrentThreadResolveScope();
 
-            return () => container!.GetVerificationOrResolveScopeForCurrentThread();
+            return () => container.GetVerificationOrResolveScopeForCurrentThread();
         }
 
         protected override Scope? GetCurrentScopeCore(Container container) =>

src/SimpleInjector/Scope.cs

@@ -689,7 +689,7 @@ private void DisposeAllRegisteredDisposables()
 
                 this.disposables = null;
 
-                DisposeInstancesInReverseOrder(instances);
+                this.DisposeInstancesInReverseOrder(instances);
             }
         }
 
@@ -721,7 +721,7 @@ private static void ThrowRecursionException() =>
 
         // This method simulates the behavior of a set of nested 'using' statements: It ensures that dispose
         // is called on each element, even if a previous instance threw an exception.
-        private static void DisposeInstancesInReverseOrder(
+        private void DisposeInstancesInReverseOrder(
             List<object> disposables, int startingAsIndex = int.MinValue)
         {
             if (startingAsIndex == int.MinValue)
@@ -733,17 +733,15 @@ private static void DisposeInstancesInReverseOrder(
             {
                 while (startingAsIndex >= 0)
                 {
-                    object instance = disposables[startingAsIndex];
+                    object instance = AsyncDisposableTypeCache.Unwrap(disposables[startingAsIndex]);
 
                     if (instance is IDisposable disposable)
                     {
                         disposable.Dispose();
                     }
                     else
                     {
-                        throw new InvalidOperationException(
-                            StringResources.TypeOnlyImplementsIAsyncDisposable(
-                                AsyncDisposableTypeCache.Unwrap(instance)));
+                        this.ThrowTypeOnlyImplementsIAsyncDisposable(instance);
                     }
 
                     startingAsIndex--;
@@ -753,11 +751,32 @@ private static void DisposeInstancesInReverseOrder(
             {
                 if (startingAsIndex >= 0)
                 {
-                    DisposeInstancesInReverseOrder(disposables, startingAsIndex - 1);
+                    this.DisposeInstancesInReverseOrder(disposables, startingAsIndex - 1);
                 }
             }
         }
 
+        private void ThrowTypeOnlyImplementsIAsyncDisposable(object instance)
+        {
+            // In case there is no active (ambient) scope, Verify creates and disposes of its own
+            // scope, but Verify() is completely synchronous and creating a VerifyAsync makes
+            // little sense because:
+            //  1. in many cases the user will call Verify() in a context where there is no option
+            //     to await (ASP.NET Core startup, ASP.NET MVC startup, etc).
+            //  2. Verify is called during a call to GetInstance when auto verification is enabled.
+            //     Adding VerifyAsync() would, therefore, mena adding an GetInstanceAsync(), but
+            //     would be a bad to add such method (see: https://stackoverflow.com/a/43240576/).
+            // At this point, we can either:
+            //  1. ignore the instance with the risk of causing a memory leak
+            //  2. call DisposeAsync().GetAwaiter().GetResult() with the possible risk of deadlock.
+            // Because 2 a really, really bad place to be in, we pick 1 and ignore it.
+            if (this.Container?.IsVerifying != true)
+            {
+                throw new InvalidOperationException(
+                    StringResources.TypeOnlyImplementsIAsyncDisposable(instance));
+            }
+        }
+
         private async DisposeAsyncTask DisposeInternalAsync()
         {
             if (this.state == DisposeState.Alive)