Allow interop resolvers to return self handle

[am11]

When user-defined resolvers return self handle via GetMainProgramHandle(), we lookup the cached handle in mono and fail to find one because internal_module is not cached in native_library_module_map.

Fix is to test the resolver returned handle against self; before the map lookup (which happens under the lock).

Close #77985.

[ghost]

Tagging subscribers to this area:
See info in area-owners.md if you want to be subscribed.

Issue Details

---

When user-defined resolvers return self handle via GetMainProgramHandle(), we lookup the cached handle in mono and fail to find one because internal_module is not cached in native_library_module_map.

Fix is to test the resolver returned handle against self; before the map lookup (which happens under the lock).

Close #77985.

Author:	am11
Assignees:	-
Labels:	area-AssemblyLoader-mono
Milestone:	-

[am11]

@lambdageek, @vargaz, candidate of backport? (GetMainProgramHandle is a new API in 7.0)

wasm failures are preexisting: #64188, #76454, #78017

[lambdageek]

@am11 @vargaz I think we should back port

[lambdageek]

/backport to release/7.0

[github-actions]

Started backporting to release/7.0: https://github.com/dotnet/runtime/actions/runs/3419652288

[am11]

@lambdageek, could you please run the Android pipeline (/azp run runtime-extra-platforms) against this PR? I think Android test runner will fail here as well. I'll then push a fix to verify it works (#78018 (comment)).

[lambdageek]

/azp run runtime-extra-platforms

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).

[akoeplinger]

On main we can also do /azp run runtime-android to only run the android jobs.
I've looked at the test results from the runtime-extra-platforms jobs that are already in and they are indeed the same as @am11 suspected.

In general we should try to avoid merging a backport for PRs that weren't yet merged to main, we'd probably have caught this earlier.

[am11]

@lambdageek, could you re-run the leg? The tests are passing locally now. For the internal call, we needed to continue looking up in the map.

[lambdageek]

/azp run runtime-android

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).

[lambdageek]

@am11 I've been meaning to ask: would it be possible to add a unit test for #77985? probably something using the remote executor would work, and looking for some C standard lib function (like _malloc ?) on non-Windows?

[am11]

For some reason, it wasn't failing the debug build ./build.sh mono+libs -os Android with missing data type (gboolean).🤔 Fixed in 4156ab6.

@lambdageek, sure. I will add a case in resolver tests.

[am11]

  [ 17%] Building C object mono/mini/CMakeFiles/monosgen-objects.dir/__/metadata/sgen-mono.c.o
  /Users/adeel/projects/runtime/src/mono/mono/metadata/native-library.c:677:138: warning: type specifier missing, defaults to 'int' [-Wimplicit-int]
  netcore_resolve_with_dll_import_resolver_nofail (MonoAssemblyLoadContext *alc, MonoAssembly *assembly, const char *scope, guint32 flags, is_internal)

Wimplicit-int should be an error in local builds as well, IMO.

[am11]

Added a test and rebased. I had to change the way we resolve self handle on Android: use dlopen(NULL, RTLD_NOW) instead of setting it to RTLD_DEFAULT, which is null on 64-bit platforms and RTLD_NOW is what they use for handle to self.

@lambdageek, please azp run again. 🙂

[lambdageek]

/azp run runtime-android

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).

[am11]

Thank you. So Android legs were green. I have disabled the new test on Windows and monointerpreter:

• free is not availabe in the self image on Windows (which is OK and aligns with @lambdageek's expectation for the test)
• monointerpreter is not OK, but that is a preexisting problem. Calling free on self handle via P/Invoke breaks IL interpreter on all mono supported OS. We should fix it (separately; not part of this PR).

[lambdageek]

/azp run runtime-android

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).

[am11]

@lambdageek, it's green (including Android AOT offsets). Good to merge & backport now? :)

[lambdageek]

/azp run runtime-android

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).

[lambdageek]

/backport to release/7.0

[github-actions]

Started backporting to release/7.0: https://github.com/dotnet/runtime/actions/runs/3534964800

[lambdageek]

/azp run runtime-androidemulator

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).

[lambdageek]

New test passed on Android emulator. LGTM. Thanks @am11