Skip results transfer in MT mode to fix ResultsNodeId race#13417
Merged
JanProvaznik merged 4 commits intodotnet:mainfrom Mar 20, 2026
Merged
Skip results transfer in MT mode to fix ResultsNodeId race#13417JanProvaznik merged 4 commits intodotnet:mainfrom
JanProvaznik merged 4 commits intodotnet:mainfrom
Conversation
) In MT mode, all in-proc nodes share the same ConfigCache and ResultsCache, so results transfer between nodes is unnecessary. The transfer mechanism races on the shared ResultsNodeId field when multiple nodes simultaneously call HandleRequestBlockedOnResultsTransfer for the same configuration, each overwriting ResultsNodeId with their own node ID. This causes intermittent crashes with 'Results for configuration X were not retrieved from node Y'. Fix: extract the transfer-needed check into a testable NeedsResultsTransfer method that returns false when MultiThreaded mode is active, skipping the entire transfer block. Tests: - Unit test (red/green validated): NeedsResultsTransfer_SkipsInMultiThreadedMode with 6 Theory cases covering all MT/ST × node ID combinations - Integration test: MultiThreadedBuild_SharedConfiguration_DoesNotCrash exercises the code path with 8 parallel projects and different target sets Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Member
Author
|
expert reviewer passe, ready to review |
Contributor
There was a problem hiding this comment.
Pull request overview
Fixes an intermittent crash in MSBuild -mt (multi-threaded, in-proc) builds by skipping the “results transfer” path when it’s unnecessary due to shared in-proc caches, eliminating a race on BuildRequestConfiguration.ResultsNodeId.
Changes:
- Added
RequestBuilder.NeedsResultsTransfer(...)and used it to bypass results-transfer blocking in MT mode. - Added unit coverage for
NeedsResultsTransferacross MT/ST and node-id combinations. - Added an integration-style MT build test exercising shared configuration + differing target sets to prevent regressions of #13188.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
src/Build/BackEnd/Components/RequestBuilder/RequestBuilder.cs |
Introduces NeedsResultsTransfer and uses it to skip results transfer in MT mode. |
src/Build.UnitTests/BackEnd/RequestBuilder_Tests.cs |
Adds a theory validating NeedsResultsTransfer behavior. |
src/Build.UnitTests/BackEnd/BuildManager_Tests.cs |
Adds an MT integration test intended to reproduce/prevent the ResultsNodeId race crash. |
You can also share your feedback on Copilot code review. Take the survey.
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
AR-May
reviewed
Mar 20, 2026
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
AR-May
approved these changes
Mar 20, 2026
This was referenced Mar 20, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fix for #13188: MT mode ResultsNodeId race condition
Problem
In MT (multi-threaded) mode, builds intermittently crash with:
\
InternalErrorException: MSB0001: Internal MSBuild Error: Results for configuration X were not retrieved from node Y
\\
Root Cause
\BuildRequestConfiguration.ResultsNodeId\ is a single \int\ field shared by all in-proc nodes via the shared \ConfigCache. When multiple in-proc nodes simultaneously need a configuration's results, they race through \HandleRequestBlockedOnResultsTransfer, which unconditionally overwrites \ResultsNodeId\ to the requesting node's ID. The last writer wins, causing other waiters to find a mismatched value when they wake up, failing the assertion in \RequestBuilder.BuildProject().
Key Insight
In MT mode, the entire results transfer mechanism is unnecessary. All in-proc nodes share the same \ConfigCache\ and \ResultsCache\ by reference. When a config is built on any node, the results are immediately accessible to all other in-proc nodes through the shared cache. MT mode also disallows out-of-proc nodes (\Scheduler.cs\ sets OOP node count to 0 when \MultiThreaded=true), so there are no cross-process transfers to worry about.
Fix
Extracted the transfer-needed check into a testable \NeedsResultsTransfer\ method that returns \ alse\ when \MultiThreaded\ mode is active, skipping the entire transfer block.
Tests
No ChangeWave Needed
This is a crash fix, not a behavioral change. The transfer was already a no-op in MT mode (copying data between shared references). Skipping it produces identical build results while preventing the race condition crash.
Fixes #13188