Fix libhost scenarios (COM, C++/CLI, custom component host) incorrect coreclr path determination #116504
Conversation
… coreclr path determination
|
Tagging subscribers to this area: @vitek-karas, @agocke, @VSadov |
There was a problem hiding this comment.
Pull Request Overview
This PR ensures that in libhost scenarios the host no longer falls back to probing the (empty) app directory, and it adds absolute-path validation for loading coreclr, hostpolicy, and hostfxr. It also adds tests to confirm that hosting components ignore artifacts in the working directory.
- Skip app-directory fallback when
host_mode_t::libhost - Add
pal::is_path_rootedchecks for coreclr, hostpolicy, and hostfxr loads - Add tests to verify working-directory components are not used in native hosting
Reviewed Changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/native/corehost/hostpolicy/standalone/coreclr_resolver.cpp | Added absolute-path check before loading coreclr |
| src/native/corehost/hostpolicy/deps_resolver.cpp | Skip adding app-local probe directories for libhost scenarios and assert app dir |
| src/native/corehost/fxr_resolver.h | Added absolute-path check for hostfxr before loading |
| src/native/corehost/fxr/standalone/hostpolicy_resolver.cpp | Added absolute-path check for hostpolicy before loading |
| src/native/corehost/apphost/standalone/hostfxr_resolver.cpp | Added absolute-path check in standalone apphost hostfxr resolver |
| src/installer/tests/HostActivation.Tests/NativeHosting/NativeHostingResultExtensions.cs | Renamed extension class and added ResolveHostFxr, ResolveHostPolicy, ResolveCoreClr |
| src/installer/tests/HostActivation.Tests/NativeHosting/LoadAssemblyAndGetFunctionPointer.cs | Added test to ensure working directory is ignored when hosting components |
| src/installer/tests/HostActivation.Tests/NativeHosting/Ijwhost.cs | Added test to ensure working directory is ignored in C++/CLI hosting |
| src/installer/tests/HostActivation.Tests/NativeHosting/Comhost.cs | Added test to ensure working directory is ignored in COM hosting |
| // For libhost scenarios, there is no app or app path | ||
| if (m_host_mode != host_mode_t::libhost && !get_app_deps().exists()) | ||
| { | ||
| assert(!m_app_dir.empty()); |
There was a problem hiding this comment.
Using assert here only triggers in debug builds; consider an explicit runtime check with proper error handling so that failures in release builds aren’t silently ignored.
| assert(!m_app_dir.empty()); | |
| if (m_app_dir.empty()) | |
| { | |
| trace::error(_X("Application directory is empty. This is required for resolving dependencies.")); | |
| return false; | |
| } |
Copilot uses AI. Check for mistakes.
| } | ||
|
|
||
| // We should always be loading hostfxr from an absolute path | ||
| if (!pal::is_path_rooted(fxr_path)) |
There was a problem hiding this comment.
[nitpick] The absolute-path validation is repeated across multiple resolvers; consider extracting this check into a shared helper to reduce duplication and improve consistency.
| if (!pal::is_path_rooted(fxr_path)) | |
| if (!fxr_resolver::is_valid_absolute_path(fxr_path)) |
Copilot uses AI. Check for mistakes.
2 participants
There is a fallback for apps with no .deps.json where the host will consider the app directory for loading coreclr. In component hosting scenarios, we do not have an app path / directory. We were incorrectly going down the path of looking for coreclr next to the (empty) app directory.
This change skips that path for libhost scenarios. It also adds checks that the paths we determine for loading coreclr, hostpolicy, and hostfxr are absolute.