Dispose services in reverse creation order

Author: pakrym

src/Microsoft.Extensions.DependencyInjection.Specification.Tests/DependencyInjectionSpecificationTests.cs

@@ -655,5 +655,30 @@ public void ServiceContainerPicksConstructorWithLongestMatches(
             Assert.Same(expected.MultipleService, actual.MultipleService);
             Assert.Same(expected.ScopedService, actual.ScopedService);
         }
+
+        [Fact]
+        public void DisposesInReverseOrderOfCreation()
+        {
+            // Arrange
+            var serviceCollection = new TestServiceCollection();
+            serviceCollection.AddSingleton<FakeDisposeCallback>();
+            serviceCollection.AddSingleton<IFakeOuterService, FakeDisposableCallbackOuterService>();
+            serviceCollection.AddSingleton<IFakeMultipleService, FakeDisposableCallbackInnerService>();
+            serviceCollection.AddSingleton<IFakeMultipleService, FakeDisposableCallbackInnerService>();
+            serviceCollection.AddSingleton<IFakeMultipleService, FakeDisposableCallbackInnerService>();
+            serviceCollection.AddSingleton<IFakeService, FakeDisposableCallbackInnerService>();
+            var serviceProvider = CreateServiceProvider(serviceCollection);
+
+            var callback = serviceProvider.GetService<FakeDisposeCallback>();
+            var outer = serviceProvider.GetService<IFakeOuterService>();
+
+            // Act
+            ((IDisposable)serviceProvider).Dispose();
+
+            // Assert
+            Assert.Equal(outer, callback.Disposed[0]);
+            Assert.Equal(outer.MultipleServices.Reverse(), callback.Disposed.Skip(1).Take(3).OfType<IFakeMultipleService>());
+            Assert.Equal(outer.SingleService, callback.Disposed[4]);
+        }
     }
 }

src/Microsoft.Extensions.DependencyInjection.Specification.Tests/Fakes/FakeService.cs

@@ -2,9 +2,61 @@
 // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
 
 using System;
+using System.Collections.Generic;
 
 namespace Microsoft.Extensions.DependencyInjection.Specification.Fakes
 {
+    public class FakeDisposeCallback
+    {
+        public List<object> Disposed { get; } = new List<object>();
+    }
+
+    public class FakeDisposableCallbackService: IDisposable
+    {
+        private static int _globalId;
+        private readonly int _id;
+        private readonly FakeDisposeCallback _callback;
+
+        public FakeDisposableCallbackService(FakeDisposeCallback callback)
+        {
+            _id = _globalId++;
+            _callback = callback;
+        }
+
+        public void Dispose()
+        {
+            _callback.Disposed.Add(this);
+        }
+
+        public override string ToString()
+        {
+            return _id.ToString();
+        }
+    }
+
+    public class FakeDisposableCallbackOuterService : FakeDisposableCallbackService, IFakeOuterService
+    {
+        public FakeDisposableCallbackOuterService(
+            IFakeService singleService,
+            IEnumerable<IFakeMultipleService> multipleServices,
+            FakeDisposeCallback callback) : base(callback)
+        {
+            SingleService = singleService;
+            MultipleServices = multipleServices;
+        }
+
+        public IFakeService SingleService { get; }
+        public IEnumerable<IFakeMultipleService> MultipleServices { get; }
+    }
+
+
+    public class FakeDisposableCallbackInnerService : FakeDisposableCallbackService, IFakeMultipleService
+    {
+        public FakeDisposableCallbackInnerService(FakeDisposeCallback callback) : base(callback)
+        {
+        }
+    }
+
     public class FakeService : IFakeEveryService, IDisposable
     {
         public PocoClass Value { get; set; }

src/Microsoft.Extensions.DependencyInjection/ServiceLookup/CallSiteExpressionBuilder.cs

@@ -184,34 +184,42 @@ protected override Expression VisitScoped(ScopedCallSite callSite, ParameterExpr
                 callSite.Key,
                 typeof(object));
 
-            var resolvedExpression = Expression.Variable(typeof(object), "resolved");
+            var resolvedVariable = Expression.Variable(typeof(object), "resolved");
 
             var resolvedServices = GetResolvedServices(provider);
 
             var tryGetValueExpression = Expression.Call(
                 resolvedServices,
                 TryGetValueMethodInfo,
                 keyExpression,
-                resolvedExpression);
+                resolvedVariable);
 
             var assignExpression = Expression.Assign(
-                resolvedExpression, VisitCallSite(callSite.ServiceCallSite, provider));
+                resolvedVariable, VisitCallSite(callSite.ServiceCallSite, provider));
 
             var addValueExpression = Expression.Call(
                 resolvedServices,
                 AddMethodInfo,
                 keyExpression,
-                resolvedExpression);
+                resolvedVariable);
+
+            Expression captureDisposableExpression = Expression.Invoke(
+                GetCaptureDisposable(provider),
+                resolvedVariable
+                );
 
             var blockExpression = Expression.Block(
                 typeof(object),
                 new[] {
-                    resolvedExpression
+                    resolvedVariable
                 },
                 Expression.IfThen(
                     Expression.Not(tryGetValueExpression),
-                    Expression.Block(assignExpression, addValueExpression)),
-                resolvedExpression);
+                    Expression.Block(
+                        assignExpression,
+                        addValueExpression,
+                        captureDisposableExpression)),
+                resolvedVariable);
 
             return blockExpression;
         }

src/Microsoft.Extensions.DependencyInjection/ServiceLookup/CallSiteRuntimeResolver.cs

@@ -49,6 +49,7 @@ protected override object VisitScoped(ScopedCallSite scopedCallSite, ServiceProv
                 if (!provider.ResolvedServices.TryGetValue(scopedCallSite.Key, out resolved))
                 {
                     resolved = VisitCallSite(scopedCallSite.ServiceCallSite, provider);
+                    provider.CaptureDisposable(resolved);
                     provider.ResolvedServices.Add(scopedCallSite.Key, resolved);
                 }
             }

src/Microsoft.Extensions.DependencyInjection/ServiceProvider.cs

@@ -169,22 +169,15 @@ public void Dispose()
                 _disposeCalled = true;
                 if (_transientDisposables != null)
                 {
-                    foreach (var disposable in _transientDisposables)
+                    for (int i = _transientDisposables.Count - 1; i >= 0; i--)
                     {
+                        var disposable = _transientDisposables[i];
                         disposable.Dispose();
                     }
 
                     _transientDisposables.Clear();
                 }
 
-                // PERF: We've enumerating the dictionary so that we don't allocate to enumerate.
-                // .Values allocates a ValueCollection on the heap, enumerating the dictionary allocates
-                // a struct enumerator
-                foreach (var entry in ResolvedServices)
-                {
-                    (entry.Value as IDisposable)?.Dispose();
-                }
-
                 ResolvedServices.Clear();
             }
         }