-
Notifications
You must be signed in to change notification settings - Fork 5.3k
Unload MsQuic after checking for QUIC support to free resources. #74749
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
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
- Loading branch information
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -1,6 +1,7 @@ | ||||||
| // Licensed to the .NET Foundation under one or more agreements. | ||||||
| // The .NET Foundation licenses this file to you under the MIT license. | ||||||
|
|
||||||
| using System.Diagnostics; | ||||||
| using System.Diagnostics.CodeAnalysis; | ||||||
| using System.Runtime.InteropServices; | ||||||
| using Microsoft.Quic; | ||||||
|
|
@@ -47,7 +48,8 @@ private MsQuicApi(QUIC_API_TABLE* apiTable) | |||||
| } | ||||||
| } | ||||||
|
|
||||||
| internal static MsQuicApi Api { get; } = null!; | ||||||
| internal static Lazy<MsQuicApi> _api = new Lazy<MsQuicApi>(AllocateMsQuicApi); | ||||||
| internal static MsQuicApi Api => _api.Value; | ||||||
|
|
||||||
| internal static bool IsQuicSupported { get; } | ||||||
|
|
||||||
|
|
@@ -58,29 +60,21 @@ private MsQuicApi(QUIC_API_TABLE* apiTable) | |||||
|
|
||||||
| static MsQuicApi() | ||||||
| { | ||||||
| IntPtr msQuicHandle; | ||||||
| if (!NativeLibrary.TryLoad($"{Interop.Libraries.MsQuic}.{MsQuicVersion.Major}", typeof(MsQuicApi).Assembly, DllImportSearchPath.AssemblyDirectory, out msQuicHandle) && | ||||||
| !NativeLibrary.TryLoad(Interop.Libraries.MsQuic, typeof(MsQuicApi).Assembly, DllImportSearchPath.AssemblyDirectory, out msQuicHandle)) | ||||||
| if (!TryLoadMsQuic(out IntPtr msQuicHandle)) | ||||||
| { | ||||||
| return; | ||||||
| } | ||||||
|
|
||||||
| try | ||||||
| { | ||||||
| if (!NativeLibrary.TryGetExport(msQuicHandle, "MsQuicOpenVersion", out IntPtr msQuicOpenVersionAddress)) | ||||||
| { | ||||||
| return; | ||||||
| } | ||||||
|
|
||||||
| QUIC_API_TABLE* apiTable = null; | ||||||
| delegate* unmanaged[Cdecl]<uint, QUIC_API_TABLE**, int> msQuicOpenVersion = (delegate* unmanaged[Cdecl]<uint, QUIC_API_TABLE**, int>)msQuicOpenVersionAddress; | ||||||
| if (StatusFailed(msQuicOpenVersion((uint)MsQuicVersion.Major, &apiTable))) | ||||||
| if (!TryOpenMsQuic(msQuicHandle, out QUIC_API_TABLE* apiTable)) | ||||||
| { | ||||||
| return; | ||||||
| } | ||||||
|
|
||||||
| try | ||||||
| { | ||||||
| // check version | ||||||
|
||||||
| // check version | |
| // Check version |
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should those be private?
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We could alternatively use [LibraryImport] for MsQuicOpenVersion and MsQuicClose, it might clean up the code a bit at the cost of not being able to unload the lib from the process I think (if we care about that)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
They already have imports in:
runtime/src/libraries/System.Net.Quic/src/System/Net/Quic/Interop/msquic_generated.cs
Line 2593 in bf47ac0
| internal static extern int MsQuicOpenVersion([NativeTypeName("uint32_t")] uint Version, [NativeTypeName("const void **")] void** QuicApi); |
I'm not an expert on this, but we control which version of libmsquic we're loading in TryLoadMsQuic and we do an attempt to load it that would not fail if the lib is not present. I'm not sure we can achieve the same with the import. Relevant history: #49973
Anyway, we might at least use nameof(MsQuicOpenVersion) from the generated interop. Though we lived with the string until now, so it's certainly not critical.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
private readonly?