Avoid StackOverflowException when cyclic Type reference#37818
Avoid StackOverflowException when cyclic Type reference#37818steveharter merged 2 commits intodotnet:masterfrom
Conversation
|
|
||
| public class TestClassWithArrayOfElementsOfTheSameClass | ||
| { | ||
| public TestClassWithArrayOfElementsOfTheSameClass[] Array { get; set; } |
There was a problem hiding this comment.
Can we add a couple more tests for different collection types, like List<T> and Dictionary, along with one where the recursive property is deeper within a .NET object graph?
There was a problem hiding this comment.
Sure. Adding a more complex test.
| //It shouldn't throw when there is no real cycle reference, and just empty object is created | ||
| string json = JsonSerializer.ToString(obj); | ||
| Assert.Equal(@"{}", json); | ||
| Assert.Equal(@"{""Array"":null}", json); |
There was a problem hiding this comment.
@rynowak previously suggested that this return an empty object (i.e. {}):
#37611 (comment)
There was a problem hiding this comment.
The array is null, so it has a null value like every other null property.
| _elementClassInfo = Options.GetOrAddClass(_elementType); | ||
| } | ||
|
|
||
| return _elementClassInfo; |
There was a problem hiding this comment.
Is it OK for this to still return null sometimes?
There was a problem hiding this comment.
Yes. Other code uses it to determine if the current property can contain elements. I'll add a comment\doc.
|
Committing this; test failures due to System.Drawing.Common.Tests |
4 participants
Fixes https://github.com/dotnet/corefx/issues/37313
This is fixed by lazy evaluating the
ElementClassInfoproperty so we don't end up in a recursive loop. To do this, theJsonPropertyInfonow holds a reference to the options class which it uses during lazy evaluation; when the code was originally written this was not possible since an instance ofJsonPropertyInfowas not linked with the options instance.Also removed a
// todo: to minimize hashtable lookups, cache JsonClassInfo:by leveraging the same lazy evaluation approach (also not possible before). This has a minor perf gain for lists and dictionaries.