An active ProjectInstance can be cached after stale /mt request cleanup
- Dominant language
- C#
- Stars
- 5.5k
- Forks
- 1.5k
- Avg merge
- 1d 8h
- Merged PRs (30d)
- 141
Description
## Description
`BuildManager.Reset()` defensively clears `BuildRequestConfiguration.ActivelyBuildingTargets` at the end of a build:
https://github.com/dotnet/msbuild/blob/main/src/Build/BackEnd/BuildManager/BuildManager.cs#L2520-L2525
This may be unsafe if a request task ignores cancellation. `WaitForCancelCompletion()` reports a timeout but cannot terminate the task, so the old `TargetBuilder` may later resume and execute its normal cleanup.
## Potential interleaving
1. An old request records `Build -> oldRequestId` in `ActivelyBuildingTargets`.
2. Cancellation times out while that request remains alive.
3. End-of-build cleanup clears `ActivelyBuildingTargets`.
4. A later build records `Build -> newRequestId` on the retained configuration.
5. The old request resumes and calls `Remove("Build")`.
6. The new request's entry is removed even though its target is still executing.
Target removal currently checks only the target name, not the owning request ID:
https://github.com/dotnet/msbuild/blob/main/src/Build/BackEnd/Components/RequestBuilder/TargetBuilder.cs#L529-L530
## Potential impact
The configuration can incorrectly report that no target is active. This could cause:
- another request to execute the same target concurrently instead of waiting;
- incorrect circular-dependency or target-blocking behavior;
- `CacheIfPossible()` to cache the `ProjectInstance` while the new target is still using it.
## Related discussion
This was identified while discussing cancellation cleanup in PR #14679:
- PR: https://github.com/dotnet/msbuild/pull/14679
- Interleaving described in review response: https://github.com/dotnet/msbuild/pull/14679#issuecomment-5267140020
Contributor guide
No contributing guide indexed for this repository
Assessment
This issue has not been assessed yet.