Breaking change: Throw when resolving on a disposed service provider (#…

…45116)

src/libraries/Microsoft.Extensions.DependencyInjection/src/ServiceLookup/ServiceProviderEngineScope.cs

@@ -17,6 +17,7 @@ internal class ServiceProviderEngineScope : IServiceScope, IServiceProvider, IAs
         private List<object> _disposables;
 
         private bool _disposed;
+        private readonly object _disposelock = new object();
 
         public ServiceProviderEngineScope(ServiceProviderEngine engine)
         {
@@ -41,17 +42,29 @@ 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))
             {
                 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<object>();
@@ -144,7 +157,7 @@ static async ValueTask Await(int i, ValueTask vt, List<object> toDispose)
         private List<object> BeginDispose()
         {
             List<object> toDispose;
-            lock (ResolvedServices)
+            lock (_disposelock)
             {
                 if (_disposed)
                 {

src/libraries/Microsoft.Extensions.DependencyInjection/tests/DI.Tests/Microsoft.Extensions.DependencyInjection.Tests.csproj

@@ -7,6 +7,8 @@
 
   <ItemGroup>
     <Compile Include="..\DI.Specification.Tests\**\*.cs" />
+    <Compile Include="$(CommonPath)..\tests\Extensions\SingleThreadedSynchronizationContext.cs"
+             Link="Shared\SingleThreadedSynchronizationContext.cs" />
   </ItemGroup>
 
   <ItemGroup>
@@ -15,4 +17,8 @@
     <ProjectReference Include="..\..\src\Microsoft.Extensions.DependencyInjection.csproj" SkipUseReferenceAssembly="true" />
   </ItemGroup>
 
+  <ItemGroup Condition="'$(TargetFramework)' == 'net461'">
+    <PackageReference Include="System.ValueTuple" Version="$(SystemValueTupleVersion)" />
+  </ItemGroup>
+
 </Project>

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<DisposeServiceProviderInCtor>();
+            IServiceProvider sp = services.BuildServiceProvider();
+            Assert.Throws<ObjectDisposedException>(() =>
+            {
+                // ctor disposes ServiceProvider
+                var service = sp.GetRequiredService<DisposeServiceProviderInCtor>();
+            });
+        }
+
+        [Fact]
+        public void GetAsyncService_DisposeAsyncOnSameThread_ThrowsAndDoesNotHangAndDisposeAsyncGetsCalled()
+        {
+            // Arrange
+            var services = new ServiceCollection();
+            var asyncDisposableResource = new AsyncDisposable();
+            services.AddSingleton<DisposeServiceProviderInCtorAsyncDisposable>(sp =>
+                new DisposeServiceProviderInCtorAsyncDisposable(asyncDisposableResource, sp));
+
+            var sp = services.BuildServiceProvider();
+            bool doesNotHang = Task.Run(() =>
+            {
+                SingleThreadedSynchronizationContext.Run(() =>
+                {
+                    // Act
+                    Assert.Throws<ObjectDisposedException>(() =>
+                    {
+                        // ctor disposes ServiceProvider
+                        var service = sp.GetRequiredService<DisposeServiceProviderInCtorAsyncDisposable>();
+                    });
+                });
+            }).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<DisposeServiceProviderInCtorDisposable>(sp =>
+                new DisposeServiceProviderInCtorDisposable(disposableResource, sp));
+
+            var sp = services.BuildServiceProvider();
+            bool doesNotHang = Task.Run(() =>
+            {
+                SingleThreadedSynchronizationContext.Run(() =>
+                {
+                    // Act
+                    Assert.Throws<ObjectDisposedException>(() =>
+                    {
+                        // ctor disposes ServiceProvider
+                        var service = sp.GetRequiredService<DisposeServiceProviderInCtorDisposable>();
+                    });
+                });
+            }).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<AsyncDisposable>();
+            services.AddSingleton<Disposable>();
+            if (includeDelayedAsyncDisposable)
+            {
+                //forces Dispose ValueTask to be asynchronous and not be immediately completed
+                services.AddSingleton<DelayedAsyncDisposableService>();
+            }
+            ServiceProvider sp = services.BuildServiceProvider();
+            var disposable = sp.GetRequiredService<Disposable>();
+            var asyncDisposable = sp.GetRequiredService<AsyncDisposable>();
+            DelayedAsyncDisposableService delayedAsyncDisposableService = null;
+            if (includeDelayedAsyncDisposable)
+            {
+                delayedAsyncDisposableService = sp.GetRequiredService<DelayedAsyncDisposableService>();
+            }
+
+            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)]