From 7a4bf1636eb68c6046f27d9376b48bd5ad3bc192 Mon Sep 17 00:00:00 2001 From: Jan Kotas Date: Tue, 2 Aug 2022 21:55:35 -0700 Subject: [PATCH 1/3] Convert GetAllMountPoints to UnmanagedCallersOnly --- .../Unix/System.Native/Interop.MountPoints.cs | 49 +++++++++++++------ .../Windows/HttpApi/Interop.HttpApi.cs | 2 +- .../Net/Windows/HttpListener.Windows.cs | 16 ++---- src/native/libs/System.Native/pal_mount.c | 8 +-- src/native/libs/System.Native/pal_mount.h | 4 +- 5 files changed, 45 insertions(+), 34 deletions(-) diff --git a/src/libraries/Common/src/Interop/Unix/System.Native/Interop.MountPoints.cs b/src/libraries/Common/src/Interop/Unix/System.Native/Interop.MountPoints.cs index 6f031a0c45f819..ece5bbc5bfca77 100644 --- a/src/libraries/Common/src/Interop/Unix/System.Native/Interop.MountPoints.cs +++ b/src/libraries/Common/src/Interop/Unix/System.Native/Interop.MountPoints.cs @@ -2,37 +2,54 @@ // The .NET Foundation licenses this file to you under the MIT license. using System; +using System.Collections.Generic; +using System.Runtime.CompilerServices; +using System.Runtime.ExceptionServices; using System.Runtime.InteropServices; internal static partial class Interop { internal static partial class Sys { - [UnmanagedFunctionPointer(CallingConvention.Cdecl)] - private unsafe delegate void MountPointFound(byte* name); + [LibraryImport(Libraries.SystemNative, EntryPoint = "SystemNative_GetAllMountPoints")] + private static unsafe partial int GetAllMountPoints(delegate* unmanaged onFound, void* context); - [LibraryImport(Libraries.SystemNative, EntryPoint = "SystemNative_GetAllMountPoints", SetLastError = true)] - private static partial int GetAllMountPoints(MountPointFound mpf); + private struct AllMountPointsContext + { + public List _results; + public ExceptionDispatchInfo? _exception; + } + + [UnmanagedCallersOnly] + private static unsafe void AddMountPoint(void* context, byte* name) + { + ref AllMountPointsContext callbackContext = ref Unsafe.As(ref *(byte*)context); + callbackContext._results = new List(); + + try + { + callbackContext._results.Add(Marshal.PtrToStringUTF8((IntPtr)name)!); + } + catch (Exception e) + { + callbackContext._exception = ExceptionDispatchInfo.Capture(e); + } + } internal static string[] GetAllMountPoints() { - int count = 0; - var found = new string[4]; + AllMountPointsContext context = default; + context._results = new List(); unsafe { - GetAllMountPoints((byte* name) => - { - if (count == found.Length) - { - Array.Resize(ref found, count * 2); - } - found[count++] = Marshal.PtrToStringAnsi((IntPtr)name)!; - }); + GetAllMountPoints(&AddMountPoint, Unsafe.AsPointer(ref context)); } - Array.Resize(ref found, count); - return found; + if (context._exception != null) + context._exception.Throw(); + + return context._results.ToArray(); } } } diff --git a/src/libraries/Common/src/Interop/Windows/HttpApi/Interop.HttpApi.cs b/src/libraries/Common/src/Interop/Windows/HttpApi/Interop.HttpApi.cs index 4cb72b927cc47d..8e004740fe97c3 100644 --- a/src/libraries/Common/src/Interop/Windows/HttpApi/Interop.HttpApi.cs +++ b/src/libraries/Common/src/Interop/Windows/HttpApi/Interop.HttpApi.cs @@ -459,7 +459,7 @@ internal struct HTTP_BINDING_INFO internal static partial uint HttpInitialize(HTTPAPI_VERSION version, uint flags, IntPtr pReserved); [LibraryImport(Libraries.HttpApi, SetLastError = true)] - internal static partial uint HttpSetUrlGroupProperty(ulong urlGroupId, HTTP_SERVER_PROPERTY serverProperty, IntPtr pPropertyInfo, uint propertyInfoLength); + internal static unsafe partial uint HttpSetUrlGroupProperty(ulong urlGroupId, HTTP_SERVER_PROPERTY serverProperty, void* pPropertyInfo, uint propertyInfoLength); [LibraryImport(Libraries.HttpApi, SetLastError = true)] internal static unsafe partial uint HttpCreateServerSession(HTTPAPI_VERSION version, ulong* serverSessionId, uint reserved); diff --git a/src/libraries/System.Net.HttpListener/src/System/Net/Windows/HttpListener.Windows.cs b/src/libraries/System.Net.HttpListener/src/System/Net/Windows/HttpListener.Windows.cs index 6f31052eaf00f3..5a2b7c5d79367a 100644 --- a/src/libraries/System.Net.HttpListener/src/System/Net/Windows/HttpListener.Windows.cs +++ b/src/libraries/System.Net.HttpListener/src/System/Net/Windows/HttpListener.Windows.cs @@ -90,10 +90,10 @@ public bool UnsafeConnectionNtlmAuthentication private Dictionary DisconnectResults => LazyInitializer.EnsureInitialized(ref _disconnectResults, () => new Dictionary()); - private void SetUrlGroupProperty(Interop.HttpApi.HTTP_SERVER_PROPERTY property, IntPtr info, uint infosize) + private unsafe void SetUrlGroupProperty(Interop.HttpApi.HTTP_SERVER_PROPERTY property, void* info, uint infosize) { Debug.Assert(_urlGroupId != 0, "SetUrlGroupProperty called with invalid url group id"); - Debug.Assert(info != IntPtr.Zero, "SetUrlGroupProperty called with invalid pointer"); + Debug.Assert(info != null, "SetUrlGroupProperty called with invalid pointer"); // // Set the url group property using Http Api. @@ -128,11 +128,9 @@ internal void SetServerTimeout(int[] timeouts, uint minSendBytesPerSecond) (ushort)timeouts[(int)Interop.HttpApi.HTTP_TIMEOUT_TYPE.HeaderWait]; timeoutinfo.MinSendRate = minSendBytesPerSecond; - IntPtr infoptr = new IntPtr(&timeoutinfo); - SetUrlGroupProperty( Interop.HttpApi.HTTP_SERVER_PROPERTY.HttpServerTimeoutsProperty, - infoptr, (uint)sizeof(Interop.HttpApi.HTTP_TIMEOUT_LIMIT_INFO)); + &timeoutinfo, (uint)sizeof(Interop.HttpApi.HTTP_TIMEOUT_LIMIT_INFO)); } public HttpListenerTimeoutManager TimeoutManager @@ -307,10 +305,8 @@ private void AttachRequestQueueToUrlGroup() info.Flags = Interop.HttpApi.HTTP_FLAGS.HTTP_PROPERTY_FLAG_PRESENT; info.RequestQueueHandle = _currentSession!.RequestQueueHandle.DangerousGetHandle(); - IntPtr infoptr = new IntPtr(&info); - SetUrlGroupProperty(Interop.HttpApi.HTTP_SERVER_PROPERTY.HttpServerBindingProperty, - infoptr, (uint)sizeof(Interop.HttpApi.HTTP_BINDING_INFO)); + &info, (uint)sizeof(Interop.HttpApi.HTTP_BINDING_INFO)); } private void DetachRequestQueueFromUrlGroup() @@ -328,11 +324,9 @@ private void DetachRequestQueueFromUrlGroup() info.Flags = Interop.HttpApi.HTTP_FLAGS.NONE; info.RequestQueueHandle = IntPtr.Zero; - IntPtr infoptr = new IntPtr(&info); - uint statusCode = Interop.HttpApi.HttpSetUrlGroupProperty(_urlGroupId, Interop.HttpApi.HTTP_SERVER_PROPERTY.HttpServerBindingProperty, - infoptr, (uint)sizeof(Interop.HttpApi.HTTP_BINDING_INFO)); + &info, (uint)sizeof(Interop.HttpApi.HTTP_BINDING_INFO)); if (statusCode != Interop.HttpApi.ERROR_SUCCESS) { diff --git a/src/native/libs/System.Native/pal_mount.c b/src/native/libs/System.Native/pal_mount.c index bf8c49a6c37118..2453bdfe9ede3f 100644 --- a/src/native/libs/System.Native/pal_mount.c +++ b/src/native/libs/System.Native/pal_mount.c @@ -29,7 +29,7 @@ #endif #endif -int32_t SystemNative_GetAllMountPoints(MountPointFound onFound) +int32_t SystemNative_GetAllMountPoints(MountPointFound onFound, void* context) { #if HAVE_MNTINFO // getmntinfo returns pointers to OS-internal structs, so we don't need to worry about free'ing the object @@ -41,7 +41,7 @@ int32_t SystemNative_GetAllMountPoints(MountPointFound onFound) int count = getmntinfo(&mounts, MNT_WAIT); for (int32_t i = 0; i < count; i++) { - onFound(mounts[i].f_mntonname); + onFound(context, mounts[i].f_mntonname); } return 0; @@ -56,7 +56,7 @@ int32_t SystemNative_GetAllMountPoints(MountPointFound onFound) struct mnttab entry; while(getmntent(fp, &entry) == 0) { - onFound(entry.mnt_mountp); + onFound(context, entry.mnt_mountp); } result = fclose(fp); @@ -79,7 +79,7 @@ int32_t SystemNative_GetAllMountPoints(MountPointFound onFound) struct mntent entry; while (getmntent_r(fp, &entry, buffer, STRING_BUFFER_SIZE) != NULL) { - onFound(entry.mnt_dir); + onFound(context, entry.mnt_dir); } result = endmntent(fp); diff --git a/src/native/libs/System.Native/pal_mount.h b/src/native/libs/System.Native/pal_mount.h index 167ed9bdd87eea..3afef9b3ca0662 100644 --- a/src/native/libs/System.Native/pal_mount.h +++ b/src/native/libs/System.Native/pal_mount.h @@ -21,7 +21,7 @@ typedef struct * Using the callback pattern allows us to limit the number of allocs we do and makes it * cleaner on the managed side since we don't have to worry about cleaning up any unmanaged memory. */ -typedef void (*MountPointFound)(const char* name); +typedef void (*MountPointFound)(void* context, const char* name); /** * Gets the space information for the given mount point and populates the input struct with the data. @@ -45,4 +45,4 @@ PALEXPORT int32_t SystemNative_GetFormatInfoForMountPoint( * function pointer once-per-mount-point to prevent heap allocs * as much as possible. */ -PALEXPORT int32_t SystemNative_GetAllMountPoints(MountPointFound onFound); +PALEXPORT int32_t SystemNative_GetAllMountPoints(MountPointFound onFound, void* context); From 76f56f664dc45e41e3dcb196ca33545380ea5af1 Mon Sep 17 00:00:00 2001 From: Jan Kotas Date: Tue, 2 Aug 2022 23:05:56 -0700 Subject: [PATCH 2/3] Fix build break --- .../src/System.IO.FileSystem.DriveInfo.csproj | 1 + 1 file changed, 1 insertion(+) diff --git a/src/libraries/System.IO.FileSystem.DriveInfo/src/System.IO.FileSystem.DriveInfo.csproj b/src/libraries/System.IO.FileSystem.DriveInfo/src/System.IO.FileSystem.DriveInfo.csproj index 690d1aeb7a94b7..61225e58e8a9b8 100644 --- a/src/libraries/System.IO.FileSystem.DriveInfo/src/System.IO.FileSystem.DriveInfo.csproj +++ b/src/libraries/System.IO.FileSystem.DriveInfo/src/System.IO.FileSystem.DriveInfo.csproj @@ -71,6 +71,7 @@ Link="Common\Interop\Unix\Interop.MountPoints.FormatInfo.cs" /> + From e1081c7980f6275c76238177252ab6e85c2da863 Mon Sep 17 00:00:00 2001 From: Jan Kotas Date: Wed, 3 Aug 2022 06:06:26 -0700 Subject: [PATCH 3/3] Apply suggestions from code review Co-authored-by: Stephen Toub --- .../src/Interop/Unix/System.Native/Interop.MountPoints.cs | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/src/libraries/Common/src/Interop/Unix/System.Native/Interop.MountPoints.cs b/src/libraries/Common/src/Interop/Unix/System.Native/Interop.MountPoints.cs index ece5bbc5bfca77..4f5f655640874f 100644 --- a/src/libraries/Common/src/Interop/Unix/System.Native/Interop.MountPoints.cs +++ b/src/libraries/Common/src/Interop/Unix/System.Native/Interop.MountPoints.cs @@ -16,15 +16,14 @@ internal static partial class Sys private struct AllMountPointsContext { - public List _results; - public ExceptionDispatchInfo? _exception; + internal List _results; + internal ExceptionDispatchInfo? _exception; } [UnmanagedCallersOnly] private static unsafe void AddMountPoint(void* context, byte* name) { ref AllMountPointsContext callbackContext = ref Unsafe.As(ref *(byte*)context); - callbackContext._results = new List(); try { @@ -46,8 +45,7 @@ internal static string[] GetAllMountPoints() GetAllMountPoints(&AddMountPoint, Unsafe.AsPointer(ref context)); } - if (context._exception != null) - context._exception.Throw(); + context._exception?.Throw(); return context._results.ToArray(); }