search

dotnet / project-system

The .NET Project System for Visual Studio
MIT License
968 stars 386 forks source link

Avoid project read lock in SDK version calculation #9347

Closed drewnoakes closed 10 months ago

drewnoakes commented 10 months ago

Use TryGetCurrentConfigurationGeneralPropertiesSnapshot rather than GetConfigurationGeneralPropertiesAsync as the latter will take a project read lock, while the former uses the most recent available snapshot. For the SDK version, this is unlikely to change throughout the lifetime of the project, so even an older version of this value will be fine.

This code runs for every project during solution load.

Microsoft Reviewers: Open in CodeFlow
adamint commented 10 months ago

Are the test failures related to this change?

drewnoakes commented 10 months ago

Getting the tests passing required some refactoring. The new use of TryGetCurrentConfigurationGeneralPropertiesSnapshot is incredibly painful to mock. Furthermore, the old SDKVersionTelemetryServiceComponent was implemented as a multi-life component on a non-dynamic component.

I addressed this by having only a single lifetime for the component, pulling the acquisition of the SDK version into a virtual method that can be mocked, and flattening two types into one (renaming it and the corresponding unit test in the process).

Also uses an xUnit theory to merge all tests into one. There was so much churn that I then converted to file-scoped namespaces.

Each of these in their own commit.