Port exhaustion issue while using ADAL 5.2.0.

[sc22github]

Which Version of ADAL are you using ?
5.2.0

Which platform has the issue?
net462

What authentication flow has the issue?

• Desktop / Mobile
  • Interactive
  • Integrated Windows Auth
  • Username Password
  • Device code flow (browserless)
• Web App
  • Authorization code
  • OBO
• Web API
  • OBO

Is this a new or existing app?

The app is in production, and I have upgraded to ADAL 5.2.0, from 4.5.1.

Repro

var authenticationContext = new AuthenticationContext(tenantAuthorityUrl, true, tokenCache);
var oboToken = await authenticationContext.AcquireTokenSilentAsync(resource, clientAssertionCert, userIdentifier);

Expected behavior
Ports used on the cloud service executing this code should not keep increasing.

Actual behavior
Ports used on cloud service keep increasing, which eventually leads to port exhaustion resulting in failure to make outbound calls.

Possible Solution
In both 4.5.1 and 5.2.0, the default behavior is to construct new HttpClient for every token request, which is not efficient. I think what regressed in 5.2.0 is that earlier new HttpClient was being created in a "using" block(HttpClientWrapper.cs), which will take care of disposing the HttpClient, whereas in 5.2.0, the HttpClient is not being created with a "using" block, neither is it being explicitly disposed (it's in HttpManager.cs).

Additional context/ Logs / Screenshots
We verified that this upgrade only is causing port exhaustion by looking at Azure load balancer metrics for ports consumption.

[bgavrilMS]

Will look at it ASAP. Using the IHttpClientFactory and providing a single HttpClient will probably workaround this.

[henrik-me]

Bogdan has additional details on this. It is a regression.

[henrik-me]

@bgavrilMS , can you pls. ensure we have an MSAL bug tracking this?

[bgavrilMS]

Root cause

So what's happening is we create a new HttpClient with each AuthenticationContext. And in web app \ web api scenarios, we are creating one of these AuthenticationContext objects for each user session.

What was happening in ADAL 4.x?

We'd create an HttpClient for each request, and dispose of it. Creating a new HttpClient per request introduced perf issues, but not this particular issue.

The guidance on MSDN on HttpClient

HttpClient is intended to be instantiated once and reused throughout the life of an application. Instantiating an HttpClient class for every request will exhaust the number of sockets available under heavy loads.
But there’s a second issue with HttpClient that you can have when you use it as singleton or static object. In this case, a singleton or static HttpClient doesn't respect DNS changes, as explained in this issue at the dotnet/corefx GitHub repository.

General .NET solution

• on net45, see Oren's solution https://github.com/dotnet/corefx/issues/11224
• on netcore, use ASP's IHttpClientFactory (this manages HttpClients without the need to dispose of them)

What to do in ADAL?

1. No code change. Document this and expect web apps to inject ASP.IHttpClientFactory via the extensible point for NET Core apps. And document how to create a scalable HttpClientFactory on net45.
2. Same as #1, but on net45 create a scalable HttpClientFactory (this is a bit of work, because we'll need to set a timeout for each endpoint ADAL contacts)

What to do in MSAL?

1. No code change + documentation for .NET Classic. But create an adapter in Microsoft.Identity.Web that from ASP.IHttpClientFactory and MSAL.IHttpClientFactory (I know, bad naming...)

It's going to be harder to implement something like solution ADAL solution 2 in MSAL, because there are so many ways of specifying the authority.

@jmprieur @henrik-me - what do you think?

[henrik-me]

I believe this is what the HttpClientFactory is meant to help with. However not really sure what the best guidance is on creating a scalable/performing HttpClientFactory.