HTTP Client socket leak with default configuration manager

[bunceg]

I think this is a leak issue with the default setup of AddJwtBearer.

We have setup our options with a MetadataAddress but not a configuration manager. Under load we get socket errors when trying to get the openid-configuration. Tracing the default code through, we get to HttpDocumentRetriever - this uses it's own HTTP Client (admittedly via static construction so it looks like it should only use one instance) but it's a suspicious candidate for our leak.

When we change the AddJwTBearer to use the default OpenIdConnectConfiguration and HttpDocumentRetrievver but with our own static instance of http client we don't get any socket exceptions.

Therefore it looks like HttpDocumentRetriever has a leak, but I can't really work out why from the code.

[Tratcher]

That HttpClient is created once at startup. https://github.com/aspnet/AspNetCore/blob/436b5461ad0337aac90089aa7ccb61dd875808e4/src/Security/src/Microsoft.AspNetCore.Authentication.JwtBearer/JwtBearerPostConfigureOptions.cs#L53

Are you doing anything interesting with AddJwtBearer? Do you have multiple? Do you add and remove them? Does BackchannelHttpHandler also mitigate your issue?

[bunceg]

@Tratcher yes I know it is - that's why I'm confused as to why injecting our own would make any difference. We're not doing anything special as far as I know. One thing that confused me a bit is that function within AddJwtBearer options seemed to be called on each api call - I wouldn't expect that, but I will double check if that is actually the case

Below is our original code

services.AddAuthentication(JwtBearerDefaults.AuthenticationScheme)
                .AddJwtBearer(options =>
                {
                    options.TokenValidationParameters = new TokenValidationParameters();
                    options.TokenValidationParameters.ValidateIssuerSigningKey = true;
                    options.TokenValidationParameters.ValidateLifetime = true;
                    options.IncludeErrorDetails = true;
                    options.RequireHttpsMetadata = authorities.All(a => a.StartsWith("https"));
                    options.TokenValidationParameters.ValidateIssuer = true;
                    options.TokenValidationParameters.ValidIssuers = authorities;
                    if (additionalAudiences != null)
                        options.TokenValidationParameters.ValidAudiences = AudienceNames.KnownAudiences.Concat(additionalAudiences);
                    else
                        options.TokenValidationParameters.ValidAudiences = AudienceNames.KnownAudiences;
                    options.MetadataAddress = $"{authorities.First()}/.well-known/openid-configuration/";
                    options.Events = new JwtBearerEvents
                    {
                        OnTokenValidated = context =>
                        {
                            var accessToken = context.SecurityToken as JwtSecurityToken;
                            if (accessToken != null)
                            {
                                ClaimsIdentity identity = context.Principal.Identity as ClaimsIdentity;
                                if (identity != null)
                                    identity.AddClaim(new Claim(CustomClaimNames.MSSPassthroughAccessToken, accessToken.RawData));
                            }

                            return Task.CompletedTask;

                        },
                        OnAuthenticationFailed = context =>
                        {
                            return Task.CompletedTask;
                        }

                    };
                });

and its setup in ConfigureServices via this:

services .AddMSSAuthentication(config.GetConnectionString("authentication.service.endpoint"))

[bunceg]

@Tratcher Any comments on this? I can confirm that the Action within the JwtBearerOptions is called on every request. I'm not sure why this is the case. Also, I can't find any examples that use BackChannelHttpHandler but since using our own HTTP Client completely fixes the issue (stress testing fails at 2 req/sec using the code above to working with 40 req/sec with the code below) I'm at a loss to understand if it's something we're doing wrong or if there is an issue within the authentication stack.

This code works fine:

options.ConfigurationManager = new ConfigurationManager<OpenIdConnectConfiguration>(
        $"{authorities.First()}/.well-known/openid-configuration/",
    new OpenIdConnectConfigurationRetriever(),
    new HttpDocumentRetriever(hc));

//options.MetadataAddress = $"{authorities.First()}/.well-known/openid-configuration/";

[Tratcher]

The AddJwtBearer config lambda shouldn't be running every request unless your app is somehow restarting or you have something very wrong with DI.

OnTokenValidated should run on every request that has a token.

[bunceg]

