Override corehost_resolve_component_dependencies and corehost_set_error_writer PInvokes in singlefile scenario.
#60046
Conversation
…error_writer` PInvokes in singlefile scenario.
|
Tagging subscribers to this area: @vitek-karas, @agocke, @VSadov |
|
We should try to backport this to 6.0 |
|
The fix for the test failures should be in VSadov#3. |
| @@ -55,11 +56,15 @@ namespace | |||
| const void* STDMETHODCALLTYPE pinvoke_override(const char* libraryName, const char* entrypointName) | |||
| { | |||
| #if defined(_WIN32) | |||
| const char* hostPolicyLib = "hostpolicy.dll"; | |||
There was a problem hiding this comment.
Is the extension needed for hostpolicy, but not for System.IO.Compression.Native on Windows and libhostpolicy on Unix?
There was a problem hiding this comment.
That is the differences between
and
hostpolicy seems to be using a naming approach that is different from System.IO.Compression.Native and I am not sure if there are reasons for that of if changing could introduce any additional risk.
It could be worth to consider removing the ".dll" in 7.0.
This fix will be likely ported to 6.0, so I think it should not be in this change.
| if (strcmp(entrypointName, "corehost_set_error_writer") == 0) | ||
| { | ||
| return (void*)corehost_set_error_writer; | ||
| } |
There was a problem hiding this comment.
An assert and a warning after this block might help us catch other cases in the future. Something like:
| } | |
| } | |
| trace::warning(_X("Hostpolicy P/Invoke override for [%s] was not found in the bundle bundle."), entrypointName); | |
| assert(false); // all cases should be handled above |
There was a problem hiding this comment.
It is ok for any user to try calling methods that do not exist in hostpolicy. It is also (although very unlikely) that unrelated hostpolicy.dll may exist and user PInvokes something from it that is real. Also there could be additional overrides registered at higher level, - in theory.
Basically, while very unlikely, there could be scenarios when not finding an override for a hostpolicy PInvoke is expected and ok.
I think, as a matter of testing in controlled environment, an assert makes sense here, but not a warning, since, strictly speaking this is not a failure.
There was a problem hiding this comment.
Actually even an assert could become a blocking nuisance if someone needs to use Checked/Debug build and there is a call to something that does not exist in hostpolicy.
Otherwise it should be a "method missing" exception that can be caught and handled/ignored.
I think this part should stay as-is.
Fixes the test failures when hostpolicy is used at runtime.
|
The windows x86 Debug failure is happening in other runs too - #60097 |
|
/backport to release/6.0 |
|
Started backporting to release/6.0: https://github.com/dotnet/runtime/actions/runs/1314783140 |
ghost
locked as resolved and limited conversation to collaborators
Re: #59961
When we embed native runtime/helpers statically in the singlefile host, the host also adds PInvoke overrides, so that PInvokes would still work and use embedded implementation.
What we missed is that the host component itself (hostpolicy) has 2 exported functions accessible from managed libraries via PInvokes. This change adds overrides for these as well.