Ensure that Native AOT libraries are not unloaded - #131960
Conversation
|
Azure Pipelines: Successfully started running 3 pipeline(s). 13 pipeline(s) were filtered out due to trigger conditions. There may be pipelines that require an authorized user to comment /azp run to run. |
There was a problem hiding this comment.
Pull request overview
This PR updates NativeAOT’s Unix PAL to prevent unloading the NativeAOT module by using RTLD_NODELETE, and updates the NativeAOT SharedLibrary smoke test to actually call dlclose on Unix.
Changes:
- Add best-effort “pin in memory” behavior on Unix by calling
dlopen(..., RTLD_NODELETE)inPalGetModuleHandleFromPointer. - Update the NativeAOT SharedLibrary smoke test to call
dlclose(handle)on Unix (matching the intent of exercising an unload attempt).
Show a summary per file
| File | Description |
|---|---|
| src/tests/nativeaot/SmokeTests/SharedLibrary/SharedLibrary.cpp | Switch Unix path to call dlclose to exercise the “attempted unload” scenario. |
| src/coreclr/nativeaot/Runtime/unix/PalUnix.cpp | Attempt to apply RTLD_NODELETE to the module containing a given pointer to prevent unload on supporting libcs. |
Copilot's findings
- Files reviewed: 2/2 changed files
- Comments generated: 1
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.
Note
This error may be related to your runner configuration. You can now configure runners for Copilot code review separately from Copilot cloud agent by creating a copilot-code-review.yml file with your setup steps. Read the docs for details.
There was a problem hiding this comment.
Copilot's findings
Suppressed comments (3)
src/tests/nativeaot/SmokeTests/SharedLibrary/SharedLibrary.cpp:80
- The test currently calls dlclose/FreeLibrary but doesn't verify the library stays loaded afterwards. Since the goal is to ensure NativeAOT shared libraries are not unloadable, add a post-unload check (GetModuleHandle on Windows; dlopen with RTLD_NOLOAD on Unix) so the test fails if the module actually unloads without crashing immediately.
// to unload the library does not to crash at least.
#ifdef TARGET_WINDOWS
FreeLibrary(handle);
#else
dlclose(handle);
src/coreclr/nativeaot/Runtime/unix/PalUnix.cpp:866
- The pinning path calls dlopen(..., RTLD_NOLOAD) but ignores whether it succeeded. If dlopen returns null, the module may still be unloadable despite pinModule=true. Consider checking the return value and falling back to a plain dlopen (which should just bump the refcount for an already-loaded image) to make pinning reliable.
if (pinModule && info.dli_fname != nullptr)
{
// NativeAOT runtime state cannot be safely unloaded.
// Keep the extra reference for the lifetime of the process.
dlopen(info.dli_fname, RTLD_LAZY | RTLD_NOLOAD);
src/coreclr/nativeaot/BuildIntegration/Microsoft.NETCore.Native.Unix.targets:294
- _targetOS is normalized to 'linux' for linux-musl and linux-bionic as well (see SingleEntry.targets). If this flag is intended for glibc only, checking for '$(_linuxLibcFlavor)' == 'glibc' won't work (glibc is represented by an empty _linuxLibcFlavor). Consider excluding the known non-glibc flavors instead (musl/bionic).
<!-- NativeAOT shared libraries should not be unloaded -->
<LinkerArg Include="-Wl,-z,nodelete" Condition="'$(_targetOS)' == 'linux' and '$(NativeLib)' == 'Shared'" />
- Files reviewed: 7/7 changed files
- Comments generated: 0 new
There was a problem hiding this comment.
Copilot's findings
Suppressed comments (2)
src/tests/nativeaot/SmokeTests/SharedLibrary/SharedLibrary.cpp:81
dlclose(handle)is now executed on every non-Windows Unix target, but the runtime-side pinning in this PR only runs onHOST_OSX(and the linker-z,nodeleteis Linux-only). This means platforms like FreeBSD/OpenBSD may actually unload the NativeAOT shared library here, which the comment above explicitly says is not supported, potentially turning this smoke test into a platform-specific crash/regression.
Consider limiting the dlclose attempt to the platforms where we have a non-unload guarantee in this PR (Apple + Linux), and keep the handle open elsewhere until/unless we add equivalent pinning.
#ifdef TARGET_WINDOWS
FreeLibrary(handle);
#else
dlclose(handle);
#endif
src/coreclr/nativeaot/Runtime/unix/PalUnix.cpp:869
pinModuleis currently only acted on forHOST_OSX, and even there it usesRTLD_NOLOADwithoutRTLD_NODELETE(and without checking the result). This makes thepinModulecontract inconsistent across Unix platforms and doesn’t match the PR description’s claim of usingRTLD_NODELETEwhere supported.
It would be more robust to honor pinModule on all non-WASM Unix platforms by taking an extra reference via dlopen(..., RTLD_NOLOAD) and, when available, also setting RTLD_NODELETE. That way Linux/glibc and other ELF Unixes get the same behavior, and the code doesn’t silently ignore pinModule on most Unix targets.
#if defined(HOST_OSX)
if (pinModule && info.dli_fname != nullptr)
{
// NativeAOT runtime state cannot be safely unloaded.
// Keep the extra reference for the lifetime of the process.
- Files reviewed: 7/7 changed files
- Comments generated: 0 new
…e.Unix.targets Co-authored-by: Jan Kotas <jkotas@microsoft.com>
Co-authored-by: Jan Kotas <jkotas@microsoft.com>
There was a problem hiding this comment.
Review details
Suppressed comments (3)
src/coreclr/nativeaot/Runtime/unix/PalUnix.cpp:866
- On macOS, the pinning path calls dlopen() with RTLD_NOLOAD only, which doesn't match the PR description of using RTLD_NODELETE to make the NativeAOT shared library non-unloadable. Adding RTLD_NODELETE here makes the intent explicit and avoids relying on reference-counting semantics of RTLD_NOLOAD.
// Unloading is disabled via `-z,nodelete` linker option on ELF platforms.
dlopen(info.dli_fname, RTLD_LAZY | RTLD_NOLOAD);
}
#endif
moduleHandle = info.dli_fbase;
}
src/tests/nativeaot/SmokeTests/SharedLibrary/SharedLibrary.cpp:81
- This test currently closes the library handle but never exercises any exported entrypoint afterward, so it doesn't actually validate that the library remained loaded/pinned (the main behavior this PR is trying to guarantee). Calling an exported function after FreeLibrary/dlclose will reliably fail (often via crash) if the library was actually unloaded.
#ifdef TARGET_WINDOWS
FreeLibrary(handle);
#else
dlclose(handle);
#endif
src/tests/nativeaot/SmokeTests/SharedLibrary/SharedLibrary.cpp:80
- Grammar: "does not to crash" should be "does not crash".
// to unload the library does not to crash at least.
#ifdef TARGET_WINDOWS
FreeLibrary(handle);
#else
dlclose(handle);
- Files reviewed: 7/7 changed files
- Comments generated: 0 new
- Review effort level: Lite
There was a problem hiding this comment.
Review details
Suppressed comments (2)
src/coreclr/nativeaot/Runtime/unix/PalUnix.cpp:863
- When
pinModuleis requested on macOS, the result ofdlopen(..., RTLD_NOLOAD)is ignored. Ifdlopenfails for any reason, the module remains unloadable and the runtime will still initialize successfully, which undermines the intent ofpinModule. Consider checking for failure and failing initialization (returningNULL) when the extra reference can’t be acquired.
// NativeAOT runtime state cannot be safely unloaded.
// Keep the extra reference for the lifetime of the process.
// Unloading is disabled via `-z,nodelete` linker option on ELF platforms.
dlopen(info.dli_fname, RTLD_LAZY | RTLD_NOLOAD);
}
src/tests/nativeaot/SmokeTests/SharedLibrary/SharedLibrary.cpp:80
- The test now calls
dlclose(handle), but it doesn’t verify that the library wasn’t actually unloaded (e.g., on a platform/config where pinning fails, unload+reload could still succeed silently here because nothing uses the library afterward). Consider re-opening the library afterdlcloseand invoking an exported function again so the test actually validates the “not unloadable” behavior.
dlclose(handle);
- Files reviewed: 7/7 changed files
- Comments generated: 0 new
- Review effort level: Lite
|
/ba-g all legs green |
As cDAC is a NativeAOT library, it cannot be safely unloaded. #131960 should prevent an actual unmapping from happening, but this makes the intent clear. --------- Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Ensure that NAOT libraries are not unloaded by opening them with the `RTLD_NODELETE` flag. Mac and glibc support this flag, and musl libc does not support unloading libraries. Fixes dotnet#64629 --------- Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> Co-authored-by: Jan Kotas <jkotas@microsoft.com>
As cDAC is a NativeAOT library, it cannot be safely unloaded. dotnet#131960 should prevent an actual unmapping from happening, but this makes the intent clear. --------- Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Ensure that NAOT libraries are not unloaded by opening them with the
RTLD_NODELETEflag. Mac and glibc support this flag, and musl libc does not support unloading libraries.Fixes #64629