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.