Implement NativeLibrary.GetEntryPointModuleHandle #57610

jkoritzinsky · 2021-08-17T22:09:34Z

I know there was a continuing discussion on naming, so we can continue that here if we so desire.

cc: @jkotas @AaronRobinsonMSFT @lambdageek

…gn to make it easier to share QCalls and to hook them up in Mono.

…can be used as a QCall entrypoint in Mono.

dotnet-issue-labeler · 2021-08-17T22:09:38Z

Note regarding the new-api-needs-documentation label:

This serves as a reminder for when your PR is modifying a ref *.cs file and adding/modifying public APIs, to please make sure the API implementation in the src *.cs file is documented with triple slash comments, so the PR reviewers can sign off that change.

src/coreclr/hosts/corerun/corerun.cpp

src/coreclr/vm/ecalllist.h

src/libraries/System.Private.CoreLib/src/System/Runtime/InteropServices/NativeLibrary.cs

src/tests/Interop/NativeLibrary/API/NativeLibraryTests.cs

src/mono/mono/metadata/qcall-def.h

src/tests/Interop/NativeLibrary/API/NativeLibraryTests.cs

jkotas · 2021-08-17T22:44:44Z

Bring in some more QCall infrastructure that matches the CoreCLR

In this specific case, it would be even better to move the unmanaged part to src/libraries/Native (it is done that way in NativeAOT already) so the whole thing can be shared. I understand that it would be more work.

jkoritzinsky · 2021-08-17T22:50:13Z

Bring in some more QCall infrastructure that matches the CoreCLR

In this specific case, it would be even better to move the unmanaged part to src/libraries/Native (it is done that way in NativeAOT already) so the whole thing can be shared. I understand that it would be more work.

I didn't want to do that initially since Mono already has some special handling for __Internal that I wanted to piggy-back on and it felt weird to have most of NativeLibrary be implemented in the runtimes but have one method implemented in src/libraries/Native, but I can move it there if we want.

If I do move it, I'd like to leave the mono QCall infra in this PR (or worst case move it into another PR) to make it easier to add QCalls to Mono in the future.

src/libraries/System.Private.CoreLib/src/System/Runtime/InteropServices/NativeLibrary.cs

…one without having to follow the pattern (makes it easier to remove this new QCall and move it to the libraries native portion)

… prototype (it's no longer exposed outside native-library.c since it's no longer used in a QCall)

…rary_entrymodule

…e on Mono

…untime into nativelibrary_entrymodule

…ble-free check against. P/Invoking messes up the ref-counts since the runtime loads the native library, so the double free test didn't work.

…nd Mono FullAOT

jkoritzinsky · 2022-03-18T02:00:58Z

All of the test failures are unrelated. This is finally ready for review!

src/tests/Interop/NativeLibrary/NativeLibraryToLoad/GlobalLoadHelper.cpp

src/tests/Interop/NativeLibrary/NativeLibraryToLoad/CMakeLists.txt

src/tests/Interop/NativeLibrary/API/GetMainProgramHandleTests.cs

jkoritzinsky · 2022-03-21T17:08:20Z

Test failures are #66852.

Merging now to make sure we get in for Preview 3

jkotas · 2022-03-21T17:34:55Z

src/native/libs/System.Native/pal_dynamicload.c

