Tests added several bug fixes. Fixes #873

src/SimpleInjector.Tests.Unit/ScopeTests.cs

@@ -1,6 +1,7 @@
 namespace SimpleInjector.Tests.Unit
 {
     using System;
+    using System.Linq;
     using System.Threading.Tasks;
     using Microsoft.VisualStudio.TestTools.UnitTesting;
     using SimpleInjector.Lifestyles;
@@ -349,7 +350,7 @@ public void Dispose_WithAsyncDisposable_ThrowsException()
             // Assert
             AssertThat.ThrowsWithExceptionMessageContains<InvalidOperationException>(
                 "AsyncDisposable only implements IAsyncDisposable, but not IDisposable. Make sure to call " +
-                "Scope.DisposeScopeAsync() instead of Dispose().",
+                "Scope.DisposeScopeAsync() instead of Scope.Dispose().",
                 action);
         }
 
@@ -420,6 +421,68 @@ public async Task GetDisposables_WithSyncAndAsyncDisposableInstances_OnlyReturns
             await scope.DisposeScopeAsync();
         }
 
+        [TestMethod]
+        public void Dispose_ContainerAndAysncDisposableResolvedAsSingletons_ThrowsExpectedException()
+        {
+            // Arrange
+            var container = new Container();
+
+            container.Register<AsyncDisposable>(Lifestyle.Singleton);
+
+            // It might seem odd to test especially in the context of having the Container resolved as
+            // dependency, but there was actually a bug with that particular case that caused Dispose to throw
+            // an ObjectDisposedException (oh the irony), instead of the correct InvalidOperationException.
+            container.Register<ServiceDependingOn<Container>>(Lifestyle.Singleton);
+
+            container.Verify();
+
+            // Act
+            Action action = () => container.Dispose();
+
+            // Assert
+            AssertThat.ThrowsWithExceptionMessageContains<InvalidOperationException>(
+                expectedMessage: "AsyncDisposable only implements IAsyncDisposable, but not IDisposable. " +
+                "Make sure to call Container.DisposeScopeAsync() instead of Container.Dispose().",
+                action);
+        }
+
+        [TestMethod]
+        public void ContainerScopeGetDisposables_WithContainerResolvedAsDependency_DoesNotContainTheContainer()
+        {
+            // Arrange
+            var container = new Container();
+
+            // It would be awkward if the container would hold a reference to itself for disposal.
+            container.Register<ServiceDependingOn<Container>>(Lifestyle.Singleton);
+
+            // Act
+            container.Verify();
+
+            // Assert
+            Assert.IsFalse(container.ContainerScope.GetAllDisposables().Contains(container));
+            Assert.IsFalse(container.ContainerScope.GetDisposables().Contains(container));
+        }
+
+        [TestMethod]
+        public void GetDisposables_WithScopeResolvedAsDependency_DoesNotContainTheScope()
+        {
+            // Arrange
+            var container = new Container();
+            container.Options.DefaultScopedLifestyle = ScopedLifestyle.Flowing;
+
+            // It would be awkward if the scope would hold a reference to itself for disposal.
+            container.Register<ServiceDependingOn<Scope>>(Lifestyle.Scoped);
+
+            var scope = new Scope(container);
+
+            // Act
+            scope.GetInstance<ServiceDependingOn<Scope>>();
+
+            // Assert
+            Assert.IsFalse(scope.GetAllDisposables().Contains(container));
+            Assert.IsFalse(scope.GetDisposables().Contains(container));
+        }
+
         public interface IFoo { }
 
         public class NonDisposable : IFoo { }

src/SimpleInjector/Container.Common.cs

@@ -183,6 +183,8 @@ public bool IsLocked
 
         internal bool HasResolveInterceptors => this.resolveInterceptors.Count > 0;
 
+        internal bool IsDisposed => this.disposed;
+
         /// <summary>
         /// Returns an array with the current registrations. This list contains all explicitly registered
         /// types, and all implicitly registered instances. Implicit registrations are  all concrete

src/SimpleInjector/ContainerScope.cs

@@ -20,7 +20,8 @@ public partial class ContainerScope : ApiObject
     {
         private readonly Scope scope;
 
-        internal ContainerScope(Container container) => this.scope = new Scope(container);
+        internal ContainerScope(Container container) =>
+            this.scope = new Scope(container) { IsContainerScope = true };
 
         /// <summary>
         /// Allows registering an <paramref name="action"/> delegate that will be called when the container
@@ -131,10 +132,25 @@ public T GetOrSetItem<T>(object key, Func<Container, object, T> valueFactory)
         /// disposed in opposite order as they appear in the list.
         /// </summary>
         /// <returns>The list of <see cref="IDisposable"/> instances that will be disposed of when thi
-        /// <see cref="SimpleInjector.Scope"/>
+        /// <see cref="Scope"/>
         /// instance is being disposed.</returns>
         public IDisposable[] GetDisposables() => this.scope.GetDisposables();
 
+        /// <summary>
+        /// Returns a copy of the list of IDisposable and IAsyncDisposable instances that will be disposed of
+        /// when this <see cref="Scope"/> instance is being disposed. The list contains scoped instances that
+        /// are cached in this <see cref="Container"/> instance, and instances explicitly registered for
+        /// disposal using <see cref="RegisterForDisposal(object)"/>. The instances are returned in order of
+        /// creation.
+        /// </summary>
+        /// <remarks>
+        /// <b>Thread safety:</b> This method is <b>not</b> thread safe and should not be used in combination
+        /// with any of the thread-safe methods.
+        /// </remarks>
+        /// <returns>The list of <see cref="IDisposable"/> instances that will be disposed of when this
+        /// <see cref="Scope"/> instance is being disposed.</returns>
+        public object[] GetAllDisposables() => this.scope.GetAllDisposables();
+
         // This method is internal, because we don't want to expose Dispose through its public API. That would
         // allow the container scope to be disposed, while the container isn't. Disposing should happen using the
         // Container API.

src/SimpleInjector/ProducerBuilders/UnregisteredTypeResolutionInstanceProducerBuilder.cs

@@ -37,7 +37,8 @@ internal UnregisteredTypeResolutionInstanceProducerBuilder(
                 () => scopeLifestyle.CreateProducer(() => scopeLifestyle.GetCurrentScope(container), container));
 
             this.resolveUnregisteredTypeRegistrations[typeof(Container)] = new LazyEx<InstanceProducer>(
-                () => Lifestyle.Singleton.CreateProducer(() => container, container));
+                () => new InstanceProducer(typeof(Container),
+                    Lifestyle.Singleton.CreateRegistration(typeof(Container), container, container)));
         }
 
         // Instead of wrapping the complete method in a lock, we lock inside the individual methods. We

src/SimpleInjector/Scope.cs

@@ -107,6 +107,8 @@ private enum DisposeState
 
         internal Scope? ParentScope { get; }
 
+        internal bool IsContainerScope { get; set; }
+
         /// <summary>Gets an instance of the given <typeparamref name="TService"/> for the current scope.</summary>
         /// <typeparam name="TService">The type of the service to resolve.</typeparam>
         /// <remarks><b>Thread safety:</b> Calls to this method are thread safe.</remarks>
@@ -375,7 +377,7 @@ public object[] GetAllDisposables()
                 return Helpers.Array<object>.Empty;
             }
             else
-            { 
+            {
                 var list = this.disposables.ToArray();
 
                 for (int index = 0; index < list.Length; index++)
@@ -758,22 +760,31 @@ private void DisposeInstancesInReverseOrder(
 
         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));
+            // Must first check for disposal because IsVerifying throws when the container is disposed of.
+            if (this.Container?.IsDisposed == true)
+            {
+                // In case there is no active (ambient) scope, Verify creates and disposes of its own scope.
+                // Verify(), however, is completely synchronous and add a Container.VerifyAsync() method 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 choose to ignore those async
+                // disposables during verification. If this is a problem the user can two things to prevent
+                // those disposables from _not_ being disposed:
+                // 1. Wrap the call to Verify() in a Scope
+                // 2. Skip the call to Verify() and rely on auto verification, which will typically be
+                //    executed within the context of an active scope.
+                if (this.Container?.IsVerifying != true)
+                {
+                    throw new InvalidOperationException(
+                        StringResources.TypeOnlyImplementsIAsyncDisposable(instance, this.IsContainerScope));
+                }
             }
         }
 

src/SimpleInjector/StringResources.cs

@@ -993,7 +993,7 @@ internal static string UnableToLoadTypesFromAssembly(Assembly assembly, Exceptio
                 assembly.FullName,
                 innerException.Message);
 
-        internal static string TypeOnlyImplementsIAsyncDisposable(object instance)
+        internal static string TypeOnlyImplementsIAsyncDisposable(object instance, bool containerScope)
         {
             string asyncDisposeMethod =
 #if NETSTANDARD2_1
@@ -1004,12 +1004,12 @@ internal static string TypeOnlyImplementsIAsyncDisposable(object instance)
 
             return Format(
                 "{0} only implements IAsyncDisposable, but not IDisposable. " +
-                "Make sure to call Scope.{1}() instead of Dispose().",
+                "Make sure to call {1}.{2}() instead of {1}.Dispose().",
                 instance.GetType().TypeName(),
+                containerScope ? nameof(Container) : nameof(Scope),
                 asyncDisposeMethod);
         }
 
-
         private static bool IsListOrArrayRelationship(KnownRelationship relationship) =>
             typeof(List<>).IsGenericTypeDefinitionOf(relationship.Consumer.Target.TargetType)
             || relationship.Consumer.Target.TargetType.IsArray;