Skip to content

Add more diagnostics to LookupProperty (more diagnostics to flaky tests) #68281

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.

Sign up for GitHub

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

Merged
krwq merged 2 commits into from
Apr 21, 2022
Merged

Add more diagnostics to LookupProperty (more diagnostics to flaky tests) #68281

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
9c5f091
add more diagnostics to LookupProperty flaky tests
krwq Apr 20, 2022
daf164d
Fix nullability issue on Release build
krwq Apr 20, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 2 additions & 11 deletions ...em.Text.Json/src/System/Text/Json/Serialization/JsonSerializer.Read.HandlePropertyName.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,8 @@ internal static JsonPropertyInfo LookupProperty(
#if DEBUG
if (state.Current.JsonTypeInfo.PropertyInfoForTypeInfo.ConverterStrategy != ConverterStrategy.Object)
{
Debug.Fail(GetLookupPropertyDebugInfo(obj, unescapedPropertyName, ref state));
string objTypeName = obj?.GetType().FullName ?? "<null>";
Debug.Fail($"obj.GetType() => {objTypeName}; {state.Current.JsonTypeInfo.GetPropertyDebugInfo(unescapedPropertyName)}");
}
#endif

Expand Down Expand Up @@ -68,16 +69,6 @@ internal static JsonPropertyInfo LookupProperty(
return jsonPropertyInfo;
}

#if DEBUG
private static string GetLookupPropertyDebugInfo(object? obj, ReadOnlySpan<byte> unescapedPropertyName, ref ReadStack state)
{
JsonTypeInfo jti = state.Current.JsonTypeInfo;
string objTypeName = obj?.GetType().FullName ?? "<null>";
string propertyName = JsonHelpers.Utf8GetString(unescapedPropertyName);
return $"ConverterStrategy is {jti.PropertyInfoForTypeInfo.ConverterStrategy}. propertyName = {propertyName}; obj.GetType() => {objTypeName}; DebugInfo={jti.GetDebugInfo()}";
}
#endif

[MethodImpl(MethodImplOptions.AggressiveInlining)]
internal static ReadOnlySpan<byte> GetPropertyName(
ref ReadStack state,
Expand Down
15 changes: 10 additions & 5 deletions ...raries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/JsonTypeInfo.Cache.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -172,17 +172,22 @@ internal JsonPropertyInfo GetProperty(
}

// No cached item was found. Try the main dictionary which has all of the properties.
Debug.Assert(PropertyCache != null);
#if DEBUG
if (PropertyCache == null)
Copy link
Member

@eiriktsarpalis eiriktsarpalis Apr 20, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IIRC this can only happen during initialization races?

Copy link
Member Author

@krwq krwq Apr 20, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think so but I'm not sure how yet, hopefully the DebugInfo will shed some light on the "how"

{
Debug.Fail($"Property cache is null. {GetPropertyDebugInfo(propertyName)}");
}
#endif

if (PropertyCache.TryGetValue(JsonHelpers.Utf8GetString(propertyName), out JsonPropertyInfo? info))
if (PropertyCache!.TryGetValue(JsonHelpers.Utf8GetString(propertyName), out JsonPropertyInfo? info))
{
Debug.Assert(info != null);
Debug.Assert(info != null, "PropertyCache contains null JsonPropertyInfo");

if (Options.PropertyNameCaseInsensitive)
{
if (propertyName.SequenceEqual(info.NameAsUtf8Bytes))
{
Debug.Assert(key == GetKey(info.NameAsUtf8Bytes.AsSpan()));
Debug.Assert(key == GetKey(info.NameAsUtf8Bytes.AsSpan()), "key does not match re-computed value for the same sequence (case-insensitive)");

// Use the existing byte[] reference instead of creating another one.
utf8PropertyName = info.NameAsUtf8Bytes!;
Expand All @@ -195,7 +200,7 @@ internal JsonPropertyInfo GetProperty(
}
else
{
Debug.Assert(key == GetKey(info.NameAsUtf8Bytes.AsSpan()));
Debug.Assert(key == GetKey(info.NameAsUtf8Bytes.AsSpan()), "key does not match re-computed value for the same sequence (case-sensitive)");
utf8PropertyName = info.NameAsUtf8Bytes;
}
}
Expand Down
6 changes: 6 additions & 0 deletions src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/JsonTypeInfo.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -241,6 +241,12 @@ internal virtual void Configure()
}

#if DEBUG
internal string GetPropertyDebugInfo(ReadOnlySpan<byte> unescapedPropertyName)
{
string propertyName = JsonHelpers.Utf8GetString(unescapedPropertyName);
return $"propertyName = {propertyName}; DebugInfo={GetDebugInfo()}";
}

internal string GetDebugInfo()
{
ConverterStrategy strat = PropertyInfoForTypeInfo.ConverterStrategy;
Expand Down