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

Fix recursive resource lookup #4095

Merged
merged 4 commits into from
Oct 27, 2022

Conversation

nohwnd
Copy link
Member

@nohwnd nohwnd commented Oct 26, 2022

Description

Check for recursive resource lookup.

This can happen when we are on non-english locale, and we try to load mscorlib.resources
(or potentially some other resources). This will trigger a new Resolve and call the method
we are currently in. If then some code in this Resolve method (like File.Exists) will again
try to access mscorlib.resources it will end up recursing forever.

So we check if the currently looked up assembly is a resource, and if it is we push it into stack
and then check the stack on additional resource lookup.

Related issue

AB#1654923

Copy link
Member

@Evangelink Evangelink left a comment

Choose a reason for hiding this comment

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

Thanks for the quick fix! Could you please add some tests?

@nohwnd
Copy link
Member Author

nohwnd commented Oct 27, 2022

Pushed, not sure if it will work, I was able to test that it fails locally, but I can't build because of some problems around manifest. So let's try to see if CI will pass.

@nohwnd
Copy link
Member Author

nohwnd commented Oct 27, 2022

@Evangelink works, approve?

@Evangelink Evangelink merged commit fbfe1e9 into microsoft:main Oct 27, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants