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 Access Violation Exception (Attempted to read or write protected memory) #40878

Closed
Turnerj opened this issue Aug 15, 2020 · 17 comments · Fixed by #40914
Closed
Assignees
Milestone

Comments

@Turnerj
Copy link

Turnerj commented Aug 15, 2020

Description

First just a quick bit of background: as I've raised in comments in #36621, I've hit a variety of InvalidProgramException and AccessViolationException while trying to bring System.Text.Json (Previews 4 through 7) to Schema.NET (RehanSaeed/Schema.NET#100). Prior to trying System.Text.Json v5 previews, I had tried v4 which worked with my converter without this exception (though I really wanted the setting to not serialize default values which is coming in v5).

Jumping to now with Preview 7, I'm hitting issues with an AccessViolationException with the error message:

Attempted to read or write protected memory. This is often an indication that other memory is corrupt.

I've created a custom solution with as little code as I reasonably could while still using a custom version of the core from Schema.NET: https://github.com/Turnerj/SchemaNet_SystemTextJson

public class CustomType : Thing, IThing
{
	public override string Type => "CustomType";

	[JsonPropertyName("name")]
	[JsonConverter(typeof(ValuesJsonConverter))]
	public OneOrMany<string> Name { get; set; }

	[JsonPropertyName("uri")]
	[JsonConverter(typeof(ValuesJsonConverter))]
	public Values<string, Uri> Uri { get; set; }
}

class Program
{
	static void Main(string[] args)
	{
		// This is the JSON of the object below
		// {"@context":"https://schema.org","@type":"CustomType","name":"Hello World"}
		var inputObj = new CustomType
		{
			Name = "Hello World"
		};

		var json = SchemaSerializer.SerializeObject(inputObj); // Exception occurs on serialization

		var outputObj = SchemaSerializer.DeserializeObject<CustomType>(json);
	}
}

Things to note:

  • OneOrMany and Values are both structs which will serialize to a single item or an array
  • Values is special because it can convert to and from any of the generic types specified on it
  • ValuesJsonConverter does the guts of the converting. In debugging, the CanConvert method is hit however Write is never actually being called on it.
  • The ValuesJsonConverter's CanConvert is: public override bool CanConvert(Type typeToConvert) => typeof(IValues).IsAssignableFrom(typeToConvert);

Configuration

  • .NET Core 3.1
  • System.Text.Json 5.0.0-preview.7.20364.11
  • Windows 10 Home (10.0.18362 Build 18362)
  • I've hit this issue in CI builds on this pull request for Ubuntu, Mac and Windows

Regression?

As mentioned, I didn't have this issue with System.Text.Json v4 (likely the last minor version) though I needed v5 as I needed the "don't serialize default values" configuration.

Other information

Full Exception

System.AccessViolationException: 'Attempted to read or write protected memory. This is often an indication that other memory is corrupt.'
at DynamicClass.get_Name(System.Object)
at System.Text.Json.JsonPropertyInfo1[[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.Converters.ObjectDefaultConverter1[[System.__Canon, System.Private.CoreLib, Version=4.0.0.0, Culture=neutral, PublicKeyToken=7cec85d7bea7798e]].OnTryWrite(System.Text.Json.Utf8JsonWriter, System.__Canon, System.Text.Json.JsonSerializerOptions, System.Text.Json.WriteStack ByRef)
at System.Text.Json.Serialization.JsonConverter1[[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.JsonConverter1[[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 SchemaNet_SystemTextJson.Program.Main(System.String[])

Additionally, when I first went to Preview 4, while I didn't hit this exception, I did hit a CLR exception ("Common Language Runtime detected an invalid program").

While this issue is happening with serialization, I have experienced similar issues with deserialization too when running CI builds of the Schema.NET PR I've linked to a few times. I'm thinking the fix for serialization might fix the deserialization issue too.

@Dotnet-GitSync-Bot Dotnet-GitSync-Bot added area-System.Text.Json untriaged New issue has not been triaged by the area owner labels Aug 15, 2020
@layomia layomia removed the untriaged New issue has not been triaged by the area owner label Aug 15, 2020
@layomia layomia added this to the 5.0.0 milestone Aug 15, 2020
@layomia layomia self-assigned this Aug 15, 2020
@Turnerj
Copy link
Author

Turnerj commented Aug 15, 2020

Found the stack trace (edited it into the issue) - VS didn't show it in the exception dialog but it was in the Visual Studio Debug Console - hope it helps!

@am11
Copy link
Member

am11 commented Aug 15, 2020

With netcoreapp3.1 and net5.0 preview 8 on macOS, it throws EntryPointNotFoundException:

Unhandled exception. System.EntryPointNotFoundException: Entry point was not found.
   at Schema.NET.IValues.get_Count()
   at Schema.NET.ValuesJsonConverter.Write(Utf8JsonWriter writer, IValues value, JsonSerializerOptions options) in /Users/am11/projects/SchemaNet_SystemTextJson/SchemaNet_SystemTextJson/Schema.NET/ValuesJsonConverter.cs:line 111
   at System.Text.Json.Serialization.JsonConverter`1.TryWrite(Utf8JsonWriter writer, T& value, JsonSerializerOptions options, WriteStack& state)
   at System.Text.Json.JsonPropertyInfo`1.GetMemberAndWriteJson(Object obj, WriteStack& state, Utf8JsonWriter writer)
   at System.Text.Json.Serialization.Converters.ObjectDefaultConverter`1.OnTryWrite(Utf8JsonWriter writer, T value, JsonSerializerOptions options, WriteStack& state)
   at System.Text.Json.Serialization.JsonConverter`1.TryWrite(Utf8JsonWriter writer, T& value, JsonSerializerOptions options, WriteStack& state)
   at System.Text.Json.Serialization.JsonConverter`1.WriteCore(Utf8JsonWriter writer, T& value, JsonSerializerOptions options, WriteStack& state)
   at System.Text.Json.Serialization.JsonConverter`1.WriteCoreAsObject(Utf8JsonWriter writer, Object value, JsonSerializerOptions options, WriteStack& state)
   at System.Text.Json.JsonSerializer.WriteCore[TValue](JsonConverter jsonConverter, Utf8JsonWriter writer, TValue& value, JsonSerializerOptions options, WriteStack& state)
   at System.Text.Json.JsonSerializer.WriteCore[TValue](Utf8JsonWriter writer, TValue& value, Type inputType, JsonSerializerOptions options)
   at System.Text.Json.JsonSerializer.Serialize[TValue](TValue& value, Type inputType, JsonSerializerOptions options)
   at System.Text.Json.JsonSerializer.Serialize[TValue](TValue value, JsonSerializerOptions options)
   at Schema.NET.SchemaSerializer.SerializeObject(Object value, JsonSerializerOptions options) in /Users/am11/projects/SchemaNet_SystemTextJson/SchemaNet_SystemTextJson/Schema.NET/SchemaSerializer.cs:line 79
   at Schema.NET.SchemaSerializer.SerializeObject(Object value) in /Users/am11/projects/SchemaNet_SystemTextJson/SchemaNet_SystemTextJson/Schema.NET/SchemaSerializer.cs:line 62
   at SchemaNet_SystemTextJson.Program.Main(String[] args) in /Users/am11/projects/SchemaNet_SystemTextJson/SchemaNet_SystemTextJson/Program.cs:line 30

The runtime type of IValues value in custom converter's Write method is string[] instead of OneOrMany<string>. Looks bad IL which is somehow bypassing System.ArgumentException or System.InvalidCastException, which (AFAICT) is not reproducible with pure C#.

@Turnerj
Copy link
Author

Turnerj commented Aug 16, 2020

Interesting find @am11 - OneOrMany<T> does have an implicit conversion from T[] which (as far as I understand) would occur outside of System.Text.Json's understanding.

If I understand you right, you're saying that it didn't implicitly convert to OneOrMany<string> thus didn't work? Though like you pointed out, the method itself takes IValues as a parameter type so I would figure if something went weird here, wouldn't have we seen a NullPointerException or the other exception types you mentioned?

On my machine, that Write method isn't even called before I hit the AccessViolationException - with that in mind, I wonder how related what you're seeing and what I am seeing are - maybe dealing with two different problems?

EDIT: Oh, I just realised you were testing with Preview 8 - may explain a difference in errors.

@am11
Copy link
Member

am11 commented Aug 16, 2020

I am getting EntryPointNotFoundException from get_Count with both netcoreapp3.1 and net5.0 preview 8 on macOS. To run your project with net5.0, I only applied this patch:

diff --git a/SchemaNet_SystemTextJson/SchemaNet_SystemTextJson.csproj b/SchemaNet_SystemTextJson/SchemaNet_SystemTextJson.csproj
index 68fd6b4..db1358e 100644
--- a/SchemaNet_SystemTextJson/SchemaNet_SystemTextJson.csproj
+++ b/SchemaNet_SystemTextJson/SchemaNet_SystemTextJson.csproj
@@ -2,11 +2,8 @@
 
   <PropertyGroup>
     <OutputType>Exe</OutputType>
-    <TargetFramework>netcoreapp3.1</TargetFramework>
+    <TargetFramework>net5.0</TargetFramework>
   </PropertyGroup>
 
-  <ItemGroup>
-    <PackageReference Include="System.Text.Json" Version="5.0.0-preview.7.20364.11" />
-  </ItemGroup>
 
 </Project>

This is because the runtime type of value (on that line 111) is string[], despite the fact that the compile-time type of value is IValues.

I have not looked into internal mechanics on S.T.J, so I am not sure; but looks like there are two issues:

  1. S.T.J is passing wrongly-typed object to the custom converter (string[] instead of OneOrMany<string> or its interface IValues).
  2. somewhere in S.T.J, it is emitting bad IL in terms of type info, which runtime is accepting, and it is bypassing the runtime type validation.
    • if we try to isolate library-free repro of this issue with reflection (or activator etc.) in pure C#, we get proper runtime exceptions, such as; Unhandled exception. System.InvalidCastException: Unable to cast object of type 'System.String[]' to type 'IValues'. or Unhandled exception. System.ArgumentException: Object of type 'System.String[]' cannot be converted to type 'IValues'. (depending on which reflection/DI approach we use).

@am11
Copy link
Member

am11 commented Aug 16, 2020

On my machine, that Write method isn't even called before I hit the AccessViolationException

Maybe because we are using different operating system (Windows, macOS)?

EntryPointNotFoundException

Btw, this mysterious EntryPointNotFoundException due to usage of generic structs has also been reported to roslyn repo few times: https://github.com/dotnet/roslyn/search?q=EntryPointNotFoundException&type=Issues. A self-contained repro is bit tricky but would be great to nib the bug.

@Turnerj
Copy link
Author

Turnerj commented Aug 16, 2020

This is because the runtime type of value (on that line 111) is string[], despite the fact that the compile-time type of value is IValues.

I wonder if this occurs for other types then like int? or more complex types like Uri. Like, do you see anything different with this for example using int??

public class CustomType : Thing, IThing
{
	public override string Type => "CustomType";

	[JsonPropertyName("number")]
	[JsonConverter(typeof(ValuesJsonConverter))]
	public OneOrMany<int?> Number { get; set; }

	[JsonPropertyName("uri")]
	[JsonConverter(typeof(ValuesJsonConverter))]
	public Values<string, Uri> Uri { get; set; }
}

class Program
{
	static void Main(string[] args)
	{
		// {"@context":"https://schema.org","@type":"CustomType","Number":123}
		var inputObj = new CustomType
		{
			Number = 123
		};

		var json = SchemaSerializer.SerializeObject(inputObj);

		var outputObj = SchemaSerializer.DeserializeObject<CustomType>(json);
	}
}

For me, it is still the same error for the same configuration as in the issue description (.NET Core 3.1, S.T.J v5 Preview 7).

Btw, this mysterious EntryPointNotFoundException due to usage of generic structs has also been reported to roslyn repo few times: https://github.com/dotnet/roslyn/search?q=EntryPointNotFoundException&type=Issues. A self-contained repro is bit tricky but would be great to nib the bug.

Interesting - this is the first I think Ive seen of an EntryPointNotFoundException - I feel like it is one of those exceptions where it only occurs when the code screws up REAL bad. Back in Preview 4 of S.T.J, I was getting exceptions for the IL that was being generated at runtime:

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

@am11
Copy link
Member

am11 commented Aug 16, 2020

I wonder if this occurs for other types then like int? or more complex types like Uri. Like, do you see anything different with this for example using int??

Tested with few alternatives (value types included) and it is throwing EntryPointNotFoundException on macOS for all T in OneOnMany<T>, with netcoreapp3.1, OOTB preview 7 S.T.J and net5 preview 8 inbox S.T.J. I have also seen it in other cases where AccessViolationException on Windows for the exact same code is something else on Unix-like OS with CoreCLR.

@devsko
Copy link
Contributor

devsko commented Aug 16, 2020

I had a look into the ReflectionEmitMemberAccessor that causes the problem in the description. The dynamically generated getter is of type IValues but leaves an OneOrMany<string> on the stack. Clearely this must go horrible wrong without boxing - what it does. It seems an additional generator.Emit(OpCodes.Box, typeof(OneOrMany<string>)); solves the problem for this case.

@Turnerj
Copy link
Author

Turnerj commented Aug 16, 2020

The dynamically generated getter is of type IValues but leaves an OneOrMany on the stack. Clearely this must go horrible wrong without boxing - what it does.

Might explain why @am11 had string[] rather than OneOrMany<string> in the Write method. In OneOrMany<T>, the first property is a readonly array of T - wouldn't surprise me if it accessed that instead under certain conditions.

@devsko
Copy link
Contributor

devsko commented Aug 16, 2020

Yes, makes sense.

@devsko
Copy link
Contributor

devsko commented Aug 16, 2020

Hi @layomia, I have a test and fix for this ready. Are you interested in a PR?

@layomia
Copy link
Contributor

layomia commented Aug 16, 2020

@Turnerj, thanks for the detailed repro.

Hi @layomia, I have a test and fix for this ready. Are you interested in a PR?

@devsko, thanks for digging into this. Yes, please open a PR and I'll take a look.

@devsko
Copy link
Contributor

devsko commented Aug 19, 2020

Hi @Turnerj
root cause of this issue is that you have a single JsonConverter<IValues> and the implementations of that interface are value types. That means the property values have to be boxed while serializing and unboxed while deserializing.

For performance reasons, STJ generates getter/setter in plain IL and explicitly without boxing. As a result, in this case the generated IL is invalid because the values (say an Int32) are treated as pointer to an object.
The AccessViolationException is just one possible consequence - I tested some scenarios and observed scary effects.

In #40914 the missing box/unbox IL is emitted, which fixes the problem, but I assume there are performance concerns and it will not be merged - at least in .NET 5. As a workaround you can create a JsonConverter<T> for each implementation type like in this PR.

@layomia Please let me know what you think about the boxing in #40914. If that is not possible, there should a NotSupportedException get thrown in this case. Fortunately the runtime is pretty robust but a AccessViolationException or InvalidProgramException can be frightening ;)

@layomia
Copy link
Contributor

layomia commented Aug 26, 2020

Re-opening until the fix is back-ported to .NET 5 - #41366.

@layomia layomia reopened this Aug 26, 2020
@layomia
Copy link
Contributor

layomia commented Aug 27, 2020

Fixed for 5.0 in #41366.

@layomia layomia closed this as completed Aug 27, 2020
@Turnerj
Copy link
Author

Turnerj commented Aug 28, 2020

Thanks for working on this so fast and solving the underlying issues @devsko and @layomia - would I expect to see this in 5.0 RC1?

@layomia
Copy link
Contributor

layomia commented Aug 28, 2020

Thanks @Turnerj. Yes, the fix will come in 5.0 RC1. Please let us know how things go with the fix.

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