WIP API Proposal - AssemblyBuilder.DefineDynamicAssembly for AssemblyLoadContext

[sdmaclea]

https://docs.microsoft.com/en-us/dotnet/api/system.reflection.emit.assemblybuilder.definedynamicassembly?view=netcore-3.0

AssemblyBuilder is implemented as a type of RuntimeAssembly, but there seems to be no mechanism to create it directly in a specific AssemblyLoadContext.

I assume we need at least one mechanism.

This issue is highly similar to that described in #29042

If we chose a similar solution, we would need 2 changes.

1. Make existing APIs sensitive to AssemblyLoadContext.CurrentContextualReflectionContext
2. Add new explicit APIs

We may be able to support 1 above in 3.0 as this may be a simple PR.
It would be nice to add the new APIs for 3.1, but this may not meet the shiproom approval process

/cc @vitek-karas @jkotas @cablooshe @elinor-fung @jeffschwMSFT @sbomer

[sdmaclea]

Maybe these automatically should get their own private AssemblyLoadContext?

[sdmaclea]

Or maybe these should not be deriving from RuntimeAssembly but rather Assembly. It seems very odd to be building an assembly while it is already loaded.

[jkotas]

deriving from RuntimeAssembly but rather Assembly

I am not sure how this would look like. RuntimeAssembly is internal impl detail of CoreLib.

[jkotas]

Make existing APIs sensitive to AssemblyLoadContext.CurrentContextualReflectionContext

This sounds good to me. AssemblyBuilder.DefineDynamicAssembly is one of those APIs that uses the implicit load context of the immediate caller: https://github.com/dotnet/coreclr/blob/master/src/vm/assembly.cpp#L561

[vitek-karas]

👍 I agree we should try to modify the existing APIs in 3.0. The new API - we'll see, this is relatively small change, so maybe it would still be possible to fit it in.

[dh1dm]

Wouldn't it be possible to provide an AssemblyLoadContext.DefineDynamicAssembly function ? The Contextual-API would require extensive locking when using thread-bound AssemblyLoadContexts, which is very common when migrating from AppDomain to AssemblyLoadContext.

[sdmaclea]

@dh1dm Adding a new API is the preferred approach. The issue was discovered too late to be included in 3.0.

require extensive locking when using thread-bound AssemblyLoadContexts

Not sure why any locking would be required as AssemblyLoadContext.CurrentContextualReflectionContext is stored in an AsyncLocal<T>.

[dh1dm]

Ok, that's true, thanks for info.

[Adriien-M]

What is the status of this issue?

It's a very good proposal because we cannot define a dynamic assembly on a new AppDomain (this documentation is by consequent completely wrong).

Moreover, regarding to this issue dotnet/docs#10149, dynamic assemblies are never collected by the garbage collector so currently, it's not possible to create assemblies and unload them which is a huge problem...

In the meantime, if this feature is not included in 3.0, is there a workaround to create assemblies and collect them? (my goal is to create types during a request lifetime)

[vitek-karas]

/cc @janvorli who might know some tricks how to unload dynamic assemblies

[jkotas]

dynamic assemblies are never collected by the garbage collector

Dynamic assemblies created with AssemblyBuilderAccess.RunAndCollect are collected by the garbage collector. Try to pass AssemblyBuilderAccess.RunAndCollect to AssemblyBuilder.DefineDynamicAssembly.

[Adriien-M]

My assemblies are created with RunAndCollect flag, however they are never collected...
This is a test class which shows this issue (last assertion fails, weak reference is still alive because assembly is not collected)

    [TestClass]
    public class TypeTest
    {
        [TestMethod]
        public void CheckIfAssemblyIsCollected()
        {
            Type type = CreateType();
            object obj = Activator.CreateInstance(type);
            WeakReference weak = new WeakReference(type);
            type = null;
            obj = null;

            Assert.IsTrue(weak.IsAlive);
            GC.Collect();
            GC.WaitForPendingFinalizers();
            Assert.IsFalse(weak.IsAlive);
        }

        static Type CreateType()
        {
            AssemblyBuilder ab = AssemblyBuilder.DefineDynamicAssembly(new AssemblyName("foo"), AssemblyBuilderAccess.RunAndCollect);
            ModuleBuilder modb = ab.DefineDynamicModule("foo.dll");
            TypeBuilder tb = modb.DefineType("Foo");

            return tb.CreateType();
        }
    }

Am I doing something wrong or is it a bug?

[vitek-karas]

I think this is probably caused by inlining. This works (prints out True and then False):

        static void Main(string[] args)
        {
            var weak = T();

            Console.WriteLine(weak.IsAlive);
            GC.Collect();
            GC.WaitForPendingFinalizers();
            Console.WriteLine(weak.IsAlive);
        }

        [MethodImpl(MethodImplOptions.NoInlining)]
        static WeakReference T()
        {
            Type type = CreateType();
            object obj = Activator.CreateInstance(type);
            WeakReference weak = new WeakReference(type);

            return weak;
        }

        static Type CreateType()
        {
            AssemblyBuilder ab = AssemblyBuilder.DefineDynamicAssembly(new AssemblyName("foo"), AssemblyBuilderAccess.RunAndCollect);
            ModuleBuilder modb = ab.DefineDynamicModule("foo.dll");
            TypeBuilder tb = modb.DefineType("Foo");

            return tb.CreateType();
        }

[Adriien-M]

@vitek-karas Thanks for your reply, your code works even without NoInlining flag. I guess this is because object instance is declared in separated function but never used in the Main one.
Now, if I change the test to use the created object (this is the goal), test fails:

    [TestClass]
    public class TypeTest
    {
        [TestMethod]
        public void CheckIfAssemblyIsCollected()
        {
            var instance = T();
            instance = null;

            GC.Collect();
            GC.WaitForPendingFinalizers();

            Assert.IsFalse(AppDomain.CurrentDomain.GetAssemblies().Any(a => a.GetName().Name == "foo"));
        }

        [MethodImpl(MethodImplOptions.NoInlining)]
        static object T()
        {
            Type type = CreateType();
            object obj = Activator.CreateInstance(type);

            return obj;
        }

        static Type CreateType()
        {
            AssemblyBuilder ab = AssemblyBuilder.DefineDynamicAssembly(new AssemblyName("foo"), AssemblyBuilderAccess.RunAndCollect);
            ModuleBuilder modb = ab.DefineDynamicModule("foo.dll");
            TypeBuilder tb = modb.DefineType("Foo");

            return tb.CreateType();
        }
    }

Here, set the instance to null is not enough, so am I obliged to always use a WeakReference? Because in this case it works...

[janvorli]

Here, set the instance to null is not enough, so am I obliged to always use a WeakReference?

setting a variable to null doesn't mean that there is no reference to the object anymore. JIT can create helper locals with longer lifetime. If you put the lines that create the instance into a separate non-inlineable function and call that from CheckIfAssemblyIsCollected, it should work too. That would ensure that the reference to your object cannot escape to some place that you don't expect.

[Adriien-M]

Hmmm, I tried to use your solution, however, as soon as I work with the instance, assembly isn't collected anymore...
I made this sample which create an expression and invoke it and the test fails. It seems really hard to track what is done on the instance and impossible to release all things related to the type or type's instances...

    [TestClass]
    public class TypeTest
    {
        [TestMethod]
        public void CheckIfAssemblyIsCollected()
        {
            var instance = T();

            GC.Collect();
            GC.WaitForPendingFinalizers();

            Assert.IsFalse(AppDomain.CurrentDomain.GetAssemblies().Any(a => a.GetName().Name == "foo"));
        }

        [MethodImpl(MethodImplOptions.NoInlining)]
        static WeakReference T()
        {
            Type type = CreateType();
            object obj = Activator.CreateInstance(type);

            var expr = CreateTestExpression(type);
            var test = expr.Compile().DynamicInvoke(obj); // Comment this line works

            return new WeakReference(obj);
        }

        static LambdaExpression CreateTestExpression(Type type)
        {
            var funcType = typeof(Func<,>).MakeGenericType(type, typeof(bool));
            var param = Expression.Parameter(type);

            return Expression.Lambda(funcType, Expression.TypeIs(param, type), new ParameterExpression[] { param });
        }

        static Type CreateType()
        {
            AssemblyBuilder ab = AssemblyBuilder.DefineDynamicAssembly(new AssemblyName("foo"), AssemblyBuilderAccess.RunAndCollect);
            ModuleBuilder modb = ab.DefineDynamicModule("foo.dll");
            TypeBuilder tb = modb.DefineType("Foo");
            
            return tb.CreateTypeInfo().AsType();
        }
    }

