JsonConverter constructor causes MissingMetadataException upon instantiation

[Sergio0694]

Description

We're currently working on migrating all our JSON serialization from Newtonsoft.Json to System.Text.Json in the Microsoft Store (see also #77897), and we're hitting some issues with trimming (we're on .NET Native). In particular, this line:

https://github.com/dotnet/runtime/blob/264d7391ec9f6e698051db0621c5e090d0ae4710/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonConverter.cs#L19

This is crashing when trimming is enabled, because the linker will remove support for getting the assembly info from types. We can fix this by adding some .rd.xml directives, but it's error prone and not really a great solution. Eg. we can use:

<Type Name="System.Text.Json.Serialization.Converters.ObjectDefaultConverter`1" Activate="Required Public" />
<Type Name="System.Text.Json.Serialization.Converters.DictionaryOfTKeyTValueConverter`3" Activate="Required Public" />
<Type Name="System.Text.Json.Serialization.Converters.ListOfTConverter`2" Activate="Required Public" />

Etc. for all converters we need. It'd be much better if this was just fixed in System.Text.Json directly. I'm aware that reflection-free mode isn't supported (see #68093), but fixing this would also benefit other scenarios (such as our case) by still allowing the linker to just trim out more metadata and reduce the binary size further.

Note: to clarify, the ask is not to support the reflection-free mode, just to make this path friendlier to trimming.

cc. @eiriktsarpalis @MichalStrehovsky

Reproduction Steps

The repro is pretty much the same as in the linked issue:

string json = """
    {
        "SomeMapping": { "A": "B" },
        "SomeList": ["A", "B"]
    }
    """;

_ = System.Text.Json.JsonSerializer.Deserialize(rawJson, MicrosoftStoreJsonSerializerContext.Default.SomeModel);

public sealed class SomeModel
{
    public Dictionary<string, string> SomeMapping { get; set; }
    public List<string> SomeList { get; set; }
}

[JsonSerializable(typeof(SomeModel))]
[JsonSourceGenerationOptions(GenerationMode = JsonSourceGenerationMode.Metadata)]
internal sealed partial class MicrosoftStoreJsonSerializerContext : JsonSerializerContext
{
}

Expected behavior

This should just work fine.

Actual behavior

We're getting a MissingMetadataException:

at System.Reflection.Runtime.TypeInfos.RuntimeNoMetadataNamedTypeInfo.get_Assembly() in f:\\dd\\ndp\\fxcore\\CoreRT\\src\\System.Private.Reflection.Core\\src\\System\\Reflection\\Runtime\\TypeInfos\\RuntimeNoMetadataNamedTypeInfo.cs:line 38 
at System.Reflection.Runtime.TypeInfos.RuntimeConstructedGenericTypeInfo.get_Assembly() in f:\\dd\\ndp\\fxcore\\CoreRT\\src\\System.Private.Reflection.Core\\src\\System\\Reflection\\Runtime\\TypeInfos\\RuntimeConstructedGenericTypeInfo.cs:line 136
at System.Text.Json.Serialization.JsonConverter`1..ctor(Boolean initialize) 
at System.Text.Json.Serialization.JsonConverter`1..ctor()
at System.Text.Json.Serialization.JsonResumableConverter`1..ctor() 
at System.Text.Json.Serialization.JsonObjectConverter`1..ctor()
at System.Text.Json.Serialization.Converters.ObjectDefaultConverter`1..ctor()
at System.Text.Json.Serialization.Metadata.SourceGenJsonTypeInfo`1.<>c.<GetConverter>b__3_1()
at System.Func`1.Invoke()
at System.Text.Json.Serialization.Converters.JsonMetadataServicesConverter`1.get_Converter() 
at System.Text.Json.Serialization.Converters.JsonMetadataServicesConverter`1.get_ElementType() 
at System.Text.Json.Serialization.Metadata.JsonTypeInfo..ctor(Type type, JsonConverter converter, JsonSerializerOptions options)
at System.Text.Json.Serialization.Metadata.JsonTypeInfo`1..ctor(JsonConverter converter, JsonSerializerOptions options) 
at System.Text.Json.Serialization.Metadata.SourceGenJsonTypeInfo`1..ctor(JsonSerializerOptions options, JsonObjectInfoValues`1 objectInfo)
at System.Text.Json.Serialization.Metadata.JsonMetadataServices.CreateObjectInfo[T](JsonSerializerOptions options, JsonObjectInfoValues`1 objectInfo) 
at SomeProject.MicrosoftStoreJsonSerializerContext.Create_SomeModel(JsonSerializerOptions options)
at SomeProject.MicrosoftStoreJsonSerializerContext.get_SomeModel()  
at SomeProject.<<TryDeserializeSomeModel>g__Foo|77_0>d.MoveNext() 

Regression?

I have a possible idea on how to fix this, by making that path entirely reflection-free. Consider this:

public abstract class JsonConverter<T>
{
    protected JsonConverter()
    {
        IsInternalType = CheckIsInternalType();
    }

    private protected virtual bool CheckIsInternalType() => false;
}

Now, all converter types in System.Text.Json would just override the method accordingly:

public class SomeSealedJsonConverter<T> : JsonConverter<T>
{
    private protected override bool CheckIsInternalType() => true;
}

public class SomeUnsealedJsonConverter<T> : JsonConverter<T>r
{
    private protected override bool CheckIsInternalType() => GetType() == typeof(SomeUnsealedJsonConverter<T>);
}

This makes sure that:

• External converters directly inheriting from JsonConverter<T> will be marked as external.
• External converters that inherit from unsealed STJ converters will also be marked as external.

Essentially this should provide a reflection-free way of checking whether a concrete converter type is from the STJ assembly.

Configuration

• System.Text.Json 7.0
• .NET Native 6.2.14

[ghost]

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

Issue Details

---

### Description We're currently working on migrating all our JSON serialization from Newtonsoft.Json to System.Text.Json in the Microsoft Store (see also #77897), and we're hitting some issues with trimming (we're on .NET Native). In particular, this line: https://github.com/dotnet/runtime/blob/264d7391ec9f6e698051db0621c5e090d0ae4710/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonConverter.cs#L19 This is crashing when trimming is enabled, because the linker will remove support for getting the assembly info from types. We can fix this by adding some .rd.xml directives, but it's error prone and not really a great solution. Eg. we can use: ```xml ``` Etc. for all converters we need. It'd be much better if this was just fixed in System.Text.Json directly. I'm aware that reflection-free mode isn't supported (see #68093), but fixing this would also benefit other scenarios (such as our case) by still allowing the linker to just trim out more metadata and reduce the binary size further. cc. @eiriktsarpalis @MichalStrehovsky ### Reproduction Steps The repro is pretty much the same as in the linked issue: ```csharp string json = """ { "SomeMapping": { "A": "B" }, "SomeList": ["A", "B"] } """; _ = System.Text.Json.JsonSerializer.Deserialize(rawJson, MicrosoftStoreJsonSerializerContext.CamelCase.DAOfferDataContract); public sealed class SomeModel { public Dictionary SomeMapping { get; set; } public List SomeList { get; set; } } [JsonSerializable(typeof(SomeModel))] [JsonSourceGenerationOptions(GenerationMode = JsonSourceGenerationMode.Metadata)] internal sealed partial class MicrosoftStoreJsonSerializerContext : JsonSerializerContext { } ``` ### Expected behavior This should just work fine. ### Actual behavior We're getting a `MissingMetadataException`: ``` at System.Reflection.Runtime.TypeInfos.RuntimeNoMetadataNamedTypeInfo.get_Assembly() in f:\\dd\\ndp\\fxcore\\CoreRT\\src\\System.Private.Reflection.Core\\src\\System\\Reflection\\Runtime\\TypeInfos\\RuntimeNoMetadataNamedTypeInfo.cs:line 38 at System.Reflection.Runtime.TypeInfos.RuntimeConstructedGenericTypeInfo.get_Assembly() in f:\\dd\\ndp\\fxcore\\CoreRT\\src\\System.Private.Reflection.Core\\src\\System\\Reflection\\Runtime\\TypeInfos\\RuntimeConstructedGenericTypeInfo.cs:line 136 at System.Text.Json.Serialization.JsonConverter`1..ctor(Boolean initialize) at System.Text.Json.Serialization.JsonConverter`1..ctor() at System.Text.Json.Serialization.JsonResumableConverter`1..ctor() at System.Text.Json.Serialization.JsonObjectConverter`1..ctor() at System.Text.Json.Serialization.Converters.ObjectDefaultConverter`1..ctor() at System.Text.Json.Serialization.Metadata.SourceGenJsonTypeInfo`1.<>c.b__3_1() at System.Func`1.Invoke() at System.Text.Json.Serialization.Converters.JsonMetadataServicesConverter`1.get_Converter() at System.Text.Json.Serialization.Converters.JsonMetadataServicesConverter`1.get_ElementType() at System.Text.Json.Serialization.Metadata.JsonTypeInfo..ctor(Type type, JsonConverter converter, JsonSerializerOptions options) at System.Text.Json.Serialization.Metadata.JsonTypeInfo`1..ctor(JsonConverter converter, JsonSerializerOptions options) at System.Text.Json.Serialization.Metadata.SourceGenJsonTypeInfo`1..ctor(JsonSerializerOptions options, JsonObjectInfoValues`1 objectInfo) at System.Text.Json.Serialization.Metadata.JsonMetadataServices.CreateObjectInfo[T](JsonSerializerOptions options, JsonObjectInfoValues`1 objectInfo) at SomeProject.MicrosoftStoreJsonSerializerContext.Create_SomeModel(JsonSerializerOptions options) at SomeProject.MicrosoftStoreJsonSerializerContext.get_SomeModel() at SomeProject.<g__Foo|77_0>d.MoveNext() ``` ### Regression? I have a possible idea on how to fix this, by making that path entirely reflection-free. Consider this: ```csharp // New internal interface in System.Text.Json internal interface IInternalConverter { Type TypeRoot { get; } } // Updated constructor for JsonConverter public abstract class JsonConverter { protected JsonConverter() { IsInternalType = GetType() == (this as IInternalConverter)?.TypeRoot; } } ``` Now, all converter types in System.Text.Json would implement the interface explicitly. like so: ```csharp // Converter in System.Text.Json public class SomeJsonConverter : JsonConverter, IInternalConverter { Type IInternalConverter.TypeRoot => typeof(SomeJsonConverter); } ``` This makes sure that: - External converters directly inheriting from `JsonConverter` won't have the interface - External converters that inherit from unsealed STJ converters won't have a matching type at runtime Essentially this should provide a reflection-free way of checking whether a concrete converter type is from the STJ assembly. ### Known Workarounds _No response_ ### Configuration - System.Text.Json 7.0 - .NET Native 6.2.14 ### Other information _No response_

Author:	Sergio0694
Assignees:	-
Labels:	`area-System.Text.Json`
Milestone:	-

[MichalStrehovsky]

For further context, in .NET Native it's possible to have a type with all of its metadata removed. So the GetType().Assembly call is likely getting a Type object for a type whose metadata was removed. Such Type objects are going to throw (because they don't know their assembly... they don't even know their names).

It's not possible to get such Type object with .NET Core trimming - either PublishAot or PublishTrimmed will always return a Type that always knows its assembly. So this is just a .NET Native issue.

But .NET Native is NetStandard 2.0 and supported so...

It can probably be worked around with:

<Assembly Name="System.Text.Json" Dynamic="All" />

Which tells the .NET Native compiler to keep the metadata for everything that was kept. It will keep more than necessary, but it should be safe for this line of code.

[Sergio0694]

That works, but causes a lot of metadata to be preserved. I narrowed that down a bit and this also seems to work:

<Type Name="System.Text.Json.Serialization.Converters.ObjectDefaultConverter`1">
    <Subtypes Activate="Required Public" />
</Type>

But even just this causes 1MB of increase on the package size, which doesn't seem worth it given the issue is just that line. Adding the individual directives like I mentioned in the OP only adds 40KB of binary size, which is great, though of course it's a bit more brittle and requires additional testing to ensure no converter type is missed.

I guess what I'm trying to say is - it feels like this issue could be fixed relatively easy by using a private protected virtual method like in my proposal, and that would benefit a lot of UWP developers (this is not the first time I see this specific issue causing problems in UWP apps, and not everyone is familiar with .rd.xml directives enough to fix this on their own).

To be clear - I'd be happy to contribute the fix myself 🙂

"It's not possible to get such Type object with .NET Core trimming"

Out of curiosity - I know the reflection-free mode isn't supported in NativeAOT, but would the same still apply to the "enhanced reflection free" mode that was in the works? Ie. the one that would remove all MethodBase support or something (can't remember all the details exactly). As in, would Type.Assembly still be available even in that case anyway?

