Repository navigation
TypeDescriptor caching blocks unloading of code which uses it from unloadable load context #30656
Description
The implementation of TypeDescription and TypeConverter uses internal caches which hold strong references to the Type object. These remain even after the descriptors and converters are released by the user code.
If these types come from collectible assemblies (either dynamic types, or custom unloadable AssemblyLoadContext), this prevents unloading of such assembly.
This is exaggerated by the fact that TypeConverter is used by Newtonsoft.Json during serilization or deserialization of JSON. This is VERY common in user code. This effectively means that any app which uses plugins and wants to be able to unload the plugins successfully would have to prevent all such plugins from using Newtonsoft.Json serialization on objects from the plugin.
This is basically a feature ask to modify all the internal caches in TypeDescriptor/TypeConverter implementation to be based on weak references, so that it's possible to collect the types.
Customer issue which ran into this, with detailed analysis of what happens: https://github.com/dotnet/coreclr/issues/26271
It was possible to workaround in this case by loading the ComponentModel assemblies many times into separate ALCs, but that comes with large performance penalty and is not an obvious solution to the problem.
Activity
vitek-karas commentedon Aug 22, 2019
/cc @janvorli
Wraith2 commentedon Aug 22, 2019
Would it make sense to make the caches ALC specific so that converters caches in each context don't leak and wait for collection to occur? [edit] I've just realised that they've internal to a third party assembly so that might not be the best solution but it might still be possible.
vitek-karas commentedon Aug 22, 2019
That could also work - not sure which solution is best for perf...
Currently there's no framework-wide mechanism to attach something to ALC, so it would have to be a custom solution (basically a weak hash table of all ALCs). It's kind of interesting because the
TypeDescriptor.s_providerTableis already aWeakHashtableand couple of others are as well, but not all. I don't know the history of this code, why it was done this way.tarekgh commentedon Aug 23, 2019
@vitek-karas I assume this issue is not meeting the 3.0 bar so I marked it as for 5.0 release. Let me know if this is not the case.
vitek-karas commentedon Aug 23, 2019
I agree it does not meet 3.0, but I think we should at least consider it for 3.1 - it blocks lot of unloadability scenarios - with no good workaround.
Wraith2 commentedon Aug 23, 2019
Just having a look through the code this change might be as simple as changing some
HashtabletoWeakHashtablebut it's going to have some performance degradations under load. There are also a number of places whereWeakReference.IsAliveis used which I know is now suggested against so could be changed at the same time.maryamariyan commentedon Oct 22, 2019
Talked offline with @vitek-karas and here is a possible action plan forward for this issue:
Possibly a simple fix is to replace one of the strong references with a weak reference. But we also want to check for perf regressions and another concern is for cases when GC after the fix clears the cache.
Also note, a sample repro for this issue is in description of https://github.com/dotnet/coreclr/issues/26271
ericstj commentedon Aug 26, 2020
Not seeing this meet the bar for 5.0 at this point either. We haven't seen a ton of interest in this outside of this report. The risk here is substantial as this code has been around for decades now and is a critical piece of windows App models like Winforms and WPF. We'd need to be very careful when touching this not to change behavior nor impact perf substantially. We're not prepared to take that amount of risk at this point in the product.
38 remaining items
zgabi commentedon Sep 12, 2025
Similar issue exists in System.Text.Json... it puts someting to the JsonSerializationOptions.s_defaultOptions._cachingContext._cache dictionary even when I create a new JsonSerializationOptions object for the serialization.
Creation stacktrace:
When I call the System.Text.Json.JsonSerializerOptionsUpdateHandler.ClearCache method with reflection, I can unload the assembly.
vitek-karas commentedon Sep 15, 2025
@zgabi The System.Text.Json problem is tracked by #65323.
alexey-zakharov commentedon Sep 16, 2025
we've used the same workaround for now. I' assume this can be fixed using the similar ConditionalWeakTable approach, however checking the IsCollectible property has currently performance overhead
ericstj commentedon Sep 25, 2025
If we backport we also need #119743.
alexey-zakharov commentedon Sep 29, 2025
yes, definitely!
@ericstj I'm happy to do the backports - would it be ok for the process? 😄
ericstj commentedon Sep 29, 2025
@jeffhandley and I were discussing if this is ready for .NET 10 or not. It's tough because this definitely feels like it still has risk, and it seems like there is a workaround. We found some of the fallout from it (perf regression) but is there more?
What we could use your help with is the template:
runtime/.github/workflows/backport.yml
Lines 25 to 45 in 50570c0
Customer impact and Risk sections.
Do you have a workaround? Is it still valuable to get this in, even without the STJ issue you mentioned above?
@jkotas -- what is your take here, are you comfortable with this change and the IsCollectible fix being backported?
jkotas commentedon Sep 30, 2025
For the servicing risk point, I would be ok with backporting. These are not high-risk changes.
From the customer impact point, I do not see a strong justification:
alexey-zakharov commentedon Sep 30, 2025
yes, we have the fix in our fork and can continue with that
zgabi commentedon Sep 30, 2025
Now there are a lot of separated reflection caches in the runtime. Why don't you use a common cache in the (Runtime)Type object?
I read recently that now the assemblyqualified name is also cached.
Maybe you could add an internal m_cache field to the RuntimeType, and all the system caches could be in this object. So no extra size when a Type is not used with reflection and no weak table needed, just access the appropriate field of the m_cache object. The aqn could be added also to this cache so the size of the Type object remains the same.
+Similar for the MethodInfos.
jkotas commentedon Sep 30, 2025
The separate reflection caches are in higher level libraries and packages. These higher-level libraries do not have access to Corelib internals. Your suggestion would require a new public API. I cannot think about a good public API shape that would be significantly better than
ConditionalWeakTable.zgabi commentedon Sep 30, 2025
It doesn’t really need to be public... you could just use something like UnsafeAccessor - that should be fast enough, right? At least faster than looking up an element in a dictionary (in a ConditionalWeakTable).
That said, even in this case it’d probably be cleaner to keep the cache class public, otherwise things get a bit messy.. but at least this way the class itself wouldn’t be directly exposed on the Type object.... or just put each cache into it's own field of the Type object, but it is increasing the size of the object
jkotas commentedon Sep 30, 2025
It needs to be public to follow the engineering practices of this project.
ericstj commentedon Oct 1, 2025
Sounds like we closed on the servicing question for this issue, as such I'm going to close this one. @zgabi if you had a proposal for reflection API can you open a new issue?