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

[System.Text.Json] System.InvalidProgramException when deserializing json. #36621

Closed
AraHaan opened this issue May 17, 2020 · 16 comments · Fixed by #40824
Closed

[System.Text.Json] System.InvalidProgramException when deserializing json. #36621

AraHaan opened this issue May 17, 2020 · 16 comments · Fixed by #40824
Assignees
Milestone

Comments

@AraHaan
Copy link
Member

AraHaan commented May 17, 2020

Description

Whenever I try to deserialize this json with System.Text.Json it throws an exception in System.Text.Json.dll as System.InvalidProgramException, then it duplicates the throw in System.Private.CoreLib.dll, then System.Private.CoreLib.dll throws an System.AggregateException
The Json data below has been changed from what was requested from an online API. So expect values not match the real user ids on the api I randomized it all so you guys get an idea what the api can and will give out.

the json: https://paste.mod.gg/ipiwosexuv.json
And the code, I am posting it as a url to avoid making this message too long as well.
The code is: https://paste.mod.gg/fokaxaxele.cpp (the thing placed it as .cpp when it is c#)

Now when you call it like so:

var result = JsonSerializer.Deserialize<MembersRoot>(json, Converter.Settings);

It should throw that exception.
Note: json is a string returned from ReadAsStringAsync() from an HttpResponseMessage from my code, however for this test have it be the string from File.ReadAllText() and that json data as the file.

Configuration

