diff --git a/.github/agents/expert-reviewer.md b/.github/agents/expert-reviewer.md
index a4fe9006f5f..f0fbd3bf2fc 100644
--- a/.github/agents/expert-reviewer.md
+++ b/.github/agents/expert-reviewer.md
@@ -40,15 +40,17 @@ Apply **all** dimensions on every review, weighted by file location (see [Folder
**Severity: BLOCKING**
**Rules:**
-1. Gate breaking behavior changes behind a ChangeWave. See `../../documentation/wiki/ChangeWaves.md`.
-2. New warnings are breaking changes for `-WarnAsError` builds. Gate behind ChangeWave or emit as `Message`.
-3. Make behavioral changes opt-in by default.
-4. SDK target changes must preserve backward compatibility with existing project files.
-5. Never remove CLI switches or aliases — deprecate with warnings first.
+1. Breaking changes are basically forbidden. Customers expect any project that built before to keep building.
+2. In cases where we expect no breaking behavior but there is a slight chance it should be gated behind a ChangeWave. See `../../documentation/wiki/ChangeWaves.md`.
+3. New warnings are breaking changes because customers set `-WarnAsError` in their builds. Gate them behind a ChangeWave or emit them as `Message`.
+4. Make behavioral changes opt-in by default.
+5. SDK target changes must preserve backward compatibility with existing project files.
+6. Never remove CLI switches or aliases — deprecate with warnings first.
**CHECK — Flag if:**
-- [ ] A behavioral change has no ChangeWave gate
-- [ ] New warnings emitted without considering `WarnAsError` impact
+- [ ] A change can be percieved by customers as breaking
+- [ ] Risky behavioral change has no ChangeWave gate
+- [ ] New warnings emitted
- [ ] A property default value has changed
- [ ] A CLI switch or alias removed rather than deprecated
- [ ] Output format changes could break downstream consumers
@@ -62,7 +64,7 @@ Apply **all** dimensions on every review, weighted by file location (see [Folder
See `../../documentation/wiki/ChangeWaves.md` and `../../documentation/wiki/ChangeWaves-Dev.md`.
**Rules:**
-1. Use the correct ChangeWave version — must match the upcoming MSBuild release, not the current one.
+1. Use the correct ChangeWave version — must match the upcoming MSBuild release, not the current one
2. Test both enabled and disabled paths — `MSBuildDisableFeaturesFromVersion` opt-out must work.
3. Document the ChangeWave in error messages, specs, and the ChangeWaves tracking file.
@@ -87,7 +89,7 @@ Hot paths: `Evaluator.cs`, `Expander.cs`, file I/O operations.
4. Profile before optimizing — claims require evidence.
**CHECK — Flag if:**
-- [ ] LINQ in a loop on a hot path
+- [ ] LINQ
- [ ] Strings allocated that could be reused or avoided
- [ ] A value recomputed on every call when cacheable
- [ ] `Dictionary` for <10 items, or `List` for frequent lookups of >100 items
@@ -123,7 +125,7 @@ See `../../documentation/assigning-msb-error-code.md`.
**Rules:**
1. Messages must be actionable — state what happened, why, and how to fix.
2. Use `MSBxxxx` format error/warning codes, each unique across the codebase.
-3. Use `ResourceUtilities.FormatResourceStringStripCodeAndKeyword` for formatting.
+3. Use `ResourceUtilities` for formatting.
4. Use correct severity levels — don't emit errors for non-fatal conditions.
**CHECK — Flag if:**
@@ -243,6 +245,7 @@ See `../../documentation/wiki/Microsoft.Build.Framework.md`.
2. Handle .NET Framework vs .NET Core differences explicitly.
3. Handle UNC paths, long paths (`\\?\`), and symlinks.
4. Build output must not differ based on build machine OS.
+5. Use Microsoft.IO Redist when targeting .NET Framework for better performance.
**CHECK — Flag if:**
- [ ] Hardcoded path separators
@@ -250,6 +253,7 @@ See `../../documentation/wiki/Microsoft.Build.Framework.md`.
- [ ] File path case sensitivity not considered
- [ ] Code assumes `\r\n` newlines
- [ ] `.NET Framework`-only API without `#if` guard
+- [ ] System.IO used instead of Microsoft.IO Redist for .NET Framework file operations
---
@@ -277,13 +281,14 @@ See `../../documentation/wiki/Microsoft.Build.Framework.md`.
**Severity: BLOCKING**
-See `../../documentation/wiki/Nodes-Orchestration.md`.
+See `../../documentation/wiki/Nodes-Orchestration.md`, `../../documentation/specs/multithreading/multithreaded-msbuild.md`
**Rules:**
1. Shared mutable state must be thread-safe (`ConcurrentDictionary`, `Interlocked`, explicit locking).
2. Synchronize access to state from multiple threads or nodes.
3. Handle in-proc vs out-of-proc node differences — behavior must be correct in both.
4. Consider IPC concurrency: message ordering, reentrancy, serialization.
+5. Check assumptions where only one thread accesses a code path.
**CHECK — Flag if:**
- [ ] Shared field read/written without synchronization
@@ -341,6 +346,7 @@ See `../../documentation/ProjectReference-Protocol.md`.
2. Match surrounding code style — MSBuild conventions may differ from general .NET guidance.
3. Handle nullability explicitly with annotations and `is null`/`is not null`.
4. Track .NET version constraints with `// TODO` for unavailable APIs.
+5. new code should have nullable enabled even if some parts of codebase don't
**CHECK — Flag if:**
- [ ] `using` block where surrounding code uses `using` declarations
@@ -453,12 +459,14 @@ See `../../documentation/High-level-overview.md` and `../../documentation/Built-
2. Verify behavior matches documented semantics.
3. Validate fixes against original repro steps.
4. Validate inputs early — fail fast with clear errors.
+5. Imagine exotic scenarios that could break assumptions. Like a red teamer trying to break MSBuild with weird project files or build environments.
**CHECK — Flag if:**
- [ ] New code path doesn't handle null/empty inputs
- [ ] Bug fix doesn't include original repro as test
- [ ] Boundary conditions not considered (off-by-one, max-length, empty)
- [ ] Input validation missing at public API boundaries
+- [ ] Code relies on assumptions that exotic inputs could break
---
@@ -483,12 +491,12 @@ See `../../documentation/High-level-overview.md` and `../../documentation/Built-
**Severity: BLOCKING**
**Rules:**
-1. Security-relaxing parameters (e.g., `TrustServerCertificate`) must require explicit user opt-in.
+1. Never regress security
2. Task type loading must unify to currently-running MSBuild assemblies.
3. Consider path traversal, symlink following, and temp file safety.
**CHECK — Flag if:**
-- [ ] Security-relaxing flag applied unconditionally
+- [ ] Security-relaxing assumption
- [ ] Task assembly loading accepts untrusted paths without validation
- [ ] File operations exploitable via symlinks or path traversal
- [ ] Credentials or tokens logged/stored insecurely
diff --git a/.github/instructions/shared-code.instructions.md b/.github/instructions/shared-code.instructions.md
index 7ff95fdf661..f6d9af0858b 100644
--- a/.github/instructions/shared-code.instructions.md
+++ b/.github/instructions/shared-code.instructions.md
@@ -29,7 +29,3 @@ Code in `src/Shared/` is linked (compiled) into multiple MSBuild assemblies. A b
* Handle .NET Framework vs .NET Core differences with appropriate `#if` guards.
* Environment variable access patterns differ across platforms — use the shared helpers.
-
-## Related Documentation
-
-* [Contributing Code](../../documentation/wiki/Contributing-Code.md)
diff --git a/.github/instructions/tasks.instructions.md b/.github/instructions/tasks.instructions.md
index ed006eb5dde..08ebfb794c3 100644
--- a/.github/instructions/tasks.instructions.md
+++ b/.github/instructions/tasks.instructions.md
@@ -33,7 +33,7 @@ Built-in tasks ship with MSBuild and cannot be independently versioned.
## Multithreaded Task Migration
-* Tasks must not hold locks across yield points.
+* All built-in tasks implement `IMultiThreadableTask` with a default `TaskEnvironment` backed by `MultiProcessTaskEnvironmentDriver.Instance`.
* Shared static state is a concurrency hazard in multi-process builds.
## Related Documentation
diff --git a/.github/skills/multithreaded-task-migration/SKILL.md b/.github/skills/multithreaded-task-migration/SKILL.md
index b098e2c49ce..2a64b6ecc8b 100644
--- a/.github/skills/multithreaded-task-migration/SKILL.md
+++ b/.github/skills/multithreaded-task-migration/SKILL.md
@@ -18,7 +18,7 @@ b. Implement `IMultiThreadableTask` **only if** the task needs `TaskEnvironment`
[MSBuildMultiThreadableTask]
public class MyTask : Task, IMultiThreadableTask
{
- public TaskEnvironment TaskEnvironment { get; set; }
+ public TaskEnvironment TaskEnvironment { get; set; } = new TaskEnvironment(MultiProcessTaskEnvironmentDriver.Instance);
...
}
```
@@ -61,7 +61,7 @@ The [`AbsolutePath`](https://github.com/dotnet/msbuild/blob/main/src/Framework/P
## Updating Unit Tests
-Every test creating a task instance must set `TaskEnvironment = TaskEnvironmentHelper.CreateForTest()`.
+Built-in MSBuild tasks now initialize `TaskEnvironment` with a `MultiProcessTaskEnvironmentDriver`-backed default. Tests creating instances of built-in tasks no longer need manual `TaskEnvironment` setup. For custom or third-party tasks that implement `IMultiThreadableTask` without a default initializer, set `TaskEnvironment = TaskEnvironmentHelper.CreateForTest()`.
## APIs to Avoid
@@ -276,7 +276,7 @@ Assertions: Execute() return value, [Output] exact string, error message content
## Sign-Off Checklist
- [ ] `[MSBuildMultiThreadableTask]` on every concrete class (not just base — `Inherited=false`)
-- [ ] `IMultiThreadableTask` only on classes that use `TaskEnvironment` APIs
+- [ ] `IMultiThreadableTask` on classes that use `TaskEnvironment` APIs, with default initializer `= new TaskEnvironment(MultiProcessTaskEnvironmentDriver.Instance)`
- [ ] Every `[Output]` property: exact string value matches pre-migration
- [ ] Every `Log.LogError`/`LogWarning`: path in message matches pre-migration (use `OriginalValue`)
- [ ] Every `GetAbsolutePath` call: null/empty exception behavior matches old code path
@@ -285,7 +285,7 @@ Assertions: Execute() return value, [Output] exact string, error message content
- [ ] Every `??` or `?.` added: verified it doesn't swallow a previously-thrown exception
- [ ] No `AbsolutePath` leaks into user-visible strings unintentionally
- [ ] Helper methods traced for internal File API usage with non-absolutized paths
-- [ ] All tests set `TaskEnvironment = TaskEnvironmentHelper.CreateForTest()`
+- [ ] Tests for custom tasks set `TaskEnvironment = TaskEnvironmentHelper.CreateForTest()` (built-in tasks have a default)
- [ ] Cross-framework: tested on both net472 and net10.0
- [ ] Concurrent execution: two tasks with different project directories produce correct results
- [ ] No forbidden APIs (`Environment.Exit`, `Console.*`, etc.)
diff --git a/azure-pipelines/vs-insertion-experimental.yml b/azure-pipelines/vs-insertion-experimental.yml
index 215e88c5ecc..05018f82c08 100644
--- a/azure-pipelines/vs-insertion-experimental.yml
+++ b/azure-pipelines/vs-insertion-experimental.yml
@@ -24,20 +24,8 @@ parameters:
displayName: 'Insertion Target Branch (select for manual insertion)'
values:
- main
- - rel/d18.6
- - rel/d18.5
- - rel/d18.4
- - rel/d18.3
- - rel/d18.0
- - rel/d17.14
- - rel/d17.13
- - rel/d17.12
- - rel/d17.11
- - rel/d17.8
- - rel/d17.6
- - rel/d17.3
- - rel/d17.0
- - rel/d16.11
+ - rel/insiders
+ - rel/stable
variables:
- name: TeamName
@@ -120,28 +108,13 @@ extends:
inputs:
targetType: inline
script: |
- # Extract VS version from branch name if it follows exp/vsXX.Y-somename pattern
- $fullBranch = "$(resources.pipeline.MSBuildExpPerf.sourceBranch)"
$parameterTargetBranch = "${{ parameters.TargetBranch }}"
- $detectedTarget = "main" # Default target branch
-
- # Try to match the pattern with regex
- if ($fullBranch -match "exp/vs(\d+)\.(\d+).*") {
- $major = $matches[1]
- $minor = $matches[2]
- $targetBranch = "rel/d$major.$minor"
- Write-Host "Detected version pattern in branch: $major.$minor"
- Write-Host "Setting target branch to $targetBranch"
- $detectedTarget = $targetBranch
- } else {
- Write-Host "No version pattern detected in branch, using default target: main"
- }
# Determine which target branch to use based on build reason
$finalTargetBranch = $parameterTargetBranch
if ("$(Build.Reason)" -eq "ResourceTrigger" -or "$(Build.Reason)" -eq "PipelineCompletedTrigger") {
- Write-Host "Build was triggered automatically, using detected target branch: $detectedTarget"
- $finalTargetBranch = $detectedTarget
+ Write-Host "Build was triggered automatically, using default target branch: main"
+ $finalTargetBranch = "main"
} else {
Write-Host "Build was triggered manually, using parameter target branch: $parameterTargetBranch"
}
diff --git a/azure-pipelines/vs-insertion.yml b/azure-pipelines/vs-insertion.yml
index f40e263cea8..e0af55a65f8 100644
--- a/azure-pipelines/vs-insertion.yml
+++ b/azure-pipelines/vs-insertion.yml
@@ -28,7 +28,7 @@ resources:
branch: main # for daily main scheduled insertion
trigger:
branches:
- include: # trigger as a followup to servicing CI
+ include: # trigger as a followup to CI on servicing branches
- vs*
repositories:
- repository: 1ESPipelineTemplates
@@ -44,20 +44,8 @@ parameters:
values:
- auto
- main
- - rel/d18.6
- - rel/d18.5
- - rel/d18.4
- - rel/d18.3
- - rel/d18.0
- - rel/d17.14
- - rel/d17.13
- - rel/d17.12
- - rel/d17.11
- - rel/d17.10
- - rel/d17.8
- - rel/d17.6
- - rel/d17.3
- - rel/d17.0
+ - rel/insiders
+ - rel/stable
- name: DropRetentionDays
default: 183
type: number
@@ -69,44 +57,11 @@ parameters:
variables:
# `auto` should work every time and selecting a branch in parameters is likely to fail due to incompatible versions in MSBuild and VS
- - name: AutoInsertTargetBranch
- ${{ if eq(variables['Build.SourceBranchName'], 'vs18.6') }}:
- value: 'rel/d18.6'
- ${{ elseif eq(variables['Build.SourceBranchName'], 'vs18.5') }}:
- value: 'rel/d18.5'
- ${{ elseif eq(variables['Build.SourceBranchName'], 'vs18.4') }}:
- value: 'rel/d18.4'
- ${{ elseif eq(variables['Build.SourceBranchName'], 'vs18.3') }}:
- value: 'rel/d18.3'
- ${{ elseif eq(variables['Build.SourceBranchName'], 'vs18.0') }}:
- value: 'rel/d18.0'
- ${{ elseif eq(variables['Build.SourceBranchName'], 'vs17.14') }}:
- value: 'rel/d17.14'
- ${{ elseif eq(variables['Build.SourceBranchName'], 'vs17.13') }}:
- value: 'rel/d17.13'
- ${{ elseif eq(variables['Build.SourceBranchName'], 'vs17.12') }}:
- value: 'rel/d17.12'
- ${{ elseif eq(variables['Build.SourceBranchName'], 'vs17.11') }}:
- value: 'rel/d17.11'
- ${{ elseif eq(variables['Build.SourceBranchName'], 'vs17.8') }}:
- value: 'rel/d17.8'
- ${{ elseif eq(variables['Build.SourceBranchName'], 'vs17.6') }}:
- value: 'rel/d17.6'
- ${{ elseif eq(variables['Build.SourceBranchName'], 'vs17.3') }}:
- value: 'rel/d17.3'
- ${{ elseif eq(variables['Build.SourceBranchName'], 'vs17.0') }}:
- value: 'rel/d17.0'
- ${{ elseif eq(variables['Build.SourceBranchName'], 'main') }}:
- value: 'main'
- ${{ elseif eq(variables['Build.SourceBranchName'], 'vs17.15') }}:
- value: 'main'
- ${{ else }}:
- value: ''
- name: InsertTargetBranch
${{ if not(eq(parameters.TargetBranch, 'auto')) }}:
value: ${{ parameters.TargetBranch }}
${{ else }}:
- value: $(AutoInsertTargetBranch)
+ value: 'main'
- name: TeamName
value: msbuild
- name: TeamEmail
@@ -227,37 +182,10 @@ extends:
$MicrosoftNETStringToolsPackageVersion = $packageFile.BaseName.TrimStart("Microsoft.NET.StringTools")
Write-Host "Setting MicrosoftNETStringToolsPackageVersion to '$MicrosoftNETStringToolsPackageVersion'"
Write-Host "##vso[task.setvariable variable=MicrosoftNETStringToolsPackageVersion]$($MicrosoftNETStringToolsPackageVersion)"
- $props = @(
- "VS.ExternalAPIs.MSBuild=$MSBuild_ExtApisPackageVersion",
- "Microsoft.Build=$MicrosoftNETStringToolsPackageVersion",
- "Microsoft.Build.Framework=$MicrosoftNETStringToolsPackageVersion",
- "Microsoft.Build.Tasks.Core=$MicrosoftNETStringToolsPackageVersion",
- "Microsoft.Build.Utilities.Core=$MicrosoftNETStringToolsPackageVersion",
- "Microsoft.NET.StringTools=$MicrosoftNETStringToolsPackageVersion"
- )
- # servicing branches until 17.12 also include Microsoft.Build.Engine and Microsoft.Build.Conversion.Core
- if ("$(InsertTargetBranch)" -in @("rel/d17.0", "rel/d17.3", "rel/d17.6", "rel/d17.8", "rel/d17.10", "rel/d17.11", "rel/d17.12"))
- {
- $props += @(
- "Microsoft.Build.Conversion.Core=$MicrosoftNETStringToolsPackageVersion",
- "Microsoft.Build.Engine=$MicrosoftNETStringToolsPackageVersion"
- )
- }
- $propsValue = $props -join ";"
-
- # For 18.0+, don't update PackageReferences so that VS packages like VSSDK
- # can choose their own versioning and don't always depend on a super fresh MSBuild
- if ("$(InsertTargetBranch)" -in @("rel/d17.10", "rel/d17.12", "rel/d17.14"))
- {
- Write-Host "Setting InsertPackagePropsValues to '$propsValue'"
- Write-Host "##vso[task.setvariable variable=InsertPackagePropsValues]$($propsValue)"
- }
- else
- {
- Write-Host "Unsetting InsertPackagePropsValues"
- Write-Host "##vso[task.setvariable variable=InsertPackagePropsValues]"
- }
+ # InsertPackagePropsValues was needed for old servicing branches (rel/d17.x) but is not used for main
+ Write-Host "Unsetting InsertPackagePropsValues"
+ Write-Host "##vso[task.setvariable variable=InsertPackagePropsValues]"
# autocomplete main
$autocomplete = "false"
diff --git a/documentation/specs/multithreading/multithreaded-msbuild.md b/documentation/specs/multithreading/multithreaded-msbuild.md
index cc224f4aa80..b21eb53c369 100644
--- a/documentation/specs/multithreading/multithreaded-msbuild.md
+++ b/documentation/specs/multithreading/multithreaded-msbuild.md
@@ -138,6 +138,8 @@ The scheduler is already capable of juggling multiple projects, and there's alre
The scheduler should be responsible for creating the appropriate combination of nodes (in-proc, out-of-proc, and thread nodes) based on the execution mode (multi-proc or multithreaded, CLI or Visual Studio scenarios). It will then coordinate projects execution through the node abstraction. Below is the diagram for cli multi-threaded mode creating all the thread nodes in the entry process for simplicity--in final production these will be in an [MSBuild Server process](#msbuild-server-integration).
+In the current implementation, enabling multithreaded mode implies that all worker nodes are in-proc. Out-of-proc worker-node topologies for multithreaded execution remain future work.
+
```mermaid
sequenceDiagram
@@ -234,7 +236,7 @@ The `MSBuildMultiThreadableTaskAttribute` is **non-inheritable** (`Inherited = f
* Derived classes cannot accidentally inherit thread-safety assumptions from base classes
* The routing decision is always explicit and visible in the task's source code
-Tasks may optionally implement `IMultiThreadableTask` to access `TaskEnvironment` APIs, but only the attribute determines routing behavior.
+Tasks may optionally implement `IMultiThreadableTask` to access `TaskEnvironment` APIs, but only the attribute determines routing behavior. If task implements `IMultiThreadableTask`, `TaskEnvironment` should be backed by `MultiProcessTaskEnvironmentDriver.Instance`, which acts as a fallback for explicit instantiation and task host scenarios.
## Tasks transition
@@ -246,6 +248,8 @@ To ease task authoring, we will provide a Roslyn analyzer that will check for kn
We need to ensure the support for multithreaded mode in Visual Studio builds. Currently, the entry node for MSBuild runs entirely within the devenv process, but the majority of the build operation are run in the MSBuild worker processes, because project systems set `BuildParameters.DisableInProcNode=true`. In multithreaded mode, all of the task execution must continue to be out of process. To address this, unlike the CLI scenario, we will move all thread nodes to the out-of-process MSBuild process, keeping only the scheduler in devenv.
+This section describes the intended Visual Studio topology, not the current implementation invariant that multithreaded mode implies all worker nodes are in-proc.
+
```mermaid
sequenceDiagram
diff --git a/documentation/specs/multithreading/taskhost-threading.md b/documentation/specs/multithreading/taskhost-threading.md
index 37116f3de74..c04d53f366b 100644
--- a/documentation/specs/multithreading/taskhost-threading.md
+++ b/documentation/specs/multithreading/taskhost-threading.md
@@ -26,7 +26,7 @@ When the main thread receives a `TaskHostConfiguration` packet, it spawns the ta
1. Sets up the environment (working directory, env vars, culture)
2. Loads the task assembly and instantiates the task
3. Sets task parameters via reflection
-4. Calls `task.Execute()`
+4. Calls `task.Execute()` — for tasks implementing `IMultiThreadableTask`, the default `TaskEnvironment` (backed by `MultiProcessTaskEnvironmentDriver`) is available, providing safe access to the task host process's working directory and environment variables
5. Collects output parameters
6. Packages the result into `TaskHostTaskComplete` and signals `_taskCompleteEvent`
diff --git a/documentation/specs/multithreading/thread-safe-tasks.md b/documentation/specs/multithreading/thread-safe-tasks.md
index a983fda8ef7..3ef98a4b215 100644
--- a/documentation/specs/multithreading/thread-safe-tasks.md
+++ b/documentation/specs/multithreading/thread-safe-tasks.md
@@ -39,10 +39,12 @@ Similar to how MSBuild provides the abstract `Task` class with default implement
namespace Microsoft.Build.Utilities;
public abstract class MultiThreadableTask : Task, IMultiThreadableTask
{
- public TaskEnvironment TaskEnvironment{ get; set; }
+ public TaskEnvironment TaskEnvironment{ get; set; } = new TaskEnvironment(MultiProcessTaskEnvironmentDriver.Instance);
}
```
+Built-in MSBuild tasks initialize `TaskEnvironment` with a `MultiProcessTaskEnvironmentDriver`-backed default. This ensures tasks have a usable `TaskEnvironment` even when explicitly instantiated outside the engine (e.g., `new Copy()`) or run in the out-of-proc task host. The engine's in-proc path (`TaskExecutionHost.InitializeForBatch`) overwrites the default with the appropriate driver before `Execute()` is called.
+
Task authors who want to support older MSBuild versions need to:
- Maintain both thread-safe and legacy implementations.
- Use conditional task declarations based on MSBuild version to select which assembly to load the task from.
diff --git a/src/Build.UnitTests/BackEnd/BuildManager_Tests.cs b/src/Build.UnitTests/BackEnd/BuildManager_Tests.cs
index 4cbc6f23c14..6d5aaeed2bc 100644
--- a/src/Build.UnitTests/BackEnd/BuildManager_Tests.cs
+++ b/src/Build.UnitTests/BackEnd/BuildManager_Tests.cs
@@ -4485,5 +4485,206 @@ public void ProjectWithNoTargetsGraph()
_logger.AssertLogContains("MSB4040");
}
+
+ ///
+ /// Verifies that MT mode builds with multiple projects referencing the same dependency
+ /// with different target sets do not crash with "Results for configuration X were not
+ /// retrieved from node Y" (GitHub issue #13188).
+ ///
+ /// The bug: in MT mode, BuildRequestConfiguration.ResultsNodeId is shared across all
+ /// in-proc nodes. When multiple nodes need to build different targets of the same config,
+ /// the results cache can't fully resolve the request, so the scheduler assigns it to a
+ /// new node. That node's BuildProject() sees ResultsNodeId pointing to the original node
+ /// and enters the results transfer block, which races on ResultsNodeId when multiple
+ /// nodes do this concurrently.
+ ///
+ /// The fix: skip the results transfer block entirely in MT mode, since all in-proc nodes
+ /// share the same ConfigCache and ResultsCache — results are already accessible.
+ ///
+ /// This test forces the scenario by having multiple parallel projects reference the same
+ /// dependency with DIFFERENT target sets, causing cache misses that force the scheduler
+ /// to assign the same config to different nodes.
+ ///
+ [Fact]
+ public void MultiThreadedBuild_SharedConfiguration_DoesNotCrash()
+ {
+ // Shared dependency project with multiple targets.
+ // Different parent projects will request different target combinations,
+ // causing partial cache misses that force re-scheduling on different nodes.
+ var sharedDep = _env.CreateFile(".proj").Path;
+ var projRoot = _env.CreateFile(".proj").Path;
+
+ // Create many parallel referencing projects to maximize the chance of
+ // scheduling the same config on different nodes.
+ const int projectCount = 8;
+ var childProjects = new string[projectCount];
+ for (int i = 0; i < projectCount; i++)
+ {
+ childProjects[i] = _env.CreateFile(".proj").Path;
+ }
+
+ // The shared dep has multiple targets. Each parent project requests a different
+ // combination so the cache can't satisfy the request from a previous build.
+ string sharedDepContents = CleanupFileContents("""
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+ """);
+ File.WriteAllText(sharedDep, sharedDepContents);
+
+ // Each child project requests a different cumulative set of targets from the shared dep.
+ // Child 0: TargetA
+ // Child 1: TargetA;TargetB (cache has TargetA but not TargetB → miss → re-schedule)
+ // Child 2: TargetA;TargetB;TargetC (cache has A,B but not C → miss → re-schedule)
+ // etc.
+ // This forces the scheduler to schedule the shared dep's config on different nodes
+ // each time a new target is needed, triggering the ResultsNodeId != currentNodeId check.
+ string[] targetNames = ["TargetA", "TargetB", "TargetC", "TargetD", "TargetE", "TargetF", "TargetG", "TargetH"];
+ for (int i = 0; i < projectCount; i++)
+ {
+ string targets = string.Join(";", targetNames.Take(i + 1));
+ string childContents = CleanupFileContents($"""
+
+
+
+
+
+
+ """);
+ File.WriteAllText(childProjects[i], childContents);
+ }
+
+ // Root project builds all children in parallel, maximizing the chance that multiple
+ // nodes simultaneously need the shared dependency with different target sets.
+ string allChildProjects = string.Join(";", childProjects);
+ string rootContents = CleanupFileContents($"""
+
+
+
+
+
+ """);
+ File.WriteAllText(projRoot, rootContents);
+
+ var buildParameters = new BuildParameters
+ {
+ MaxNodeCount = 8,
+ EnableNodeReuse = false,
+ MultiThreaded = true,
+ DisableInProcNode = false,
+ // MT mode needs SaveOperatingEnvironment=false to allow multiple in-proc nodes.
+ // With it enabled, the operating environment semaphore limits to one in-proc node.
+ SaveOperatingEnvironment = false,
+ Loggers = [_logger],
+ };
+
+ var data = new BuildRequestData(projRoot, new Dictionary(), null,
+ ["Build"], null);
+
+ // Use DefaultBuildManager for MT mode (needs proper in-proc node setup).
+ BuildManager.DefaultBuildManager.Dispose();
+ BuildResult result = BuildManager.DefaultBuildManager.Build(buildParameters, data);
+
+ result.OverallResult.ShouldBe(BuildResultCode.Success);
+
+ // Verify all targets of the shared dep were built.
+ foreach (string target in targetNames)
+ {
+ _logger.AssertLogContains($"{target} built");
+ }
+
+ // Verify all child projects completed.
+ for (int i = 0; i < projectCount; i++)
+ {
+ _logger.AssertLogContains($"Child{i} built");
+ }
+ }
+
+ ///
+ /// Regression test: when MSBUILDDEBUGSCHEDULER is enabled in multithreaded (-mt) mode,
+ /// multiple in-proc node engines write trace output to files. Previously, all engines
+ /// wrote to the same EngineTrace_{PID}.txt file with FileShare.Read, causing IOException
+ /// when two engines traced concurrently. The IOException fatally crashed the ActionBlock
+ /// work queue, silently dropping subsequent request completions and causing a deadlock.
+ ///
+ /// The fix uses per-node trace files (EngineTrace_{PID}_{NodeId}.txt) and catches IO
+ /// exceptions in TraceEngine() so they can never crash the build engine.
+ ///
+ [Fact]
+ public void MultiThreadedBuild_WithDebugSchedulerTracing_DoesNotDeadlock()
+ {
+ string debugPath = _env.CreateFolder().Path;
+ _env.SetEnvironmentVariable("MSBUILDDEBUGSCHEDULER", "1");
+ _env.SetEnvironmentVariable("MSBUILDDEBUGPATH", debugPath);
+
+ // Create a root project that builds two independent child projects in parallel.
+ // This forces multiple in-proc nodes to run concurrently, which triggers
+ // concurrent TraceEngine() calls.
+ string child1 = _env.CreateFile(".proj").Path;
+ string child2 = _env.CreateFile(".proj").Path;
+ string root = _env.CreateFile(".proj").Path;
+
+ string childContent = CleanupFileContents("""
+
+
+
+
+
+ """);
+ File.WriteAllText(child1, childContent);
+ File.WriteAllText(child2, childContent);
+
+ string rootContent = CleanupFileContents($"""
+
+
+
+
+
+
+ """);
+ File.WriteAllText(root, rootContent);
+
+ var buildParameters = new BuildParameters
+ {
+ MaxNodeCount = 4,
+ EnableNodeReuse = false,
+ MultiThreaded = true,
+ DisableInProcNode = false,
+ SaveOperatingEnvironment = false,
+ Loggers = [_logger],
+ };
+
+ var data = new BuildRequestData(root, new Dictionary(), null,
+ ["Build"], null);
+
+ BuildManager.DefaultBuildManager.Dispose();
+ BuildResult result = BuildManager.DefaultBuildManager.Build(buildParameters, data);
+
+ result.OverallResult.ShouldBe(BuildResultCode.Success);
+ _logger.AssertLogContains("Root completed");
+ }
}
}
diff --git a/src/Build.UnitTests/BackEnd/RequestBuilder_Tests.cs b/src/Build.UnitTests/BackEnd/RequestBuilder_Tests.cs
index 80e9c990ba6..bb5b64ced35 100644
--- a/src/Build.UnitTests/BackEnd/RequestBuilder_Tests.cs
+++ b/src/Build.UnitTests/BackEnd/RequestBuilder_Tests.cs
@@ -12,6 +12,7 @@
using Microsoft.Build.Framework;
using Microsoft.Build.Shared;
using Microsoft.Build.Unittest;
+using Shouldly;
using Xunit;
using TaskItem = Microsoft.Build.Execution.ProjectItemInstance.TaskItem;
@@ -340,6 +341,26 @@ private NodeLoggingContext GetNodeLoggingContext()
{
return new NodeLoggingContext(_host, 1, false);
}
+
+ ///
+ /// Verifies NeedsResultsTransfer returns false in MT mode, even when ResultsNodeId
+ /// points to a different node. In MT mode all in-proc nodes share the same cache,
+ /// so results are already accessible without transfer.
+ /// See https://github.com/dotnet/msbuild/issues/13188
+ ///
+ [Theory]
+ [InlineData(true, 1, 2, false)] // MT mode, results on different node → skip transfer (shared cache)
+ [InlineData(true, 1, 1, false)] // MT mode, results on same node → no transfer needed
+ [InlineData(true, Scheduler.InvalidNodeId, 2, false)] // MT mode, results unset → no transfer needed
+ [InlineData(false, 1, 2, true)] // ST mode, results on different node → transfer needed
+ [InlineData(false, 1, 1, false)] // ST mode, results on same node → no transfer needed
+ [InlineData(false, Scheduler.InvalidNodeId, 2, false)] // ST mode, results unset → no transfer needed
+ public void NeedsResultsTransfer_SkipsInMultiThreadedMode(
+ bool isMultiThreaded, int resultsNodeId, int currentNodeId, bool expectedNeedsTransfer)
+ {
+ bool result = RequestBuilder.NeedsResultsTransfer(resultsNodeId, currentNodeId, isMultiThreaded);
+ result.ShouldBe(expectedNeedsTransfer);
+ }
}
internal sealed class TestTargetBuilder : ITargetBuilder, IBuildComponent
diff --git a/src/Build.UnitTests/BackEnd/TaskHost_MultiThreadableTask_Tests.cs b/src/Build.UnitTests/BackEnd/TaskHost_MultiThreadableTask_Tests.cs
new file mode 100644
index 00000000000..f1e59ba557b
--- /dev/null
+++ b/src/Build.UnitTests/BackEnd/TaskHost_MultiThreadableTask_Tests.cs
@@ -0,0 +1,122 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+
+using System;
+using System.Collections.Generic;
+using System.IO;
+using System.Reflection;
+using Microsoft.Build.Execution;
+using Microsoft.Build.Framework;
+using Microsoft.Build.Tasks;
+using Microsoft.Build.UnitTests;
+using Microsoft.Build.Utilities;
+using Shouldly;
+using Xunit;
+using Xunit.Abstractions;
+
+namespace Microsoft.Build.Engine.UnitTests.BackEnd
+{
+ ///
+ /// Tests that IMultiThreadableTask implementations always have a usable TaskEnvironment,
+ /// even when explicitly instantiated or run in the out-of-proc task host.
+ ///
+ public class TaskHost_MultiThreadableTask_Tests : IDisposable
+ {
+ private readonly ITestOutputHelper _output;
+ private readonly TestEnvironment _env;
+ private readonly string _testProjectsDir;
+
+ public TaskHost_MultiThreadableTask_Tests(ITestOutputHelper output)
+ {
+ _output = output;
+ _env = TestEnvironment.Create(output);
+ _testProjectsDir = _env.CreateFolder().Path;
+ }
+
+ public void Dispose()
+ {
+ _env.Dispose();
+ }
+
+ ///
+ /// A subclass of a built-in IMultiThreadableTask (MakeDir) should inherit the
+ /// non-null default TaskEnvironment. This covers the scenario where someone
+ /// derives from a built-in task and explicitly instantiates it.
+ ///
+ [Fact]
+ public void ExplicitlyInstantiated_InheritedTask_HasNonNullTaskEnvironment()
+ {
+ var task = new DerivedMakeDirTask();
+ IMultiThreadableTask multiThreadable = task;
+
+ multiThreadable.TaskEnvironment.ShouldNotBeNull();
+ }
+
+ ///
+ /// When a task that inherits from a built-in IMultiThreadableTask (MakeDir) runs in
+ /// the out-of-proc task host (via TaskHostFactory), the inherited default TaskEnvironment
+ /// should be usable. The task accesses TaskEnvironment.ProjectDirectory in Execute() —
+ /// without the default it would NRE.
+ ///
+ [Fact]
+ public void InheritedTask_InTaskHost_HasUsableTaskEnvironment()
+ {
+ string projectContent = $"""
+
+
+
+
+
+
+
+ """;
+
+ string projectFile = Path.Combine(_testProjectsDir, "TaskEnvTest.proj");
+ File.WriteAllText(projectFile, projectContent);
+
+ var logger = new MockLogger(_output);
+ var buildParameters = new BuildParameters
+ {
+ Loggers = [logger],
+ DisableInProcNode = false,
+ EnableNodeReuse = false,
+ };
+
+ var buildRequestData = new BuildRequestData(
+ projectFile,
+ new Dictionary(),
+ null,
+ ["TestTarget"],
+ null);
+
+ var result = BuildManager.DefaultBuildManager.Build(buildParameters, buildRequestData);
+
+ _output.WriteLine(logger.FullLog);
+
+ result.OverallResult.ShouldBe(BuildResultCode.Success);
+
+ // Verify the task actually ran in the task host
+ TaskRouterTestHelper.AssertTaskUsedTaskHost(logger, "DerivedMakeDirTask");
+
+ // Verify the task was able to read ProjectDirectory without NRE
+ logger.FullLog.ShouldContain("TaskEnvironment.ProjectDirectory=");
+ }
+ }
+
+ ///
+ /// Task that inherits from the built-in MakeDir (which implements IMultiThreadableTask).
+ /// Does NOT declare its own TaskEnvironment — it relies on the inherited default from MakeDir.
+ /// Overrides Execute() to log TaskEnvironment.ProjectDirectory, proving the default works.
+ ///
+ public class DerivedMakeDirTask : MakeDir
+ {
+ public override bool Execute()
+ {
+ string projectDir = TaskEnvironment.ProjectDirectory;
+ Log.LogMessage(MessageImportance.High, $"TaskEnvironment.ProjectDirectory={projectDir}");
+ return true;
+ }
+ }
+}
diff --git a/src/Build.UnitTests/Telemetry/Telemetry_Tests.cs b/src/Build.UnitTests/Telemetry/Telemetry_Tests.cs
index d81776f1d77..0361e99ca0d 100644
--- a/src/Build.UnitTests/Telemetry/Telemetry_Tests.cs
+++ b/src/Build.UnitTests/Telemetry/Telemetry_Tests.cs
@@ -6,11 +6,14 @@
using System.Diagnostics;
using System.Linq;
using System.Threading;
+using System.Threading.Tasks;
+using Microsoft.Build.BackEnd.Logging;
using Microsoft.Build.Execution;
using Microsoft.Build.Framework;
using Microsoft.Build.Framework.Telemetry;
using Microsoft.Build.TelemetryInfra;
using Microsoft.Build.UnitTests;
+using Microsoft.Build.UnitTests.BackEnd;
using Shouldly;
using Xunit;
using Xunit.Abstractions;
@@ -470,5 +473,67 @@ public void BuildIncrementalityInfo_NoTargets_ClassifiedAsUnknown()
incrementality.TotalTargetsCount.ShouldBe(0);
incrementality.IncrementalityRatio.ShouldBe(0.0);
}
+
+ [Fact]
+ public void IsEmpty_TrueForDefault_FalseAfterAdd()
+ {
+ var data = new WorkerNodeTelemetryData();
+ data.IsEmpty.ShouldBeTrue();
+
+ var targetKey = new TaskOrTargetTelemetryKey("Target1", isCustom: false, isFromNugetCache: false, isFromMetaProject: false);
+ data.AddTarget(targetKey, wasExecuted: true);
+ data.IsEmpty.ShouldBeFalse();
+ }
+
+ [Fact]
+ public void FinalizeProcessing_AfterMerge_ResetsState()
+ {
+ var forwarder = new TelemetryForwarderProvider.TelemetryForwarder();
+ var loggingService = new EventRecordingLoggingService();
+
+ var loggingContext = new MockLoggingContext(
+ loggingService,
+ new BuildEventContext(1, 2, BuildEventContext.InvalidProjectContextId, 4));
+
+ // Merge some data.
+ var localData = new WorkerNodeTelemetryData();
+ var key = new TaskOrTargetTelemetryKey("TestTarget", isCustom: true, isFromNugetCache: false, isFromMetaProject: false);
+ localData.AddTarget(key, wasExecuted: true);
+ forwarder.MergeWorkerData(localData);
+
+ // First FinalizeProcessing should emit a telemetry event.
+ forwarder.FinalizeProcessing(loggingContext);
+ var telemetryEvents = loggingService.RecordedEvents.OfType().ToList();
+ telemetryEvents.Count.ShouldBe(1);
+ telemetryEvents[0].WorkerNodeTelemetryData.TargetsExecutionData.ShouldContainKey(key);
+
+ // Second FinalizeProcessing on an empty forwarder should be a no-op (state was reset).
+ forwarder.FinalizeProcessing(loggingContext);
+ loggingService.RecordedEvents.OfType().Count().ShouldBe(1, "No new event should be emitted after reset");
+
+ // Merge new data after reset — forwarder should still work.
+ var localData2 = new WorkerNodeTelemetryData();
+ var key2 = new TaskOrTargetTelemetryKey("TestTarget2", isCustom: false, isFromNugetCache: false, isFromMetaProject: false);
+ localData2.AddTarget(key2, wasExecuted: false, skipReason: TargetSkipReason.ConditionWasFalse);
+ forwarder.MergeWorkerData(localData2);
+
+ // Third FinalizeProcessing should emit only the new data.
+ forwarder.FinalizeProcessing(loggingContext);
+ var allTelemetryEvents = loggingService.RecordedEvents.OfType().ToList();
+ allTelemetryEvents.Count.ShouldBe(2);
+ allTelemetryEvents[1].WorkerNodeTelemetryData.TargetsExecutionData.ShouldContainKey(key2);
+ allTelemetryEvents[1].WorkerNodeTelemetryData.TargetsExecutionData.ShouldNotContainKey(key, "Old data should not appear after reset");
+ }
+
+ ///
+ /// that records all calls
+ /// so tests can inspect emitted build events.
+ ///
+ private sealed class EventRecordingLoggingService : MockLoggingService, ILoggingService
+ {
+ public List RecordedEvents { get; } = [];
+
+ void ILoggingService.LogBuildEvent(BuildEventArgs buildEvent) => RecordedEvents.Add(buildEvent);
+ }
}
}
diff --git a/src/Build/BackEnd/Components/BuildRequestEngine/BuildRequestEngine.cs b/src/Build/BackEnd/Components/BuildRequestEngine/BuildRequestEngine.cs
index 42d90d11290..8a024f39848 100644
--- a/src/Build/BackEnd/Components/BuildRequestEngine/BuildRequestEngine.cs
+++ b/src/Build/BackEnd/Components/BuildRequestEngine/BuildRequestEngine.cs
@@ -1561,13 +1561,27 @@ private void TraceEngine(string format, params object[] stuff)
{
lock (this)
{
- FileUtilities.EnsureDirectoryExists(_debugDumpPath);
+ try
+ {
+ FileUtilities.EnsureDirectoryExists(_debugDumpPath);
+
+ // Use per-node trace files to avoid file contention when multiple in-proc
+ // nodes run in the same process (multithreaded / -mt mode).
+ int nodeId = _componentHost?.BuildParameters?.NodeId ?? 0;
+ string traceFile = string.Format(CultureInfo.CurrentCulture, Path.Combine(_debugDumpPath, @"EngineTrace_{0}_{1}.txt"), EnvironmentUtilities.CurrentProcessId, nodeId);
- using (StreamWriter file = FileUtilities.OpenWrite(string.Format(CultureInfo.CurrentCulture, Path.Combine(_debugDumpPath, @"EngineTrace_{0}.txt"), EnvironmentUtilities.CurrentProcessId), append: true))
+ using (StreamWriter file = FileUtilities.OpenWrite(traceFile, append: true))
+ {
+ string message = String.Format(CultureInfo.CurrentCulture, format, stuff);
+ file.WriteLine("{0}({1})-{2}: {3}", Thread.CurrentThread.Name, Environment.CurrentManagedThreadId, DateTime.UtcNow.Ticks, message);
+ file.Flush();
+ }
+ }
+ catch (IOException)
{
- string message = String.Format(CultureInfo.CurrentCulture, format, stuff);
- file.WriteLine("{0}({1})-{2}: {3}", Thread.CurrentThread.Name, Environment.CurrentManagedThreadId, DateTime.UtcNow.Ticks, message);
- file.Flush();
+ // Trace file IO failures must never crash the build engine.
+ // In multithreaded mode, file contention can occur if multiple engines
+ // happen to share a trace file path despite the per-node naming.
}
}
}
diff --git a/src/Build/BackEnd/Components/RequestBuilder/RequestBuilder.cs b/src/Build/BackEnd/Components/RequestBuilder/RequestBuilder.cs
index 409ee34f360..5f337c99148 100644
--- a/src/Build/BackEnd/Components/RequestBuilder/RequestBuilder.cs
+++ b/src/Build/BackEnd/Components/RequestBuilder/RequestBuilder.cs
@@ -19,6 +19,7 @@
using Microsoft.Build.Experimental.BuildCheck;
using Microsoft.Build.Experimental.BuildCheck.Infrastructure;
using Microsoft.Build.Framework;
+using Microsoft.Build.Framework.Telemetry;
using Microsoft.Build.Internal;
using Microsoft.Build.Shared;
using Microsoft.Build.Shared.Debugging;
@@ -1057,6 +1058,38 @@ private void RaiseBuildRequestCompleted(BuildRequestEntry entryToComplete)
OnBuildRequestCompleted?.Invoke(entryToComplete);
}
+ ///
+ /// Determines whether results need to be transferred from another node before building.
+ ///
+ /// In order for the check for target completeness for this project to be valid, all of the
+ /// target results from the project must be present in the results cache. It is possible that
+ /// this project has been moved from its original node and when it was, its results did not come
+ /// with it. This would be signified by the ResultsNodeId value in the configuration pointing to
+ /// a different node than the current one. In that case we will need to request those results be
+ /// moved from their original node to this one.
+ ///
+ ///
+ /// In MT mode, all worker nodes share the same process and caches, so transfer is unnecessary
+ /// and would race on the shared ResultsNodeId field (see #13188). This relies on the invariant
+ /// that MultiThreaded == true implies all worker nodes share caches (see
+ /// multithreaded-msbuild.md). If a future mode mixes in-proc and out-of-proc worker nodes,
+ /// revisit this check — see https://github.com/dotnet/msbuild/issues/11939.
+ ///
+ ///
+ /// The node ID where results currently reside.
+ /// The node ID of the current node.
+ /// Whether MT mode is active.
+ /// True if results must be transferred from another node before building.
+ internal static bool NeedsResultsTransfer(int resultsNodeId, int currentNodeId, bool isMultiThreaded)
+ {
+ if (isMultiThreaded)
+ {
+ return false;
+ }
+
+ return resultsNodeId != Scheduler.InvalidNodeId && resultsNodeId != currentNodeId;
+ }
+
///
/// Invokes the OnBlockedRequest event
///
@@ -1190,13 +1223,11 @@ private async Task BuildProject()
// Set the current directory to that required by the project.
SetProjectDirectory();
- // Transfer results and state from the previous node, if necessary.
- // In order for the check for target completeness for this project to be valid, all of the target results from the project must be present
- // in the results cache. It is possible that this project has been moved from its original node and when it was its results did not come
- // with it. This would be signified by the ResultsNode value in the configuration pointing to a different node than the current one. In that
- // case we will need to request those results be moved from their original node to this one.
- if ((_requestEntry.RequestConfiguration.ResultsNodeId != Scheduler.InvalidNodeId) &&
- (_requestEntry.RequestConfiguration.ResultsNodeId != _componentHost.BuildParameters.NodeId))
+ // Transfer results from the previous node if necessary — see NeedsResultsTransfer.
+ if (NeedsResultsTransfer(
+ _requestEntry.RequestConfiguration.ResultsNodeId,
+ _componentHost.BuildParameters.NodeId,
+ _componentHost.BuildParameters.MultiThreaded))
{
// This indicates to the system that we will block waiting for a results transfer. We will block here until those results become available.
await BlockOnTargetInProgress(Microsoft.Build.BackEnd.BuildRequest.InvalidGlobalRequestId, null);
@@ -1285,6 +1316,9 @@ private void UpdateStatisticsPostBuild()
return;
}
+ // Accumulate all telemetry into a local instance, then merge into the shared singleton once.
+ WorkerNodeTelemetryData telemetryData = new();
+
foreach (var projectTargetInstance in _requestEntry.RequestConfiguration.Project.Targets)
{
bool wasExecuted =
@@ -1315,20 +1349,16 @@ private void UpdateStatisticsPostBuild()
(isFromNuget && FileClassifier.Shared.IsMicrosoftPackageInNugetCache(projectTargetInstance.Value.FullPath));
}
- telemetryForwarder.AddTarget(
- projectTargetInstance.Key,
- // would we want to distinguish targets that were executed only during this execution - we'd need
- // to remember target names from ResultsByTarget from before execution
- wasExecuted,
- isCustom,
- isMetaprojTarget,
- isFromNuget,
- skipReason);
+ var key = new TaskOrTargetTelemetryKey(
+ projectTargetInstance.Key, isCustom, isFromNuget, isMetaprojTarget);
+ telemetryData.AddTarget(key, wasExecuted, skipReason);
}
TaskRegistry taskReg = _requestEntry.RequestConfiguration.Project.TaskRegistry;
CollectTasksStats(taskReg);
+ telemetryForwarder.MergeWorkerData(telemetryData);
+
void CollectTasksStats(TaskRegistry taskRegistry)
{
if (taskRegistry == null)
@@ -1338,13 +1368,16 @@ void CollectTasksStats(TaskRegistry taskRegistry)
foreach (TaskRegistry.RegisteredTaskRecord registeredTaskRecord in taskRegistry.TaskRegistrations.Values.SelectMany(record => record))
{
- telemetryForwarder.AddTask(
+ var key = new TaskOrTargetTelemetryKey(
registeredTaskRecord.TaskIdentity.Name,
+ registeredTaskRecord.ComputeIfCustom(),
+ registeredTaskRecord.IsFromNugetCache,
+ isFromMetaProject: false);
+ telemetryData.AddTask(
+ key,
registeredTaskRecord.Statistics.ExecutedTime,
registeredTaskRecord.Statistics.ExecutedCount,
registeredTaskRecord.Statistics.TotalMemoryConsumption,
- registeredTaskRecord.ComputeIfCustom(),
- registeredTaskRecord.IsFromNugetCache,
registeredTaskRecord.TaskFactoryAttributeName,
registeredTaskRecord.TaskFactoryParameters.Runtime);
diff --git a/src/Build/Collections/MultiDictionary.cs b/src/Build/Collections/MultiDictionary.cs
index ef8c02c8c8e..388ac065b47 100644
--- a/src/Build/Collections/MultiDictionary.cs
+++ b/src/Build/Collections/MultiDictionary.cs
@@ -68,6 +68,15 @@ internal MultiDictionary(IEqualityComparer keyComparer)
_backing = new Dictionary>(keyComparer);
}
+ ///
+ /// Constructor taking a specified comparer and initial capacity for the keys.
+ /// Pre-sizing avoids repeated dictionary resizes when the number of keys is known.
+ ///
+ internal MultiDictionary(int capacity, IEqualityComparer keyComparer)
+ {
+ _backing = new Dictionary>(capacity, keyComparer);
+ }
+
///
/// Number of keys
///
diff --git a/src/Build/Instance/ProjectInstance.cs b/src/Build/Instance/ProjectInstance.cs
index 662fe6ff516..b7dea631294 100644
--- a/src/Build/Instance/ProjectInstance.cs
+++ b/src/Build/Instance/ProjectInstance.cs
@@ -3398,7 +3398,7 @@ private void CreateEvaluatedIncludeSnapshotIfRequested(bool keepEvaluationCache,
return;
}
- var multiDictionary = new MultiDictionary(StringComparer.OrdinalIgnoreCase);
+ var multiDictionary = new MultiDictionary(items.Count, StringComparer.OrdinalIgnoreCase);
foreach (var item in items)
{
multiDictionary.Add(item.EvaluatedInclude, projectItemToInstanceMap[item]);
diff --git a/src/Build/TelemetryInfra/ITelemetryForwarder.cs b/src/Build/TelemetryInfra/ITelemetryForwarder.cs
index 916661a7aa2..18000c48280 100644
--- a/src/Build/TelemetryInfra/ITelemetryForwarder.cs
+++ b/src/Build/TelemetryInfra/ITelemetryForwarder.cs
@@ -1,9 +1,8 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.
-using System;
using Microsoft.Build.BackEnd.Logging;
-using Microsoft.Build.Framework;
+using Microsoft.Build.Framework.Telemetry;
namespace Microsoft.Build.TelemetryInfra;
@@ -15,26 +14,13 @@ internal interface ITelemetryForwarder
{
bool IsTelemetryCollected { get; }
- void AddTask(
- string name,
- TimeSpan cumulativeExecutionTime,
- short executionsCount,
- long totalMemoryConsumed,
- bool isCustom,
- bool isFromNugetCache,
- string? taskFactoryName,
- string? taskHostRuntime);
-
///
- /// Add info about target execution to the telemetry.
+ /// Merges a batch of telemetry data into this forwarder's accumulated state.
///
- /// The target name.
- /// Whether the target was executed (not skipped).
- /// Whether this is a custom target.
- /// Whether the target is from a meta project.
- /// Whether the target is from a NuGet package.
- /// The reason the target was skipped, if applicable.
- void AddTarget(string name, bool wasExecuted, bool isCustom, bool isMetaproj, bool isFromNugetCache, TargetSkipReason skipReason = TargetSkipReason.None);
+ void MergeWorkerData(IWorkerNodeTelemetryData data);
+ ///
+ /// Sends accumulated telemetry and resets internal state.
+ ///
void FinalizeProcessing(LoggingContext loggingContext);
}
diff --git a/src/Build/TelemetryInfra/TelemetryForwarderProvider.cs b/src/Build/TelemetryInfra/TelemetryForwarderProvider.cs
index 92175ef4a71..dd851779bd9 100644
--- a/src/Build/TelemetryInfra/TelemetryForwarderProvider.cs
+++ b/src/Build/TelemetryInfra/TelemetryForwarderProvider.cs
@@ -1,10 +1,8 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.
-using System;
using Microsoft.Build.BackEnd;
using Microsoft.Build.BackEnd.Logging;
-using Microsoft.Build.Framework;
using Microsoft.Build.Framework.Telemetry;
using Microsoft.Build.Shared;
@@ -49,32 +47,48 @@ public void ShutdownComponent()
_instance = null;
}
+ ///
+ /// Active telemetry forwarder that accumulates worker node telemetry.
+ ///
+ ///
+ /// Thread-safe: in /m /mt mode, multiple instances share a single
+ /// singleton, so and
+ /// may be called concurrently from different node threads.
+ ///
public class TelemetryForwarder : ITelemetryForwarder
{
- private readonly WorkerNodeTelemetryData _workerNodeTelemetryData = new();
+ private WorkerNodeTelemetryData _workerNodeTelemetryData = new();
+ private readonly LockType _lock = new();
// in future, this might be per event type
public bool IsTelemetryCollected => true;
- public void AddTask(string name, TimeSpan cumulativeExecutionTime, short executionsCount, long totalMemoryConsumed, bool isCustom, bool isFromNugetCache, string? taskFactoryName, string? taskHostRuntime)
+ public void MergeWorkerData(IWorkerNodeTelemetryData data)
{
- var key = GetKey(name, isCustom, false, isFromNugetCache);
- _workerNodeTelemetryData.AddTask(key, cumulativeExecutionTime, executionsCount, totalMemoryConsumed, taskFactoryName, taskHostRuntime);
- }
-
- public void AddTarget(string name, bool wasExecuted, bool isCustom, bool isMetaproj, bool isFromNugetCache, TargetSkipReason skipReason = TargetSkipReason.None)
- {
- var key = GetKey(name, isCustom, isMetaproj, isFromNugetCache);
- _workerNodeTelemetryData.AddTarget(key, wasExecuted, skipReason);
+ lock (_lock)
+ {
+ _workerNodeTelemetryData.Add(data);
+ }
}
- private static TaskOrTargetTelemetryKey GetKey(string name, bool isCustom, bool isMetaproj,
- bool isFromNugetCache)
- => new TaskOrTargetTelemetryKey(name, isCustom, isFromNugetCache, isMetaproj);
public void FinalizeProcessing(LoggingContext loggingContext)
{
- WorkerNodeTelemetryEventArgs telemetryArgs = new(_workerNodeTelemetryData)
+ WorkerNodeTelemetryData snapshot;
+
+ lock (_lock)
+ {
+ // Nothing accumulated since the last call — skip sending.
+ if (_workerNodeTelemetryData.IsEmpty)
+ {
+ return;
+ }
+
+ snapshot = _workerNodeTelemetryData;
+ _workerNodeTelemetryData = new();
+ }
+
+ WorkerNodeTelemetryEventArgs telemetryArgs = new(snapshot)
{ BuildEventContext = loggingContext.BuildEventContext };
loggingContext.LogBuildEvent(telemetryArgs);
}
@@ -84,9 +98,7 @@ public class NullTelemetryForwarder : ITelemetryForwarder
{
public bool IsTelemetryCollected => false;
- public void AddTask(string name, TimeSpan cumulativeExecutionTime, short executionsCount, long totalMemoryConsumed, bool isCustom, bool isFromNugetCache, string? taskFactoryName, string? taskHostRuntime) { }
-
- public void AddTarget(string name, bool wasExecuted, bool isCustom, bool isMetaproj, bool isFromNugetCache, TargetSkipReason skipReason = TargetSkipReason.None) { }
+ public void MergeWorkerData(IWorkerNodeTelemetryData data) { }
public void FinalizeProcessing(LoggingContext loggingContext) { }
}
diff --git a/src/Framework/Telemetry/WorkerNodeTelemetryData.cs b/src/Framework/Telemetry/WorkerNodeTelemetryData.cs
index d127a280d54..4d1f50bfc6e 100644
--- a/src/Framework/Telemetry/WorkerNodeTelemetryData.cs
+++ b/src/Framework/Telemetry/WorkerNodeTelemetryData.cs
@@ -14,6 +14,9 @@ public WorkerNodeTelemetryData(Dictionary
+ /// Merges all data from another into this instance.
+ ///
public void Add(IWorkerNodeTelemetryData other)
{
foreach (var task in other.TasksExecutionData)
@@ -27,10 +30,12 @@ public void Add(IWorkerNodeTelemetryData other)
}
}
+ ///
+ /// Adds or aggregates task execution data.
+ ///
public void AddTask(TaskOrTargetTelemetryKey task, TimeSpan cumulativeExecutionTime, int executionsCount, long totalMemoryConsumption, string? factoryName, string? taskHostRuntime)
{
- TaskExecutionStats? taskExecutionStats;
- if (!TasksExecutionData.TryGetValue(task, out taskExecutionStats))
+ if (!TasksExecutionData.TryGetValue(task, out TaskExecutionStats? taskExecutionStats))
{
taskExecutionStats = new(cumulativeExecutionTime, executionsCount, totalMemoryConsumption, factoryName, taskHostRuntime);
TasksExecutionData[task] = taskExecutionStats;
@@ -45,6 +50,9 @@ public void AddTask(TaskOrTargetTelemetryKey task, TimeSpan cumulativeExecutionT
}
}
+ ///
+ /// Adds or updates target execution data.
+ ///
public void AddTarget(TaskOrTargetTelemetryKey target, bool wasExecuted, TargetSkipReason skipReason = TargetSkipReason.None)
{
if (TargetsExecutionData.TryGetValue(target, out var existingStats))
@@ -71,6 +79,8 @@ public void AddTarget(TaskOrTargetTelemetryKey target, bool wasExecuted, TargetS
public WorkerNodeTelemetryData() : this([], []) { }
+ public bool IsEmpty => TasksExecutionData.Count == 0 && TargetsExecutionData.Count == 0;
+
public Dictionary TasksExecutionData { get; }
public Dictionary TargetsExecutionData { get; }
diff --git a/src/Tasks/AssignTargetPath.cs b/src/Tasks/AssignTargetPath.cs
index f8f42c9945d..43e39efd7a2 100644
--- a/src/Tasks/AssignTargetPath.cs
+++ b/src/Tasks/AssignTargetPath.cs
@@ -23,7 +23,7 @@ public class AssignTargetPath : TaskExtension, IMultiThreadableTask
///
/// Gets or sets the task execution environment for thread-safe path resolution.
///
- public TaskEnvironment TaskEnvironment { get; set; }
+ public TaskEnvironment TaskEnvironment { get; set; } = new TaskEnvironment(MultiProcessTaskEnvironmentDriver.Instance);
///
diff --git a/src/Tasks/Copy.cs b/src/Tasks/Copy.cs
index 36d2a3d20f9..58ff7cfc6ae 100644
--- a/src/Tasks/Copy.cs
+++ b/src/Tasks/Copy.cs
@@ -187,7 +187,7 @@ public Copy()
public bool FailIfNotIncremental { get; set; }
///
- public TaskEnvironment TaskEnvironment { get; set; }
+ public TaskEnvironment TaskEnvironment { get; set; } = new TaskEnvironment(MultiProcessTaskEnvironmentDriver.Instance);
#endregion
diff --git a/src/Tasks/Delete.cs b/src/Tasks/Delete.cs
index d687b9c0006..b928bf858af 100644
--- a/src/Tasks/Delete.cs
+++ b/src/Tasks/Delete.cs
@@ -65,7 +65,7 @@ public ITaskItem[] Files
public bool FailIfNotIncremental { get; set; }
///
- public TaskEnvironment TaskEnvironment { get; set; }
+ public TaskEnvironment TaskEnvironment { get; set; } = new TaskEnvironment(MultiProcessTaskEnvironmentDriver.Instance);
///
/// Verify that the inputs are correct.
diff --git a/src/Tasks/DownloadFile.cs b/src/Tasks/DownloadFile.cs
index 5da52626eed..2386d482d8c 100644
--- a/src/Tasks/DownloadFile.cs
+++ b/src/Tasks/DownloadFile.cs
@@ -69,7 +69,7 @@ public sealed class DownloadFile : TaskExtension, ICancelableTask, IIncrementalT
public bool FailIfNotIncremental { get; set; }
///
- public TaskEnvironment TaskEnvironment { get; set; }
+ public TaskEnvironment TaskEnvironment { get; set; } = new TaskEnvironment(MultiProcessTaskEnvironmentDriver.Instance);
///
/// Gets or sets a to use. This is used by unit tests to mock a connection to a remote server.
diff --git a/src/Tasks/FileIO/GetFileHash.cs b/src/Tasks/FileIO/GetFileHash.cs
index 655ff3a6de8..9aa91f99ad0 100644
--- a/src/Tasks/FileIO/GetFileHash.cs
+++ b/src/Tasks/FileIO/GetFileHash.cs
@@ -66,7 +66,7 @@ internal static readonly Dictionary> SupportedAlgori
public ITaskItem[] Items { get; set; }
///
- public TaskEnvironment TaskEnvironment { get; set; }
+ public TaskEnvironment TaskEnvironment { get; set; } = new TaskEnvironment(MultiProcessTaskEnvironmentDriver.Instance);
public override bool Execute()
{
diff --git a/src/Tasks/FileIO/ReadLinesFromFile.cs b/src/Tasks/FileIO/ReadLinesFromFile.cs
index 1b80db354aa..552db96d7ed 100644
--- a/src/Tasks/FileIO/ReadLinesFromFile.cs
+++ b/src/Tasks/FileIO/ReadLinesFromFile.cs
@@ -25,7 +25,7 @@ public class ReadLinesFromFile : TaskExtension, IMultiThreadableTask
public ITaskItem File { get; set; }
///
- public TaskEnvironment TaskEnvironment { get; set; }
+ public TaskEnvironment TaskEnvironment { get; set; } = new TaskEnvironment(MultiProcessTaskEnvironmentDriver.Instance);
///
/// Receives lines from file.
diff --git a/src/Tasks/FileIO/VerifyFileHash.cs b/src/Tasks/FileIO/VerifyFileHash.cs
index 8dab7a858b2..9b6dd12d3d0 100644
--- a/src/Tasks/FileIO/VerifyFileHash.cs
+++ b/src/Tasks/FileIO/VerifyFileHash.cs
@@ -17,7 +17,7 @@ namespace Microsoft.Build.Tasks
public sealed class VerifyFileHash : TaskExtension, ICancelableTask, IMultiThreadableTask
{
///
- public TaskEnvironment TaskEnvironment { get; set; }
+ public TaskEnvironment TaskEnvironment { get; set; } = new TaskEnvironment(MultiProcessTaskEnvironmentDriver.Instance);
///
/// The file path.
diff --git a/src/Tasks/FileIO/WriteLinesToFile.cs b/src/Tasks/FileIO/WriteLinesToFile.cs
index 82c6517eb93..0cced7ad137 100644
--- a/src/Tasks/FileIO/WriteLinesToFile.cs
+++ b/src/Tasks/FileIO/WriteLinesToFile.cs
@@ -23,7 +23,7 @@ public class WriteLinesToFile : TaskExtension, IIncrementalTask, IMultiThreadabl
private static readonly Encoding s_defaultEncoding = new UTF8Encoding(encoderShouldEmitUTF8Identifier: false, throwOnInvalidBytes: true);
///
- public TaskEnvironment TaskEnvironment { get; set; }
+ public TaskEnvironment TaskEnvironment { get; set; } = new TaskEnvironment(MultiProcessTaskEnvironmentDriver.Instance);
///
/// File to write lines to.
diff --git a/src/Tasks/ListOperators/FindUnderPath.cs b/src/Tasks/ListOperators/FindUnderPath.cs
index 46d280dd332..1347cbd3528 100644
--- a/src/Tasks/ListOperators/FindUnderPath.cs
+++ b/src/Tasks/ListOperators/FindUnderPath.cs
@@ -21,7 +21,7 @@ public class FindUnderPath : TaskExtension, IMultiThreadableTask
///
/// Gets or sets the task execution environment for thread-safe path resolution.
///
- public TaskEnvironment TaskEnvironment { get; set; }
+ public TaskEnvironment TaskEnvironment { get; set; } = new TaskEnvironment(MultiProcessTaskEnvironmentDriver.Instance);
///
/// Filter based on whether items fall under this path or not.
diff --git a/src/Tasks/MakeDir.cs b/src/Tasks/MakeDir.cs
index b16b810d5b2..cc3f4a33ccf 100644
--- a/src/Tasks/MakeDir.cs
+++ b/src/Tasks/MakeDir.cs
@@ -35,7 +35,7 @@ public ITaskItem[] Directories
public bool FailIfNotIncremental { get; set; }
///
- public TaskEnvironment TaskEnvironment { get; set; }
+ public TaskEnvironment TaskEnvironment { get; set; } = new TaskEnvironment(MultiProcessTaskEnvironmentDriver.Instance);
private ITaskItem[] _directories;
diff --git a/src/Tasks/Move.cs b/src/Tasks/Move.cs
index 5e0a13b3742..e764d89cfe1 100644
--- a/src/Tasks/Move.cs
+++ b/src/Tasks/Move.cs
@@ -76,7 +76,7 @@ public class Move : TaskExtension, ICancelableTask, IIncrementalTask, IMultiThre
public bool FailIfNotIncremental { get; set; }
///
- public TaskEnvironment TaskEnvironment { get; set; }
+ public TaskEnvironment TaskEnvironment { get; set; } = new TaskEnvironment(MultiProcessTaskEnvironmentDriver.Instance);
///
/// Stop and return (in an undefined state) as soon as possible.
diff --git a/src/Tasks/RemoveDir.cs b/src/Tasks/RemoveDir.cs
index 5eeda7f53d4..83b4569c8d2 100644
--- a/src/Tasks/RemoveDir.cs
+++ b/src/Tasks/RemoveDir.cs
@@ -25,7 +25,7 @@ public class RemoveDir : TaskExtension, IIncrementalTask, IMultiThreadableTask
private ITaskItem[] _directories;
///
- public TaskEnvironment TaskEnvironment { get; set; }
+ public TaskEnvironment TaskEnvironment { get; set; } = new TaskEnvironment(MultiProcessTaskEnvironmentDriver.Instance);
[Required]
public ITaskItem[] Directories
diff --git a/src/Tasks/Touch.cs b/src/Tasks/Touch.cs
index 3ca82c7c951..b524a842d16 100644
--- a/src/Tasks/Touch.cs
+++ b/src/Tasks/Touch.cs
@@ -50,7 +50,7 @@ public class Touch : TaskExtension, IIncrementalTask, IMultiThreadableTask
public ITaskItem[] TouchedFiles { get; set; }
///
- public TaskEnvironment TaskEnvironment { get; set; }
+ public TaskEnvironment TaskEnvironment { get; set; } = new TaskEnvironment(MultiProcessTaskEnvironmentDriver.Instance);
///
/// Importance: high, normal, low (default normal)
diff --git a/src/Tasks/Unzip.cs b/src/Tasks/Unzip.cs
index fd262b1007c..e1a0dfa0de3 100644
--- a/src/Tasks/Unzip.cs
+++ b/src/Tasks/Unzip.cs
@@ -77,7 +77,7 @@ public sealed class Unzip : TaskExtension, ICancelableTask, IIncrementalTask, IM
public bool FailIfNotIncremental { get; set; }
///
- public TaskEnvironment TaskEnvironment { get; set; }
+ public TaskEnvironment TaskEnvironment { get; set; } = new TaskEnvironment(MultiProcessTaskEnvironmentDriver.Instance);
///
public void Cancel()
diff --git a/src/Tasks/WriteCodeFragment.cs b/src/Tasks/WriteCodeFragment.cs
index c0f58314d0f..7fd8815cbdd 100644
--- a/src/Tasks/WriteCodeFragment.cs
+++ b/src/Tasks/WriteCodeFragment.cs
@@ -34,7 +34,7 @@ namespace Microsoft.Build.Tasks
public class WriteCodeFragment : TaskExtension, IMultiThreadableTask
{
///
- public TaskEnvironment TaskEnvironment { get; set; }
+ public TaskEnvironment TaskEnvironment { get; set; } = new TaskEnvironment(MultiProcessTaskEnvironmentDriver.Instance);
private const string TypeNameSuffix = "_TypeName";
private const string IsLiteralSuffix = "_IsLiteral";
private static readonly string[] NamespaceImports = ["System", "System.Reflection"];
diff --git a/src/Tasks/XmlPeek.cs b/src/Tasks/XmlPeek.cs
index b156df8b0a5..026be8fcf65 100644
--- a/src/Tasks/XmlPeek.cs
+++ b/src/Tasks/XmlPeek.cs
@@ -25,7 +25,7 @@ public class XmlPeek : TaskExtension, IMultiThreadableTask
#region Properties
///
- public TaskEnvironment TaskEnvironment { get; set; }
+ public TaskEnvironment TaskEnvironment { get; set; } = new TaskEnvironment(MultiProcessTaskEnvironmentDriver.Instance);
///
/// The XPath Query.
diff --git a/src/Tasks/XmlPoke.cs b/src/Tasks/XmlPoke.cs
index 59b368c6dcc..1ca7f718c91 100644
--- a/src/Tasks/XmlPoke.cs
+++ b/src/Tasks/XmlPoke.cs
@@ -24,7 +24,7 @@ public class XmlPoke : TaskExtension, IMultiThreadableTask
#region Properties
///
- public TaskEnvironment TaskEnvironment { get; set; }
+ public TaskEnvironment TaskEnvironment { get; set; } = new TaskEnvironment(MultiProcessTaskEnvironmentDriver.Instance);
///
/// The XML input as file path.
diff --git a/src/Tasks/XslTransformation.cs b/src/Tasks/XslTransformation.cs
index bacc8c910ac..1d760bb619b 100644
--- a/src/Tasks/XslTransformation.cs
+++ b/src/Tasks/XslTransformation.cs
@@ -29,7 +29,7 @@ public class XslTransformation : TaskExtension, IMultiThreadableTask
#region Members
///
- public TaskEnvironment TaskEnvironment { get; set; }
+ public TaskEnvironment TaskEnvironment { get; set; } = new TaskEnvironment(MultiProcessTaskEnvironmentDriver.Instance);
///
/// The output files.
diff --git a/src/Tasks/ZipDirectory.cs b/src/Tasks/ZipDirectory.cs
index a93097933e8..30ed730be31 100644
--- a/src/Tasks/ZipDirectory.cs
+++ b/src/Tasks/ZipDirectory.cs
@@ -59,7 +59,7 @@ public sealed class ZipDirectory : TaskExtension, IIncrementalTask, IMultiThread
public string? CompressionLevel { get; set; }
///
- public TaskEnvironment TaskEnvironment { get; set; } = null!;
+ public TaskEnvironment TaskEnvironment { get; set; } = new TaskEnvironment(MultiProcessTaskEnvironmentDriver.Instance);
public override bool Execute()
{