diff --git a/src/libraries/Microsoft.Extensions.DependencyInjection/src/ServiceLookup/ServiceProviderEngineScope.cs b/src/libraries/Microsoft.Extensions.DependencyInjection/src/ServiceLookup/ServiceProviderEngineScope.cs index b8a330642cf40..48335806c9e06 100644 --- a/src/libraries/Microsoft.Extensions.DependencyInjection/src/ServiceLookup/ServiceProviderEngineScope.cs +++ b/src/libraries/Microsoft.Extensions.DependencyInjection/src/ServiceLookup/ServiceProviderEngineScope.cs @@ -17,6 +17,7 @@ internal class ServiceProviderEngineScope : IServiceScope, IServiceProvider, IAs private List _disposables; private bool _disposed; + private readonly object _disposelock = new object(); public ServiceProviderEngineScope(ServiceProviderEngine engine) { @@ -41,8 +42,6 @@ public object GetService(Type serviceType) internal object CaptureDisposable(object service) { - Debug.Assert(!_disposed); - _captureDisposableCallback?.Invoke(service); if (ReferenceEquals(this, service) || !(service is IDisposable || service is IAsyncDisposable)) @@ -50,8 +49,22 @@ internal object CaptureDisposable(object service) return service; } - lock (ResolvedServices) + lock (_disposelock) { + if (_disposed) + { + if (service is IDisposable disposable) + { + disposable.Dispose(); + } + else + { + // sync over async, for the rare case that an object only implements IAsyncDisposable and may end up starving the thread pool. + Task.Run(() => ((IAsyncDisposable)service).DisposeAsync().AsTask()).GetAwaiter().GetResult(); + } + ThrowHelper.ThrowObjectDisposedException(); + } + if (_disposables == null) { _disposables = new List(); @@ -144,7 +157,7 @@ static async ValueTask Await(int i, ValueTask vt, List toDispose) private List BeginDispose() { List toDispose; - lock (ResolvedServices) + lock (_disposelock) { if (_disposed) { diff --git a/src/libraries/Microsoft.Extensions.DependencyInjection/tests/DI.Tests/Microsoft.Extensions.DependencyInjection.Tests.csproj b/src/libraries/Microsoft.Extensions.DependencyInjection/tests/DI.Tests/Microsoft.Extensions.DependencyInjection.Tests.csproj index 762664f48f733..bb3a008f3684d 100644 --- a/src/libraries/Microsoft.Extensions.DependencyInjection/tests/DI.Tests/Microsoft.Extensions.DependencyInjection.Tests.csproj +++ b/src/libraries/Microsoft.Extensions.DependencyInjection/tests/DI.Tests/Microsoft.Extensions.DependencyInjection.Tests.csproj @@ -7,6 +7,8 @@ + @@ -15,4 +17,8 @@ + + + + diff --git a/src/libraries/Microsoft.Extensions.DependencyInjection/tests/DI.Tests/ServiceProviderContainerTests.cs b/src/libraries/Microsoft.Extensions.DependencyInjection/tests/DI.Tests/ServiceProviderContainerTests.cs index 0ede07896f5b5..d83a52d4f9e19 100644 --- a/src/libraries/Microsoft.Extensions.DependencyInjection/tests/DI.Tests/ServiceProviderContainerTests.cs +++ b/src/libraries/Microsoft.Extensions.DependencyInjection/tests/DI.Tests/ServiceProviderContainerTests.cs @@ -4,11 +4,13 @@ using System; using System.Collections.Generic; using System.Linq; +using System.Threading; using System.Threading.Tasks; using Microsoft.Extensions.DependencyInjection.Fakes; using Microsoft.Extensions.DependencyInjection.Specification; using Microsoft.Extensions.DependencyInjection.Specification.Fakes; using Microsoft.Extensions.DependencyInjection.Tests.Fakes; +using Microsoft.Extensions.Internal; using Xunit; namespace Microsoft.Extensions.DependencyInjection.Tests @@ -265,6 +267,145 @@ public void ScopeDispose_PreventsServiceResolution() Assert.NotNull(provider.CreateScope()); } + [Fact] + public void GetService_DisposeOnSameThread_Throws() + { + var services = new ServiceCollection(); + services.AddSingleton(); + IServiceProvider sp = services.BuildServiceProvider(); + Assert.Throws(() => + { + // ctor disposes ServiceProvider + var service = sp.GetRequiredService(); + }); + } + + [Fact] + public void GetAsyncService_DisposeAsyncOnSameThread_ThrowsAndDoesNotHangAndDisposeAsyncGetsCalled() + { + // Arrange + var services = new ServiceCollection(); + var asyncDisposableResource = new AsyncDisposable(); + services.AddSingleton(sp => + new DisposeServiceProviderInCtorAsyncDisposable(asyncDisposableResource, sp)); + + var sp = services.BuildServiceProvider(); + bool doesNotHang = Task.Run(() => + { + SingleThreadedSynchronizationContext.Run(() => + { + // Act + Assert.Throws(() => + { + // ctor disposes ServiceProvider + var service = sp.GetRequiredService(); + }); + }); + }).Wait(TimeSpan.FromSeconds(10)); + + Assert.True(doesNotHang); + Assert.True(asyncDisposableResource.DisposeAsyncCalled); + } + + [Fact] + public void GetService_DisposeOnSameThread_ThrowsAndDoesNotHangAndDisposeGetsCalled() + { + // Arrange + var services = new ServiceCollection(); + var disposableResource = new Disposable(); + services.AddSingleton(sp => + new DisposeServiceProviderInCtorDisposable(disposableResource, sp)); + + var sp = services.BuildServiceProvider(); + bool doesNotHang = Task.Run(() => + { + SingleThreadedSynchronizationContext.Run(() => + { + // Act + Assert.Throws(() => + { + // ctor disposes ServiceProvider + var service = sp.GetRequiredService(); + }); + }); + }).Wait(TimeSpan.FromSeconds(10)); + + Assert.True(doesNotHang); + Assert.True(disposableResource.Disposed); + } + + private class DisposeServiceProviderInCtor : IDisposable + { + public DisposeServiceProviderInCtor(IServiceProvider sp) + { + (sp as IDisposable).Dispose(); + } + public void Dispose() { } + } + + [Theory] + [InlineData(true)] + [InlineData(false)] + public async Task AddDisposablesAndAsyncDisposables_DisposeAsync_AllDisposed(bool includeDelayedAsyncDisposable) + { + var services = new ServiceCollection(); + services.AddSingleton(); + services.AddSingleton(); + if (includeDelayedAsyncDisposable) + { + //forces Dispose ValueTask to be asynchronous and not be immediately completed + services.AddSingleton(); + } + ServiceProvider sp = services.BuildServiceProvider(); + var disposable = sp.GetRequiredService(); + var asyncDisposable = sp.GetRequiredService(); + DelayedAsyncDisposableService delayedAsyncDisposableService = null; + if (includeDelayedAsyncDisposable) + { + delayedAsyncDisposableService = sp.GetRequiredService(); + } + + await sp.DisposeAsync(); + + Assert.True(disposable.Disposed); + Assert.True(asyncDisposable.DisposeAsyncCalled); + if (includeDelayedAsyncDisposable) + { + Assert.Equal(1, delayedAsyncDisposableService.DisposeCount); + } + } + + private class DisposeServiceProviderInCtorAsyncDisposable : IFakeService, IAsyncDisposable + { + private readonly AsyncDisposable _asyncDisposable; + + public DisposeServiceProviderInCtorAsyncDisposable(AsyncDisposable asyncDisposable, IServiceProvider sp) + { + _asyncDisposable = asyncDisposable; + (sp as IAsyncDisposable).DisposeAsync(); + } + public async ValueTask DisposeAsync() + { + await _asyncDisposable.DisposeAsync(); + await Task.Yield(); + } + } + + private class DisposeServiceProviderInCtorDisposable : IFakeService, IDisposable + { + private readonly Disposable _disposable; + + public DisposeServiceProviderInCtorDisposable(Disposable disposable, IServiceProvider sp) + { + _disposable = disposable; + (sp as IDisposable).Dispose(); + } + public void Dispose() + { + _disposable.Dispose(); + } + } + [ActiveIssue("https://github.com/dotnet/runtime/issues/42160")] // We don't support value task services currently [Theory] [InlineData(ServiceLifetime.Transient)]