-
Notifications
You must be signed in to change notification settings - Fork 5.2k
Consolidate direct paths of NativeLibrary API to managed implementation #121058
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
base: main
Are you sure you want to change the base?
Conversation
|
Tagging subscribers to this area: @dotnet/interop-contrib |
...System.Private.CoreLib/src/System/Runtime/InteropServices/NativeLibrary.NativeAot.Windows.cs
Show resolved
Hide resolved
src/libraries/System.Private.CoreLib/src/System/Runtime/InteropServices/NativeLibrary.cs
Outdated
Show resolved
Hide resolved
src/libraries/System.Private.CoreLib/src/System/Runtime/InteropServices/NativeLibrary.cs
Outdated
Show resolved
Hide resolved
src/libraries/System.Private.CoreLib/src/System/Runtime/InteropServices/NativeLibrary.cs
Outdated
Show resolved
Hide resolved
src/libraries/System.Private.CoreLib/src/System/Runtime/InteropServices/NativeLibrary.cs
Outdated
Show resolved
Hide resolved
cc8e0ca to
fd7dca3
Compare
fd7dca3 to
4907c5a
Compare
| #include "nativelibrarynative.h" | ||
|
|
||
| // static | ||
| extern "C" INT_PTR QCALLTYPE NativeLibrary_LoadFromPath(LPCWSTR path, BOOL throwOnError) |
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.
Would it be better to split this into unifying NativeLibrary_LoadFromPath/NativeLibrary_FreeLib/NativeLibrary_GetSymbol in one straightforward PR, and do the policy-heave APIs in a separate PR that is unlikely to be straightforward and that will need very careful review?
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.
Yeah I think so. The detailed behavior of LoadBySearch are not exactly the same. If we feel too complex we can also keep it unmanaged for coreclr.
src/libraries/System.Private.CoreLib/src/System/Runtime/Loader/AssemblyLoadContext.cs
Outdated
Show resolved
Hide resolved
src/libraries/System.Private.CoreLib/src/System/Runtime/InteropServices/NativeLibrary.cs
Show resolved
Hide resolved
|
Thanks! @AaronRobinsonMSFT @jkoritzinsky Could you please review this as well? |
|
@elinor-fung or @agocke Do you know if the hosting test legs were triggered above? |
Yes - they are in the |
Taking NativeAOT implementation and adapting coreclr behavior. The remaining code for P/Invoke resolver isn't changed.
Mono implementation can be update too, but I'm unsure about how the fallback registered for wasm should be handled.