Convert non-blittable delegate usage in System.DirectoryServices to function pointers. #65319
Conversation
…unction pointers. All of the delegates used very simple marshalling and they were all used in one or two places immediately after a GetProcAddress call for them, so it seemed like it was worthwhile to go all the way to function pointers instead of just manually pinning the parameters and still allocating delegates. Fixes dotnet#65259
ghost
assigned
|
Tagging subscribers to this area: @dotnet/area-system-directoryservices, @jay98014 Issue DetailsAll of the delegates used very simple marshalling and they were all used in one or two places immediately after a GetProcAddress call for them, so it seemed like it was worthwhile to go all the way to function pointers instead of just manually pinning the parameters and still allocating delegates. Fixes #65259 @danmoseley is there a particular CI pipeline I can trigger to validate that the failing tests are passing now? cc: @dotnet/interop-contrib
|
| // call DsReplicaSyncAllW | ||
| IntPtr functionPtr = global::Interop.Kernel32.GetProcAddress(DirectoryContext.ADHandle, "DsListDomainsInSiteW"); | ||
| if (functionPtr == (IntPtr)0) | ||
| var dsListDomainsInSiteW = (delegate* unmanaged<IntPtr, char*, IntPtr*, int>)global::Interop.Kernel32.GetProcAddress(DirectoryContext.ADHandle, "DsListDomainsInSiteW"); |
There was a problem hiding this comment.
Is it desirable that this is IntPtr* rather than DS_NAME_RESULT*?
What about int rather than uint (since its a DWORD)?
There was a problem hiding this comment.
I wanted to match the existing delegate signatures to ensure I wouldn't have to add a ton of casts.
| // call DsReplicaSyncAllW | ||
| IntPtr functionPtr = global::Interop.Kernel32.GetProcAddress(DirectoryContext.ADHandle, "DsListDomainsInSiteW"); | ||
| if (functionPtr == (IntPtr)0) | ||
| var dsListDomainsInSiteW = (delegate* unmanaged<IntPtr, char*, IntPtr*, int>)global::Interop.Kernel32.GetProcAddress(DirectoryContext.ADHandle, "DsListDomainsInSiteW"); |
There was a problem hiding this comment.
This library targets netstandard2.0. The default unmanaged calling convention is not supported for netstandard2.0. I think these needs to be explicit StdCall to make it work for netstandard2.0.
There was a problem hiding this comment.
It looks like the netstandard2.0 implementation is a PlatformNotSupported-stubbed out implementation, so it doesn't compile this code.
There was a problem hiding this comment.
It looks like the
netstandard2.0implementation is a PlatformNotSupported-stubbed out implementation, so it doesn't compile this code.
That is good. I had thought that targeting netstandard2.0 without stating the calling convention would result in a compiler error. Right?
There was a problem hiding this comment.
Yep. The feature is conditionally available based on a RuntimeFeature field/property.
|
Both failing legs are issues that are occurring outside of this PR as well. |
4 participants
All of the delegates used very simple marshalling and they were all used in one or two places immediately after a GetProcAddress call for them, so it seemed like it was worthwhile to go all the way to function pointers instead of just manually pinning the parameters and still allocating delegates.
Fixes #65259
@danmoseley is there a particular CI pipeline I can trigger to validate that the failing tests are passing now?
cc: @dotnet/interop-contrib