Multithreading: add an opt-in strict mode that runs the build from a sentinel current directory
- Dominant language
- C#
- Stars
- 5.5k
- Forks
- 1.5k
- Avg merge
- 1d 8h
- Merged PRs (30d)
- 141
Description
### Summary
Static analysis cannot close the multithreading migration on its own, and we now have data on that. I propose an opt-in runtime strict mode that makes unresolved-path bugs *fail loudly and immediately* instead of surfacing as rare, load-dependent flakiness.
### Why: the empirical case
Across dotnet/arcade, dotnet/source-build-assets and the dotnet/dotnet VMR I migrated ~150 tasks to `[MSBuildMultiThreadableTask]`. `Microsoft.Build.TaskAuthoring.Analyzer` was enabled in **all three** repos and every build was **0 warnings, 0 errors**.
A manual audit afterwards found **9 real defects**. Categorised by why the analyzer could not see them:
| Cause | Count | Example |
|---|---|---|
| Analysis scope excluded the code | 2 | defect in an unannotated base class (#14783) |
| Monitored-API list gap | 3 | `AssemblyName.GetAssemblyName` |
| Outside the analyzer's model entirely | 4 | task-object race, memoized failure, nested task construction, path flowing through a DI abstraction |
The last row is the important one. Those are dataflow and lifetime problems, not banned-API problems. No practical amount of allowlist tuning reaches them.
And this is a *favourable* sample: I had context, motivation and a purpose-built auditing script. Thousands of task authors doing this migration will have the analyzer and nothing else.
### Why the failure mode is unusually nasty
An unresolved relative path in MT mode does not reliably throw. It resolves against whatever the shared node's current directory happens to be, which depends on which project scheduled first. So the same task:
- passes locally, single-threaded, every time
- passes in CI most of the time
- fails occasionally, on one machine, under load
Worse, some of these bugs never throw at all. In `CheckForPoison` an unresolved path threw `FileNotFoundException` into a `catch` that means "not an assembly", silently skipping source-build's poison checks. The build stayed green and the leak-detection gate quietly stopped detecting leaks.
### Proposal
Under an opt-in switch (`MSBUILDMULTITHREADEDSTRICT=1`, or `-mt:strict`), when the engine runs in-process multithreaded:
1. **Set the process current directory to a sentinel directory** — empty, or containing only a marker file — for the duration of the build. Any task resolving a relative path against the CWD then fails deterministically and immediately, on the first run, on every machine, with a stack trace pointing at the offending call. The whole class of bugs collapses into one reproducible failure.
2. Optionally, throw with a targeted message when a task annotated as multithreadable calls `Directory.SetCurrentDirectory` or reads `Environment.CurrentDirectory`.
Point 1 is the high-value half and is cheap. It converts a probabilistic, machine-dependent, sometimes-silent failure into a deterministic one — which is the property CI needs and static analysis cannot provide.
### Suggested rollout
- Off by default; on in the MSBuild repo's own CI, in Arcade's, and recommended in the migration skill
- Document it as the *verification* step of the migration, with the analyzer as the *authoring* step
The analyzer catches what it can see. This catches what it cannot.
Related: #14772 (analyzer epic), #14783, #14784.
Contributor guide
No contributing guide indexed for this repository
Research direction
No files or tests are named in the issue. Start by tracing the in-process multithreaded engine and how the build process manages its current directory, then examine the proposed environment-variable and command-line switch entry points. Done means an opt-in strict mode uses a sentinel directory during the build, remains off by default, and covers the deterministic failure behavior.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- csharp
- Domain
- build-system
- Issue type
- Feature
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Active
- Clarity
- Mostly clear
- Newbie friendliness
- 35/100