Preserve virtual task environments in TaskHosts

documentation/MSBuild-Coordinator.md

@@ -179,14 +179,16 @@ Without nested grants, the child process could request a new root grant while th
 Nested grants avoid this by treating child processes as participants in the parent grant:
 
 1. The root build receives a grant token in `NodeGrantMessage`.
-2. `BuildManager` records that token in the build process environment as `MSBUILDCOORDINATORGRANTID`, so task-launched child processes inherit it.
+2. `BuildManager` records that token as `MSBUILDCOORDINATORGRANTID` in the build's environment snapshot, not the parent process's global environment. In multithreaded mode, each request receives it through `TaskEnvironment`; tasks dispatched to a TaskHost receive that environment in their configuration. Task-launched child processes then inherit the token from the task's execution environment.
 3. A child process that sees the token and a server that supports `nested-grants` sends `JoinGrantMessage`.
 4. The coordinator validates that the root grant is still active.
 5. If valid, the child receives a grant capped by the root grant's node count without consuming additional global budget.
 6. Releasing a nested grant does not release global budget or invalidate the root grant token.
 
 Nested grants do not implement a scheduler within the root grant. Each nested process is capped by the root grant's node count, but the coordinator does not track combined concurrency across the root process and all nested participants. The root build is expected to coordinate its own nested work so it does not oversubscribe the resources it was granted.
 
+Starting with Change Wave 18.12, TaskHost startup reconciliation preserves ordinary task-environment variables, including the grant token, instead of treating every difference from process startup as a bitness adjustment. Opting out of that wave restores the legacy reconciliation and can reintroduce a deadlock when a task-hosted restore process loses the token.
+
 Nested grant validation happens when the nested process joins the root grant. If the root grant is released later, the coordinator rejects new joins for that grant ID, but it does not revoke nested grants that were already issued. Those nested builds continue until they release or disconnect.
 
 Nested grants are capability-gated. Older peers that do not advertise `nested-grants` continue to exchange `NodeGrantMessage` using the legacy int-only wire shape (`GrantId` is `Guid.Empty` and never sent on the wire), and clients only send `JoinGrantMessage` when the server advertises the capability.

documentation/specs/multithreading/taskhost-threading.md

@@ -180,7 +180,7 @@ Each new `TaskHostConfiguration` carries a full environment snapshot, task param
 **Reset per task:** `_currentConfiguration`, `_debugCommunications`, `_updateEnvironment`, `_warningsAsErrors`/`_warningsNotAsErrors`/`_warningsAsMessages`, `_fileAccessData`, per-task `TaskExecutionContext`
 
 **Persists across tasks (within a single build):**
-- `s_mismatchedEnvironmentValues` (static) -- environment variable fixups for bitness differences, computed once per process
+- `s_mismatchedEnvironmentValues` (static) -- environment variable fixups for bitness differences, computed once per process. Starting with Change Wave 18.12, only the documented Windows WOW64 variables (`PROCESSOR_ARCHITECTURE`, `PROCESSOR_ARCHITEW6432`, `ProgramFiles`, `ProgramW6432`, `CommonProgramFiles`, and `CommonProgramW6432`) are reconciled. Unix task hosts do not apply these Windows-specific fixups. Other variables come from the task's environment, including additions, replacements, and removals that differ from process startup. They also pass back to the parent without reverse fixups, so an earlier task or build cannot overwrite later virtualized state through this table. Opting out of Wave 18.12 restores the legacy behavior of reconciling all startup differences.
 - `_registeredTaskObjectCache` -- task object cache with `Build` lifetime scope, disposed at end of each build (in `HandleShutdown()`), recreated fresh on the next `Run()` call
 - `_pendingCallbackRequests` / `_nextCallbackRequestId` -- callback tracking (should be empty between tasks)
 

documentation/wiki/ChangeWaves.md

@@ -36,6 +36,7 @@ Change wave checks around features will be removed in the release that accompani
 ## Current Rotation of Change Waves
 
 ### 18.12