@Tratcher ok thanks for the pointers. I think we have something very wrong with DI - can you elaborate on that in any way? When I use the in-built DI everything works as expected, when I use the Unity DI (latest version 2.1.3) it exhibits the above behavior. I've no idea why Unity would cause this and I can't raise a bug report with them until I understand what is going on.

How does DI impact the AddJwtBearer extension method?

[Tratcher]

AddJwtBearer depends on several singleton DI services. If those aren't being treated as singletons by your container then your config will keep getting re-run and cause the issues you've seen.

It may just be a usage issue rather than a bug in container itself. How do you have it wired up? There are examples for using most containers with AspNetCore. E.g. https://docs.microsoft.com/en-us/aspnet/core/migration/proper-to-2x/mvc2?view=aspnetcore-2.2#native-dependency-injection

[bunceg]

@Tratcher I'm using the package Unity.Microsoft.DependencyInjection described here https://github.com/unitycontainer/examples/tree/master/src/AspNetCoreExample that's meant to implement all the plumbing in the example you provide,

[bunceg]

@Tratcher I've had a look at the code for AddJwtBearer... the first singleton registration I can see is for IPostConfigureOptions.... unfortunately when I look at the known registrations within Unity on completion of starttup I can see this correctly registered as a Singleton... because this works, I'm assuming the rest are working too.

There is an instance of JwtBearerOptions is marked as a Transient though... not sure if this is appropriate or not or where this comes from.

Any other ideas?

[Tratcher]

@Haok do you have a good explanation for how a bad DI setup would cause auth options to get reset per request?

[HaoK]

So generally auth uses IOptionsMonitor<TOptions> which is usually a singleton which uses a IOptionsFactory<TOptions> and IOptionsMonitorCache<TOptions>.

There's not too much going on, basically the monitor returns whatever is in the cache, if its a miss, it uses the factory to create a new instance.

https://github.com/aspnet/Extensions/blob/master/src/Options/Options/src/OptionsMonitor.cs

Where does the transient instance of jwtbearer come from? That's most likely the issue

[ENikS]

@Tratcher @HaoK
If jwtbearer is configured as a factory, it is always transient.

[brentschmaltz]

@Tratcher @HaoK does @ENikS response give you any insight?

[ENikS]

When reproducing this issue I see no Unity code involved at the moment:

>   UnityInjection.Api.dll!UnityInjection.Api.Startup.ConfigureServices.AnonymousMethod__7_2(Microsoft.AspNetCore.Authentication.JwtBearer.JwtBearerOptions options) Line 58    C#
    Microsoft.Extensions.Options.dll!Microsoft.Extensions.Options.ConfigureNamedOptions<System.__Canon>.Configure(string name, System.__Canon options) Line 52  C#
    Microsoft.Extensions.Options.dll!Microsoft.Extensions.Options.OptionsFactory<Microsoft.AspNetCore.Authentication.JwtBearer.JwtBearerOptions>.Create(string name) Line 35    C#
    Microsoft.Extensions.Options.dll!Microsoft.Extensions.Options.OptionsMonitor<Microsoft.AspNetCore.Authentication.JwtBearer.JwtBearerOptions>.Get.AnonymousMethod__0() Line 64   C#
    System.Private.CoreLib.dll!System.Lazy<Microsoft.AspNetCore.Authentication.JwtBearer.JwtBearerOptions>.ViaFactory(System.Threading.LazyThreadSafetyMode mode)   Unknown
    System.Private.CoreLib.dll!System.Lazy<System.__Canon>.ExecutionAndPublication(System.LazyHelper executionAndPublication, bool useDefaultConstructor)   Unknown
    System.Private.CoreLib.dll!System.Lazy<Microsoft.AspNetCore.Authentication.JwtBearer.JwtBearerOptions>.CreateValue()    Unknown
    Microsoft.Extensions.Options.dll!Microsoft.Extensions.Options.OptionsCache<Microsoft.AspNetCore.Authentication.JwtBearer.JwtBearerOptions>.GetOrAdd(string name, System.Func<Microsoft.AspNetCore.Authentication.JwtBearer.JwtBearerOptions> createOptions) Line 35 C#
    Microsoft.Extensions.Options.dll!Microsoft.Extensions.Options.OptionsMonitor<Microsoft.AspNetCore.Authentication.JwtBearer.JwtBearerOptions>.Get(string name) Line 64   C#
    Microsoft.AspNetCore.Authentication.dll!Microsoft.AspNetCore.Authentication.AuthenticationHandler<Microsoft.AspNetCore.Authentication.JwtBearer.JwtBearerOptions>.InitializeAsync(Microsoft.AspNetCore.Authentication.AuthenticationScheme scheme, Microsoft.AspNetCore.Http.HttpContext context)   Unknown
    System.Private.CoreLib.dll!System.Runtime.CompilerServices.AsyncMethodBuilderCore.Start<Microsoft.AspNetCore.Authentication.AuthenticationHandler<Microsoft.AspNetCore.Authentication.JwtBearer.JwtBearerOptions>.<InitializeAsync>d__42>(ref Microsoft.AspNetCore.Authentication.AuthenticationHandler<Microsoft.AspNetCore.Authentication.JwtBearer.JwtBearerOptions>.<InitializeAsync>d__42 stateMachine)    Unknown
    Microsoft.AspNetCore.Authentication.dll!Microsoft.AspNetCore.Authentication.AuthenticationHandler<Microsoft.AspNetCore.Authentication.JwtBearer.JwtBearerOptions>.InitializeAsync(Microsoft.AspNetCore.Authentication.AuthenticationScheme scheme, Microsoft.AspNetCore.Http.HttpContext context)   Unknown
    Microsoft.AspNetCore.Authentication.Core.dll!Microsoft.AspNetCore.Authentication.AuthenticationHandlerProvider.GetHandlerAsync(Microsoft.AspNetCore.Http.HttpContext context, string authenticationScheme)  Unknown
    System.Private.CoreLib.dll!System.Runtime.CompilerServices.AsyncMethodBuilderCore.Start<Microsoft.AspNetCore.Authentication.AuthenticationHandlerProvider.<GetHandlerAsync>d__5>(ref Microsoft.AspNetCore.Authentication.AuthenticationHandlerProvider.<GetHandlerAsync>d__5 stateMachine)  Unknown
    Microsoft.AspNetCore.Authentication.Core.dll!Microsoft.AspNetCore.Authentication.AuthenticationHandlerProvider.GetHandlerAsync(Microsoft.AspNetCore.Http.HttpContext context, string authenticationScheme)  Unknown
    Microsoft.AspNetCore.Authentication.Core.dll!Microsoft.AspNetCore.Authentication.AuthenticationService.AuthenticateAsync(Microsoft.AspNetCore.Http.HttpContext context, string scheme)  Unknown
    System.Private.CoreLib.dll!System.Runtime.CompilerServices.AsyncMethodBuilderCore.Start<Microsoft.AspNetCore.Authentication.AuthenticationService.<AuthenticateAsync>d__10>(ref Microsoft.AspNetCore.Authentication.AuthenticationService.<AuthenticateAsync>d__10 stateMachine)    Unknown
    Microsoft.AspNetCore.Authentication.Core.dll!Microsoft.AspNetCore.Authentication.AuthenticationService.AuthenticateAsync(Microsoft.AspNetCore.Http.HttpContext context, string scheme)  Unknown
    Microsoft.AspNetCore.Authentication.Abstractions.dll!Microsoft.AspNetCore.Authentication.AuthenticationHttpContextExtensions.AuthenticateAsync(Microsoft.AspNetCore.Http.HttpContext context, string scheme)    Unknown
    Microsoft.AspNetCore.Authentication.dll!Microsoft.AspNetCore.Authentication.AuthenticationMiddleware.Invoke(Microsoft.AspNetCore.Http.HttpContext context)  Unknown
    System.Private.CoreLib.dll!System.Runtime.CompilerServices.AsyncMethodBuilderCore.Start<Microsoft.AspNetCore.Authentication.AuthenticationMiddleware.<Invoke>d__6>(ref Microsoft.AspNetCore.Authentication.AuthenticationMiddleware.<Invoke>d__6 stateMachine)  Unknown
    Microsoft.AspNetCore.Authentication.dll!Microsoft.AspNetCore.Authentication.AuthenticationMiddleware.Invoke(Microsoft.AspNetCore.Http.HttpContext context)  Unknown
    Microsoft.AspNetCore.Diagnostics.dll!Microsoft.AspNetCore.Diagnostics.DeveloperExceptionPageMiddleware.Invoke(Microsoft.AspNetCore.Http.HttpContext context)    Unknown
    System.Private.CoreLib.dll!System.Runtime.CompilerServices.AsyncMethodBuilderCore.Start<Microsoft.AspNetCore.Diagnostics.DeveloperExceptionPageMiddleware.<Invoke>d__7>(ref Microsoft.AspNetCore.Diagnostics.DeveloperExceptionPageMiddleware.<Invoke>d__7 stateMachine)    Unknown
    Microsoft.AspNetCore.Diagnostics.dll!Microsoft.AspNetCore.Diagnostics.DeveloperExceptionPageMiddleware.Invoke(Microsoft.AspNetCore.Http.HttpContext context)    Unknown
    Microsoft.AspNetCore.Server.IISIntegration.dll!Microsoft.AspNetCore.Server.IISIntegration.IISMiddleware.Invoke(Microsoft.AspNetCore.Http.HttpContext httpContext)   Unknown
    System.Private.CoreLib.dll!System.Runtime.CompilerServices.AsyncMethodBuilderCore.Start<Microsoft.AspNetCore.Server.IISIntegration.IISMiddleware.<Invoke>d__13>(ref Microsoft.AspNetCore.Server.IISIntegration.IISMiddleware.<Invoke>d__13 stateMachine)    Unknown
    Microsoft.AspNetCore.Server.IISIntegration.dll!Microsoft.AspNetCore.Server.IISIntegration.IISMiddleware.Invoke(Microsoft.AspNetCore.Http.HttpContext httpContext)   Unknown
    Microsoft.AspNetCore.HttpOverrides.dll!Microsoft.AspNetCore.HttpOverrides.ForwardedHeadersMiddleware.Invoke(Microsoft.AspNetCore.Http.HttpContext context)  Unknown
    Microsoft.AspNetCore.HostFiltering.dll!Microsoft.AspNetCore.HostFiltering.HostFilteringMiddleware.Invoke(Microsoft.AspNetCore.Http.HttpContext context) Unknown
    Microsoft.AspNetCore.Hosting.dll!Microsoft.AspNetCore.Hosting.Internal.RequestServicesContainerMiddleware.Invoke(Microsoft.AspNetCore.Http.HttpContext httpContext) Unknown
    Microsoft.AspNetCore.Hosting.dll!Microsoft.AspNetCore.Hosting.Internal.HostingApplication.ProcessRequestAsync(Microsoft.AspNetCore.Hosting.Internal.HostingApplication.Context context) Unknown
    Microsoft.AspNetCore.Server.Kestrel.Core.dll!Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http.HttpProtocol.ProcessRequests<Microsoft.AspNetCore.Hosting.Internal.HostingApplication.Context>(Microsoft.AspNetCore.Hosting.Server.IHttpApplication<Microsoft.AspNetCore.Hosting.Internal.HostingApplication.Context> application)  Unknown
    System.Private.CoreLib.dll!System.Threading.ExecutionContext.RunInternal(System.Threading.ExecutionContext executionContext, System.Threading.ContextCallback callback, object state)   Unknown
    System.Private.CoreLib.dll!System.Runtime.CompilerServices.AsyncTaskMethodBuilder<System.Threading.Tasks.VoidTaskResult>.AsyncStateMachineBox<Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http.HttpProtocol.<ProcessRequests>d__188<Microsoft.AspNetCore.Hosting.Internal.HostingApplication.Context>>.MoveNext() Unknown
    System.Private.CoreLib.dll!System.Threading.ExecutionContext.RunInternal(System.Threading.ExecutionContext executionContext, System.Threading.ContextCallback callback, object state)   Unknown
    System.Private.CoreLib.dll!System.Threading.ThreadPoolWorkQueue.Dispatch()  Unknown

