[release/6.0] Close MsQuic after checking for QUIC support to free resources #80785
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
ghost
assigned 
ManickaP
force-pushed
the
release/6.0
branch
4 times, most recently
from
2adc359
to
a94c18f
Compare
rbhanda
added
Servicing-approved
halter73
approved these changes
Jan 31, 2023
|
Approved by Tactics for 6.0.15. |
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Manual backport of #75521 in release/7.0 (which is in turn a backport of #75163 and #75441 in main)
Fixes #74629
Customer Impact
Memory (and number of threads) regression in Kestrel .NET 6.0 against .NET 5.0 for all applications (on Win11+/Win2022+ and Linux with OpenSsl 1.1):
Even if app is not using HTTP/3 (or QUIC), Kestrel initializes native MsQuic.dll early on, which always allocates threads (2* number of logical cores). Thus, causes unnecessary resource increase even if the process does not end up using HTTP/3 at all (HTTP/3 is opt-in like HTTP/2).
Note: HttpClient does not have this problem on .NET 6.0, only Kestrel does. HttpClient regression was first in .NET 7.0 - which was addressed by PR #75521.
We had 1 customer with ~50-core machine who noticed the problem in a dump and were surprised to see so many extra threads they didn't know about / they didn't need. At minimum it will help avoid developer confusion in dump investigations in such cases.
The fix closes all MsQuic resources after their initialization.
Affected platforms include Windows 11, Windows Server 2022, many Linux platforms with msquic package installed.
Testing
Risk
Low, the fix consists of gracefully de-initializing MsQuic library in the process after checking QUIC support. It is backport of fix from 7.0 GA - conceptually the changes are doing the same, but the code is different between 6.0 and 7.0.
The library is re-initialized only when actually needed.