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
Register the assembly/library resolvers at early stage #21361
Conversation
This PR has Quantification details
Why proper sizing of changes matters
Optimal pull request sizes drive a better predictable PR flow as they strike a
What can I do to optimize my changes
How to interpret the change counts in git diff output
Was this comment helpful? 👍 :ok_hand: :thumbsdown: (Email) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
This comment was marked as outdated.
This comment was marked as outdated.
1 similar comment
This comment was marked as outdated.
This comment was marked as outdated.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can you think of a way to tickle this to validate the throw behavior?I'm not sure it can be done easily without a hosting scenario, but I wonder if we should. This isn't blocking or imperative for this PR, but I wonder if it would be useful.
@JamesWTruher It's easy to validate the throw-on-reentry behavior. The method |
📣 Hey @daxian-dbw, how did we do? We would love to hear your feedback with the link below! 🗣️ 🔗 https://aka.ms/PSRepoFeedback |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for your contributions.
PR Summary
Fix #20740, Fix #21319
This is an unexpected regression caused by #18356, specifically this line of change in
ExternalScriptInfo
.The native lib resolver and GAC assembly resolvers are registered within the static constructor of
ClrFacade
, and that change reduced the reference surface ofClrFacade
, which caused them to not be registered when runningpwsh -exe bypass -nop -c "ipmo pki"
.The fix is to do the registration to an early stage.
PowerShellAssemblyLoadContext.InitializeSingleton(string.Empty)
is added to the static constructor ofRunspaceBase
, which will run as soon as anything in the startup code path referencesLocalRunspace
orRunspaceBase
. This is to make sure we register the resolvers before PowerShell is ready to run any scripts/cmdlets.However, we still need to keep the singleton initialization in
ClrFacade
becauseClrFacade
can be used without a Runspace created, for example, by calling the type conversion methods inLanguagePrimitive
. So, we attempt to do the singleton initialization at both places.Many thanks to @SeeminglyScience for the help in investigation and validation.
PR Checklist
.h
,.cpp
,.cs
,.ps1
and.psm1
files have the correct copyright headerWIP:
or[ WIP ]
to the beginning of the title (theWIP
bot will keep its status check atPending
while the prefix is present) and remove the prefix when the PR is ready.Resolving
event of the default ALC. It's verified that after this change, the behavior (order of the registered handler) is the same as in PowerShell v7.3 -- PowerShell's handler is registered first and then the VSCode extension's handler.