diff --git a/src/Microsoft.Framework.DependencyInjection/Properties/Resources.Designer.cs b/src/Microsoft.Framework.DependencyInjection/Properties/Resources.Designer.cs index ce83a23a..fc40e62f 100644 --- a/src/Microsoft.Framework.DependencyInjection/Properties/Resources.Designer.cs +++ b/src/Microsoft.Framework.DependencyInjection/Properties/Resources.Designer.cs @@ -106,6 +106,14 @@ internal static string FormatTypeCannotBeActivated(object p0, object p1) return string.Format(CultureInfo.CurrentCulture, GetString("TypeCannotBeActivated"), p0, p1); } + /// + /// Pooled type '{0}' reused before correct disposal. + /// + internal static string FormatPooledTypeReusedBeforeDisposal(object p0) + { + return string.Format(CultureInfo.CurrentCulture, GetString("PooledTypeReusedBeforeDisposal"), p0); + } + private static string GetString(string name, params string[] formatterNames) { var value = _resourceManager.GetString(name); diff --git a/src/Microsoft.Framework.DependencyInjection/Resources.resx b/src/Microsoft.Framework.DependencyInjection/Resources.resx index 5cfd5693..9ad3ddd4 100644 --- a/src/Microsoft.Framework.DependencyInjection/Resources.resx +++ b/src/Microsoft.Framework.DependencyInjection/Resources.resx @@ -137,4 +137,7 @@ Cannot instantiate implementation type '{0}' for service type '{1}'. + + Pooled type '{0}' reused before correct disposal. + \ No newline at end of file diff --git a/src/Microsoft.Framework.DependencyInjection/ServiceLookup/ServiceScope.cs b/src/Microsoft.Framework.DependencyInjection/ServiceLookup/ServiceScope.cs index 8cf3348b..f6fe2223 100644 --- a/src/Microsoft.Framework.DependencyInjection/ServiceLookup/ServiceScope.cs +++ b/src/Microsoft.Framework.DependencyInjection/ServiceLookup/ServiceScope.cs @@ -8,10 +8,12 @@ namespace Microsoft.Framework.DependencyInjection.ServiceLookup internal class ServiceScope : IServiceScope { private readonly ServiceProvider _scopedProvider; - - public ServiceScope(ServiceProvider scopedProvider) + private readonly ServiceScopeFactory _parentFactory; + + public ServiceScope(ServiceProvider scopedProvider, ServiceScopeFactory parentFactory) { _scopedProvider = scopedProvider; + _parentFactory = parentFactory; } public IServiceProvider ServiceProvider @@ -19,9 +21,24 @@ public IServiceProvider ServiceProvider get { return _scopedProvider; } } + internal void Reset() + { + _scopedProvider.Reset(); + } + public void Dispose() { + if (_scopedProvider.Disposed) + { + return; + } + _scopedProvider.Dispose(); + + if (_parentFactory != null) + { + _parentFactory.PoolScope(this); + } } } } diff --git a/src/Microsoft.Framework.DependencyInjection/ServiceLookup/ServiceScopeFactory.cs b/src/Microsoft.Framework.DependencyInjection/ServiceLookup/ServiceScopeFactory.cs index fc9cb773..2f4fcca3 100644 --- a/src/Microsoft.Framework.DependencyInjection/ServiceLookup/ServiceScopeFactory.cs +++ b/src/Microsoft.Framework.DependencyInjection/ServiceLookup/ServiceScopeFactory.cs @@ -1,20 +1,46 @@ // Copyright (c) .NET Foundation. All rights reserved. // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. +using System; +using System.Collections.Concurrent; + namespace Microsoft.Framework.DependencyInjection.ServiceLookup { internal class ServiceScopeFactory : IServiceScopeFactory { + private readonly int _capacity; + private readonly int _halfCapacity; + private readonly ConcurrentQueue _scopePool = new ConcurrentQueue(); + private readonly ServiceProvider _provider; public ServiceScopeFactory(ServiceProvider provider) { + _capacity = 32 * Environment.ProcessorCount; + _halfCapacity = _capacity / 2; _provider = provider; } public IServiceScope CreateScope() { - return new ServiceScope(new ServiceProvider(_provider)); + ServiceScope scope; + // maintain unused buffer of _halfCapacity as partial defense against badly behaving user code + if (_scopePool.Count > _halfCapacity && _scopePool.TryDequeue(out scope)) + { + scope.Reset(); + return scope; + } + return new ServiceScope(new ServiceProvider(_provider), this); + } + + internal void PoolScope(ServiceScope scope) + { + // Benign race condition + if (_scopePool.Count < _capacity) + { + _scopePool.Enqueue(scope); + } } + } } diff --git a/src/Microsoft.Framework.DependencyInjection/ServiceProvider.cs b/src/Microsoft.Framework.DependencyInjection/ServiceProvider.cs index ffc05654..3698bc72 100644 --- a/src/Microsoft.Framework.DependencyInjection/ServiceProvider.cs +++ b/src/Microsoft.Framework.DependencyInjection/ServiceProvider.cs @@ -41,7 +41,6 @@ internal ServiceProvider(ServiceProvider parent) _root = parent._root; _table = parent._table; } - // Reusing _resolvedServices as an implementation detail of the lock private object SyncObject => _resolvedServices; @@ -139,10 +138,27 @@ internal IServiceCallSite GetResolveCallSite(IService service, ISet callSi } } + internal bool Disposed { get; private set; } + + internal void Reset() + { + lock (SyncObject) + { + if (!Disposed) + { + throw new InvalidOperationException( + Resources.FormatPooledTypeReusedBeforeDisposal( + nameof(ServiceProvider))); + } + Disposed = false; + } + } + public void Dispose() { lock (SyncObject) { + Disposed = true; if (_transientDisposables != null) { foreach (var disposable in _transientDisposables) @@ -151,6 +167,7 @@ public void Dispose() } _transientDisposables.Clear(); + _transientDisposables = null; } // PERF: We've enumerating the dictionary so that we don't allocate to enumerate. @@ -179,6 +196,10 @@ private object CaptureDisposable(object service) { lock (SyncObject) { + if (Disposed) + { + throw new ObjectDisposedException(nameof(ServiceProvider)); + } if (_transientDisposables == null) { _transientDisposables = new List(); @@ -279,6 +300,10 @@ public virtual object Invoke(ServiceProvider provider) object resolved; lock (provider._resolvedServices) { + if (provider.Disposed) + { + throw new ObjectDisposedException(nameof(ServiceProvider)); + } if (!provider._resolvedServices.TryGetValue(_key, out resolved)) { resolved = _serviceCallSite.Invoke(provider); diff --git a/test/Microsoft.Framework.DependencyInjection.Tests/ScopingContainerTestBase.cs b/test/Microsoft.Framework.DependencyInjection.Tests/ScopingContainerTestBase.cs index 1e836ff1..4c363137 100644 --- a/test/Microsoft.Framework.DependencyInjection.Tests/ScopingContainerTestBase.cs +++ b/test/Microsoft.Framework.DependencyInjection.Tests/ScopingContainerTestBase.cs @@ -121,6 +121,34 @@ public void SelfResolveThenDispose() (container as IDisposable)?.Dispose(); } + [Fact] + public void PooledDisposedScopesResolveDifferentInstances() + { + var container = CreateContainer(); + FakeService disposableService1; + FakeService disposableService2; + + var scopeFactory = container.GetService(); + using (var scope = scopeFactory.CreateScope()) + { + disposableService1 = (FakeService)scope.ServiceProvider.GetService(); + + Assert.False(disposableService1.Disposed); + } + + Assert.True(disposableService1.Disposed); + + using (var scope = scopeFactory.CreateScope()) + { + disposableService2 = (FakeService)scope.ServiceProvider.GetService(); + + Assert.False(disposableService2.Disposed); + } + + Assert.True(disposableService2.Disposed); + Assert.NotEqual(disposableService1, disposableService2); + } + [Fact] public void SingletonServicesComeFromRootContainer() {