dotnet / project-system

The .NET Project System for Visual Studio
MIT License
971 stars 388 forks source link

DesignTimeBuild should not run concurrently with Build of same csproj #8680

Closed jzabroski closed 1 year ago

jzabroski commented 2 years ago

Visual Studio Version

Microsoft Visual Studio Professional 2022 Version 17.1.3 VisualStudio.17.Release/17.1.3+32328.378 Microsoft .NET Framework Version 4.8.03761 Installed Version: Professional .NET Core Debugging with WSL 1.0 .NET Core Debugging with WSL ADL Tools Service Provider 1.0 This package contains services used by Data Lake tools ASA Service Provider 1.0 ASP.NET and Web Tools 2019 17.1.358.51495 ASP.NET and Web Tools 2019 ASP.NET Web Frameworks and Tools 2019 17.1.358.51495 For additional information, visit https://www.asp.net/ Azure App Service Tools v3.0.0 17.1.358.51495 Azure App Service Tools v3.0.0 Azure Data Lake Tools for Visual Studio 2.6.5000.0 Microsoft Azure Data Lake Tools for Visual Studio Azure Functions and Web Jobs Tools 17.1.358.51495 Azure Functions and Web Jobs Tools Azure Stream Analytics Tools for Visual Studio 2.6.5000.0 Microsoft Azure Stream Analytics Tools for Visual Studio C# Tools 4.1.0-5.22165.10+e555772db77ca828b02b4bd547c318387f11d01f C# components used in the IDE. Depending on your project type and settings, a different version of the compiler may be used. Common Azure Tools 1.10 Provides common services for use by Azure Mobile Services and Microsoft Azure Tools. Cookiecutter 17.0.21344.1 Provides tools for finding, instantiating and customizing templates in cookiecutter format. Microsoft Azure Hive Query Language Service 2.6.5000.0 Language service for Hive query Microsoft Azure Stream Analytics Language Service 2.6.5000.0 Language service for Azure Stream Analytics Microsoft JVM Debugger 1.0 Provides support for connecting the Visual Studio debugger to JDWP compatible Java Virtual Machines Microsoft Library Manager 2.1.161+abc97ecc7d.RR Install client-side libraries easily to any web project Microsoft MI-Based Debugger 1.0 Provides support for connecting Visual Studio to MI compatible debuggers Microsoft Visual Studio Tools for Containers 1.2 Develop, run, validate your ASP.NET Core applications in the target environment. F5 your application directly into a container with debugging, or CTRL + F5 to edit & refresh your app without having to rebuild the container. Node.js Tools 1.5.40105.1 Commit Hash:1822ee94b29c6cf748a19825f14cc26d30b0b871 Adds support for developing and debugging Node.js apps in Visual Studio NuGet Package Manager 6.1.0 NuGet Package Manager in Visual Studio. For more information about NuGet, visit https://docs.nuget.org/ Project System Tools 1.0 Tools for working with C#, VisualBasic, and F# projects. Python - Django support 17.0.21344.1 Provides templates and integration for the Django web framework. Python - Profiling support 17.0.21344.1 Profiling support for Python projects. Python with Pylance 17.0.21344.1 Provides IntelliSense, projects, templates, debugging, interactive windows, and other support for Python developers. Razor (ASP.NET Core) 17.0.0.2206201+62a2c1d6162f828801565a7ca26d9d48b810a05b Provides languages services for ASP.NET Core Razor. Redgate SQL Prompt 1.0.0.0 Write, format, and refactor SQL effortlessly SQL Server Data Tools 17.0.62203.25080 Microsoft SQL Server Data Tools SQL Server Reporting Services 16.0.19875.0 Microsoft SQL Server Reporting Services Designers Version 16.0.19875.0 ToolWindowHostedEditor 1.0 Hosting json editor into a tool window TypeScript Tools 17.0.1229.2001 TypeScript Tools for Microsoft Visual Studio Visual Basic Tools 4.1.0-5.22165.10+e555772db77ca828b02b4bd547c318387f11d01f Visual Basic components used in the IDE. Depending on your project type and settings, a different version of the compiler may be used. Visual F# Tools 17.1.0-beta.21610.4+07b5673e4f2fa7630e78abe37f16b372353a7242 Microsoft Visual F# Tools Visual Studio Code Debug Adapter Host Package 1.0 Interop layer for hosting Visual Studio Code debug adapters in Visual Studio Visual Studio Container Tools Extensions 1.0 View, manage, and diagnose containers within Visual Studio. Visual Studio IntelliCode 2.2 AI-assisted development for Visual Studio. Visual Studio Tools for Containers 1.0 Visual Studio Tools for Containers

Summary

Installing the Visual Studio Project System Tools for Visual Studio 2022 revealed that DesignTimeBuild and Build were running concurrently, and it is this concurrent execution that was causing spurious "file is opened by another process" errors during build.

Steps to Reproduce

  1. Use a nuget package like Grpc.Tools that injects its own targets into the csproj
  2. Add enough .proto files that are sufficiently long and full of random DTOs to keep the compiler busy for a few seconds
  3. Do a "Full Clean" (including design time build metadata in the obj directory) and rebuild several times until you get the "file is opened by another process" error on the Protobuf Compiler
  4. (optional, may be required for full repro) Have another project that imports the generated Protobuf DTOs and services and references them. In this way, if you rebuild the solution, it may be the case that Roslyn DesignTimeBuild is happening in order to satisfy understanding what the services are. Note, that is totally OK, but doing it concurrently with Build of the same csproj is not OK. The Concurrency & Coordination robotics SDK Runtime that George built into MSBuild years ago should allow you to not do things like this, but it's been awhile since I understood how all this works.

Expected Behavior

DesignTimeBuild should not run concurrently with Build of same csproj. It is super difficult to debug these problems when they occur, and waste a ton of time. Core Tools should be simple to comprehend.

Actual Behavior

DesignTimeBuild runs concurrently with Build. This can cause spurious failures when tasks run by DesignTimeBuild try to access mutex'd resources Build tasks are running, and vice versa.

User Impact

I have to explain to every developer in my organization a workaround for this problem, or they will hit productivity road blocks whenever they encounter this error and don't know what the root cause is.

drewnoakes commented 1 year ago

Triage notes: I've asked the CPS team about this.

drewnoakes commented 1 year ago

CPS confirmed that this is a problem and there's currently no way to prevent these from overlapping.

@jzabroski what files are experiencing this issue? I would expect design-time builds should not be modifying files on disk, and any file reads during build should be made permitting shared reads. This problem isn't something I've seen reported elsewhere, so perhaps there's something that can be improved in the Protobuf compiler you're using that would avoid this (by modifying how files are locked, modifying how long file locks are held for (eg: buffering in memory then flushing rather than streaming to disk), or perhaps by adding retries so that builds complete successfully, albeit with a delay.

drewnoakes commented 1 year ago

@jzabroski we're going to close this out for now as there's nothing clearly actionable here yet. We can reopen later if that changes. Thanks.

jzabroski commented 1 year ago

@jzabroski what files are experiencing this issue?

I missed this question.

This happens with Google Protobuf Compiler MSBuild hooks. Some of these targets have been worked on by Microsoft interns but they did not clearly document why they disabled DesignTimeBuild calls to certain Protobuf targets: They added DisableProtobufDesignTimeBuild property, with not much clarity as to why (although I think I was able to figure it out over many weeks of on and off debugging: CPS doesn't intelligently run things concurrently).

In any case, it is pretty bad that the "design" of MSBuild and CPS is that all tasks must be process re-entrant safe. Totally nuts if you ask me. I'm also a bit shocked there is no way to prevent it. Feels like a house of cards to close this.

jzabroski commented 1 year ago

@drewnoakes One follow up question if you can spare a few minutes. Can you explain why the CPS can't just avoid running a DesignTimeBuild during another Build from within Visual Studio? It seems like a straightforward mutex to me. I figure some additional color may help me understand this better.

drewnoakes commented 1 year ago

My understanding is that design-time builds are expected to not change anything on disk, which would make the safe to run concurrent with real builds.

I am not deeply familiar with how CPS schedules DTBs but I do know there are several ways to run them (for example, depending upon whether there are in-memory changes that aren't saved to disk yet), meaning they can run in process or out of process. This wouldn't just be adding a mutex and calling it done.

Either way, the request doesn't fall on this repo. It'd have to be a feedback ticket, which could then be assigned to the relevant team internally. If you want to file one, I'll make sure it's forwarded appropriately. You'd have to make a strong case for this change though, as the engineering cost associated with it is likely high, and given there hasn't been much signal for this I can't imagine it getting to the top of the priority list.

I suspect it'd be easier to update the protobuf package's behaviour during design-time builds. This documentation may be helpful: https://github.com/dotnet/project-system/blob/main/docs/design-time-builds.md#determining-whether-a-target-is-running-in-a-design-time-build

jzabroski commented 1 year ago

Thanks; agree, not on the priority list but just another MSBuild foot-gun to be mindful of.

Was a little confused to hear project-system repo shouldnt be used for CPS issues. What is the difference between dotnet/project-system repo and the CPS?

drewnoakes commented 1 year ago

dotnet/project-system is built on top of CPS. Organisationally, this repo belongs to .NET, and CPS belongs to the VS platform. CPS is not only used for .NET projects. Our teams work closely together. Customer-facing CPS issues are tracked in feedback tickets.