-
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
Revert "Unload MsQuic after checking for QUIC support to free resources. (#74749)" #74984
Conversation
…es. (dotnet#74749)" This reverts commit 7a45201.
Tagging subscribers to this area: @dotnet/ncl Issue DetailsThis reverts commit 7a45201.
|
It turns out the test failure may be something environmental, because it is caused by MsQuic returning QUIC_STATUS_OUT_OF_MEMORY, so I might close this PR if we decide to mitigate this in other way (like temporarily disabling quic test runs on affected docker images) |
Have you ever seen It can be very well a real problem with the change where loading/unloading/loading_again native library or starting/shutting_down/starting_again a lot of threads is hitting some pathological behaviors. |
No. I find it quite weird, doing the unload/load dance immediately works well, but when we load+unload as part of QUIC support checking and re-load and reopen the library again later, it failed deterministically on the machine I was testing it. I will try to see where the status comes from in MsQuic, do you perhaps have some other ideas how to investigate further? |
If you have a machine where it failed deterministically, it would be the place to start. My guess is that it will be a bug in msquic library unloading. Library unloading is typically buggy and it does not work for most libraries out there. We may want to keep msquic loaded once it gets loaded (ie do not ever call NativeLibrary.Free on it). The resource hit from keeping the library loaded should be fairly small. You can still close the Apis to shutdown the msquic threadpool. |
…et#75163) * Revert "Revert "Unload MsQuic after checking for QUIC support to free resources. (dotnet#74749)" (dotnet#74984)" This reverts commit 953f524. * update helix images * update helix images * Improve diagnostics when opening MsQuic Co-authored-by: Radek Zikmund <radekzikmund@microsoft.com>
…et#75163) * Revert "Revert "Unload MsQuic after checking for QUIC support to free resources. (dotnet#74749)" (dotnet#74984)" This reverts commit 953f524. * update helix images * update helix images * Improve diagnostics when opening MsQuic Co-authored-by: Radek Zikmund <radekzikmund@microsoft.com>
…sources (#75163, #75441) (#75521) * Unload MsQuic after checking for QUIC support to free resources (#75163) * Revert "Revert "Unload MsQuic after checking for QUIC support to free resources. (#74749)" (#74984)" This reverts commit 953f524. * update helix images * update helix images * Improve diagnostics when opening MsQuic Co-authored-by: Radek Zikmund <radekzikmund@microsoft.com> * Don't unload MsQuic from the process (#75441) * Revert helix queues change (to be done in another PR) * Code review feedback
Contributes to #74952.
This reverts commit 7a45201 to unblock CI.
Reopens #74629