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

AssemblyLoadContext: Consider adding AssemblyLoadContext.Current API #7642

Closed
gokarnm opened this issue Mar 16, 2017 · 4 comments
Closed

AssemblyLoadContext: Consider adding AssemblyLoadContext.Current API #7642

gokarnm opened this issue Mar 16, 2017 · 4 comments
Assignees
Labels
area-AssemblyLoader-coreclr enhancement Product code improvement that does NOT require public API changes/additions
Milestone

Comments

@gokarnm
Copy link

gokarnm commented Mar 16, 2017

Right now the AssemblyLoadContext class exposes a Default static property that returns the default load context (AppPathAssemblyLoadContext). Consider adding an AssemblyLoadContext.Current static property that returns the current AssemblyLoadContext from which the calling type's assembly is resolved. Here are some data points to support this API.

  • ALC is poorly documented and generally not very well understood. Usually third party library/framework developers don't need to deal with this class unless they are loading dynamic assemblies or loading assemblies outside of the TPA list. It's easy to incorrectly use AssemblyLoadContext.Default to load assemblies instead of using the current ALC, as AssemblyLoadContext.Default is discoverable and the developer may not know the implications of loading into default vs. current ALC. A developer may not be even aware of 'current ALC' as a concept.
  • Bugs related to loading in default ALC instead of current ALC may surface only when someone loads the affected library in a custom ALC and subsequently sees assembly resolution errors. This may not be a popular use case, may not have been envisioned and tested by the library developer.
  • By providing a new AssemblyLoadContext.Current API with appropriate documentation, it would be clearer to the library developer and possibly force to think about using default or current ALC as per their use case and intended behavior.

As an example, here is an issue in MVC due to use of incorrect ALC for loading - aspnet/Mvc#5960 .

@rahku
Copy link
Contributor

rahku commented Mar 17, 2017

In order for this to work, api would need to find the assembly of the caller. I am not aware of a reliable way to find the assembly of the caller when jit can do cross module inlining & tailcall optimizations.
@jkotas do you know

@jkotas
Copy link
Member

jkotas commented Mar 17, 2017

Right, getting caller assembly has always been a problematic construct.

I am not sure whether Current property would add clarity. It does not reflect the fact that it is important which assembly the code is executing in. For example, it is easy to miss that fact during refactoring when moving the code from one assembly to another. I would need to be something like AssemblyLoadContext.GetAssemblyLoadContextOfExecutingAssembly().

It is not that different from AssemblyLoadContext.GetLoadContext(Assembly.GetExecutingAssembly()) that you can write with NS 2.0.

@rahku
Copy link
Contributor

rahku commented Mar 17, 2017

Closing this we already have a PR dotnet/coreclr#10271 out for documentation

@rahku rahku closed this as completed Mar 17, 2017
@gokarnm
Copy link
Author

gokarnm commented Mar 18, 2017

Thanks for the feedback and the ALC documentation!

@msftgits msftgits transferred this issue from dotnet/coreclr Jan 31, 2020
@msftgits msftgits added this to the 2.0.0 milestone Jan 31, 2020
@ghost ghost locked as resolved and limited conversation to collaborators Dec 25, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-AssemblyLoader-coreclr enhancement Product code improvement that does NOT require public API changes/additions
Projects
None yet
Development

No branches or pull requests

4 participants