+- [Task hosts preserve ordinary task-environment variables instead of undoing them as startup differences; Windows architecture-specific variables retain their bitness adjustments. This prevents coordinator/static-graph restore deadlocks in multithreaded builds. Opting out restores the legacy reconciliation and can reintroduce the deadlock.](https://github.com/dotnet/msbuild/issues/14986)
 - [Events that a task logs from a TaskHost - extended errors, warnings and messages, critical messages, telemetry, and any other event kind the router did not enumerate - reach the parent process instead of being dropped.](https://github.com/dotnet/msbuild/pull/14876)
 
 ### 18.11

src/Build.UnitTests/BackEnd/TaskHostEnvironment_E2E_Tests.cs

@@ -0,0 +1,238 @@
+// 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.Diagnostics;
+using System.Globalization;
+using Microsoft.Build.Execution;
+using Microsoft.Build.Framework;
+using Microsoft.Build.UnitTests;
+using Microsoft.Build.UnitTests.Shared;
+using Shouldly;
+using Xunit;
+
+namespace Microsoft.Build.Engine.UnitTests.BackEnd;
+
+public class TaskHostEnvironment_E2E_Tests(ITestOutputHelper output)
+{
+    [Theory]
+    [InlineData(false, false, null, "build", false)]
+    [InlineData(false, false, "process", "build", false)]
+    [InlineData(false, false, "process", null, false)]
+    [InlineData(false, true, null, "build", false)]
+    [InlineData(false, true, "process", "build", false)]
+    [InlineData(false, true, "process", null, false)]
+    [InlineData(true, false, null, "build", false)]
+    [InlineData(true, false, "process", "build", false)]
+    [InlineData(true, false, "process", null, false)]
+    [InlineData(true, false, null, "build", true)]
+    [InlineData(true, false, "process", "build", true)]
+    [InlineData(true, false, "process", null, true)]
+    public void TaskEnvironmentSurvivesTaskHostReconciliation(
+        bool multiThreaded, bool explicitTaskHost, string? processValue, string? buildValue, bool disableChangeWave)
+    {
+        using TestEnvironment env = TestEnvironment.Create(output);
+        env.SetEnvironmentVariable("MSBUILDDISABLEFEATURESFROMVERSION", disableChangeWave ? "18.12" : null);
+        string variable = $"MSBUILD_TASK_ENVIRONMENT_{Guid.NewGuid():N}";
+        env.SetEnvironmentVariable(variable, processValue);
+        string taskFactory = explicitTaskHost ? """TaskFactory="TaskHostFactory" """ : string.Empty;
+
+        TransientTestFile project = env.CreateFile("environment.proj", $"""
+            <Project>
+              <UsingTask TaskName="ReadEnvironmentVariableTask" AssemblyFile="{typeof(ReadEnvironmentVariableTask).Assembly.Location}" {taskFactory}/>
+              <UsingTask TaskName="SetEnvironmentVariableTask" AssemblyFile="{typeof(SetEnvironmentVariableTask).Assembly.Location}" {taskFactory}/>
+              <Target Name="Build">
+                <ReadEnvironmentVariableTask VariableName="{variable}">
+                  <Output TaskParameter="Value" PropertyName="FirstValue" />
+                  <Output TaskParameter="Pid" PropertyName="FirstPid" />
+                </ReadEnvironmentVariableTask>
+                <ReadEnvironmentVariableTask VariableName="{variable}">
+                  <Output TaskParameter="Value" PropertyName="SecondValue" />
+                  <Output TaskParameter="Pid" PropertyName="SecondPid" />
+                </ReadEnvironmentVariableTask>
+                <SetEnvironmentVariableTask VariableName="{variable}" Value="changed-by-task" />
+                <ReadEnvironmentVariableTask VariableName="{variable}">
+                  <Output TaskParameter="Value" PropertyName="ChangedValue" />
+                  <Output TaskParameter="Pid" PropertyName="LastPid" />
+                </ReadEnvironmentVariableTask>
+              </Target>
+            </Project>
+            """);
+
+        MockLogger logger = new(output);
+        BuildParameters parameters = new()
+        {
+            MultiThreaded = multiThreaded,
+            DisableInProcNode = false,
+            EnableNodeReuse = false,
+            Loggers = [logger],
+        };
+        parameters.SetBuildProcessEnvironmentVariable(variable, buildValue);
+
+        using BuildManager manager = new();
+        BuildResult result = manager.Build(parameters, new BuildRequestData(
+            project.Path, new Dictionary<string, string?>(), null, ["Build"], null,
+            BuildRequestDataFlags.ProvideProjectStateAfterBuild));
+
+        result.ShouldHaveSucceeded();
+        ProjectInstance? state = result.ProjectStateAfterBuild;
+        state.ShouldNotBeNull();
+        string expectedValue = (disableChangeWave ? processValue : buildValue) ?? string.Empty;
+        state.GetPropertyValue("FirstValue").ShouldBe(expectedValue);
+        state.GetPropertyValue("SecondValue").ShouldBe(expectedValue);
+        state.GetPropertyValue("ChangedValue").ShouldBe("changed-by-task");
+        string firstPid = state.GetPropertyValue("FirstPid");
+        firstPid.ShouldBe(state.GetPropertyValue("SecondPid"));
+        firstPid.ShouldBe(state.GetPropertyValue("LastPid"));
+        using Process currentProcess = Process.GetCurrentProcess();
+        string currentPid = currentProcess.Id.ToString(CultureInfo.InvariantCulture);
+        if (multiThreaded || explicitTaskHost)
+        {
+            firstPid.ShouldNotBe(currentPid);
+        }
+        else
+        {
+            firstPid.ShouldBe(currentPid);
+        }
+
+        Environment.GetEnvironmentVariable(variable).ShouldBe(processValue);
+    }
+
+    [Theory]
+    [InlineData(null)]
+    [InlineData("second-build")]
+    public void ReusedTaskHostObservesTheNextBuildEnvironment(string? nextValue)
+    {
+        using TestEnvironment env = TestEnvironment.Create(output);
+        env.SetEnvironmentVariable("MSBUILDDISABLEFEATURESFROMVERSION", null);
+        string variable = $"MSBUILD_REUSED_TASK_ENVIRONMENT_{Guid.NewGuid():N}";
+        env.SetEnvironmentVariable(variable, "process");
+
+        TransientTestFile project = env.CreateFile("reuse.proj", $"""
+            <Project>
+              <UsingTask TaskName="ReadEnvironmentVariableTask" AssemblyFile="{typeof(ReadEnvironmentVariableTask).Assembly.Location}" />
+              <Target Name="Build">
+                <ReadEnvironmentVariableTask VariableName="{variable}">
+                  <Output TaskParameter="Value" PropertyName="ObservedValue" />
+                  <Output TaskParameter="Pid" PropertyName="TaskPid" />
+                </ReadEnvironmentVariableTask>
+              </Target>
+            </Project>
+            """);
+
+        using BuildManager manager = new();
+        ProjectInstance first = Build("first-build");
+        ProjectInstance second = Build(nextValue);
+
+        first.GetPropertyValue("ObservedValue").ShouldBe("first-build");
+        second.GetPropertyValue("ObservedValue").ShouldBe(nextValue ?? string.Empty);
+        second.GetPropertyValue("TaskPid").ShouldBe(first.GetPropertyValue("TaskPid"));
+        Environment.GetEnvironmentVariable(variable).ShouldBe("process");
+
+        ProjectInstance Build(string? value)
+        {
+            MockLogger logger = new(output);
+            BuildParameters parameters = new()
+            {
+                MultiThreaded = true,
+                DisableInProcNode = false,
+                EnableNodeReuse = true,
+                Loggers = [logger],
+            };
+            parameters.SetBuildProcessEnvironmentVariable(variable, value);
+            BuildResult result = manager.Build(parameters, new BuildRequestData(
+                project.Path, new Dictionary<string, string?>(), null, ["Build"], null,
+                BuildRequestDataFlags.ProvideProjectStateAfterBuild));
+            result.ShouldHaveSucceeded();
+            ProjectInstance? state = result.ProjectStateAfterBuild;
+            state.ShouldNotBeNull();
+            int taskPid = int.Parse(state.GetPropertyValue("TaskPid"), CultureInfo.InvariantCulture);
+            using Process currentProcess = Process.GetCurrentProcess();
+            taskPid.ShouldNotBe(currentProcess.Id);
+            env.WithTransientProcess(taskPid);
+            return state;
+        }
+    }
+
+    [Fact]
+    public void ConcurrentBuildManagersKeepTaskEnvironmentsIsolated()
+    {
+        using TestEnvironment env = TestEnvironment.Create(output);
+        env.SetEnvironmentVariable("MSBUILDDISABLEFEATURESFROMVERSION", null);
+        string variable = $"MSBUILD_PARALLEL_TASK_ENVIRONMENT_{Guid.NewGuid():N}";
+        env.SetEnvironmentVariable(variable, "process");
+        TransientTestFile project = env.CreateFile("parallel.proj", $"""
+            <Project>
+              <UsingTask TaskName="ReadEnvironmentVariableTask" AssemblyFile="{typeof(ReadEnvironmentVariableTask).Assembly.Location}" />
+              <Target Name="Build">
+                <ReadEnvironmentVariableTask VariableName="{variable}">
+                  <Output TaskParameter="Value" PropertyName="ObservedValue" />
+                </ReadEnvironmentVariableTask>
+              </Target>
+            </Project>
+            """);
+
+        using BuildManager firstManager = new();
+        using BuildManager secondManager = new();
+        firstManager.BeginBuild(CreateParameters("first-build"));
+        secondManager.BeginBuild(CreateParameters("second-build"));
+        Environment.GetEnvironmentVariable(variable).ShouldBe("process");
+        BuildRequestData request = new(
+            project.Path, new Dictionary<string, string?>(), null, ["Build"], null,
+            BuildRequestDataFlags.ProvideProjectStateAfterBuild);
+        BuildSubmission first = firstManager.PendBuildRequest(request);
+        BuildSubmission second = secondManager.PendBuildRequest(request);
+        first.ExecuteAsync(null, null);
+        second.ExecuteAsync(null, null);
+        firstManager.EndBuild();
+        secondManager.EndBuild();
+
+        BuildResult firstResult = first.BuildResult.ShouldNotBeNull();
+        BuildResult secondResult = second.BuildResult.ShouldNotBeNull();
+        firstResult.ShouldHaveSucceeded();
+        secondResult.ShouldHaveSucceeded();
+        firstResult.ProjectStateAfterBuild.ShouldNotBeNull().GetPropertyValue("ObservedValue").ShouldBe("first-build");
+        secondResult.ProjectStateAfterBuild.ShouldNotBeNull().GetPropertyValue("ObservedValue").ShouldBe("second-build");
+        Environment.GetEnvironmentVariable(variable).ShouldBe("process");
+
+        BuildParameters CreateParameters(string value)
+        {
+            BuildParameters parameters = new()
+            {
+                MultiThreaded = true,
+                DisableInProcNode = false,
+                EnableNodeReuse = false,
+                SaveOperatingEnvironment = false,
+                Loggers = [new MockLogger(output)],
+            };
+            parameters.SetBuildProcessEnvironmentVariable(variable, value);
+            return parameters;
+        }
+    }
+
+    [WindowsFullFrameworkOnlyFact]
+    public void CrossBitnessTaskHostPreservesArchitectureEnvironment()
+    {
+        using TestEnvironment env = TestEnvironment.Create(output);
+        env.SetEnvironmentVariable("MSBUILDDISABLEFEATURESFROMVERSION", null);
+        string? nativeProgramFiles = Environment.GetEnvironmentVariable("ProgramW6432");
+        nativeProgramFiles.ShouldNotBeNullOrEmpty();
+        TransientTestFile project = env.CreateFile("architecture.proj", """
+            <Project>
+              <UsingTask TaskName="Exec"
+                         AssemblyFile="$([System.IO.Path]::Combine('$(MSBuildToolsPath)', 'Microsoft.Build.Tasks.Core.dll'))"
+                         TaskFactory="TaskHostFactory" Runtime="CLR4" Architecture="x64" />
+              <Target Name="Build">
+                <Exec Command="echo HOST_ARCHITECTURE=%PROCESSOR_ARCHITECTURE% &amp; echo HOST_PROGRAMFILES=%ProgramFiles%" />
+              </Target>
+            </Project>
+            """);
+
+        string buildOutput = RunnerUtilities.ExecBootstrapedMSBuild(
+            $"\"{project.Path}\" /mt /nr:false /v:n", out bool success, outputHelper: output);
+        success.ShouldBeTrue(buildOutput);
+        buildOutput.ShouldContain("HOST_ARCHITECTURE=AMD64");
+        buildOutput.ShouldContain($"HOST_PROGRAMFILES={nativeProgramFiles}");
+    }
+}