Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Deadlock when creating typed HttpClient with DI and HttpClientFactory #35986

Closed
meareindel opened this issue Apr 10, 2020 · 7 comments · Fixed by #46157
Closed

Deadlock when creating typed HttpClient with DI and HttpClientFactory #35986

meareindel opened this issue Apr 10, 2020 · 7 comments · Fixed by #46157

Comments

@meareindel
Copy link

meareindel commented Apr 10, 2020

Describe the bug

We have a .net core web service with, for simplicity, two controllers, each with one route. One of the controllers, let's say 'scoped' controller, has typed http client injected directly. Other one, - 'singleton' controller, - has some singleton service injected, which, in its turn, has the same typed http client injected. The typed http client itself is registered with headers propagation - but it could be any additional .AddHttpMessageHandler(Func<IServiceProvider, DelegatingHandler>) registered which makes calls similar to IServiceProvider.GetRequiredService for any singleton dependency.

At first time after web service start when there is two (or more) concurrent requests on routes in both controllers, and route in 'scoped' controller starts processing just a bit earlier, than route in 'singleton' controller, there is a deadlock in DI.

To Reproduce

Take the solution from https://github.com/meareindel/DI-deadlock/tree/master and run the single test in it. It could be needed to modify the value of seconds to wait between requesting tasks depending on a machine configuration. The value also needs to be other for debugging (a little bit more, around 10ms on my machine, for example).

Expected behavior

No deadlock occurs, http client created successfully --OR-- validation error when injecting transient service into singleton.

Screenshots

If applicable, add screenshots to help explain your problem.

Additional context

Add any other context about the problem here.
Include the output of dotnet --info

stacks of route threads
'singleton' controller route:

Lazy<ActiveHandlerTrackingEntry>.ExecutionAndPublication()
Lazy<ActiveHandlerTrackingEntry>.CreateValue()
Lazy<__Canon>.get_Value()
DefaultHttpClientFactory.CreateHandler()
DefaultHttpClientFactory.CreateClient()
HttpClientBuilderExtensions.<>c__DisplayClass10_0<TypedHttpClient>.<AddTypedClientCore>b__0()
CallSiteRuntimeResolver.VisitFactory()
CallSiteVisitor<RuntimeResolverContext, object>.VisitCallSiteMain() [2]
CallSiteRuntimeResolver.VisitDisposeCache()
CallSiteVisitor<RuntimeResolverContext, object>.VisitCallSite() [2]
CallSiteRuntimeResolver.VisitConstructor()
CallSiteVisitor<RuntimeResolverContext, object>.VisitCallSiteMain() [1]
CallSiteRuntimeResolver.VisitCache()
CallSiteRuntimeResolver.VisitRootCache()
CallSiteVisitor<RuntimeResolverContext, object>.VisitCallSite() [1]
CallSiteRuntimeResolver.Resolve()
DynamicServiceProviderEngine.<>c__DisplayClass1_0.<RealizeService>b__0()
ServiceProviderEngine.GetService()
ServiceProviderEngineScope.GetService()
ActivatorUtilities.GetService()
[Lightweight Method Call]
ControllerActivatorProvider.<>c__DisplayClass4_0.<CreateActivator>b__0()
ControllerFactoryProvider.<>c__DisplayClass5_0.<CreateControllerFactory>g__CreateController|0()
ControllerActionInvoker.Next()
ControllerActionInvoker.InvokeInnerFilterAsync()
ResourceInvoker.Next()
ResourceInvoker.InvokeFilterPipelineAsync()
ResourceInvoker.<<InvokeAsync>g__Logged|17_1>d.MoveNext()
AsyncMethodBuilderCore.Start<Microsoft.AspNetCore.Mvc.Infrastructure.ResourceInvoker.<<InvokeAsync>g__Logged|17_1>d>()
AsyncTaskMethodBuilder.Start<Microsoft.AspNetCore.Mvc.Infrastructure.ResourceInvoker.<<InvokeAsync>g__Logged|17_1>d>()
ResourceInvoker.<InvokeAsync>g__Logged|17_1()
ResourceInvoker.InvokeAsync()
ActionEndpointFactory.<>c__DisplayClass7_0.<CreateRequestDelegate>b__0()
EndpointMiddleware.Invoke()
AuthorizationMiddleware.<Invoke>d__5.MoveNext()
AsyncMethodBuilderCore.Start<Microsoft.AspNetCore.Authorization.AuthorizationMiddleware.<Invoke>d__5>()
AsyncTaskMethodBuilder.Start<Microsoft.AspNetCore.Authorization.AuthorizationMiddleware.<Invoke>d__5>()
AuthorizationMiddleware.Invoke()
EndpointRoutingMiddleware.SetRoutingAndContinue()
EndpointRoutingMiddleware.Invoke()
HeaderPropagationMiddleware.<Invoke>d__4.MoveNext()
AsyncMethodBuilderCore.Start<Microsoft.AspNetCore.HeaderPropagation.HeaderPropagationMiddleware.<Invoke>d__4>()
AsyncTaskMethodBuilder.Start<Microsoft.AspNetCore.HeaderPropagation.HeaderPropagationMiddleware.<Invoke>d__4>()
HeaderPropagationMiddleware.Invoke()
HttpsRedirectionMiddleware.Invoke()
DeveloperExceptionPageMiddleware.<Invoke>d__9.MoveNext()
AsyncMethodBuilderCore.Start<Microsoft.AspNetCore.Diagnostics.DeveloperExceptionPageMiddleware.<Invoke>d__9>()
AsyncTaskMethodBuilder.Start<Microsoft.AspNetCore.Diagnostics.DeveloperExceptionPageMiddleware.<Invoke>d__9>()
DeveloperExceptionPageMiddleware.Invoke()
HostFilteringMiddleware.Invoke()
HostingApplication.ProcessRequestAsync()
ApplicationWrapper<HostingApplication.Context>.Microsoft.AspNetCore.Hosting.Server.IHttpApplication<TContext>.ProcessRequestAsync()
ApplicationWrapper<HostingApplication.Context>.ProcessRequestAsync()
HttpContextBuilder.<>c__DisplayClass23_0.<<SendAsync>g__RunRequestAsync|0>d.MoveNext()
AsyncMethodBuilderCore.Start<Microsoft.AspNetCore.TestHost.HttpContextBuilder.<>c__DisplayClass23_0.<<SendAsync>g__RunRequestAsync|0>d>()
AsyncTaskMethodBuilder.Start<Microsoft.AspNetCore.TestHost.HttpContextBuilder.<>c__DisplayClass23_0.<<SendAsync>g__RunRequestAsync|0>d>()
HttpContextBuilder.<>c__DisplayClass23_0.<SendAsync>g__RunRequestAsync|0()
HttpContextBuilder.<>c__DisplayClass23_0.<SendAsync>b__1()
QueueUserWorkItemCallbackDefaultContext.Execute()
ThreadPoolWorkQueue.Dispatch()
_ThreadPoolWaitCallback.PerformWaitCallback()
[Native to Managed Transition]

'scoped' controller route:

Monitor.Enter()
CallSiteRuntimeResolver.VisitCache()
CallSiteRuntimeResolver.VisitRootCache()
CallSiteVisitor<RuntimeResolverContext, object>.VisitCallSite() [2]
CallSiteRuntimeResolver.Resolve() [2]
DynamicServiceProviderEngine.<>c__DisplayClass1_0.<RealizeService>b__0() [2]
ServiceProviderEngine.GetService() [2]
ServiceProviderEngineScope.GetService() [2]
ServiceProviderServiceExtensions.GetRequiredService()
ServiceProviderServiceExtensions.GetRequiredService<Microsoft.Extensions.Options.IOptions<Microsoft.AspNetCore.HeaderPropagation.HeaderPropagationOptions>>()
HeaderPropagationHttpClientBuilderExtensions.<>c.<AddHeaderPropagation>b__0_0()
HttpClientBuilderExtensions.<>c__DisplayClass3_0.<AddHttpMessageHandler>b__1()
DefaultHttpClientFactory.<>c__DisplayClass17_0.<CreateHandlerEntry>g__Configure|0()
LoggingHttpMessageHandlerBuilderFilter.<>c__DisplayClass2_0.<Configure>b__0()
DefaultHttpClientFactory.CreateHandlerEntry()
DefaultHttpClientFactory.<>c__DisplayClass14_0.<.ctor>b__1()
Lazy<ActiveHandlerTrackingEntry>.ViaFactory()
Lazy<__Canon>.ExecutionAndPublication()
Lazy<ActiveHandlerTrackingEntry>.CreateValue()
Lazy<__Canon>.get_Value()
DefaultHttpClientFactory.CreateHandler()
DefaultHttpClientFactory.CreateClient()
HttpClientBuilderExtensions.<>c__DisplayClass10_0<TypedHttpClient>.<AddTypedClientCore>b__0()
CallSiteRuntimeResolver.VisitFactory()
CallSiteVisitor<RuntimeResolverContext, object>.VisitCallSiteMain()
CallSiteRuntimeResolver.VisitDisposeCache()
CallSiteVisitor<RuntimeResolverContext, object>.VisitCallSite() [1]
CallSiteRuntimeResolver.Resolve() [1]
DynamicServiceProviderEngine.<>c__DisplayClass1_0.<RealizeService>b__0() [1]
ServiceProviderEngine.GetService() [1]
ServiceProviderEngineScope.GetService() [1]
ActivatorUtilities.GetService()
[Lightweight Method Call]
ControllerActivatorProvider.<>c__DisplayClass4_0.<CreateActivator>b__0()
ControllerFactoryProvider.<>c__DisplayClass5_0.<CreateControllerFactory>g__CreateController|0()
ControllerActionInvoker.Next()
ControllerActionInvoker.InvokeInnerFilterAsync()
ResourceInvoker.Next()
ResourceInvoker.InvokeFilterPipelineAsync()
ResourceInvoker.<<InvokeAsync>g__Logged|17_1>d.MoveNext()
AsyncMethodBuilderCore.Start<Microsoft.AspNetCore.Mvc.Infrastructure.ResourceInvoker.<<InvokeAsync>g__Logged|17_1>d>()
AsyncTaskMethodBuilder.Start<Microsoft.AspNetCore.Mvc.Infrastructure.ResourceInvoker.<<InvokeAsync>g__Logged|17_1>d>()
ResourceInvoker.<InvokeAsync>g__Logged|17_1()
ResourceInvoker.InvokeAsync()
ActionEndpointFactory.<>c__DisplayClass7_0.<CreateRequestDelegate>b__0()
EndpointMiddleware.Invoke()
AuthorizationMiddleware.<Invoke>d__5.MoveNext()
AsyncMethodBuilderCore.Start<Microsoft.AspNetCore.Authorization.AuthorizationMiddleware.<Invoke>d__5>()
AsyncTaskMethodBuilder.Start<Microsoft.AspNetCore.Authorization.AuthorizationMiddleware.<Invoke>d__5>()
AuthorizationMiddleware.Invoke()
EndpointRoutingMiddleware.SetRoutingAndContinue()
EndpointRoutingMiddleware.Invoke()
HeaderPropagationMiddleware.<Invoke>d__4.MoveNext()
AsyncMethodBuilderCore.Start<Microsoft.AspNetCore.HeaderPropagation.HeaderPropagationMiddleware.<Invoke>d__4>()
AsyncTaskMethodBuilder.Start<Microsoft.AspNetCore.HeaderPropagation.HeaderPropagationMiddleware.<Invoke>d__4>()
HeaderPropagationMiddleware.Invoke()
HttpsRedirectionMiddleware.Invoke()
DeveloperExceptionPageMiddleware.<Invoke>d__9.MoveNext()
AsyncMethodBuilderCore.Start<Microsoft.AspNetCore.Diagnostics.DeveloperExceptionPageMiddleware.<Invoke>d__9>()
AsyncTaskMethodBuilder.Start<Microsoft.AspNetCore.Diagnostics.DeveloperExceptionPageMiddleware.<Invoke>d__9>()
DeveloperExceptionPageMiddleware.Invoke()
HostFilteringMiddleware.Invoke()
HostingApplication.ProcessRequestAsync()
ApplicationWrapper<HostingApplication.Context>.Microsoft.AspNetCore.Hosting.Server.IHttpApplication<TContext>.ProcessRequestAsync()
ApplicationWrapper<HostingApplication.Context>.ProcessRequestAsync()
HttpContextBuilder.<>c__DisplayClass23_0.<<SendAsync>g__RunRequestAsync|0>d.MoveNext()
AsyncMethodBuilderCore.Start<Microsoft.AspNetCore.TestHost.HttpContextBuilder.<>c__DisplayClass23_0.<<SendAsync>g__RunRequestAsync|0>d>()
AsyncTaskMethodBuilder.Start<Microsoft.AspNetCore.TestHost.HttpContextBuilder.<>c__DisplayClass23_0.<<SendAsync>g__RunRequestAsync|0>d>()
HttpContextBuilder.<>c__DisplayClass23_0.<SendAsync>g__RunRequestAsync|0()
HttpContextBuilder.<>c__DisplayClass23_0.<SendAsync>b__1()
QueueUserWorkItemCallbackDefaultContext.Execute()
ThreadPoolWorkQueue.Dispatch()
_ThreadPoolWaitCallback.PerformWaitCallback()
[Native to Managed Transition]