[eiriktsarpalis]

it feels like this issue could be fixed relatively easy by using a private protected virtual method like in my proposal

I don't believe that would work in the general case. Apart from the fact that each internal derived type would need to be explicitly marked as internal (allowing for the possibility of new internal types potentially missing that declaration), it doesn't account for custom converters deriving from built-in converters that are both unsealed and public. Currently there's only one such class, JsonStringEnumConverter (which is actually a factory, so the value of IsInternalConverter will be imprecise but not necessarily carry any performance impact) but we are planning on adding more public converters like that, see https://github.com/dotnet/runtime/issues/73124#issuecomment-1303267422 and https://github.com/dotnet/runtime/issues/63791.

[Sergio0694]

"it doesn't account for custom converters deriving from built-in converters that are both unsealed and public"

I'm not sure I understand why this wouldn't work, could you elaborate? Consider this case:

// This is public and unsealed
public class InternalConverter<T> : JsonConverter<T>
{
    private protected override bool CheckIsInternalType() => GetType() == typeof(InternalConverter<T>);
}

// This is an external converter that inherits from it
public class ExternalConverter<T> : InternalConverter<T>
{
}

When either is instantiated, you can have two cases:

• If InternalConverter<T> is instantiated (say, it's InternalConverter<string>), that CheckIsInternalType will return true, because it will result in InternalConverter<string> == typeof(InternalConverter<string>).
• If ExternalConverter<T> is instantiated (say, it's ExternalConverter<string>), that CheckIsInternalType will return false, because the check will now result in ExternalConverter<string> == InternalConverter<string>.

Essentially, this solution accounts both cases just fine, as far as I can tell. 🙂

Note: this is also a very similar approach to what MemoryStream does to check against derived types.

"allowing for the possibility of new internal types potentially missing that declaration"

I can see that, but I think we can address that with tests, especially because we know the full set of converters. We might even be able to just come up with a way to fetch and validate all tests via reflection (eg. get all non abstract internal converters, instantiate them with random type arguments, invoke CheckIsInternalType and verify it returns true).

To clarify again, I'm happy to contribute this improvement myself, since it'd benefit us directly (and NativeAOT users) 😄

[eiriktsarpalis]

Consider this case:

I assumed the virtual would simply set a flag, but this is running an equality comparison against the declared type. Would this work if reflection metadata has been stripped?

[Sergio0694]

Yup this will work just fine, even in reflection-free mode you can directly compare Type instances for checking equality (which is exactly what we need), that is explicitly allowed. As in, that would work both on .NET Native (where the entire metadata is stripped) and in NativeAOT reflection-free mode (where the entire reflection stack is gone) 🙂

If performance is a concern, such a type comparison is actually just a direct comparison, so it's also faster than the current code (as another side bonus on top of not using reflection). See sharplab example, where the whole check just compiles to:

JsonSerializer`1[[System.Int32, System.Private.CoreLib]].M()
    L0000: mov rax, 0x7ffb7e5ad460
    L000a: cmp [rcx], rax
    L000d: sete al
    L0010: movzx eax, al
    L0013: ret

[eiriktsarpalis]

Performance shouldn't be a concern, the virtual would be invoked only once by the constructor anyways. What does concern me is maintainability and risk of regression -- clearly our testing pipelines don't cover that particular scenario.

[MichalStrehovsky]

But even just this causes 1MB of increase on the package size, which doesn't seem worth it given the issue is just that line

Try dropping the Required part and leave it at <Subtypes Activate="Public" />. Required roots things that would otherwise be unused.

[Sergio0694]

That doesn't seem to be enough, I tried but unfortunately I still got MissingMetadataException-s with that 🥲

[MichalStrehovsky]

Out of curiosity - I know the reflection-free mode isn't supported in NativeAOT, but would the same still apply to the "enhanced reflection free" mode that was in the works?

The compiler doesn't generate such data structures and I deleted the representation for this from the reflection stack in https://github.com/dotnet/runtime/pull/73612.

[Sergio0694]

"What does concern me is maintainability and risk of regression -- clearly our testing pipelines don't cover that particular scenario."

@eiriktsarpalis if we want to make sure to not miss any types, could we add a test that does the following:

• Go through all public types in System.Text.Json's assembly
• Filter those that are deriving from JsonConverter<T>
• If they're generic, call MakeGenericType on them with some random type (say, object)
• Get a default instance via RuntimeHelpers.GetUninitializedObject
• Invoke CheckIsInternalType on them via reflection
• Verify that it returns true

And we can then also define a couple of converter types in the same test project and check those return false.

Seems like that would give us a reliable way to automatically test all existing converters to ensure this works? What do you think? 🙂

[eiriktsarpalis]

I mean we'll do that if we absolutely have to, but hoping for a cleaner solution or a potential workaround in other platforms that do need this.

[ghost]

Tagging subscribers to 'linkable-framework': @eerhardt, @vitek-karas, @LakshanF, @sbomer, @joperezr See info in area-owners.md if you want to be subscribed.

Issue Details

---

### Description We're currently working on migrating all our JSON serialization from Newtonsoft.Json to System.Text.Json in the Microsoft Store (see also #77897), and we're hitting some issues with trimming (we're on .NET Native). In particular, this line: https://github.com/dotnet/runtime/blob/264d7391ec9f6e698051db0621c5e090d0ae4710/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonConverter.cs#L19 This is crashing when trimming is enabled, because the linker will remove support for getting the assembly info from types. We can fix this by adding some .rd.xml directives, but it's error prone and not really a great solution. Eg. we can use: ```xml ``` Etc. for all converters we need. It'd be much better if this was just fixed in System.Text.Json directly. I'm aware that reflection-free mode isn't supported (see #68093), but fixing this would also benefit other scenarios (such as our case) by still allowing the linker to just trim out more metadata and reduce the binary size further. > **Note:** to clarify, the ask is not to support the reflection-free mode, just to make this path friendlier to trimming. cc. @eiriktsarpalis @MichalStrehovsky ### Reproduction Steps The repro is pretty much the same as in the linked issue: ```csharp string json = """ { "SomeMapping": { "A": "B" }, "SomeList": ["A", "B"] } """; _ = System.Text.Json.JsonSerializer.Deserialize(rawJson, MicrosoftStoreJsonSerializerContext.CamelCase.DAOfferDataContract); public sealed class SomeModel { public Dictionary SomeMapping { get; set; } public List SomeList { get; set; } } [JsonSerializable(typeof(SomeModel))] [JsonSourceGenerationOptions(GenerationMode = JsonSourceGenerationMode.Metadata)] internal sealed partial class MicrosoftStoreJsonSerializerContext : JsonSerializerContext { } ``` ### Expected behavior This should just work fine. ### Actual behavior We're getting a `MissingMetadataException`: ``` at System.Reflection.Runtime.TypeInfos.RuntimeNoMetadataNamedTypeInfo.get_Assembly() in f:\\dd\\ndp\\fxcore\\CoreRT\\src\\System.Private.Reflection.Core\\src\\System\\Reflection\\Runtime\\TypeInfos\\RuntimeNoMetadataNamedTypeInfo.cs:line 38 at System.Reflection.Runtime.TypeInfos.RuntimeConstructedGenericTypeInfo.get_Assembly() in f:\\dd\\ndp\\fxcore\\CoreRT\\src\\System.Private.Reflection.Core\\src\\System\\Reflection\\Runtime\\TypeInfos\\RuntimeConstructedGenericTypeInfo.cs:line 136 at System.Text.Json.Serialization.JsonConverter`1..ctor(Boolean initialize) at System.Text.Json.Serialization.JsonConverter`1..ctor() at System.Text.Json.Serialization.JsonResumableConverter`1..ctor() at System.Text.Json.Serialization.JsonObjectConverter`1..ctor() at System.Text.Json.Serialization.Converters.ObjectDefaultConverter`1..ctor() at System.Text.Json.Serialization.Metadata.SourceGenJsonTypeInfo`1.<>c.b__3_1() at System.Func`1.Invoke() at System.Text.Json.Serialization.Converters.JsonMetadataServicesConverter`1.get_Converter() at System.Text.Json.Serialization.Converters.JsonMetadataServicesConverter`1.get_ElementType() at System.Text.Json.Serialization.Metadata.JsonTypeInfo..ctor(Type type, JsonConverter converter, JsonSerializerOptions options) at System.Text.Json.Serialization.Metadata.JsonTypeInfo`1..ctor(JsonConverter converter, JsonSerializerOptions options) at System.Text.Json.Serialization.Metadata.SourceGenJsonTypeInfo`1..ctor(JsonSerializerOptions options, JsonObjectInfoValues`1 objectInfo) at System.Text.Json.Serialization.Metadata.JsonMetadataServices.CreateObjectInfo[T](JsonSerializerOptions options, JsonObjectInfoValues`1 objectInfo) at SomeProject.MicrosoftStoreJsonSerializerContext.Create_SomeModel(JsonSerializerOptions options) at SomeProject.MicrosoftStoreJsonSerializerContext.get_SomeModel() at SomeProject.<g__Foo|77_0>d.MoveNext() ``` ### Regression? I have a possible idea on how to fix this, by making that path entirely reflection-free. Consider this: ```csharp public abstract class JsonConverter { protected JsonConverter() { IsInternalType = CheckIsInternalType(); } private protected virtual bool CheckIsInternalType() => false; } ``` Now, all converter types in System.Text.Json would just override the method accordingly: ```csharp public class SomeSealedJsonConverter : JsonConverter { private protected override bool CheckIsInternalType() => true; } public class SomeUnsealedJsonConverter : JsonConverterr { private protected override bool CheckIsInternalType() => GetType() == typeof(SomeUnsealedJsonConverter); } ``` This makes sure that: - External converters directly inheriting from `JsonConverter` will be marked as external. - External converters that inherit from unsealed STJ converters will also be marked as external. Essentially this should provide a reflection-free way of checking whether a concrete converter type is from the STJ assembly. ### Configuration - System.Text.Json 7.0 - .NET Native 6.2.14

Author:	Sergio0694
Assignees:	-
Labels:	`enhancement`, `area-System.Text.Json`, `linkable-framework`
Milestone:	8.0.0

[Sergio0694]

The workarounds we have are either to preserve all converter types, which adds too much binary size as it preserves any converter type anywhere even if unused, or manually annotating the ones you need, which only adds a bit more metadata than needed but is acceptable on this front. The downside to this though is that it's a rather brittle solution, as it's fairly easy to miss one especially if a given converter is only used on some specific JSON models. I'd feel much more comfortable if this was just fixed at the source, and it'd allow us to proceed with more confidence to switch over the whole Store to System.Text.Json 😅

Also if it's any help (I know the team is busy), I really wouldn't mind contributing this myself 🙂

[eiriktsarpalis]

Another possible solution is to investigate current usage of the IsInternalConverter flag and possibly replace it with something that doesn't require reflection.

[Sergio0694]

That sounds like an even better idea 😄 Just glancing at the code it's not immediately obvious to me what this property is even for (there's also no comments).

[Sergio0694]

Great to see this being added to the new AOT user story for .NET 8! 🎉

@eiriktsarpalis should we add the partner-impact tag here too, since we're hitting this in the Store? This specific reflection dependency being addressed in the next release would be a pretty nice win for us there 😄

[dongle-the-gadget]

One thing I also want to mention, System.Text.Json apparently treats List<A<B>> as requiring converter JsonConverter<List<A<B>>>, which has its own reflection code: https://github.com/dotnet/runtime/blob/ec9fb02a2c6b606ef06acc911a0b104fd3d2a9a3/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonConverterOfT.cs#L22 That means Sergio's previously mentioned workaround to add possibly every converter System.Text.Json implements needs to be changed to

<Type Name="System.Text.Json.Serialization.JsonConverter`1">
    <Subtypes Activate="Required Public" />
</Type>

for this scenario to work.