-
Notifications
You must be signed in to change notification settings - Fork 4.7k
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
[release/6.0] Close MsQuic after checking for QUIC support to free resources (#75521) #80623
Comments
Tagging subscribers to this area: @dotnet/ncl Issue DetailsDescriptionManual backport of #75521 (which is in turn a backport of #75163 and #75441) cc: @wfurt Customer ImpactMemory (and number of processes) regression in .NET 7.0 from .NET 6.0 for all applications (on Win11+/Win2022+ and Linux with OpenSsl 1.1): The fix closes all MsQuic resources after their initialization. Affected platforms include Windows 11, Windows Server 2022, many Linux platforms with msquic package installed. Note: The same problem exists also in 6.0, but hidden under opt-in switch into HTTP/3 support. We had 1 customer with ~50-core machine who noticed that 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. TestingFunctional tests suite passes as part of the CI, resource consumption was checked manually. RiskLow, the fix consists of gracefully de-initializing MsQuic library in the process after checking QUIC support. The library is re-initialized only when actually needed. Reproduction StepsExpected behaviorActual behaviorRegression?No response Known WorkaroundsNo response ConfigurationNo response Other informationNo response
|
Did you want to open a Pull Request? 😅 |
It can be backported, but the question is how much impact will it make. Since the additional threads are allocated only when the user opts in for HTTP3 support which implies he intends to use it. So fixing this would improve only the situation when the user enables HTTP3, but does not use it, which seems kind of niche. BTW. Users should not be using HTTP3 in production in .NET 6 anyway, because it implements only draft versions of HTTP3 and QUIC. At the very least, talking to 3rd party servers is unlikely to work since most public implementations don't support draft versions anymore. |
Both Quic & H3 are preview and pretty rough in 6.0 IMHO. We should encourage users to migrate... |
@rzikm The problem is that Kestrel is always checks whether QUIC is supported in release/6.0 whether or not HTTP/3 is actually used. The theory behind why the suppression was okay is that Unfortunately, due to layering, it's not easy to prevent Kestrel from checking whether quic is supported when HTTP/3 is disabled. That's because HTTP/3 could be enabled after all the services have been added, and Kestrel uses the presence of the Fundamentally, even if it would be best if Kestrel could avoid checking it more, Kestrel will always need to check IsSupported sometimes, and this shouldn't spawn threads that never get closed. |
I see, then backporting makes sense. |
Duplicate of #74629 |
Description
Manual backport of #75521 (which is in turn a backport of #75163 and #75441)
Fixes #74629
Replaces #75330
cc: @wfurt
Customer Impact
Memory (and number of processes) regression in .NET 7.0 from .NET 6.0 for all applications (on Win11+/Win2022+ and Linux with OpenSsl 1.1):
Even if app is not using HTTP/3 (or QUIC), we initialize early on native MsQuic.dll 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).
The fix closes all MsQuic resources after their initialization.
The change builds on fix in msquic 2.1.1 (Helix images were updated by PR #75479).
Affected platforms include Windows 11, Windows Server 2022, many Linux platforms with msquic package installed.
Note: The same problem exists also in 6.0, but hidden under opt-in switch into HTTP/3 support. We had 1 customer with ~50-core machine who noticed that 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.
Testing
Functional tests suite passes as part of the CI, resource consumption was checked manually.
Risk
Low, the fix consists of gracefully de-initializing MsQuic library in the process after checking QUIC support. The library is re-initialized only when actually needed.
The text was updated successfully, but these errors were encountered: