TaskHost file-access data is corrupted under multithreaded MSBuild
- Dominant language
- C#
- Stars
- 5.5k
- Forks
- 1.5k
- Avg merge
- 1d 8h
- Merged PRs (30d)
- 141
Description
### Summary
When an unannotated task runs in a sidecar TaskHost under experimental multithreaded MSBuild (`-mt`) and reports file accesses through `EngineServices.ReportFileAccess`, MSBuild can crash while processing the TaskHost completion packet.
This was exposed by `Microsoft.MSBuildCache.SharedCompilation`'s `ResolveFileAccesses` task, but the defect is in MSBuild's TaskHost packet deserialization rather than in project-cache lookup or materialization.
### Environment
- MSBuild 18.11 prerelease from .NET SDK `11.0.100-rc.1.26402.102`
- Windows
- Build invoked with `-mt`
- An unannotated task is routed to a sidecar TaskHost and reports file accesses
### Actual behavior
The build fails with:
```text
MSB4018: The "ResolveFileAccesses" task failed unexpectedly.
System.NullReferenceException
at Microsoft.Build.BackEnd.FileAccessManager.ReportFileAccess(...)
at Microsoft.Build.BackEnd.TaskHostTask.HandleTaskHostTaskComplete(...)
```
`FileAccessManager.ReportFileAccess` receives a `FileAccessData` value whose `Path` is null and dereferences it while checking path prefixes.
### Root cause
`TaskHostTaskComplete.Translate` deserializes each `FileAccessData` struct through an `ITranslatable` reference. Assigning the struct to the interface boxes a copy. The translator mutates that boxed copy, while the struct value subsequently added to the result list remains its default value. Consequently, fields such as `Path` are lost during the TaskHost packet round trip.
The relevant flow is:
1. A task running in TaskHost records valid `FileAccessData` through `EngineServices.ReportFileAccess`.
2. `TaskHostTaskComplete` serializes the values successfully.
3. During deserialization, interface boxing causes mutations to apply to a boxed struct copy.
4. Default `FileAccessData` entries are replayed by the owning process.
5. `FileAccessManager.ReportFileAccess` crashes on the null path.
### Expected behavior
All `FileAccessData` fields should survive TaskHost packet serialization, and the owning MSBuild process should be able to replay accesses reported by an out-of-process task without crashing.
### Suggested fix and regression coverage
- Deserialize `FileAccessData` by reference without converting a mutable struct to `ITranslatable` in a way that boxes a copy.
- Add a round-trip serialization test for `TaskHostTaskComplete` that verifies `Path` and the other `FileAccessData` fields are preserved.
- An integration test should run an unannotated task under `-mt`, report a file access, and verify that the build completes and the access reaches the owning process.
### Project-cache impact
Project-cache services appear centralized and concurrency-compatible, so this does not indicate a cache lookup/materialization race. However, cache population relies on correct file-access attribution. Any cache-related task that reports accesses from a sidecar TaskHost can hit this engine defect under `-mt`.
As a temporary mitigation, we are keeping MSBuildCache graph/cache-population builds process-based and enabling `-mt` only for builds that do not import the cache shared-compilation support package.
Contributor guide
No contributing guide indexed for this repository
Research direction
Start with TaskHostTaskComplete.Translate and the FileAccessData deserialization path, then inspect FileAccessManager.ReportFileAccess to understand the null-path failure. Add the requested round-trip test verifying Path and other fields, and an integration test covering an unannotated task under -mt; done means accesses survive deserialization and the build completes without crashing.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- csharp
- Domain
- build-system
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Active
- Clarity
- Clearly specified
- Newbie friendliness
- 68/100