System.Text.Json should support unloadable assemblies correctly

[eiriktsarpalis]

I think this should make the cache work reasonably well with collectible assemblies, but it's hard to tell for sure. Could you please add a test which:

• Creates a new unloadable ALC and loads some assembly with test types into it
• Serializes/deserializes objects of the types from the assembly above
• Unloads the ALC
• Validates that the ALC was actually unloaded

Good guide what to do is here: https://docs.microsoft.com/en-us/dotnet/standard/assembly/unloadability

In short what we want to avoid:
Global GC root which holds onto anything from unloadable ALCs as that will prevent the ALC from unloading. We already have quite a few caches in the FX which do this, so let's not add another one.

Originally posted by @vitek-karas in #64646 (comment)

Tagging subscribers to this area: @vitek-karas, @agocke, @VSadov
See info in area-owners.md if you want to be subscribed.

Issue Details

---

I think this should make the cache work reasonably well with collectible assemblies, but it's hard to tell for sure. Could you please add a test which:

• Creates a new unloadable ALC and loads some assembly with test types into it
• Serializes/deserializes objects of the types from the assembly above
• Unloads the ALC
• Validates that the ALC was actually unloaded

Good guide what to do is here: https://docs.microsoft.com/en-us/dotnet/standard/assembly/unloadability

In short what we want to avoid:
Global GC root which holds onto anything from unloadable ALCs as that will prevent the ALC from unloading. We already have quite a few caches in the FX which do this, so let's not add another one.

Originally posted by @vitek-karas in #64646 (comment)

Author:	eiriktsarpalis
Assignees:	-
Labels:	area-AssemblyLoader-coreclr, untriaged
Milestone:	-

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

Issue Details

---

I think this should make the cache work reasonably well with collectible assemblies, but it's hard to tell for sure. Could you please add a test which:

• Creates a new unloadable ALC and loads some assembly with test types into it
• Serializes/deserializes objects of the types from the assembly above
• Unloads the ALC
• Validates that the ALC was actually unloaded

Good guide what to do is here: https://docs.microsoft.com/en-us/dotnet/standard/assembly/unloadability

In short what we want to avoid:
Global GC root which holds onto anything from unloadable ALCs as that will prevent the ALC from unloading. We already have quite a few caches in the FX which do this, so let's not add another one.

Originally posted by @vitek-karas in #64646 (comment)

Author:	eiriktsarpalis
Assignees:	-
Labels:	area-System.Text.Json, untriaged
Milestone:	-

[eiriktsarpalis]

From #64646 (comment)

In short what we want to avoid:
Global GC root which holds onto anything from unloadable ALCs as that will prevent the ALC from unloading. We already have quite a few caches in the FX which do this, so let's not add another one.

System.Text.Json already suffers from that issue since we're rooting the default JsonSerializerOptions instance (and any caches it may create):

runtime/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializerOptions.cs

Line 35 in cff924f

public static JsonSerializerOptions Default { get; } = new JsonSerializerOptions { _haveTypesBeenCreated = true };

We need to work out an eviction policy that takes into account the rooted instance as well, however that's not in scope for the current PR, which only addresses the performance issue.

[eiriktsarpalis]

I'm not sure what the best solution would be here. @stephentoub added JsonSerializerOptionsUpdateHandler to support hot reload. Perhaps we should expose something like this for users to call into when unloading their ALC?

[teo-tsirpanis]

Perhaps using ConditionalWeakTable to store the types will help. I will prepare a PR at a later time.

[stephentoub]

Perhaps using ConditionalWeakTable to store the types will help. I will prepare a PR at a later time.

CWT could easily result in types getting dropped from the cache way earlier than they should, just because the GC happened to run, in turn resulting in huge increases in costs for JsonSerializer.

It's possible it could play a role, but any such changes will require very careful measurement.

[teo-tsirpanis]

Can a Type instance get collected before it gets unloaded? I didn't know that. And I see the CWT was considered but turned down in

runtime/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializerOptions.Caching.cs

Line 111 in a25aa66

/// Defines a cache of CachingContexts; instead of using a ConditionalWeakTable which can be slow to traverse

Another thing I thought is to add an Unload event handler on the unloadable ALC of a type whose serialization info is going to get cached, but it needs some care to add only one handler per ALC.