[loader] Set domain->entry_assembly earlier in startup #31814
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
CoffeeFlux
force-pushed
the
ensure-entry-assembly
branch
from
6990628
to
b37d6cb
Compare
lambdageek
approved these changes
Feb 5, 2020
EgorBo
approved these changes
Feb 6, 2020
When locating AppContext.BaseDirectory on netcore, if APP_CONTEXT_BASE_DIRECTORY is not set we call into the runtime for a fallback location, which checks domain->entry_assembly. On legacy this is fine because we set that property in `prepare_thread_to_exec_main`, but on netcore the new managed assembly loading algorithm will call into managed and check the property before that function is run, which will result in AppContext.BaseDirectory returning an empty string. The solution is to set entry_assembly a bit earlier in initialization. This is the appropriate location because it's what embedding API users will call to run an assembly. In the process, remove some code that would bypass the usual lookup algorithm unnecessarily.
CoffeeFlux
force-pushed
the
ensure-entry-assembly
branch
from
b37d6cb
to
dd140b2
Compare
|
Rebased on top of Egor's work; confirmed that it can run the CoreCLR tests without issue. I'm still somewhat hesitant on whether this is the right place to set this, but after sleeping on it I think it's okay with an in-code comment. Will merge once CI is green. |
ghost
added
the
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
When locating AppContext.BaseDirectory on netcore, if APP_CONTEXT_BASE_DIRECTORY is not set we call into the runtime for a fallback location, which checks domain->entry_assembly. On legacy this is fine because we set that property in
prepare_thread_to_exec_main, but on netcore the new managed assembly loading algorithm will call into managed and check the property before that function is run, which will result in AppContext.BaseDirectory returning an empty string. The solution is to set entry_assembly a bit earlier in initialization. This is the appropriate location because it's what embedding API users will call to run an assembly.In the process, I got rid of some code that would bypass the usual lookup algorithm unnecessarily.
Fixes #31800