So, the issue is with how it is created. If you could point me to that part in your code I could verify how is it different from native DI.

[Tratcher]

@HaoK it looks like an issue with OptionsMonitor failing to cache, no?

[ENikS]

Between refresh request start and that line in the code few types are resolved from container.

Types Unity resolves:

Microsoft.AspNetCore.Authentication.IAuthenticationHandlerProvider
Microsoft.AspNetCore.Authentication.IAuthenticationService
Microsoft.AspNetCore.Authentication.JwtBearer.JwtBearerHandler

On the same page refresh these are the types native DI resolves:

Microsoft.AspNetCore.Authentication.IAuthenticationHandlerProvider
Microsoft.AspNetCore.Authentication.IAuthenticationService
Microsoft.AspNetCore.Mvc.Infrastructure.IActionResultExecutor`1[[Microsoft.AspNetCore.Mvc.ObjectResult]]

[Tratcher]

Which of those are created fresh each resolve, and which return cached singletons?

[ENikS]

It should not matter.

If these requests are consistently different with native DI it means execution path is different as well. So, either Unity does not comply with registration convention or native DI uses some kind of magic trick to create something. If someone could help me to find where the execution path diverges we could see the issue...

But to answer your question, neither one of these is singleton in Unity.

[ENikS]

I just cloned entire Extensions Repo, built sample with project references instead of nuget packages and tried to reproduce the error:

I could not! I am assuming it was fixed in the later version and no longer a problem.

@bunceg please update to the latest and verify.

[grahambunce]

I will double check the later released versions tomorrow but hopefully it’s patched on the 2.1 LTS branch rather than the 2.2 branch....

[bunceg]

@Tratcher @ENikS I've used the 2.1.7 package and the latest 2.2.1 Microsoft.AspNetCore.App package (with associated .net SDK) and the latest version of unity (5.9.1)

None of these updates have fixed the problem.

Since @ENikS did his test from source, is there an outstanding change that hasn't been packaged up yet and, if so, will it be available as a 2.1.8 release (due to 2.1.x being under LTS) ?

[Tratcher]

If @ENikS built from source it was likely the master branch which is developing 3.0.0. You can try a nightly build at https://github.com/dotnet/core-sdk#installers-and-binaries

We can't talk about patching until we track down what the cause and solution were.

[ENikS]

It was master branch.

[bunceg]

@Tratcher I'm a bit stuck here. I'm updating my test app to the latest NuGet Microsoft.AspNetCore.App version 3.0.0-preview-18579-0056 and it's failing to update. I've set TargetFramework to netcoreapp3.0 and installed the .NET SDK from the link you gave me (.NET Core SDK 3.0.100-preview) as below.

VS (2017) won't build the solution with an error "the current .NET SDK does not support targeting .NET Core 3.0.... etc.

command line dotnet build fails with errors below.

So not sure where to go next to help you work out whether the issue is really fixed in 3.0 or not.

[ENikS]

You need Visual Studio 2019

[bunceg]

I see.... a veritable hornets nest of installing of preview versions then.... @Tratcher is this something I need to pursue independently of the analysis @ENikS and I have already done? it's not a Unity issue, it is an issue in .net core 2.1 / 2.2 but (apparently) not in 3.0.. so it's directly pointing at a bug in .net core itself. I am surprised that this hasn't come up before but I presume the majority of use cases is to use the in-built DI

[Tratcher]

If you want to pursue a patch then we still need to narrow down what the issue was and how it was fixed. That doesn't necessarily require installing the previews, that was more for confirmation of the fix.

I see two options: 1) further debugging 2.x to figure out the original issue, or 2) diffing 2.x and 3.0 code to figure out what changed. At the very least this discussion should move to https://github.com/aspnet/aspnetcore since the issue seams to be in that code base. Can you open a new issue there and summarize the findings so far?

[bunceg]

@Tratcher ok done. I do want a patch pursued at this is causing us load issues on our platform, and we are locked to the 2.1 LTS branch for now. The only alternative appears to be to use in-built DI, which isn't really fair on Unity as apparently is not their fault, and causes us problems as we have a lot of custom wrapper code around unity around decorators etc. that we'd need to re-write to use custom DI.

[Tratcher]

@brentschmaltz you can close this issue as a duplicate of https://github.com/aspnet/AspNetCore/issues/7030.

[brentschmaltz]

@Tratcher sure duplicate of https://github.com/aspnet/AspNetCore/issues/7030