.net core 3.1, It uses the latest pre-release System.Text.Json as I was told somewhere that the 3.1 version was broken so I tried to 5.0 pre-release one to see if it works.
I am using the latest insider preview of Windows 10 and it is x64, the program targets Any CPU with no preference at all.
I tried this in debug mode in VS2019 that is when I noticed the exception and the program not doing what it should do when it did not have this exception (that was when it used Newtonsoft.Json's DeserializeObject which worked fine for this).

I do not think it is specific to the configuration but something wrong on either System.Text.Json side, or something wrong with my converters or something.

Regression?

I do not know for sure.

Other information

I do not know yet of any workarrounds other than using Newtonsoft.Json which I would like to squash out for System.Text.Json. 1 less package means for me faster restore which means maybe slightly faster build.

Sadly I only got what VS2019 printed, it never gave me the stack traces to the exceptions.

Exception thrown: 'System.InvalidProgramException' in System.Text.Json.dll
Exception thrown: 'System.InvalidProgramException' in System.Private.CoreLib.dll
Exception thrown: 'System.AggregateException' in System.Private.CoreLib.dll
@Dotnet-GitSync-Bot Dotnet-GitSync-Bot added area-System.Text.Json untriaged New issue has not been triaged by the area owner labels May 17, 2020
@AraHaan
Copy link
Member Author

AraHaan commented May 17, 2020

cc: @jozkee @steveharter

@AraHaan
Copy link
Member Author

AraHaan commented May 17, 2020

I think I figured it out, It seems to be the readers in my converters for some reason.

The switch expressions and even doing manual if , else if, else will still result in the same exception results.

No clue why as it expects the type returned by the Read method.

Odd thing about this is that the code seems valid to me.

Edit: It seems the issue was that I defined 1 enum value in a class that gets serialized to as nullable.

@AraHaan AraHaan closed this as completed May 17, 2020
@GrabYourPitchforks
Copy link
Member

Reopening this issue. System.Text.Json team, can you take a look at this? The InvalidProgramException is occurring when the JIT tries to compile the dynamic method that System.Text.Json has created to wrap the DatumAttributes.PatronStatus property setter. I haven't looked at the raw IL, but the property is typed as Nullable<PatronStatus>, and the Set delegate is typed as Func<object, PatronStatus>. So my guess is that we're producing invalid IL in that dynamic method: we're accepting a local of type T and trying to assign it to a property of type Nullable<T>, and the JIT is detecting the mismatch and failing.

The failure is happening on line 174 in the snippet below. The first time this particular Set delegate is invoked, the JIT tries to compile the IL, and the JIT itself is throwing the exception.

else
{
success = Converter.TryRead(ref reader, RuntimePropertyType!, Options, ref state, out T value);
if (success)
{
if (!IgnoreNullValues || (!isNullToken && value != null))
{
Set!(obj, value);
}
}
}

@AraHaan
Copy link
Member Author

AraHaan commented May 18, 2020

I think another issue is there also is no form to override on Read where the return value can be nullable if CanConvert does:

public override bool CanConvert(Type t)
        => t == typeof(T) || t == typeof(T?);

Like there is no

public T? Read(ref Utf8JsonReader reader, Type typeToConvert,
    JsonSerializerOptions options)

variant of Read.

@layomia layomia removed the untriaged New issue has not been triaged by the area owner label May 20, 2020
@layomia layomia self-assigned this May 20, 2020
@layomia layomia added this to the 5.0 milestone May 20, 2020
@Turnerj
Copy link

Turnerj commented May 23, 2020

I've additionally encountered this with deserialization too for Schema.NET (RehanSaeed/Schema.NET#100) with Preview 4 of System.Text.Json. It seems that all of the errors generated in our tests are for the same bit of invalid IL code generated by System.Text.Json.

  Error Message:
   System.InvalidProgramException : Invalid IL code in (wrapper dynamic-method) object:get_Property (object): IL_000b: ret       

  Stack Trace:
    at (wrapper managed-to-native) System.Delegate.CreateDelegate_internal(System.Type,object,System.Reflection.MethodInfo,bool)
  at System.Delegate.CreateDelegate (System.Type type, System.Object firstArgument, System.Reflection.MethodInfo method, System.Boolean throwOnBindFailure, System.Boolean allowClosed) [0x002f0] in <f9d1b832704f410aa8ec771f4fe80552>:0 
  at System.Delegate.CreateDelegate (System.Type type, System.Object firstArgument, System.Reflection.MethodInfo method) [0x00000] in <f9d1b832704f410aa8ec771f4fe80552>:0 
  at System.Reflection.Emit.DynamicMethod.CreateDelegate (System.Type delegateType) [0x00029] in <f9d1b832704f410aa8ec771f4fe80552>:0 
  at System.Text.Json.Serialization.ReflectionEmitMemberAccessor.CreatePropertyGetter (System.Reflection.PropertyInfo propertyInfo, System.Type classType, System.Type propertyType) [0x000ae] in <baabb804b3c244839f55b9e878301bf6>:0 
  at System.Text.Json.Serialization.ReflectionEmitMemberAccessor.CreatePropertyGetter[TProperty] (System.Reflection.PropertyInfo propertyInfo) [0x00007] in <baabb804b3c244839f55b9e878301bf6>:0 
  at System.Text.Json.JsonPropertyInfo`1[TTypeToConvert].Initialize (System.Type parentClassType, System.Type declaredPropertyType, System.Type runtimePropertyType, System.Text.Json.ClassType runtimeClassType, System.Reflection.PropertyInfo propertyInfo, System.Text.Json.Serialization.JsonConverter converter, System.Nullable`1[T] ignoreCondition, System.Text.Json.JsonSerializerOptions options) [0x00052] in <baabb804b3c244839f55b9e878301bf6>:0 
  at System.Text.Json.JsonClassInfo.CreateProperty (System.Type declaredPropertyType, System.Type runtimePropertyType, System.Reflection.PropertyInfo propertyInfo, System.Type parentClassType, System.Text.Json.Serialization.JsonConverter converter, System.Text.Json.JsonSerializerOptions options, System.Nullable`1[T] ignoreCondition) [0x00013] in <baabb804b3c244839f55b9e878301bf6>:0 
  at System.Text.Json.JsonClassInfo.AddProperty (System.Reflection.PropertyInfo propertyInfo, System.Type parentClassType, System.Text.Json.JsonSerializerOptions options) [0x00057] in <baabb804b3c244839f55b9e878301bf6>:0 
  at System.Text.Json.JsonClassInfo..ctor (System.Type type, System.Text.Json.JsonSerializerOptions options) [0x00123] in <baabb804b3c244839f55b9e878301bf6>:0 
  at System.Text.Json.JsonSerializerOptions.GetOrAddClass (System.Type type) [0x00025] in <baabb804b3c244839f55b9e878301bf6>:0 
  at System.Text.Json.ReadStack.Initialize (System.Type type, System.Text.Json.JsonSerializerOptions options, System.Boolean supportContinuation) [0x00000] in <baabb804b3c244839f55b9e878301bf6>:0 
  at System.Text.Json.JsonSerializer.ReadCore[TValue] (System.Text.Json.Utf8JsonReader& reader, System.Type returnType, System.Text.Json.JsonSerializerOptions options) [0x00008] in <baabb804b3c244839f55b9e878301bf6>:0 
  at System.Text.Json.JsonSerializer.Deserialize[TValue] (System.String json, System.Type returnType, System.Text.Json.JsonSerializerOptions options) [0x0007a] in <baabb804b3c244839f55b9e878301bf6>:0 
  at System.Text.Json.JsonSerializer.Deserialize[TValue] (System.String json, System.Text.Json.JsonSerializerOptions options) [0x0000e] in <baabb804b3c244839f55b9e878301bf6>:0 
  at Schema.NET.SchemaSerializer.DeserializeObject[T] (System.String value) [0x0000a] in <4ea8786ac13043f48b5de492acdcae8a>:0 
  at Schema.NET.Test.ValuesJsonConverterTest.DeserializeObject[T] (System.String json) [0x00000] in <f475aebe1e8a43f4a2ced41d511263ce>:0 
  at Schema.NET.Test.ValuesJsonConverterTest.ReadJson_Values_SingleValue_Enum_HttpsSchema () [0x00006] in <f475aebe1e8a43f4a2ced41d511263ce>:0 
  at (wrapper managed-to-native) System.Reflection.RuntimeMethodInfo.InternalInvoke(System.Reflection.RuntimeMethodInfo,object,object[],System.Exception&)

Full test error log which includes multiple same failures.

@Turnerj
Copy link

Turnerj commented May 24, 2020

One thing that might be of note, in my case it doesn't seem to be limited to value types being nullable, we have the error also happening on type string.

CI Test Result: https://ci.appveyor.com/project/RehanSaeed/schema-net/builds/33064765/job/62lxs509wsfre175#L2698
Test Code: https://github.com/RehanSaeed/Schema.NET/blob/77413e5e5594130960fec1fe7f7198c985ed937f/Tests/Schema.NET.Test/ValuesJsonConverterTest.cs#L469-L474

@Turnerj
Copy link

Turnerj commented Jun 26, 2020

FYI, just tested Preview 6 out of a hope that this might have been co-incidentally fixed. Unfortunately this issue still occurs in the same way as outlined before.

eg. System.InvalidProgramException : Invalid IL code in (wrapper dynamic-method) object:get_Property (object): IL_000b: ret

@Turnerj
Copy link

Turnerj commented Jul 22, 2020

Just tested out Preview 7, now with a related (but not the same) error.

System.AccessViolationException: Attempted to read or write protected memory. This is often an indication that other memory is corrupt.
   at DynamicClass.get_I
temListElement(System.Object)
   at System.Text.Json.JsonPropertyInfo`1[[System.__Canon, System.Private.CoreLib, Version=4.0.0.0, Culture=neutral, PublicKeyToken=7cec85d7bea7798e]].GetMemberAndWriteJson(System.Object, System.Text.Json.WriteStack ByRef, System.Text.Json.Utf8JsonWriter)
   at System.Text.Json.Serialization.JsonConverter`1[[System.__Canon, System.Private.CoreLib, Version=4.0.0.0, Culture=neutral, PublicKeyToken=7cec85d7bea7798e]].TryWrite(System.Text.Json.Utf8JsonWriter, System.__Canon ByRef, System.Text.Json.JsonSerializerOptions, System.Text.Json.WriteStack ByRef)
   at System.Text.Json.Serialization.JsonConverter`1[[System.__Canon, System.Private.CoreLib, Version=4.0.0.0, Culture=neutral, PublicKeyToken=7cec85d7bea7798e]].WriteCore(System.Text.Json.Utf8JsonWriter, System.__Canon ByRef, System.Text.Json.JsonSerializerOptions, System.Text.Json.WriteStack ByRef)
   at System.Text.Json.Serialization.JsonConverter`1[[System.__Canon, System.Private.CoreLib, Version=4.0.0.0, Culture=neutral, PublicKeyToken=7cec85d7bea7798e]].WriteCoreAsObject(System.Text.Json.Utf8JsonWriter, System.Object, System.Text.Json.JsonSerializerOptions, System.Text.Json.WriteStack ByRef)
   at System.Text.Json.JsonSerializer.WriteCore[[System.__Canon, System.Private.CoreLib, Version=4.0.0.0, Culture=neutral, PublicKeyToken=7cec85d7bea7798e]](System.Text.Json.Serialization.JsonConverter, System.Text.Json.Utf8JsonWriter, System.__Canon ByRef, System.Text.Json.JsonSerializerOptions, System.Text.Json.WriteStack ByRef)
   at System.Text.Json.JsonSerializer.WriteCore[[System.__Canon, System.Private.CoreLib, Version=4.0.0.0, Culture=neutral, PublicKeyToken=7cec85d7bea7798e]](System.Text.Json.Utf8JsonWriter, System.__Canon ByRef, System.Type, System.Text.Json.JsonSerializerOptions)
   at System.Text.Json.JsonSerializer.Serialize[[System.__Canon, System.Private.CoreLib, Version=4.0.0.0, Culture=neutral, PublicKeyToken=7cec85d7bea7798e]](System.__Canon ByRef, System.Type, System.Text.Json.JsonSerializerOptions)
   at System.Text.Json.JsonSerializer.Serialize[[System.__Canon, System.Private.CoreLib, Version=4.0.0.0, Culture=neutral, PublicKeyToken=7cec85d7bea7798e]](System.__Canon, System.Text.Json.JsonSerializerOptions)
   at Schema.NET.SchemaSerializer.SerializeObject(System.Object, System.Text.Json.JsonSerializerOptions)
   at Schema.NET.SchemaSerializer.SerializeObject(System.Object)
   at Schema.NET.Thing.ToString()
   at Schema.NET.Test.ItemListTest.Deserializing_ItemListJsonLd_ReturnsMatchingItemList()
   at System.RuntimeMethodHandle.InvokeMethod(System.Object, System.Object[], System.Signature, Boolean, Boolean)
   at System.Reflection.RuntimeMethodInfo.Invoke(System.Object, System.Reflection.BindingFlags, System.Reflection.Binder, System.Object[], System.Globalization.CultureInfo)
   at System.Reflection.MethodBase.Invoke(System.Object, System.Object[])
   at Xunit.Sdk.TestInvoker`1[[System.__Canon, System.Private.CoreLib, Version=4.0.0.0, Culture=neutral, PublicKeyToken=7cec85d7bea7798e]].CallTestMethod(System.Object)
   at Xunit.Sdk.TestInvoker`1+<>c__DisplayClass48_1+<<InvokeTestMethodAsync>b__1>d[[System.__Canon, System.Private.CoreLib, Version=4.0.0.0, Culture=neutral, PublicKeyToken=7cec85d7bea7798e]].MoveNext()
   at System.Runtime.CompilerServices.AsyncMethodBuilderCore.Start[[Xunit.Sdk.TestInvoker`1+<>c__DisplayClass48_1+<<InvokeTestMethodAsync>b__1>d[[System.__Canon, System.Private.CoreLib, Version=4.0.0.0, Culture=neutral, PublicKeyToken=7cec85d7bea7798e]], xunit.execution.dotnet, Version=2.4.1.0, Culture=neutral, PublicKeyToken=8d05b1bb7a6fdb6c]](<<InvokeTestMethodAsync>b__1>d<System.__Canon> ByRef)

CI Test Result: https://dev.azure.com/schema-net/Schema.NET/_build/results?buildId=776&view=logs&j=c7dcf16a-b484-548d-e1f9-8e1c62e0ef67&t=882dcaa3-8da3-56bd-4ce5-e13b1ad98e80&l=23

@layomia
Copy link
Contributor

layomia commented Jul 22, 2020

@Turnerj - do you have project/code sample that you can share that can repro this error?

@Turnerj
Copy link

Turnerj commented Jul 22, 2020

@layomia - that latest error was generated by the latest commit in this PR: RehanSaeed/Schema.NET#100

The test that failed in the latest error was this one: https://github.com/Turnerj/Schema.NET/blob/5df8731a317c5173a1ace4870188ef82b270c95a/Tests/Schema.NET.Test/core/ItemListTest.cs#L59-L63

My various other comments earlier about IL errors are also from the same PR on earlier preview versions.

@steveharter
Copy link
Member

For the original case there is a custom converter:

    internal class PatronStatusConverter : JsonConverter<PatronStatus>
    {
        /// <inheritdoc />
        public override bool CanConvert(Type t)
            => t == typeof(PatronStatus) || t == typeof(PatronStatus?);

A custom converter should not include the nullable version (PatronStatus?) in CanConvert as that is taken care of automatically by the serializer, at least if the standard "null" semantics are desired.

If you want to override null, override HandleNull and return true from the converter. This feature was added in 5.0.

@layomia
Copy link
Contributor

layomia commented Aug 14, 2020

A custom converter should not include the nullable version (PatronStatus?) in CanConvert as that is taken care of automatically by the serializer, at least if the standard "null" semantics are desired.

👍

If the nullable version is included in the CanConvert check, the serializer will thow an InvalidOperationException saying that the implementation is redundant. The expected behavior from the converter is not to include the nullable version, or omit the override entirely. Without the redundant check, the converter is picked up and used as expected.

@Turnerj
Copy link

Turnerj commented Aug 14, 2020

Hmmmmm - in my case, my CanConvert looks like the following:

https://github.com/Turnerj/Schema.NET/blob/5df8731a317c5173a1ace4870188ef82b270c95a/Source/Schema.NET/ValuesJsonConverter.cs#L33

public override bool CanConvert(Type typeToConvert) => typeof(IValues).IsAssignableFrom(typeToConvert);

The types that implement IValues are structs of OneOrMany<T>, Values<T1, T2>, Values<T1, T2, T3> etc.

I'll note that this converter is specifically added to every property where it is applicable so I don't know if CanConvert is even used in that case.

@Turnerj
Copy link

Turnerj commented Aug 15, 2020

Now that this is closed, should I be creating a new issue for the InvalidProgramException errors I've received that I've mentioned in my previous comments? (As my CanConvert doesn't do a nullable check)

@layomia
Copy link
Contributor

layomia commented Aug 15, 2020

@Turnerj, yes pease open a new issue and kindly provide a repro that shows the converter, all types being (de)serialized, what the JSON payload looks like, and what the call to the serializer looks like.

@Turnerj
Copy link

Turnerj commented Aug 15, 2020

@layomia - Done #40878

@ghost ghost locked as resolved and limited conversation to collaborators Dec 9, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
6 participants