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

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

Merged
merged 2 commits into from
Apr 21, 2022

Conversation

krwq
Copy link
Member

@krwq krwq commented Apr 20, 2022

Contributes to: #67816

Seems there was more asserts in LookupProperty I missed last time because AggressiveInlining which caused the stack trace to skip the correct method name

@ghost
Copy link

ghost commented Apr 20, 2022

Tagging subscribers to this area: @dotnet/area-system-text-json, @gregsdennis
See info in area-owners.md if you want to be subscribed.

Issue Details

Contributes to: #67816

Seems there was more asserts in LookupProperty I missed last time because AggressiveInlining which caused the stack trace to skip the correct method name

Author: krwq
Assignees: -
Labels:

area-System.Text.Json

Milestone: -

@krwq krwq changed the title Add more diagnostics to LookupProperty flaky tests Add more diagnostics to LookupProperty (more diagnostics to flaky tests) Apr 20, 2022
@@ -172,17 +172,22 @@ public partial class JsonTypeInfo
}

// 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

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

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"

@krwq
Copy link
Member Author

krwq commented Apr 21, 2022

The failure System.Text.Json.SourceGeneration.Tests.ReferenceHandlerTests_Metadata_String.ValueTypesShouldNotContainId is not related to the changes I'm making here - it might be another manifestation of same or similar bug (uber issue is here: #67761). I'm merging because this will possibly help us diagnose these kind of issues

@krwq krwq merged commit a4d45b2 into dotnet:main Apr 21, 2022
directhex pushed a commit to directhex/runtime that referenced this pull request Apr 21, 2022
…ts) (dotnet#68281)

* add more diagnostics to LookupProperty flaky tests

* Fix nullability issue on Release build
@ghost ghost locked as resolved and limited conversation to collaborators May 21, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants