Cannot unload a collectible AssemblyLoadContext if JsonConvert.Serialize() has been called on a custom object

[jvuoti]

If you execute Newtonsoft.Json.JsonConvert.Serialize() on a custom object defined in an assembly in a collectible AssemblyLoadContext, the AssemblyLoadContext can no longer be unloaded.

We noticed this issue in our testing as we are planning to use the collectible AssemblyLoadContext feature to port our plugin code from .NET framework to .NET Core 3.0. Our plugin code currently uses AppDomains and Json.NET for serializing the parameters and results for the plugin calls. Furthermore, the parameter structure classes are defined in the plugin assemblies, so this issue now blocks our .NET Core plugins from unloading.

The unload problem seems to be caused by TypeDescriptor caching: JsonConvert.Serialize() internally seems to call TypeDescriptor.GetConverter(type) on the custom object type, which adds it to the static TypeDescriptor caches. As the TypeDescriptor is loaded in the default LoadContext, the TypeDescriptor caches will keep the references to the custom types alive, and block the plugin assembly from unloading.

A simple reproduction based on the Unloading sample can be found here: https://github.com/jvuoti/samples/tree/jsonconvert_blocking_assemblyloadcontext_unload/core/tutorials/Unloading

In the sample, the Logger plugin dependency has been modified to serialize a CustomLogMessage object, also defined in the same plugin assembly:

    public class Logger
    {
        public class CustomLogMessage
        {
            public string LogMessage { get; set; }
        }

        public static void LogMessage(string msg)
        {
            var logMsg = JsonConvert.SerializeObject(new CustomLogMessage {LogMessage = msg});
            Console.WriteLine(logMsg);
        }
    }

Once the modified plugin code is called, the sample can no longer unload the AssemblyLoadContext.

The only way we have found to get the AssemblyLoadContext to unload is to first clear the internal TypeDescriptor caches via reflection, like this:

    var typeConverterAssembly = typeof(TypeConverter).Assembly;
    var reflectTypeDescriptionProviderType = typeConverterAssembly.GetType("System.ComponentModel.ReflectTypeDescriptionProvider");

    var reflectTypeDescriptorProviderTable = reflectTypeDescriptionProviderType.GetField("s_attributeCache", BindingFlags.Static | BindingFlags.NonPublic);
    var attributeCacheTable = (Hashtable)reflectTypeDescriptorProviderTable.GetValue(null);
    attributeCacheTable.Clear();

However, this does not really feel right :)

So are we just doing things wrong, and would there be a simpler workaround for this? E.g. is there some way we could force a copy of the TypeConverter to be loaded inside the collectible AssemblyLoadContext, so the caches would also be inside it?

[hoyosjs]

@janvorli

[josalem]

CC @sdmaclea @vitek-karas

[vitek-karas]

This is one of the known issues with the unload in .NET Core. Caches. They tend to hold strong references to things they really should only have weak references to.

There's no ideal solution other than fixing Newtonsoft.Json in this case (to use weak refs).

A workaround would be to load the Newtonsoft.Json into every load context separately. You can do this by overriding the Load method on the AssemblyLoadContext and calling this.LoadFromAssemblyPath when there's a request for Newtonsoft.Json.

The downside of this approach is:

• performance impact - every copy of Newtonsoft.Json will be JITTed again - also more memory and so on.
• type differences - if you're actually passing around types from Newtonsoft.Json between the plugins and the app (like JValue and so on), then this won't work, since the host will see different types than the plugin and you'll get weird InvalidCastException: Can't cast JValue to JValue.

[jvuoti]

Thanks, I now tried to explicitly force Newtonsoft.Json to be loaded in collectible AssemblyLoadContext, but it did not help.

I updated the HostAssemblyLoadContext in the repro sample like so:

    protected override Assembly Load(AssemblyName name)
    {
        if (name.Name == "Newtonsoft.Json")
        {
            string jsonNetAssemblyPath = _resolver.ResolveAssemblyToPath(name);
            Console.WriteLine($"Loading Json.NET assembly {jsonNetAssemblyPath} into the HostAssemblyLoadContext");
            return LoadFromAssemblyPath(jsonNetAssemblyPath);
        }

        if (name.Name == "System.ComponentModel.TypeConverter")
        {
            // never goes here!
            throw new FileNotFoundException();
        }
        ...

When running the sample, the console shows that Newtonsoft.Json.dll is loaded to the HostAssemblyLoadContext, but still the unload fails.

I debugged the sample with dotMemory and it shows that the reference to the System.Reflection.LoaderAllocator instance is still being held in HashTables in System.ComponentModel.TypeDescriptor and System.ComponentModel.ReflectTypeDescriptionProvider.

If I understand correctly, TypeDescriptor and ReflectTypeDescriptionProvider are loaded from the framework assembly System.ComponentModel.TypeConverter - and the framework assemblies always seem to get loaded to the default load context.

I also tried forcing the System.ComponentModel.TypeConverter assembly to be loaded to the HostAssemblyLoadContext as in the above sample, but the load is never attempted.

Soo, any chance in fixing System.ComponentModel.TypeDescriptor to use weak refs? ;) Although that would probably then be an issue for the corefx repo...

Or could there be some way of loading such framework assemblies to the collectible AssemblyLoadContexts?

[janvorli]

You should be able to load everything from the framework / runtime except the System.Private.CoreLib.dll into the unloadable context (or maybe just the System.ComponentModel.TypeConverter). That's something that our nightly unloadability tests do. It is not recommended for general usage, but it should solve your issue, hopefully not introducing other problems.
However, the AssemblyDependencyResolver would never attempt to do that, you'd need to resolve the path to the assemblies yourself.

[jvuoti]

Thanks, after studying the test code here (as well as some trial and error), we finally got the unload working.

The trick was to load the netstandard.dll to the collectible AssemblyLoadContext, too. Otherwise System.ComponentModel.TypeConverter.dll was always loaded from the default context. Perhaps the type forwards in the netstandard assembly always point to the same load context?

Both assemblies also needed to be loaded to the AssemblyLoadContext when creating it, as was done in the tests. Trying to resolve them in the Load method seems to happen too late.

I updated the sample, and now the unload works there. Our own plugin load contexts also now seems unload cleanly after the changes. We'll need to test it more to see if the explicit load of the assemblies causes causes issues, but so far everything seems to work.

Thanks for all the help!

[jkotas]

load everything from the framework / runtime except the System.Private.CoreLib.dll into the unloadable context

Loading netcoreapp framework multiple times in the same process or loading it as unloadable is unsupported configuration. Many framework assemblies (e.g. networking stack) are incompatible with unloading. They won't unload, leak or crash.

[paviad]

@jvuoti Brilliant workaround! thank you so much!

This should be resolved in the framework tho.

[rrs]

Can you point me in the direction of the tests that helped you please @jvuoti

[jvuoti]

Can you point me in the direction of the tests that helped you please @jvuoti

Sorry, don't remember the exact details anymore, but we were probably looking at the System.Runtime.Loader tests. However, we were just mostly doing trial and error, running the repro sample in a memory profiler to see where the references were being held. I think the tests pointed us to try loading the referenced assemblies explicitly to the AssemblyLoadContext on program init, which then worked.

[rrs]

I ended up experimenting in the end and this works in my proof on concept, it might help anyone else with the same issue

/// <summary>
/// Work around for https://github.com/dotnet/runtime/issues/13283
/// </summary>
class NewtonsoftIsolationHelper
{
    private static readonly string[] RequiredFrameworkAssemblyNames = new[]
    {
        "netstandard",
        "System.ComponentModel.TypeConverter"
    };

    private static readonly IEnumerable<string> RequireFrameworkAssemblyLocations = AssemblyLoadContext.Default.Assemblies
        .Where(a => RequiredFrameworkAssemblyNames.Any(n => string.Equals(a.GetName().Name, n, StringComparison.OrdinalIgnoreCase))).Select(o => o.Location);

    public static void LoadRequiredFrameworkAssemblies(AssemblyLoadContext loadContext)
    {
        foreach (var a in RequireFrameworkAssemblyLocations)
        {
            loadContext.LoadFromAssemblyPath(a);
        }
    }
}

and just invoke it on any AssemblyLoadContext you wish (in my case all of them) at creation like so

var loadContext = new AssemblyLoadContext("/path/to/plugin.dll");
NewtonsoftIsolationHelper.LoadRequiredFrameworkAssemblies(loadContext);

[groogiam]

@rrs I am trying to get your code working but I cannot seem to get the netstandard dll loaded. It does not seem to exist in AssemblyLoadContext.Default.Assemblies. Any insights you can provide on how to get this loaded?

[rrs]

@groogiam
I only did a basic proof of concept.
Its a netcoreapp3.1 wpf project.
what framework are you targeting?
at what point are you using the code?

[groogiam]

.netcoreapp3.1 in an asp.net core service originally but I've since moved to just trying to get it working in a console application. I was trying to use the NewtonsoftIsolationHelper above but netstandard does not get loaded by default in my netcoreapp3.1 console app.

[rrs]

looked into it, and its a bit of a pain.
They aren't loaded until they are required, which in this case is by the non default assembly load context (ALC). Sadly there aren't the right hooks on the default ALC to grab them and load them on the non default ALC after the fact.

solution below is a bit hacky but I see no other way.

/// <summary>
/// Work around for https://github.com/dotnet/runtime/issues/13283
/// </summary>
public class NewtonsoftIsolationHelper
{
    private static readonly string[] RequiredFrameworkAssemblyNames = new[]
    {
        "netstandard",
        "System.ComponentModel.TypeConverter"
    };

    private static Lazy<IEnumerable<string>> RequireFrameworkAssemblyLocations = new Lazy<IEnumerable<string>>(() =>
    {
        // force newtonsoft to load in the default assembly load context, so that it in turn loads the required assemblies
        // there aren't the right hooks into the default assembly load context to get them on natural resolution
        JsonConvert.SerializeObject(new { });
        return AssemblyLoadContext.Default.Assemblies
            .Where(a => RequiredFrameworkAssemblyNames.Any(n => string.Equals(a.GetName().Name, n, StringComparison.OrdinalIgnoreCase)))
            .Select(o => o.Location);

    });

    public static void LoadRequiredFrameworkAssemblies(AssemblyLoadContext loadContext)
    {
        foreach (var a in RequireFrameworkAssemblyLocations.Value)
        {
            loadContext.LoadFromAssemblyPath(a);
        }
    }
}