dotnet / dotnet/msbuild

TaskHost file-access data is corrupted under multithreaded MSBuild

Open
#14,824 2 comments 0 reactions 0 assignees View on GitHub
Area: Multithreaded Priority:1 triaged
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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.