+static void* g_defaultSearchOrderPseudoHandle = NULL;
+void* SystemNative_GetDefaultSearchOrderPseudoHandle(void)
+{
+    if (g_defaultSearchOrderPseudoHandle == NULL)


This has potential race condition depending on how the C++ decides to compile it. The C/C++ compiler is free to reorder the if check and reading of g_defaultSearchOrderPseudoHandle to return. It means that the method can return NULL instead of the actual value.

This is real problem. We had many bugs like that over the years - they are always very hard to trace down.

This can be fixed by making g_defaultSearchOrderPseudoHandle volatile, correct?

Yup. Also, you may want to cache the value in a local so that it does not need to be fetched from memory twice.

jkoritzinsky added 5 commits August 10, 2021 14:54

Bring in some more QCall infrastructure that matches the CoreCLR desi…

e4e799c

…gn to make it easier to share QCalls and to hook them up in Mono.

Expose method from CoreRun for use in testing for desktop scenarios.

5f16ec8

Refactor out the implementation of the lookup for "__Internal" so it …

e0542ee

…can be used as a QCall entrypoint in Mono.

First pass implementing API on corelcr and mono.

57643bc

Fix build and test failures.

4843165

jkoritzinsky requested review from lambdageek, thaystg and vargaz as code owners August 17, 2021 22:09

dotnet-issue-labeler bot added area-System.Runtime.InteropServices new-api-needs-documentation labels Aug 17, 2021

jkoritzinsky requested review from AaronRobinsonMSFT and elinor-fung August 17, 2021 22:09

AaronRobinsonMSFT reviewed Aug 17, 2021

View reviewed changes

src/coreclr/hosts/corerun/corerun.cpp Outdated Show resolved Hide resolved

src/coreclr/vm/ecalllist.h Outdated Show resolved Hide resolved

src/libraries/System.Private.CoreLib/src/System/Runtime/InteropServices/NativeLibrary.cs Show resolved Hide resolved

AaronRobinsonMSFT added this to the 7.0.0 milestone Aug 17, 2021

jkotas reviewed Aug 17, 2021

View reviewed changes

src/libraries/System.Private.CoreLib/src/System/Runtime/InteropServices/NativeLibrary.cs Outdated Show resolved Hide resolved

AaronRobinsonMSFT reviewed Aug 17, 2021

View reviewed changes

src/tests/Interop/NativeLibrary/API/NativeLibraryTests.cs Outdated Show resolved Hide resolved

jkotas reviewed Aug 17, 2021

View reviewed changes

src/mono/mono/metadata/qcall-def.h Outdated Show resolved Hide resolved

jkotas reviewed Aug 17, 2021

View reviewed changes

src/tests/Interop/NativeLibrary/API/NativeLibraryTests.cs Outdated Show resolved Hide resolved

vargaz reviewed Aug 18, 2021

View reviewed changes

src/libraries/System.Private.CoreLib/src/System/Runtime/InteropServices/NativeLibrary.cs Outdated Show resolved Hide resolved

lambdageek mentioned this pull request Aug 18, 2021

[Mono] Split QCall table declarations from their implementations #57639

Open

jkoritzinsky added 7 commits August 18, 2021 11:32

Fix typos and do basic cleanup.

376cd45

Enhance testing.

d19ed4a

Update comment for mono QCalls so the comment explains how to define …

d50f07f

…one without having to follow the pattern (makes it easier to remove this new QCall and move it to the libraries native portion)

Fix corerun build on non-Windows.

d338682

Move API implementation to libraries.

3710af5

Make netcore_lookup_self_native_handle static so it doesn't require a…

93fc783

… prototype (it's no longer exposed outside native-library.c since it's no longer used in a QCall)

Compile pal_dl.c in the System.Native shim

58554e0

jkoritzinsky added 3 commits March 15, 2022 15:51

Merge branch 'main' of github.com:jkoritzinsky/runtime into nativelib…

c9a9dba

…rary_entrymodule

Rename attribute

7ffac68

Fix duplicate source file include

a175e5c

runfoapp bot mentioned this pull request Mar 16, 2022

ThreadPoolTests.CooperativeBlockingCanCreateThreadsFaster timed out #64964

Closed

jkoritzinsky added 7 commits March 16, 2022 09:20

Fix allocation in NativeLibrary.Free to handle the raw lib handle cas…

81f9daa

…e on Mono

Fix Apple test builds

a6117ba

Merge branch 'nativelibrary_entrymodule' of github.com:jkoritzinsky/r…

2d4959a

…untime into nativelibrary_entrymodule

Fix typo in ConditionalFact condition

f17ec5a

Don't P/Invoke into the same NativeLibrary that we're testing our dou…

b8aa4fe

…ble-free check against. P/Invoking messes up the ref-counts since the runtime loads the native library, so the double free test didn't work.

Fix GetCurrentClrDetails exporting on win x86

4e7a69c

Use delegate interop to avoid hitting issues with function pointers a…

c58228b

…nd Mono FullAOT

jkoritzinsky marked this pull request as ready for review March 18, 2022 02:00

jkoritzinsky requested a review from MichalStrehovsky as a code owner March 18, 2022 02:00

AaronRobinsonMSFT reviewed Mar 18, 2022

View reviewed changes

src/tests/Interop/NativeLibrary/NativeLibraryToLoad/GlobalLoadHelper.cpp Show resolved Hide resolved

src/tests/Interop/NativeLibrary/NativeLibraryToLoad/CMakeLists.txt Outdated Show resolved Hide resolved

This was referenced Mar 18, 2022

Failing test system.tests.gcextendedtests.getallocatedbytesforcurrentthread #65183

Closed

[WASM] BlazorWasmBuildPublishTests getting oomkill'ed during linking #66725

Closed

Clean up CMakeLists.txt for NativeLibrary tests

6bc7f4c

runfoapp bot mentioned this pull request Mar 18, 2022

Threadpool test CooperativeBlockingCanCreateThreadsFaster failing on Mac #66852

Closed

AaronRobinsonMSFT reviewed Mar 18, 2022

View reviewed changes

src/tests/Interop/NativeLibrary/API/GetMainProgramHandleTests.cs Show resolved Hide resolved

Add a test for freeing the result of GetMainProgramHandle

4266aab

AaronRobinsonMSFT approved these changes Mar 21, 2022

View reviewed changes

jkoritzinsky merged commit e22ebdf into dotnet:main Mar 21, 2022

jkoritzinsky deleted the nativelibrary_entrymodule branch March 21, 2022 17:08

jkotas reviewed Mar 21, 2022

View reviewed changes

jkoritzinsky mentioned this pull request Mar 21, 2022

Fix memory load ordering bug #66952

Merged

radekdoulik pushed a commit to radekdoulik/runtime that referenced this pull request Mar 30, 2022

Implement NativeLibrary.GetEntryPointModuleHandle (dotnet#57610)

b4abdb2

ghost locked as resolved and limited conversation to collaborators Apr 20, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement NativeLibrary.GetEntryPointModuleHandle #57610

Implement NativeLibrary.GetEntryPointModuleHandle #57610

jkoritzinsky commented Aug 17, 2021 •

edited by AaronRobinsonMSFT

Loading

dotnet-issue-labeler bot commented Aug 17, 2021

jkotas commented Aug 17, 2021

jkoritzinsky commented Aug 17, 2021

jkoritzinsky commented Mar 18, 2022

jkoritzinsky commented Mar 21, 2022

jkotas Mar 21, 2022

jkoritzinsky Mar 21, 2022

jkotas Mar 21, 2022

Implement NativeLibrary.GetEntryPointModuleHandle #57610

Implement NativeLibrary.GetEntryPointModuleHandle #57610

Conversation

jkoritzinsky commented Aug 17, 2021 • edited by AaronRobinsonMSFT Loading

dotnet-issue-labeler bot commented Aug 17, 2021

jkotas commented Aug 17, 2021

jkoritzinsky commented Aug 17, 2021

jkoritzinsky commented Mar 18, 2022

jkoritzinsky commented Mar 21, 2022

jkotas Mar 21, 2022

Choose a reason for hiding this comment

jkoritzinsky Mar 21, 2022

Choose a reason for hiding this comment

jkotas Mar 21, 2022

Choose a reason for hiding this comment

jkoritzinsky commented Aug 17, 2021 •

edited by AaronRobinsonMSFT

Loading