Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow using active host context in hostfxr_get_runtime_delegate #86460

Merged
merged 7 commits into from
May 20, 2023

Conversation

jander-msft
Copy link
Member

These changes allow for passing nullptr for the handle parameter of the hostfxr_get_runtime_delegate function. This allows in-process callers who do not have the real host context handle to invoke this function and get a runtime delegate for the primary runtime instance. Calling hostfxr_get_runtime_delegate without the handle will only work if there is an active host context, otherwise it will return StatusCode::HostInvalidState.

For testing, I decided to add a new test specifically for ensuring that calling hostfxr_get_runtime_delegate without the handle will work instead of updating the existing tests. Most of the other tests would not have worked with this enablement because it requires an active context handle to be set, which only largely occurs if calling hostfxr_get_runtime_delegate with an existing handle or calling fx_muxer_t::run_app; rather than forcing these to unnecessarily invoke hostfxr_get_runtime_delegate twice (one with a handle, the next without), I created a separate test to do just that.

Fixes #84143

@ghost ghost added the community-contribution Indicates that the PR has been added by a community member label May 18, 2023
@ghost
Copy link

ghost commented May 18, 2023

Tagging subscribers to this area: @vitek-karas, @agocke, @VSadov
See info in area-owners.md if you want to be subscribed.

Issue Details

These changes allow for passing nullptr for the handle parameter of the hostfxr_get_runtime_delegate function. This allows in-process callers who do not have the real host context handle to invoke this function and get a runtime delegate for the primary runtime instance. Calling hostfxr_get_runtime_delegate without the handle will only work if there is an active host context, otherwise it will return StatusCode::HostInvalidState.

For testing, I decided to add a new test specifically for ensuring that calling hostfxr_get_runtime_delegate without the handle will work instead of updating the existing tests. Most of the other tests would not have worked with this enablement because it requires an active context handle to be set, which only largely occurs if calling hostfxr_get_runtime_delegate with an existing handle or calling fx_muxer_t::run_app; rather than forcing these to unnecessarily invoke hostfxr_get_runtime_delegate twice (one with a handle, the next without), I created a separate test to do just that.

Fixes #84143

Author: jander-msft
Assignees: -
Labels:

area-Host

Milestone: -

Copy link
Member

@vitek-karas vitek-karas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good - I'll leave the final call to @elinor-fung .

src/native/corehost/fxr/fx_muxer.cpp Outdated Show resolved Hide resolved
src/native/corehost/test/nativehost/host_context_test.cpp Outdated Show resolved Hide resolved
Copy link
Member

@elinor-fung elinor-fung left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good - just some minor comments. Thank you for doing this!

src/native/corehost/fxr/fx_muxer.cpp Outdated Show resolved Hide resolved
src/native/corehost/fxr/fx_muxer.h Outdated Show resolved Hide resolved
src/native/corehost/fxr/fx_muxer.cpp Outdated Show resolved Hide resolved
src/native/corehost/test/nativehost/host_context_test.cpp Outdated Show resolved Hide resolved
@jander-msft
Copy link
Member Author

jander-msft commented May 19, 2023

I'm going to hazard a guess that the single failure is not really due to my change. I don't see a way of rerunning that one build leg in GH or AzDO (maybe I just don't have permissions). Is there some way to do it without starting an entirely new build?

@vitek-karas
Copy link
Member

Yes - they're unrelated. If you go to Checks and then on the left open Build Analysis, it shows that all the failures are known issues.

Thanks a lot for this PR!

@elinor-fung elinor-fung merged commit 408729c into dotnet:main May 20, 2023
@elinor-fung
Copy link
Member

Thanks, @jander-msft!

@ghost ghost locked as resolved and limited conversation to collaborators Jun 19, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-Host community-contribution Indicates that the PR has been added by a community member
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Unable to use hosting APIs to load managed assemblies from an ICorProfilerCallback implementation
3 participants