Repository navigation
Web Cmdlet Socket Exhaustion Concern #9977
Description
This issue was originally opened in the https://github.com/Azure/azure-functions-powershell-worker repo by Mark Kraus (@markekraus).
One thing we do not handle well in the web cmdlets in PS Core is the singleton nature of HttpClient. HttpClient is intended to be instantiated once and then reused. In PS Core we instantiate a new HttpClient on every call to IWR/IRM. This is somewhat OK for a shell and scripting language, but could be problematic for Functions. A common pattern is to create a azure Function to combine data from multiple endpoints as a single data set. In a high volume environment, the repeated calls to web cmdlets could result in socket exhaustion. More Info
For conversation details please see Azure/azure-functions-powershell-worker#174
Activity
vexx32 commentedon Jun 21, 2019
From what I've read on the subject, the fix could more or less be as simple as maintaining a static
HttpClientinstance that is reused for all web cmdlet calls to the API. If thread safety is a concern, locks or possibly [ThreadStatic] members may be needed as well.Mark may have other ideas as well. 🙂
markekraus commentedon Jun 21, 2019
Unfortunately, it's not that simple. lets say the following 2 commands run in different runspaces:
Because both of those make a modification to the
HttpClientHandler.ServerCertificateCustomValidationCallback(which would need to be static between threads to reduce the risk of port exhaustion) it is possible that theHttpClientHandler.DangerousAcceptAnyServerCertificateValidatorcould be applied when the call tohttps://sensitiveexternalsite.comis made.Basically, there are a few options. 1 is to create an
HttpClientandHttpClientHandlerpair per thread. Another is to implement our ownHttpMessageHandlerthat includes the logic for handling our use cases (this is a pretty massive undertaking). Another is to create some kind of smart logic to create a newHttpClientonly when needed (not as easy as it sounds).vexx32 commentedon Jun 22, 2019
.NET Core uses an implementation of a StringBuilderCache that is similar to what it sounds like you're suggesting; we could possibly take some pointers from their implementation of that and go from there....
JustinGrote commentedon Jun 22, 2019
Since there is a fairly limited amount of HttpClientHandler configurations, and it's unlikely a user is going to instantiate multiple different clienthandler configurations per session (maybe 2-3 to accomodate different host requirements), as discussed on another thread it's probably best to instantiate one httpclient per httpclienthandler configuration, which will drastically reduce the number of httpclients instantiated and bring the socket count down, especially for multiple concurrent requests to the same host using the same httpclienthandler configuration.
iSazonov commentedon Jun 26, 2019
I see that Dongbo Wang (@daxian-dbw) suggest per runspace caching Azure/azure-functions-powershell-worker#174 (comment)
It is easy to check that it improves performance without feature regression.
JustinGrote commentedon Jun 26, 2019
The problem is that there still needs to allow multiple httpclienthandlers for different iwr parameter configurations, and it's a 1:1 relationship for httpclienthandler to httpclient.
Not sure what he means by the race conditions either, httpclient is thread safe
Edit: for these reasons I think a process wide singleton per httpclienthandler would be sufficient. Alternatively, allow users to "opt in" to this behavior with a parameter pair similar to -sessionvariable and - websession, but that is less intuitive.
iSazonov commentedon Jun 27, 2019
So we need cache like config -> httpclienthandler or config -> httpclienthandler -> httpclient
JustinGrote commentedon Jun 27, 2019
I'll defer to Mark Kraus (@markekraus) on the particular implementation, I'm not a c# wizard but I'd envision taking a hash of the config and using it as the key in a key value cache, the value being the httpclient that was constructed with the particular httpclienthandler, and instantiate a new one if there is a cache miss and save it there.
The cache would dispose with the process, since there's a fairly limited group of configurations especially in a single use case so a memory leak isn't a major concern.
iSazonov commentedon Jan 15, 2021
We could use a (global static) cache like Tokenizer.GetStringBuilder().
vexx32 commentedon Jan 15, 2021
Not a bad idea. It's probably OK to just have the one handler to start off, and if the user requests a different configuration, the method goes off to create the necessary handler, store it in the lookup, and then return it.
iSazonov commentedon Jan 20, 2021
For reference dotnet/runtime#44818 - what we could cache.
CarloToso commentedon Mar 15, 2023
Ilya (@iSazonov) Duplicate of #13109 and #6156 related to #12764 might be fixed by #19249
iSazonov commentedon Mar 16, 2023
The issue resolved if WebSession is used but not otherwise.But I doubt that it makes sense to cache the session in the shadow always because requests can be to different servers and with different parameters, and it can be too expensive to create a cache for each target server.
JustinGrote commentedon Mar 16, 2023
Don't all httpclients now just use socketshttphandler that coordinates for socket exhaustion now since .net core 2.1? If so then the default behavior shouldn't really be a worry anymore, unless there's special handling going on.
iSazonov commentedon Mar 16, 2023
Yes. And now (7.4 Preview2) we explicitly dispose HttpClient and HttpClientHandler/SocketsHttpHandler.
microsoft-github-policy-service commentedon Nov 16, 2023
This issue has not had any activity in 6 months, if this is a bug please try to reproduce on the latest version of PowerShell and reopen a new issue and reference this issue if this is still a blocker for you.
microsoft-github-policy-service commentedon Nov 16, 2023
This issue has not had any activity in 6 months, if this is a bug please try to reproduce on the latest version of PowerShell and reopen a new issue and reference this issue if this is still a blocker for you.
microsoft-github-policy-service commentedon Nov 16, 2023
This issue has not had any activity in 6 months, if this is a bug please try to reproduce on the latest version of PowerShell and reopen a new issue and reference this issue if this is still a blocker for you.
JustinGrote commentedon Nov 16, 2023
Francisco Gamino (@Francisco-Gamino) I would argue this can be closed as resolved then
microsoft-github-policy-service commentedon Nov 24, 2023
This issue has been marked as "No Activity" as there has been no activity for 6 months. It has been closed for housekeeping purposes.