GetCustomAttributes throws Exception when attribute contains a type defined in a dynamic assembly.

ceresgalax commented 6 months ago

Description

When GetCustomAttributes<T>() is called on a type defined in a dynamic assembly created with AssemblyBuilder which has a custom attribute referencing another type in the dynamic assembly, a System.IO.FileNotFoundException exception is thrown.

Also note that when adding a Resolve callback to the AppDomain or ApplicationLoadContext and returning the AssemblyBuilder that contains the dynamic assembly, an exception will be thrown: System.InvalidOperationException: Dynamically emitted assemblies are unsupported during host-based resolution., so this is not a possible workaround.

Reproduction Steps

using System.Reflection;
using System.Reflection.Emit;

namespace ReproDAGetCustomAttributes;

[AttributeUsage(AttributeTargets.Class)]
public class MyCustomAttribute(Type type) : Attribute
{
    private readonly Type _type = type;
}

public static class Entry
{
    public static void Main()
    {
        AssemblyName an = new("MyDnamicAssembly");
        AssemblyBuilder ab = AssemblyBuilder.DefineDynamicAssembly(an, AssemblyBuilderAccess.RunAndCollect);
        ModuleBuilder mb = ab.DefineDynamicModule(an.Name!);

        TypeBuilder typeABuilder = mb.DefineType("TypeA", TypeAttributes.Class | TypeAttributes.Public);
        Type typeA = typeABuilder.CreateType();

        TypeBuilder typeBBuilder = mb.DefineType("TypeB", TypeAttributes.Class | TypeAttributes.Public);
        CustomAttributeBuilder cab = new(typeof(MyCustomAttribute).GetConstructor([typeof(Type)])!, [typeA]);
        typeBBuilder.SetCustomAttribute(cab);
        Type typeB = typeBBuilder.CreateType();

        typeB.GetCustomAttributes<MyCustomAttribute>(); // Throws exception
    }
}

Expected behavior

typeB.GetCustomAttributes should not throw an exception, and should return an instance of MyCustomAttribute constructed with the type of TypeA from the dynamic assembly.

Actual behavior

An exception is thrown when attempting to get the custom attributes of TypeB:

System.IO.FileNotFoundException: Could not load file or assembly 'MyDnamicAssembly, Version=0.0.0.0, Culture=neutral, PublicKeyToken=null'. The system cannot find the file specified.
File name: 'MyDnamicAssembly, Version=0.0.0.0, Culture=neutral, PublicKeyToken=null'
   at System.Reflection.RuntimeAssembly.InternalLoad(AssemblyName assemblyName, StackCrawlMark& stackMark, AssemblyLoadContext assemblyLoadContext, RuntimeAssembly requestingAssembly, Boolean throwOnFileNotFound)
   at System.Reflection.TypeNameParser.ResolveAssembly(String assemblyName)
   at System.Reflection.TypeNameParser.GetType(String typeName, ReadOnlySpan`1 nestedTypeNames, String assemblyNameIfAny)
   at System.Reflection.TypeNameParser.Parse()
   at System.Reflection.TypeNameParser.GetTypeHelper(Char* pTypeName, RuntimeAssembly requestingAssembly, Boolean throwOnError, Boolean requireAssemblyQualifiedName)
   at System.Reflection.CustomAttribute._CreateCaObject(RuntimeModule pModule, RuntimeType type, IRuntimeMethodInfo pCtor, Byte** ppBlob, Byte* pEndBlob, Int32* pcNamedArgs)
   at System.Reflection.CustomAttribute.AddCustomAttributes(ListBuilder`1& attributes, RuntimeModule decoratedModule, Int32 decoratedMetadataToken, RuntimeType attributeFilterType, Boolean mustBeInheritable, ListBuilder`1 derivedAttributes)
   at System.Reflection.CustomAttribute.GetCustomAttributes(RuntimeType type, RuntimeType caType, Boolean inherit)
   at System.Attribute.GetCustomAttributes(MemberInfo element, Type attributeType, Boolean inherit)
   at System.Reflection.CustomAttributeExtensions.GetCustomAttributes[T](MemberInfo element)
   at ReproDAGetCustomAttributes.Entry.Main()

Regression?

No response

Known Workarounds

No response

Configuration

Tested with .NET 8.0.0, & 8.0.1 Windows 10 (x64) & macOS 14.3.1 (23D60) (ARM64)

Other information

No response

dotnet-policy-service[bot] commented 6 months ago

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

buyaa-n commented 6 months ago

Closing as duplicate of https://github.com/dotnet/runtime/issues/95829

The solution is discussed on the issue, look https://github.com/dotnet/runtime/issues/95829#issuecomment-1947072758 for example.

ceresgalax commented 6 months ago

Thank you for directing me to that issue.

Unfortunately, using AppDomain.CurrentDomain.AssemblyResolve is not a solution for me, since I am defining my dynamic assembly into a collectible AssemblyLoadContext. Here is an example:

AssemblyLoadContext alc = new AssemblyLoadContext(name: null, isCollectible: true);
AssemblyBuilder ab;
Type typeB;

using (_currentAssemblyLoadContext.EnterContextualReflection()) 
{
    AssemblyName an = new("MyDnamicAssembly");
    ab = AssemblyBuilder.DefineDynamicAssembly(an, AssemblyBuilderAccess.RunAndCollect);
    ModuleBuilder mb = ab.DefineDynamicModule(an.Name!);

    TypeBuilder typeABuilder = mb.DefineType("TypeA", TypeAttributes.Class | TypeAttributes.Public);
    Type typeA = typeABuilder.CreateType();

    TypeBuilder typeBBuilder = mb.DefineType("TypeB", TypeAttributes.Class | TypeAttributes.Public);
    CustomAttributeBuilder cab = new(typeof(MyCustomAttribute).GetConstructor([typeof(Type)])!, [typeA]);
    typeBBuilder.SetCustomAttribute(cab);
    typeB = typeBBuilder.CreateType();
}

AppDomain.CurrentDomain.AssemblyResolve += (sender, args) => 
{
    return args.Name == ab.FullName ? ab : null;
};

typeB.GetCustomAttributes<MyCustomAttribute>(); // Throws exception

In this example, the exception thrown is: System.NotSupportedException: Resolving to a collectible assembly is not supported.

I am using a collectible AssemblyLoadContext so that I can re-create this dynamic assembly during runtime and unload the prior dynamic assembly after a new one is created.

I'd expect GetCustomAttributes to be able to use AssemblyLoadContext.CurrentContextualReflectionContext to find the dynamic assembly. I'm not aware of any workarounds for this.

jkotas commented 6 months ago

This looks like a bug to me that we should fix. It should just work, without any workarounds.

buyaa-n commented 6 months ago

Unfortunately, using AppDomain.CurrentDomain.AssemblyResolve is not a solution for me, since I am defining my dynamic assembly into a collectible AssemblyLoadContext. Here is an example:

Ah, thank you for the example, debugging the sample code shows that RuntimeAssemblyBuilder loads the correct (CurrentContextualReflectionContext). I suspect the issue in the AssemblyLoader not in reflection emit

https://github.com/dotnet/runtime/blob/71f8fb65a5a28018901823c19de57fe9451ab3b1/src/coreclr/System.Private.CoreLib/src/System/Reflection/Emit/RuntimeAssemblyBuilder.cs#L57-L58

@elinor-fung could you take a look and change the area if needed?

elinor-fung commented 6 months ago

The underlying reason here is that AssemblyBuilder.DefineDynamicAssembly does not add the dynamic assembly to the ALC's cache of loaded assemblies that gets used when resolving assemblies by name. I don't think we'd want to start adding them to the cache, since anyone could emit a dynamic assembly (or multiple even with the same name) - that could break any resolution for a different assembly that happens to have the same name.

elinor-fung commented 6 months ago

I'm actually not entirely sure why AssemblyLoadContext.Resolving blocks returning dynamic assemblies (seems to be that way since coreclr existed) - @jkotas do you happen to know?

One reason I see would be the caching. The only thing I could think of would be to try to relax that if possible and internally avoid what that block is guarding against - like making sure dynamic assemblies returned don't get added to the cache, other things?

jkotas commented 6 months ago

I don't think we'd want to start adding them to the cache, since anyone could emit a dynamic assembly (or multiple even with the same name) - that could break any resolution for a different assembly that happens to have the same name.

I agree. On the other hand, it is very unexpected that assembly self-reference via type reference in a custom attribute does not work.

The cleanest would be to give each dynamic assembly its own ALC. What would break if we have done that?

why AssemblyLoadContext.Resolving blocks returning dynamic assemblies (seems to be that way since coreclr existed) - @jkotas do you happen to know?

I do not know. My guess that there was some problem with caching or lifetime tracking as you have said.

jkotas commented 6 months ago

workarounds

On .NET 9, you can use PersistedAssemblyBuilder, save it into a MemoryStream, and load the MemoryStream into any assembly load context you want. It will avoid the odd corner cases in handling of dynamic assemblies in the runtime.

elinor-fung commented 6 months ago

The cleanest would be to give each dynamic assembly its own ALC. What would break if we have done that?

I imagine dependencies on other ALCs would have problems or confusing behaviour. Currently, AssemblyLoadContext.EnterContextualReflection() gets the assembly for AssemblyBuilder.DefineDynamicAssembly to be associated with / resolve dependencies against a specific ALC. If dynamic assemblies are always loaded into their own ALC, it would effectively stop respecting that contextual setting and potentially lead to similar confusion as what exists around Assembly.LoadFile always loading into a new ALC.

jkotas commented 6 months ago

We should be able to respect the contextual setting in the dedicated ALC. The Load method of the dedicated ALC can forward to the contextual assemblyLoadContext from DefineDynamicAssembly.

steveharter commented 6 months ago

Is this an accurate summary?

This is a bug and should be fixed without a workaround
Jan suggests that each AssemblyBuilder should have its own ALC (pending whether or not this breaks stuff too badly)
An ALC should be able to have a dependency on another ALC with shared types (supported today)
~The user still needs to call ALC.EnterContextualReflection (is this correct?)~
- ~Fix "System.NotSupportedException: Resolving to a collectible assembly is not supported."~
A potential workaround is to leverage the 9.0 AssemblyBuilder.Save feature.

jkotas commented 6 months ago

The user still needs to call ALC.EnterContextualReflection (is this correct?)

This is not correct - I have edited your comment. I agree with the rest.

buyaa-n commented 3 months ago

@elinor-fung is this still planned to be fixed in 9.0?

elinor-fung commented 3 months ago

I am not working on this. I don't believe anyone else is/has either.

Also note that giving each AssemblyBuilder its own ALC, assuming it doesn't break common scenarios, would still be an observable change (for example, it would still be seen in the API for getting the assembly's ALC), so I think it would fall under a breaking change.

buyaa-n commented 3 months ago

OK, moving to future for now

dotnet / runtime