Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ArgumentNullException in System.Text.Json.Serialization.Metadata.JsonTypeInfo #58690

Closed
Tracked by #63918
JakenVeina opened this issue Sep 5, 2021 · 7 comments · Fixed by #73241
Closed
Tracked by #63918

ArgumentNullException in System.Text.Json.Serialization.Metadata.JsonTypeInfo #58690

JakenVeina opened this issue Sep 5, 2021 · 7 comments · Fixed by #73241
Assignees
Labels
area-System.Text.Json bug linkable-framework Issues associated with delivering a linker friendly framework Priority:3 Work that is nice to have
Milestone

Comments

@JakenVeina
Copy link
Contributor

JakenVeina commented Sep 5, 2021

Description

I am receiving an ArgumentNullException from JsonSerializer.Serialize<T>(Utf8JsonWriter, T value, JsonSerializerOptions options) that appears to be the result of a faulty null-check-override within JsonTypeInfo.

   at System.ThrowHelper.ThrowArgumentNullException(ExceptionArgument argument)
   at System.OrdinalIgnoreCaseComparer.GetHashCode(String obj)
   at System.Text.Json.Serialization.Metadata.JsonTypeInfo.ParameterLookupKey.GetHashCode()
   at System.Collections.Generic.Dictionary`2.FindValue(TKey key)
   at System.Collections.Generic.Dictionary`2.TryGetValue(TKey key, TValue& value)
   at System.Text.Json.Serialization.Metadata.JsonTypeInfo.InitializeConstructorParameters(ConstructorInfo constructorInfo)
   at System.Text.Json.Serialization.Metadata.JsonTypeInfo..ctor(Type type, JsonConverter converter, Type runtimeType, JsonSerializerOptions options)
   at System.Text.Json.JsonSerializerOptions.<RootBuiltInConvertersAndTypeInfoCreator>g__CreateJsonTypeInfo|107_0(Type type, JsonSerializerOptions options)
   at System.Text.Json.JsonSerializer.GetTypeInfo(Type runtimeType, JsonSerializerOptions options)
   at System.Text.Json.JsonSerializer.Serialize[TValue](Utf8JsonWriter writer, TValue& value, Type runtimeType, JsonSerializerOptions options)
   at System.Text.Json.JsonSerializer.Serialize[TValue](Utf8JsonWriter writer, TValue value, JsonSerializerOptions options)
   at Modix.Host.EntryPoint.EndpointWriteOnlyJsonConverter.Write(Utf8JsonWriter writer, Endpoint value, JsonSerializerOptions options) in D:\Projects\JakenVeina\MODiX\MODiX.Host\EntryPoint.cs:line 90

The null value in question is coming from System.Reflection.ParameterInfo.Name, retrieved from the constructor of System.Runtime.CompilerServices.NullableContextAttribute.

The NullableContextAttribute instance is coming from Microsoft.AspNetCore.Http.Endpoint which is being passed to Microsoft.Extensions.Logging.ILogger.Log calls as TState state.

NullableContextAttribute is an internal and compiler-generated type, so it's been proving difficult to implement a workaround where I just skip or customize serialization of the type. It also strikes me as a slight design defect that all this work is being done to analyze this type for deserialization, when I am only attempting to serialize it, and have no intention of ever deserializing it.

Configuration

.NET SDK 6.0.100-preview.7.21379.14
System.Text.Json 6.0.0-preview.7.21377.19

@dotnet-issue-labeler dotnet-issue-labeler bot added area-System.Text.Json untriaged New issue has not been triaged by the area owner labels Sep 5, 2021
@eiriktsarpalis
Copy link
Member

Hi @JakenVeina, would it be possible to provide a minimal app that reproduces the issue? Thanks.

@JakenVeina
Copy link
Contributor Author

It looks like a "minimal" reproduction might not be possible. I have replicated the scenario in a simple console program, using the same runtime and package versions, and the exception does not occur. I.E...

JsonSerializer.Serialize(writer, value, options);

...where value is a NullableContextAttribute object, which, when reflected upon, produces a ParameterInfo object with a Name value of null. This line throws in the full solution, but not in the console program. The options object is not meaningfully different between the two. I noticed there's a decent amount of static caching of JsonTypeInfo objects going on, so perhaps that's somehow the culprit?

