dotnet / dotnet/msbuild

An active ProjectInstance can be cached after stale  /mt  request cleanup

Open
#14,694 0 comments 1 reaction 2 assignees Claimed by @AR-May View on GitHub
triaged
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.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.