dotnet --info:
Пакет SDK для .NET Core (отражающий любой global.json):
Version: 3.1.101
Commit: b377529961

Среда выполнения:
OS Name: Windows
OS Version: 10.0.18363
OS Platform: Windows
RID: win10-x64
Base Path: C:\Program Files\dotnet\sdk\3.1.101\

Host (useful for support):
Version: 3.1.1
Commit: a1388f194c

.NET Core SDKs installed:
2.1.600 [C:\Program Files\dotnet\sdk]
2.1.601 [C:\Program Files\dotnet\sdk]
2.1.602 [C:\Program Files\dotnet\sdk]
2.1.604 [C:\Program Files\dotnet\sdk]
2.1.700 [C:\Program Files\dotnet\sdk]
2.1.701 [C:\Program Files\dotnet\sdk]
2.1.801 [C:\Program Files\dotnet\sdk]
2.1.802 [C:\Program Files\dotnet\sdk]
2.2.105 [C:\Program Files\dotnet\sdk]
3.1.101 [C:\Program Files\dotnet\sdk]

.NET Core runtimes installed:
Microsoft.AspNetCore.All 2.1.8 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.All]
Microsoft.AspNetCore.All 2.1.9 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.All]
Microsoft.AspNetCore.All 2.1.11 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.All]
Microsoft.AspNetCore.All 2.1.12 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.All]
Microsoft.AspNetCore.All 2.1.13 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.All]
Microsoft.AspNetCore.All 2.2.3 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.All]
Microsoft.AspNetCore.All 2.2.8 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.All]
Microsoft.AspNetCore.App 2.1.8 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
Microsoft.AspNetCore.App 2.1.9 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
Microsoft.AspNetCore.App 2.1.11 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
Microsoft.AspNetCore.App 2.1.12 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
Microsoft.AspNetCore.App 2.1.13 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
Microsoft.AspNetCore.App 2.2.3 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
Microsoft.AspNetCore.App 2.2.8 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
Microsoft.AspNetCore.App 3.1.1 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
Microsoft.NETCore.App 2.1.8 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
Microsoft.NETCore.App 2.1.9 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
Microsoft.NETCore.App 2.1.11 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
Microsoft.NETCore.App 2.1.12 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
Microsoft.NETCore.App 2.1.13 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
Microsoft.NETCore.App 2.2.3 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
Microsoft.NETCore.App 2.2.8 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
Microsoft.NETCore.App 3.1.1 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
Microsoft.WindowsDesktop.App 3.1.1 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App]

To install additional .NET Core runtimes or SDKs:
https://aka.ms/dotnet-download

The workaround is to register singleton service as scoped, of course. But I mainly expects some guidelines/documentation notes about such case. Is it a real bug to fix or is it unsupported servces scopes configuration - then the DI runtime validation could be expanded to such cases?

@analogrelay
Copy link
Contributor

Triage: This may not be HttpClientFactory specific, it might be a DI deadlock that HttpClientFactory is manifesting. (cc @davidfowl for any spicy thoughts).

@analogrelay analogrelay transferred this issue from dotnet/extensions May 7, 2020
@Dotnet-GitSync-Bot Dotnet-GitSync-Bot added area-System.Net.Http untriaged New issue has not been triaged by the area owner labels May 7, 2020
@ghost
Copy link

ghost commented May 7, 2020

Tagging subscribers to this area: @dotnet/ncl
Notify danmosemsft if you want to be subscribed.

@davidfowl
Copy link
Member

OK I was able to reproduce the issue in isolation. This is an issue in the http client factory implementation and it's use of lazy.

using System;
using System.Threading;
using System.Threading.Tasks;
using Microsoft.Extensions.DependencyInjection;

namespace ConsoleApp38
{
    class Program
    {
        [ThreadStatic]
        public static int ThreadId;

        static async Task Main(string[] args)
        {
            // Thread 1: Thing1 (transient) -> Thing0 (singleton)
            // Thread 2: Thing2 (singleton) -> Thing1 (transient) -> Thing0 (singleton)

            // This reproduces the dead lock between the Lazy<T> and the IServiceProvider. Whenever singleton or scoped
            // services are resolved a lock is taken (either globally for singletons or per scope for scoped).

            // 1. Thread 1 resolves the Thing1 which is a transient service
            // 2. In parallel, Thread 2 resolves Thing2 which is a singleton
            // 3. Thread 1 enters the factory callback for Thing1 and takes the lazy lock
            // 4. Thread 2 takes the singleton lock for the container when it resolves Thing2
            // 5. Thread 2 enters the factory callback for Thing1 and waits on the lazy lock
            // 6. Thread 1 calls GetRequiredService<Thing0> on the service provider, this waits for the singleton lock that Thread1 has
            
            // Thread 1 has the lazy lock and is waiting on the singleton lock
            // Thread 2 has the singleton lock an is waiting on the lazy

            var services = new ServiceCollection();

            IServiceProvider sp = null;

            var lazy = new Lazy<Thing1>(() =>
            {
                // Tries to take the singleton lock (global)
                var thing0 = sp.GetRequiredService<Thing0>();
                return new Thing1(thing0);
            });

            services.AddSingleton<Thing0>();
            services.AddTransient(sp =>
            {
                if (ThreadId == 1)
                {
                    Thread.Sleep(1000);
                }
                else
                {
                    // Let Thread 1 over take Thread 2
                    Thread.Sleep(3000);
                }

                return lazy.Value;
            });
            services.AddSingleton<Thing2>();

            sp = services.BuildServiceProvider();

            var t1 = Task.Run(() =>
            {
                ThreadId = 1;
                using var scope1 = sp.CreateScope();
                scope1.ServiceProvider.GetRequiredService<Thing1>();
            });

            var t2 = Task.Run(() =>
            {
                ThreadId = 2;
                using var scope2 = sp.CreateScope();
                scope2.ServiceProvider.GetRequiredService<Thing2>();
            });

            await t1;
            await t2;
        }

        public class Thing2
        {
            public Thing2(Thing1 thing1)
            {

            }
        }

        public class Thing1
        {
            public Thing1(Thing0 thing2)
            {
            }
        }

        public class Thing0
        {
            public Thing0()
            {
            }
        }
    }
}

@maryamariyan
Copy link
Member

maryamariyan commented Aug 18, 2020

@meareindel are you still getting the repro on your machine? could you then perhaps check if adding

app.ApplicationServices.GetRequiredService<HttpMessageHandlerBuilder>();

to the beginning of Configure call in Startup class perhaps allows for avoiding this deadlock?

@maryamariyan
Copy link
Member

Moving the fix to 6.0, this is not a regression and I am assuming the above workaround in #35986 (comment) should enable us to avoid the deadlock explained in this issue.

@maryamariyan maryamariyan modified the milestones: 5.0.0, 6.0.0 Aug 20, 2020
@halter73
Copy link
Member

Note for our future selves:

@davidfowl and I talked a bit about this offline, and we were thinking a per-service lock instead of a global (technically per-scope) lock is likely the solution to this problem.

@meareindel
Copy link
Author

@meareindel are you still getting the repro on your machine? could you then perhaps check if adding

app.ApplicationServices.GetRequiredService<HttpMessageHandlerBuilder>();

to the beginning of Configure call in Startup class perhaps allows for avoiding this deadlock?

I've just moved from Singleton to Scoped lifetime to avoid, considering that there was no need in singleton lifetime at all.

I could try the workaround above on original test example for clarity, but could you please elaborate how would it work? As far as I see, the DefaultHttpMessageHandlerBuilder got registered as transient in AddHttpClient extension of IHttpClientFactory and has no explicit singleton dependencies which could then be resolved and cached.

@ghost ghost added the in-pr There is an active PR which will close this issue when it is merged label Dec 16, 2020
@ghost ghost removed the in-pr There is an active PR which will close this issue when it is merged label Jan 21, 2021
@ghost ghost locked as resolved and limited conversation to collaborators Feb 20, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.