I will attempt to clone my full application and trim as much out of it as possible, without affecting the exception that throws.

@JakenVeina
Copy link
Contributor Author

Here we go: https://github.com/JakenVeina/DotnetRuntimeIssue58690

I identified the issue with reproducing it locally: I was inadvertently calling .Serialize<Attribute>() instead of .Serialize(), resulting in the library reflecting upon System.Attribute instead of System.Runtime.CompilerServices.NullableContextAttribute.

@eiriktsarpalis
Copy link
Member

I can reproduce. Minimal reproduction:

using System;
using System.Reflection;
using System.Text.Json;

Type type = Assembly.GetExecutingAssembly().GetType("System.Runtime.CompilerServices.NullableContextAttribute")!;
object value = Activator.CreateInstance(type, (byte)0)!;
JsonSerializer.Serialize(value);

// Force generation of a NullableContextAttribute in this assembly
static string? MethodWithNullableAnnotations() => throw new NotImplementedException();

Throws

Unhandled exception. System.ArgumentNullException: Value cannot be null. (Parameter 'obj')
   at System.OrdinalIgnoreCaseComparer.GetHashCode(String obj)
   at System.Text.Json.Serialization.Metadata.JsonTypeInfo.ParameterLookupKey.GetHashCode()
   at System.Collections.Generic.Dictionary`2.FindValue(TKey key)
   at System.Collections.Generic.Dictionary`2.TryGetValue(TKey key, TValue& value)
   at System.Text.Json.Serialization.Metadata.JsonTypeInfo.InitializeConstructorParameters(JsonParameterInfoValues[] jsonParameters)
   at System.Text.Json.Serialization.Metadata.JsonTypeInfo..ctor(Type type, JsonConverter converter, Type runtimeType, JsonSerializerOptions options)
   at System.Text.Json.JsonSerializerOptions.<InitializeForReflectionSerializer>g__CreateJsonTypeInfo|112_0(Type type, JsonSerializerOptions options)
   at System.Text.Json.JsonSerializerOptions.GetClassFromContextOrCreate(Type type)
   at System.Text.Json.JsonSerializerOptions.GetOrAddClass(Type type)
   at System.Text.Json.JsonSerializer.GetTypeInfo(JsonSerializerOptions options, Type runtimeType)
   at System.Text.Json.JsonSerializer.Serialize[TValue](TValue value, JsonSerializerOptions options)

I did not deduce the root cause -- for some reason attempting to replicate the implementation of NullableContextAttribute in source resulted in a different type of failure much later in the implementation. We should still investigate and at least attempt to fail more gracefully if this scenario cannot be supported.

This seems like a very niche use case. Moving to 7.0.0.

@eiriktsarpalis eiriktsarpalis removed needs more info untriaged New issue has not been triaged by the area owner labels Sep 10, 2021
@eiriktsarpalis eiriktsarpalis added this to the 7.0.0 milestone Sep 10, 2021
@eiriktsarpalis eiriktsarpalis added bug Priority:3 Work that is nice to have labels Sep 10, 2021
@layomia layomia added the source-generator Indicates an issue with a source generator feature label Sep 22, 2021
BobLd added a commit to BobLd/lean-monitor-2 that referenced this issue Nov 25, 2021
@eiriktsarpalis eiriktsarpalis modified the milestones: 7.0.0, Future Apr 18, 2022
@eiriktsarpalis
Copy link
Member

eiriktsarpalis commented Apr 18, 2022

Moving to Future, as we won't have time to work on this in the .NET 7 timeframe.

@eiriktsarpalis
Copy link
Member

Root cause appears to be IL #73188 (comment) containing parameters with stripped names. Given that the linker can trim parameter names to save size we might want to add detection logic for ParameterInfo with null Name properties (and either fail gracefully or use a placeholder name like ILSpy is doing).

