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

Skip Init and Static helpers when looking for a caller #111446

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

jkotas
Copy link
Member

@jkotas jkotas commented Jan 15, 2025

We have learned that real world code may call Assembly.GetCallingAssembly in a static constructor. It returns different value now that there is an extra managed frame introduced by HMF removal. The fix adds the managed helper types to the long ad-hoc list of types that should be skipped for this and similar APIs. It does not change the fact that Assembly.GetCallingAssembly is a slow fragile unreliable API that should not be used.

Fixes #111399

We have learned that real world code may call Assembly.GetCallingAssembly in a static constructor. It returns different value now that there is an extra managed frame introduced by HMF removal. The fix adds the managed helper types to the long ad-hoc list of types that should be skipped for this and similar APIs. It does not change the fact that Assembly.GetCallingAssembly is a slow fragile unreliable API that should not be used.

Fixes dotnet#111399
Copy link
Contributor

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

@davidwrighton
Copy link
Member

@jkotas do you want to write a regression test for this? This is a rather odd corner case that looks easy to break in the future to me.

Copy link
Member

@davidwrighton davidwrighton left a comment

Choose a reason for hiding this comment

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

Looks like this should fix the problem, but wow the use of GetCallingAssembly here is a bad idea.

@AaronRobinsonMSFT
Copy link
Member

@jkotas do you want to write a regression test for this? This is a rather odd corner case that looks easy to break in the future to me.

It sounds like we might want a test for the reflection list that Jan added to. I agree this seems very fragile and the API should be avoided.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants