Don't allocate disposable list until used

- Only use it for transient objects
- Use _resolvedServices as a lockObject to avoid allocating a lockObj

Author: davidfowl

src/Microsoft.Framework.DependencyInjection/ServiceProvider.cs

@@ -21,7 +21,7 @@ internal class ServiceProvider : IServiceProvider, IDisposable
         private readonly ServiceTable _table;
 
         private readonly Dictionary<IService, object> _resolvedServices = new Dictionary<IService, object>();
-        private readonly List<IDisposable> _disposables = new List<IDisposable>();
+        private List<IDisposable> _transientDisposables;
 
         private static readonly Func<Type, ServiceProvider, Func<ServiceProvider, object>> _createServiceAccessor = CreateServiceAccessor;
 
@@ -42,6 +42,9 @@ internal ServiceProvider(ServiceProvider parent)
             _table = parent._table;
         }
 
+        // Reusing _resolvedServices as an implementation detail of the lock
+        private object SyncObject => _resolvedServices;
+
         /// <summary>
         /// Gets the service object of the specified type.
         /// </summary>
@@ -138,14 +141,24 @@ internal IServiceCallSite GetResolveCallSite(IService service, ISet<Type> callSi
 
         public void Dispose()
         {
-            lock (_disposables)
+            lock (SyncObject)
             {
-                foreach (var disposable in _disposables)
+                if (_transientDisposables != null)
+                {
+                    foreach (var disposable in _transientDisposables)
+                    {
+                        disposable.Dispose();
+                    }
+
+                    _transientDisposables.Clear();
+                }
+
+                // Nuke the other service types
+                foreach (var item in _resolvedServices.Values)
                 {
-                    disposable.Dispose();
+                    (item as IDisposable)?.Dispose();
                 }
 
-                _disposables.Clear();
                 _resolvedServices.Clear();
             }
         }
@@ -163,9 +176,14 @@ private object CaptureDisposable(object service)
                 var disposable = service as IDisposable;
                 if (disposable != null)
                 {
-                    lock (_disposables)
+                    lock (SyncObject)
                     {
-                        _disposables.Add(disposable);
+                        if (_transientDisposables == null)
+                        {
+                            _transientDisposables = new List<IDisposable>();
+                        }
+
+                        _transientDisposables.Add(disposable);
                     }
                 }
             }
@@ -262,7 +280,7 @@ public virtual object Invoke(ServiceProvider provider)
                 {
                     if (!provider._resolvedServices.TryGetValue(_key, out resolved))
                     {
-                        resolved = provider.CaptureDisposable(_serviceCallSite.Invoke(provider));
+                        resolved = _serviceCallSite.Invoke(provider);
                         provider._resolvedServices.Add(_key, resolved);
                     }
                 }
@@ -287,12 +305,8 @@ public virtual Expression Build(Expression providerExpression)
                     keyExpression,
                     resolvedExpression);
 
-                var captureDisposableExpression = Expression.Assign(
-                    resolvedExpression,
-                    Expression.Call(
-                        providerExpression,
-                        CaptureDisposableMethodInfo,
-                        _serviceCallSite.Build(providerExpression)));
+                var assignExpression = Expression.Assign(
+                    resolvedExpression, _serviceCallSite.Build(providerExpression));
 
                 var addValueExpression = Expression.Call(
                     resolvedServicesExpression,
@@ -305,7 +319,7 @@ public virtual Expression Build(Expression providerExpression)
                     new[] { resolvedExpression },
                     Expression.IfThen(
                         Expression.Not(tryGetValueExpression),
-                        Expression.Block(captureDisposableExpression, addValueExpression)),
+                        Expression.Block(assignExpression, addValueExpression)),
                     resolvedExpression);
 
                 return Lock(providerExpression, blockExpression);

test/Microsoft.Framework.DependencyInjection.Tests/ScopingContainerTestBase.cs

@@ -78,22 +78,33 @@ public void DisposingScopeDisposesService()
             FakeService disposableService;
             FakeService transient1;
             FakeService transient2;
+            FakeService singleton;
 
             var scopeFactory = container.GetService<IServiceScopeFactory>();
             using (var scope = scopeFactory.CreateScope())
             {
                 disposableService = (FakeService)scope.ServiceProvider.GetService<IFakeScopedService>();
                 transient1 = (FakeService)scope.ServiceProvider.GetService<IFakeService>();
                 transient2 = (FakeService)scope.ServiceProvider.GetService<IFakeService>();
+                singleton = (FakeService)scope.ServiceProvider.GetService<IFakeSingletonService>();
 
                 Assert.False(disposableService.Disposed);
                 Assert.False(transient1.Disposed);
                 Assert.False(transient2.Disposed);
+                Assert.False(singleton.Disposed);
             }
 
             Assert.True(disposableService.Disposed);
             Assert.True(transient1.Disposed);
             Assert.True(transient2.Disposed);
+            Assert.False(singleton.Disposed);
+
+            var disposableContainer = container as IDisposable;
+            if (disposableContainer != null)
+            {
+                disposableContainer.Dispose();
+                Assert.True(singleton.Disposed);
+            }
         }
 
         [Fact]