dotnet / runtime

.NET is a cross-platform runtime for cloud, mobile, desktop, and IoT apps.
https://docs.microsoft.com/dotnet/core/
MIT License
15.47k stars 4.76k forks source link

[API Proposal]: MetadataLoadContext.GetLoadContext(Assembly) #98756

Open IS4Code opened 9 months ago

IS4Code commented 9 months ago

Background and motivation

Unlike AssemblyLoadContext, the similarly named MetadataLoadContext does not have the GetLoadContext method that allows retrieving the original load context from its corresponding Assembly instance. This is inconvenient in situations such as when calling assembly.GetReferencedAssemblies() and attempting to resolve the AssemblyName instances.

The relevant property is already there, just internal and thus inaccessible.

API Proposal

using System.Reflection.TypeLoading;

namespace System.Reflection;

public class MetadataLoadContext
{
    public static MetadataLoadContext? GetLoadContext(Assembly assembly)
    {
        ArgumentNullException.ThrowIfNull(assembly);

        return (assembly as RoAssembly)?.Loader;
    }
}

API Usage

var context = new MetadataLoadContext(...);
var asm = context.LoadFromAssemblyName(...);
Assert.AreEqual(MetadataLoadContext.GetLoadContext(asm), context);

Alternative Designs

Another option, with arguably greater impact but also requirements, would be to make the Assembly class expose its loader through a public property, probably as an interface which both AssemblyLoadContext and MetadataLoadContext would implement. This would work with any potential new load context type, but requires modifying Assembly and thus would not work easily with older platforms, unlike just modifying MetadataLoadContext which is released as a package. It could however remain a possibility even with this API added.

Risks

By accessing the MetadataLoadContext of an Assembly, code may load any arbitrary assemblies into the context, but the related AssemblyLoadContext already provides this functionality where it may be more dangerous than here, and the Loader property is accessible by reflection anyway.

ghost commented 9 months ago

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

Issue Details
### Background and motivation Unlike `AssemblyLoadContext`, the similarly named `MetadataLoadContext` does not have the `GetLoadContext` method that allows retrieving the original load context from its corresponding `Assembly` instance. This is inconvenient in situations such as when calling `assembly.GetReferencedAssemblies()` and attempting to resolve the `AssemblyName` instances. The relevant property is already there, just internal and thus inaccessible. ### API Proposal ```csharp using System.Reflection.TypeLoading; namespace System.Reflection; class MetadataLoadContext { public static MetadataLoadContext? GetLoadContext(Assembly assembly) { ArgumentNullException.ThrowIfNull(assembly); return (assembly as RoAssembly)?.Inner; } } ``` ### API Usage ```csharp var context = new MetadataLoadContext(...); var asm = context.LoadFromAssemblyName(...); Assert.AreEqual(MetadataLoadContext.GetLoadContext(asm), context); ``` ### Alternative Designs Another option, with arguably greater impact but also requirements, would be to make the `Assembly` class expose its loader through a public property, probably as an interface which both `AssemblyLoadContext` and `MetadataLoadContext` would implement. This would work with any potential new load context type, but requires modifying `Assembly` and thus would not work easily with older platforms, unlike just modifying `MetadataLoadContext` which is released as a package. It could however remain a possibility even with this API added. ### Risks By accessing the `MetadataLoadContext` of an `Assembly`, code may load any arbitrary assemblies into the context, but the related `AssemblyLoadContext` already provides this functionality where it may be more dangerous than here, and the `Loader` property is accessible by reflection anyway.
Author: IS4Code
Assignees: -
Labels: `api-suggestion`, `area-AssemblyLoader-coreclr`, `untriaged`
Milestone: -
ghost commented 9 months ago

Tagging subscribers to this area: @dotnet/area-system-reflection See info in area-owners.md if you want to be subscribed.

Issue Details
### Background and motivation Unlike `AssemblyLoadContext`, the similarly named `MetadataLoadContext` does not have the `GetLoadContext` method that allows retrieving the original load context from its corresponding `Assembly` instance. This is inconvenient in situations such as when calling `assembly.GetReferencedAssemblies()` and attempting to resolve the `AssemblyName` instances. The relevant property is already there, just internal and thus inaccessible. ### API Proposal ```csharp using System.Reflection.TypeLoading; namespace System.Reflection; class MetadataLoadContext { public static MetadataLoadContext? GetLoadContext(Assembly assembly) { ArgumentNullException.ThrowIfNull(assembly); return (assembly as RoAssembly)?.Inner; } } ``` ### API Usage ```csharp var context = new MetadataLoadContext(...); var asm = context.LoadFromAssemblyName(...); Assert.AreEqual(MetadataLoadContext.GetLoadContext(asm), context); ``` ### Alternative Designs Another option, with arguably greater impact but also requirements, would be to make the `Assembly` class expose its loader through a public property, probably as an interface which both `AssemblyLoadContext` and `MetadataLoadContext` would implement. This would work with any potential new load context type, but requires modifying `Assembly` and thus would not work easily with older platforms, unlike just modifying `MetadataLoadContext` which is released as a package. It could however remain a possibility even with this API added. ### Risks By accessing the `MetadataLoadContext` of an `Assembly`, code may load any arbitrary assemblies into the context, but the related `AssemblyLoadContext` already provides this functionality where it may be more dangerous than here, and the `Loader` property is accessible by reflection anyway.
Author: IS4Code
Assignees: -
Labels: `api-suggestion`, `area-System.Reflection`, `area-AssemblyLoader-coreclr`, `untriaged`
Milestone: -
jkotas commented 9 months ago

@steveharter This proposal looks good to me. Mark is as api-ready-to-review?

bartonjs commented 7 months ago

Video

namespace System.Reflection;

public partial class MetadataLoadContext
{
    public static MetadataLoadContext? GetLoadContext(Assembly assembly);
}