If yourself or someone else have another suggestion I'm listening because currently, I'm really stuck and the initial proposal seems to be the only way to achieve this...

[sdmaclea]

@Adriien-M The path you are on seems like it may eventually work.

If you want to also try the unloadable AssemblyLoadContext, that should also work. See https://github.com/dotnet/corefx/issues/38426#issuecomment-500723986. Basically if the caller is running in an unloadable AssemblyLoadContext The new assembly created by AssemblyBuilder.DefineDynamicAssembly will also be in the same unloadable AssemblyLoadContext.

You may run into similar issues as you are experiencing.

[janvorli]

The problem is that AppDomain.CurrentDomain.GetAssemblies keeps the assembly alive as it generates an array of assemblies. If you put the check into a separate non-inlineable function and call it, it will work (I've verified that):

        [MethodImpl(MethodImplOptions.NoInlining)]
	public static bool CheckAssemblyExists()
        {
	    return AppDomain.CurrentDomain.GetAssemblies().Any(a => a.GetName().Name == "foo");
        }

Also, you may need to call the GC.Collect(); GC.WaitForPendingFinalizers(); in a loop until the CheckAssemblyExists returns false and limit the loop count to say 10. If it doesn't succeed in those 10 iterations, it means failure.

[janvorli]

In fact, you will need to have the loop there, as it is clear that one iteration was not enough from the behavior of your code.

[Adriien-M]

@janvorli Thanks indeed it works if I have a loop, can I assume that Gabrage Collector will always collect my dynamic assemblies even if I don't call it explicitly?

@sdmaclea I do not understand completely the strategy, can you give me a sample to try? :)
I have to create a class extending AssemblyLoadContext and then call AssemblyBuilder.DefineDynamicAssembly in the Load method? Afterwards, as .NET Core 2 doesn't implement Unload method, the garbage collector will unload it itself?

[vitek-karas]

The loop calling GC.Collect is only to make GC happened right then. The dynamic assemblies will be cleaned up whenever GC does run. So without the loop, it will happen eventually. In theory the dynamic assemblies left around should only be adding to memory pressure - which GC will notice and eventually clean it up. Do you have other reasons why you're looking for unload to be deterministic?

The AssemblyBuilder.DefineDynamicAssembly will use the ALC of the caller - so the workaround is to call it from a method which itself is in assembly loaded into the separate ALC. That is not the Load method. So for example let's have an assembly Test.dll which has a method CreateCode which then calls DefineDynamicAssembly. If you make sure that Test.dll is loaded into a separate ALC and call the CreateCode on it, the dynamic assembly created should also go into that separate ALC.

[Adriien-M]

Ok I get it, what solution seems to be the best for you? Use ALC along with separated dll or continue to create my dynamic assemblies in my application?

The goal is to execute an expression on dynamic object. More precisely, expression and object structure are parsed from an XML file, so I create the type dynamically (recursively for nested types), then I build an Expression with this type as a parameter. From there, I can compile my expression and invoke it on an object that I converted previously from JSON to this dynamic type....
It's a little tricky but it works great, my only concern is to be sure that my generated assemblies are well collected, otherwise, the memory usage will grow indefinitely... Especially that these assemblies are used only at the expression invocation, once I get the result, they become useless

[vitek-karas]

I would rely on dynamic assemblies alone - adding ALCs is just gonna complicate things - I think.

If in your case the XML is static - I would probably try to precompute all of that and if it's not too big, just leave it all in memory. Creating types per-request feels very expensive. The runtime should be able to free them and so on, but it's not going to be fast.

[Adriien-M]

I agree with you, unfortunately, types can change and new types are often defined.
So I didn't want to create and store a type as soon as the type was a new one or an evolution, but it probably remains the best solution.

[Ap0k]

Please see this PR #48072

[vitek-karas]

I filed dotnet/dotnet-api-docs#5321 for some of the documentation issues around this (mentioned by several comments above).

[vitek-karas]

#48072 implemented the change in existing APIs. I'm leaving this issue opened to track the work to add a new API which can explicitly state the load context to use.

[Ap0k]

@vitek-karas DefineDynamicAssembly API Extension PR #48353