@eiriktsarpalis eiriktsarpalis self-assigned this Aug 2, 2022
@eiriktsarpalis eiriktsarpalis modified the milestones: Future, 7.0.0 Aug 2, 2022
@jkotas jkotas added the linkable-framework Issues associated with delivering a linker friendly framework label Aug 2, 2022
@ghost
Copy link

ghost commented Aug 2, 2022

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

I am receiving an ArgumentNullException from JsonSerializer.Serialize<T>(Utf8JsonWriter, T value, JsonSerializerOptions options) that appears to be the result of a faulty null-check-override within JsonTypeInfo.

   at System.ThrowHelper.ThrowArgumentNullException(ExceptionArgument argument)
   at System.OrdinalIgnoreCaseComparer.GetHashCode(String obj)
   at System.Text.Json.Serialization.Metadata.JsonTypeInfo.ParameterLookupKey.GetHashCode()
   at System.Collections.Generic.Dictionary`2.FindValue(TKey key)
   at System.Collections.Generic.Dictionary`2.TryGetValue(TKey key, TValue& value)
   at System.Text.Json.Serialization.Metadata.JsonTypeInfo.InitializeConstructorParameters(ConstructorInfo constructorInfo)
   at System.Text.Json.Serialization.Metadata.JsonTypeInfo..ctor(Type type, JsonConverter converter, Type runtimeType, JsonSerializerOptions options)
   at System.Text.Json.JsonSerializerOptions.<RootBuiltInConvertersAndTypeInfoCreator>g__CreateJsonTypeInfo|107_0(Type type, JsonSerializerOptions options)
   at System.Text.Json.JsonSerializer.GetTypeInfo(Type runtimeType, JsonSerializerOptions options)
   at System.Text.Json.JsonSerializer.Serialize[TValue](Utf8JsonWriter writer, TValue& value, Type runtimeType, JsonSerializerOptions options)
   at System.Text.Json.JsonSerializer.Serialize[TValue](Utf8JsonWriter writer, TValue value, JsonSerializerOptions options)
   at Modix.Host.EntryPoint.EndpointWriteOnlyJsonConverter.Write(Utf8JsonWriter writer, Endpoint value, JsonSerializerOptions options) in D:\Projects\JakenVeina\MODiX\MODiX.Host\EntryPoint.cs:line 90

The null value in question is coming from System.Reflection.ParameterInfo.Name, retrieved from the constructor of System.Runtime.CompilerServices.NullableContextAttribute.

The NullableContextAttribute instance is coming from Microsoft.AspNetCore.Http.Endpoint which is being passed to Microsoft.Extensions.Logging.ILogger.Log calls as TState state.

NullableContextAttribute is an internal and compiler-generated type, so it's been proving difficult to implement a workaround where I just skip or customize serialization of the type. It also strikes me as a slight design defect that all this work is being done to analyze this type for deserialization, when I am only attempting to serialize it, and have no intention of ever deserializing it.

Configuration

.NET SDK 6.0.100-preview.7.21379.14
System.Text.Json 6.0.0-preview.7.21377.19

Author: JakenVeina
Assignees: eiriktsarpalis
Labels:

bug, area-System.Text.Json, linkable-framework, Priority:3

Milestone: 7.0.0

@ghost ghost added the in-pr There is an active PR which will close this issue when it is merged label Aug 2, 2022
eiriktsarpalis added a commit to eiriktsarpalis/runtime that referenced this issue Aug 2, 2022
* Add runtime detection for constructor parameters with missing names. Fix dotnet#58690.
* Extend NotSupportedTypeConverter to MemberInfo types from System.Type types. Contributes to dotnet#58947.
eiriktsarpalis added a commit that referenced this issue Aug 3, 2022
* Makes a couple of changes on unsupported types:

* Add runtime detection for constructor parameters with missing names. Fix #58690.
* Extend NotSupportedTypeConverter to MemberInfo types from System.Type types. Contributes to #58947.

* Fix failing test in mono
@ghost ghost removed the in-pr There is an active PR which will close this issue when it is merged label Aug 3, 2022
@ghost ghost locked as resolved and limited conversation to collaborators Sep 2, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-System.Text.Json bug linkable-framework Issues associated with delivering a linker friendly framework Priority:3 Work that is nice to have
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants