From fc791d1c6d416544a94f7be4c7dd75275a25f7c7 Mon Sep 17 00:00:00 2001 From: Radek Zikmund Date: Mon, 12 Sep 2022 11:02:45 +0200 Subject: [PATCH 1/3] Don't unload MsQuic from the process --- .../src/System/Net/Quic/Internal/MsQuicApi.cs | 115 +++++++++--------- 1 file changed, 55 insertions(+), 60 deletions(-) diff --git a/src/libraries/System.Net.Quic/src/System/Net/Quic/Internal/MsQuicApi.cs b/src/libraries/System.Net.Quic/src/System/Net/Quic/Internal/MsQuicApi.cs index e3e4728610b14f..1655d3cac5934c 100644 --- a/src/libraries/System.Net.Quic/src/System/Net/Quic/Internal/MsQuicApi.cs +++ b/src/libraries/System.Net.Quic/src/System/Net/Quic/Internal/MsQuicApi.cs @@ -20,6 +20,8 @@ internal sealed unsafe partial class MsQuicApi private static readonly Version MsQuicVersion = new Version(2, 1); + private static readonly IntPtr MsQuicHandle = TryLoadMsQuic(); + public MsQuicSafeHandle Registration { get; } public QUIC_API_TABLE* ApiTable { get; } @@ -60,75 +62,68 @@ private MsQuicApi(QUIC_API_TABLE* apiTable) static MsQuicApi() { - if (!TryLoadMsQuic(out IntPtr msQuicHandle)) + if (MsQuicHandle == IntPtr.Zero) + { + // MsQuic library not loaded + return; + } + + if (!TryOpenMsQuic(out QUIC_API_TABLE* apiTable, out _)) { return; } try { - if (!TryOpenMsQuic(msQuicHandle, out QUIC_API_TABLE* apiTable, out _)) + // Check version + int arraySize = 4; + uint* libVersion = stackalloc uint[arraySize]; + uint size = (uint)arraySize * sizeof(uint); + if (StatusFailed(apiTable->GetParam(null, QUIC_PARAM_GLOBAL_LIBRARY_VERSION, &size, libVersion))) { return; } - try + var version = new Version((int)libVersion[0], (int)libVersion[1], (int)libVersion[2], (int)libVersion[3]); + if (version < MsQuicVersion) { - // Check version - int arraySize = 4; - uint* libVersion = stackalloc uint[arraySize]; - uint size = (uint)arraySize * sizeof(uint); - if (StatusFailed(apiTable->GetParam(null, QUIC_PARAM_GLOBAL_LIBRARY_VERSION, &size, libVersion))) + if (NetEventSource.Log.IsEnabled()) { - return; - } - - var version = new Version((int)libVersion[0], (int)libVersion[1], (int)libVersion[2], (int)libVersion[3]); - if (version < MsQuicVersion) - { - if (NetEventSource.Log.IsEnabled()) - { - NetEventSource.Info(null, $"Incompatible MsQuic library version '{version}', expecting '{MsQuicVersion}'"); - } - return; + NetEventSource.Info(null, $"Incompatible MsQuic library version '{version}', expecting '{MsQuicVersion}'"); } + return; + } - // Assume SChannel is being used on windows and query for the actual provider from the library if querying is supported - QUIC_TLS_PROVIDER provider = OperatingSystem.IsWindows() ? QUIC_TLS_PROVIDER.SCHANNEL : QUIC_TLS_PROVIDER.OPENSSL; - size = sizeof(QUIC_TLS_PROVIDER); - apiTable->GetParam(null, QUIC_PARAM_GLOBAL_TLS_PROVIDER, &size, &provider); - UsesSChannelBackend = provider == QUIC_TLS_PROVIDER.SCHANNEL; + // Assume SChannel is being used on windows and query for the actual provider from the library if querying is supported + QUIC_TLS_PROVIDER provider = OperatingSystem.IsWindows() ? QUIC_TLS_PROVIDER.SCHANNEL : QUIC_TLS_PROVIDER.OPENSSL; + size = sizeof(QUIC_TLS_PROVIDER); + apiTable->GetParam(null, QUIC_PARAM_GLOBAL_TLS_PROVIDER, &size, &provider); + UsesSChannelBackend = provider == QUIC_TLS_PROVIDER.SCHANNEL; - if (UsesSChannelBackend) + if (UsesSChannelBackend) + { + // Implies windows platform, check TLS1.3 availability + if (!IsWindowsVersionSupported()) { - // Implies windows platform, check TLS1.3 availability - if (!IsWindowsVersionSupported()) + if (NetEventSource.Log.IsEnabled()) { - if (NetEventSource.Log.IsEnabled()) - { - NetEventSource.Info(null, $"Current Windows version ({Environment.OSVersion}) is not supported by QUIC. Minimal supported version is {MinWindowsVersion}"); - } - - return; + NetEventSource.Info(null, $"Current Windows version ({Environment.OSVersion}) is not supported by QUIC. Minimal supported version is {MinWindowsVersion}"); } - Tls13ServerMayBeDisabled = IsTls13Disabled(isServer: true); - Tls13ClientMayBeDisabled = IsTls13Disabled(isServer: false); + return; } - IsQuicSupported = true; - } - finally - { - // Gracefully close the API table to free resources. The API table will be allocated lazily again if needed - bool closed = TryCloseMsQuic(msQuicHandle, apiTable); - Debug.Assert(closed, "Failed to close MsQuic"); + Tls13ServerMayBeDisabled = IsTls13Disabled(isServer: true); + Tls13ClientMayBeDisabled = IsTls13Disabled(isServer: false); } + + IsQuicSupported = true; } finally { - // Unload the library, we will load it again when we actually use QUIC - NativeLibrary.Free(msQuicHandle); + // Gracefully close the API table to free resources. The API table will be allocated lazily again if needed + bool closed = TryCloseMsQuic(apiTable); + Debug.Assert(closed, "Failed to close MsQuic"); } } @@ -136,28 +131,26 @@ private static MsQuicApi AllocateMsQuicApi() { Debug.Assert(IsQuicSupported); - int openStatus = MsQuic.QUIC_STATUS_INTERNAL_ERROR; - - if (TryLoadMsQuic(out IntPtr msQuicHandle) && - TryOpenMsQuic(msQuicHandle, out QUIC_API_TABLE* apiTable, out openStatus)) + if (!TryOpenMsQuic(out QUIC_API_TABLE* apiTable, out int openStatus)) { - return new MsQuicApi(apiTable); + throw ThrowHelper.GetExceptionForMsQuicStatus(openStatus); } - ThrowHelper.ThrowIfMsQuicError(openStatus); - - // this should unreachable as TryOpenMsQuic returns non-success status on failure - throw new Exception("Failed to create MsQuicApi instance"); + return new MsQuicApi(apiTable); } - private static bool TryLoadMsQuic(out IntPtr msQuicHandle) => - 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); + private static IntPtr TryLoadMsQuic() => + (NativeLibrary.TryLoad($"{Interop.Libraries.MsQuic}.{MsQuicVersion.Major}", typeof(MsQuicApi).Assembly, DllImportSearchPath.AssemblyDirectory, out IntPtr msQuicHandle) || + NativeLibrary.TryLoad(Interop.Libraries.MsQuic, typeof(MsQuicApi).Assembly, DllImportSearchPath.AssemblyDirectory, out msQuicHandle)) + ? msQuicHandle + : IntPtr.Zero; - private static bool TryOpenMsQuic(IntPtr msQuicHandle, out QUIC_API_TABLE* apiTable, out int openStatus) + private static bool TryOpenMsQuic(out QUIC_API_TABLE* apiTable, out int openStatus) { + Debug.Assert(MsQuicHandle != IntPtr.Zero); + apiTable = null; - if (!NativeLibrary.TryGetExport(msQuicHandle, "MsQuicOpenVersion", out IntPtr msQuicOpenVersionAddress)) + if (!NativeLibrary.TryGetExport(MsQuicHandle, "MsQuicOpenVersion", out IntPtr msQuicOpenVersionAddress)) { if (NetEventSource.Log.IsEnabled()) { @@ -185,9 +178,11 @@ private static bool TryOpenMsQuic(IntPtr msQuicHandle, out QUIC_API_TABLE* apiTa return true; } - private static bool TryCloseMsQuic(IntPtr msQuicHandle, QUIC_API_TABLE* apiTable) + private static bool TryCloseMsQuic(QUIC_API_TABLE* apiTable) { - if (NativeLibrary.TryGetExport(msQuicHandle, "MsQuicClose", out IntPtr msQuicClose)) + Debug.Assert(MsQuicHandle != IntPtr.Zero); + + if (NativeLibrary.TryGetExport(MsQuicHandle, "MsQuicClose", out IntPtr msQuicClose)) { ((delegate* unmanaged[Cdecl])msQuicClose)(apiTable); return true; From 53e8c268896962cfd4326271f49c993367d30c1c Mon Sep 17 00:00:00 2001 From: Radek Zikmund Date: Mon, 12 Sep 2022 12:09:33 +0200 Subject: [PATCH 2/3] Code review feedback --- .../src/System/Net/Quic/Internal/MsQuicApi.cs | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/src/libraries/System.Net.Quic/src/System/Net/Quic/Internal/MsQuicApi.cs b/src/libraries/System.Net.Quic/src/System/Net/Quic/Internal/MsQuicApi.cs index 1655d3cac5934c..ed36925f7d1974 100644 --- a/src/libraries/System.Net.Quic/src/System/Net/Quic/Internal/MsQuicApi.cs +++ b/src/libraries/System.Net.Quic/src/System/Net/Quic/Internal/MsQuicApi.cs @@ -20,7 +20,7 @@ internal sealed unsafe partial class MsQuicApi private static readonly Version MsQuicVersion = new Version(2, 1); - private static readonly IntPtr MsQuicHandle = TryLoadMsQuic(); + private static readonly IntPtr MsQuicHandle = TryLoadMsQuic(out IntPtr handle) ? handle : IntPtr.Zero; public MsQuicSafeHandle Registration { get; } @@ -139,11 +139,9 @@ private static MsQuicApi AllocateMsQuicApi() return new MsQuicApi(apiTable); } - private static IntPtr TryLoadMsQuic() => - (NativeLibrary.TryLoad($"{Interop.Libraries.MsQuic}.{MsQuicVersion.Major}", typeof(MsQuicApi).Assembly, DllImportSearchPath.AssemblyDirectory, out IntPtr msQuicHandle) || - NativeLibrary.TryLoad(Interop.Libraries.MsQuic, typeof(MsQuicApi).Assembly, DllImportSearchPath.AssemblyDirectory, out msQuicHandle)) - ? msQuicHandle - : IntPtr.Zero; + private static bool TryLoadMsQuic(out IntPtr msQuicHandle) => + 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); private static bool TryOpenMsQuic(out QUIC_API_TABLE* apiTable, out int openStatus) { From b43b31092b5360d164f05b3170ac30798c7f67b7 Mon Sep 17 00:00:00 2001 From: Radek Zikmund Date: Mon, 12 Sep 2022 17:46:25 +0200 Subject: [PATCH 3/3] More code review feedback --- .../src/System/Net/Quic/Internal/MsQuicApi.cs | 50 ++++++------------- 1 file changed, 14 insertions(+), 36 deletions(-) diff --git a/src/libraries/System.Net.Quic/src/System/Net/Quic/Internal/MsQuicApi.cs b/src/libraries/System.Net.Quic/src/System/Net/Quic/Internal/MsQuicApi.cs index ed36925f7d1974..8e476eb91b146d 100644 --- a/src/libraries/System.Net.Quic/src/System/Net/Quic/Internal/MsQuicApi.cs +++ b/src/libraries/System.Net.Quic/src/System/Net/Quic/Internal/MsQuicApi.cs @@ -20,7 +20,8 @@ internal sealed unsafe partial class MsQuicApi private static readonly Version MsQuicVersion = new Version(2, 1); - private static readonly IntPtr MsQuicHandle = TryLoadMsQuic(out IntPtr handle) ? handle : IntPtr.Zero; + private static readonly delegate* unmanaged[Cdecl] MsQuicOpenVersion; + private static readonly delegate* unmanaged[Cdecl] MsQuicClose; public MsQuicSafeHandle Registration { get; } @@ -60,16 +61,22 @@ private MsQuicApi(QUIC_API_TABLE* apiTable) internal static bool Tls13ServerMayBeDisabled { get; } internal static bool Tls13ClientMayBeDisabled { get; } +#pragma warning disable CA1810 // Initialize all static fields in 'MsQuicApi' when those fields are declared and remove the explicit static constructor static MsQuicApi() { - if (MsQuicHandle == IntPtr.Zero) + if (!NativeLibrary.TryLoad($"{Interop.Libraries.MsQuic}.{MsQuicVersion.Major}", typeof(MsQuicApi).Assembly, DllImportSearchPath.AssemblyDirectory, out IntPtr msQuicHandle) && + !NativeLibrary.TryLoad(Interop.Libraries.MsQuic, typeof(MsQuicApi).Assembly, DllImportSearchPath.AssemblyDirectory, out msQuicHandle)) { // MsQuic library not loaded return; } + MsQuicOpenVersion = (delegate* unmanaged[Cdecl])NativeLibrary.GetExport(msQuicHandle, nameof(MsQuicOpenVersion)); + MsQuicClose = (delegate* unmanaged[Cdecl])NativeLibrary.GetExport(msQuicHandle, nameof(MsQuicClose)); + if (!TryOpenMsQuic(out QUIC_API_TABLE* apiTable, out _)) { + // Too low version of the library (likely pre-2.0) return; } @@ -122,10 +129,10 @@ static MsQuicApi() finally { // Gracefully close the API table to free resources. The API table will be allocated lazily again if needed - bool closed = TryCloseMsQuic(apiTable); - Debug.Assert(closed, "Failed to close MsQuic"); + MsQuicClose(apiTable); } } +#pragma warning restore CA1810 private static MsQuicApi AllocateMsQuicApi() { @@ -139,29 +146,12 @@ private static MsQuicApi AllocateMsQuicApi() return new MsQuicApi(apiTable); } - private static bool TryLoadMsQuic(out IntPtr msQuicHandle) => - 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); - private static bool TryOpenMsQuic(out QUIC_API_TABLE* apiTable, out int openStatus) { - Debug.Assert(MsQuicHandle != IntPtr.Zero); - - apiTable = null; - if (!NativeLibrary.TryGetExport(MsQuicHandle, "MsQuicOpenVersion", out IntPtr msQuicOpenVersionAddress)) - { - if (NetEventSource.Log.IsEnabled()) - { - NetEventSource.Info(null, "Failed to get MsQuicOpenVersion export in msquic library."); - } - - openStatus = MsQuic.QUIC_STATUS_NOT_FOUND; - return false; - } + Debug.Assert(MsQuicOpenVersion != null); QUIC_API_TABLE* table = null; - delegate* unmanaged[Cdecl] msQuicOpenVersion = (delegate* unmanaged[Cdecl])msQuicOpenVersionAddress; - openStatus = msQuicOpenVersion((uint)MsQuicVersion.Major, &table); + openStatus = MsQuicOpenVersion((uint)MsQuicVersion.Major, &table); if (StatusFailed(openStatus)) { if (NetEventSource.Log.IsEnabled()) @@ -169,6 +159,7 @@ private static bool TryOpenMsQuic(out QUIC_API_TABLE* apiTable, out int openStat NetEventSource.Info(null, $"MsQuicOpenVersion returned {openStatus} status code."); } + apiTable = null; return false; } @@ -176,19 +167,6 @@ private static bool TryOpenMsQuic(out QUIC_API_TABLE* apiTable, out int openStat return true; } - private static bool TryCloseMsQuic(QUIC_API_TABLE* apiTable) - { - Debug.Assert(MsQuicHandle != IntPtr.Zero); - - if (NativeLibrary.TryGetExport(MsQuicHandle, "MsQuicClose", out IntPtr msQuicClose)) - { - ((delegate* unmanaged[Cdecl])msQuicClose)(apiTable); - return true; - } - - return false; - } - private static bool IsWindowsVersionSupported() => OperatingSystem.IsWindowsVersionAtLeast(MinWindowsVersion.Major, MinWindowsVersion.Minor, MinWindowsVersion.Build, MinWindowsVersion.Revision);