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

BuildManager instances acquire its own BuildTelemetry instance (#8444) #8552

Merged
merged 4 commits into from
Mar 11, 2023

Conversation

rokonec
Copy link
Contributor

@rokonec rokonec commented Mar 10, 2023

Summary

Originall implementation did not expect multiple instances of BuildManager called concurrently. But in VS DTB and normal build are run concurrently.

This is backported from main PR #8444

Customer Impact

In rare cases Dictionary data structure is corrupted and can cause infinite loop. This affect only VS scenarios.
It is currently #7 ranked VS hang issue.

Regression?

Yes, introduced in VS 17.4.

Testing

Manual validation by @rokonec and automated tests. Additionally it has been in bleeding edge VS for about three weeks.

Risk

Low

…t#8444)

Fixes https://devdiv.visualstudio.com/DevDiv/_workitems/edit/1708215

Context
In VS there are multiple instances of BuildManager called asynchronously. For DTB and normal build and maybe other which I have not identified yet.

Changes Made
BuildManager instances acquire its own BuildTelemetry instance as oppose to sharing single BuildTelemetry instance in non thread safe manner.

Testing
Locally
AR-May and others added 3 commits March 10, 2023 15:44
Our CI builds fails because of bug NuGet/Home#12373. 
It is fixed in NuGet/NuGet.Client#5010. 
We are waiting for it to flow to CI machines. Meanwhile this PR applies a workaround.

Note: This PR needs to be reverted once it happens.
@rokonec rokonec merged commit f6fdcf5 into dotnet:vs17.5 Mar 11, 2023
@rokonec rokonec deleted the vs17.5 branch March 11, 2023 00:30
@rokonec
Copy link
Contributor Author

rokonec commented Mar 11, 2023

I am merging it as it was approved for servicing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants