[vs18.3] Fix the lifecycle of task host factory

documentation/High-level-overview.md

@@ -149,6 +149,15 @@ TaskHost can be opted-in via `TaskFactory="TaskHostFactory"` in the [`UsingTask`
 - If a task's source code is in the same repository that is being built, and the repository's build needs to use that task during the build process. Using a Task Host makes sure the DLLs are not locked at the end of the build (as MSBuild uses long living worker nodes that survives single build execution)
 - As an isolation mechanism - separating the execution from the engine execution process.
 
+When `TaskHostFactory` is specified as the task factory, the task always runs out-of-process and short lived. See the below matrix:
+
+| Does TaskHost match executing MSBuild Runtime? | Is TaskHostFactory requested for the Task? | Expected task execution type |
+| :-: | :-: | --- |
+| ✅ | :x: | in-process execution |
+| ✅ | ✅ | short-lived out-of-proc execution |
+| :x: | ✅ | short-lived out-of-proc execution |
+| :x: | :x: | long-lived out-of-proc execution |
+
 ## Caches
 ### Project result cache
 The project Result Cache refers to the cache used by the scheduler that keeps the build results of already executed project. The result of a target is success, failure, and a list of items that succeeded. Beyond that, the `Returns` and `Outputs` attributes from targets are also serialized with the build result, as to be used by other targets for their execution.

src/Build.UnitTests/TaskHostFactoryLifecycle_E2E_Tests.cs

@@ -0,0 +1,120 @@
+// 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.IO;
+using System.Runtime.InteropServices;
+using Microsoft.Build.UnitTests;
+using Microsoft.Build.UnitTests.Shared;
+using Shouldly;
+using Xunit;
+using Xunit.Abstractions;
+
+namespace Microsoft.Build.Engine.UnitTests
+{
+    /// <summary>
+    /// End-to-end tests for task host factory lifecycle behavior.
+    /// 
+    /// Tests validate the behavior based on whether the TaskHost runtime matches 
+    /// the executing MSBuild runtime and whether TaskHostFactory is explicitly requested.
+    /// 
+    /// This is a regression test for https://github.com/dotnet/msbuild/issues/13013
+    /// </summary>
+    public class TaskHostFactoryLifecycle_E2E_Tests
+    {
+        private static string AssemblyLocation { get; } = Path.Combine(Path.GetDirectoryName(typeof(TaskHostFactoryLifecycle_E2E_Tests).Assembly.Location) ?? System.AppContext.BaseDirectory);
+
+        private static string TestAssetsRootPath { get; } = Path.Combine(AssemblyLocation, "TestAssets", "TaskHostLifecycle");
+
+        private readonly ITestOutputHelper _output;
+
+        public TaskHostFactoryLifecycle_E2E_Tests(ITestOutputHelper output)
+        {
+            _output = output;
+        }
+
+        /// <summary>
+        /// Validates task host lifecycle behavior for all scenarios.
+        /// 
+        /// Test scenarios:
+        /// 1. Runtime matches + TaskHostFactory requested → short-lived out of proc (nodereuse:False)
+        /// 2. Runtime matches + TaskHostFactory NOT requested → in-proc execution
+        /// 3. Runtime doesn't match + TaskHostFactory requested → short-lived out of proc (nodereuse:False)
+        /// 4. Runtime doesn't match + TaskHostFactory NOT requested → long-lived sidecar out of proc (nodereuse:True)
+        /// </summary>
+        /// <param name="runtimeToUse">The runtime to use for the task (CurrentRuntime or NET)</param>
+        /// <param name="taskFactoryToUse">The task factory to use (TaskHostFactory or AssemblyTaskFactory)</param>
+        [Theory]
+#if NET
+        [InlineData("CurrentRuntime", "AssemblyTaskFactory")] // Match + No Explicit → in-proc
+        [InlineData("CurrentRuntime", "TaskHostFactory")]     // Match + Explicit → short-lived out-of-proc
+#endif
+        [InlineData("NET", "AssemblyTaskFactory")]            // No Match + No Explicit → long-lived sidecar out-of-proc
+        [InlineData("NET", "TaskHostFactory")]                // No Match + Explicit → short-lived out-of-proc
+        public void TaskHostLifecycle_ValidatesAllScenarios(
+            string runtimeToUse,
+            string taskFactoryToUse)
+        {
+            bool? expectedNodeReuse;
+
+            // TaskHostFactory is always short lived and out-of-proc
+            if (taskFactoryToUse == "TaskHostFactory")
+            {
+                expectedNodeReuse = false;
+            }
+            // AssemblyTaskFactory behavior depends on runtime
+            else if (taskFactoryToUse == "AssemblyTaskFactory")
+            {
+                if (runtimeToUse == "CurrentRuntime")
+                {
+                    // in-proc
+                    expectedNodeReuse = null;
+                }
+                else if (runtimeToUse == "NET")
+                {
+                    // When running on .NET Framework: out-of-proc, otherwise on .NET in-proc.
+                    expectedNodeReuse = RuntimeInformation.FrameworkDescription.StartsWith(".NET Framework", StringComparison.OrdinalIgnoreCase) ? true : null;
+                }
+                else
+                {
+                    throw new ArgumentOutOfRangeException(nameof(runtimeToUse), "Unknown runtime to use: " + runtimeToUse);
+                }
+            }
+            else
+            {
+                throw new ArgumentOutOfRangeException(nameof(taskFactoryToUse), "Unknown task factory to use: " + taskFactoryToUse);
+            }
+
+            using TestEnvironment env = TestEnvironment.Create(_output);
+            string testProjectPath = Path.Combine(TestAssetsRootPath, "TaskHostLifecycleTestApp.csproj");
+
+            string testTaskOutput = RunnerUtilities.ExecBootstrapedMSBuild(
+                $"{testProjectPath} -v:n -restore /p:RuntimeToUse={runtimeToUse} /p:TaskFactoryToUse={taskFactoryToUse}", 
+                out bool successTestTask,
+                outputHelper: _output);
+
+            successTestTask.ShouldBeTrue();
+
+            // Verify execution mode (out-of-proc vs in-proc) and node reuse behavior
+            if (expectedNodeReuse.HasValue)
+            {
+                // For out-of-proc scenarios, validate the task runs in a separate process
+                // by checking for the presence of command-line arguments that indicate task host execution
+                testTaskOutput.ShouldContain("/nodemode:", 
+                    customMessage: "Task should run out-of-proc and have /nodemode: in its command-line arguments");
+
+                // Validate the nodereuse flag in the task's command-line arguments
+                string expectedFlag = expectedNodeReuse.Value ? "/nodereuse:True" : "/nodereuse:False";
+                testTaskOutput.ShouldContain(expectedFlag, 
+                    customMessage: $"Task should have {expectedFlag} in its command-line arguments");
+            }
+            else
+            {
+                // For in-proc scenarios, validate the task does NOT run in a task host
+                // by ensuring task host specific command-line flags are not present
+                testTaskOutput.ShouldNotContain("/nodemode:", 
+                    customMessage: "Task should run in-proc and not have task host command-line arguments like /nodemode:");
+            }
+        }
+    }
+}

src/Build.UnitTests/TestAssets/ExampleNetTask/ExampleTask/ExampleTask.cs

@@ -20,7 +20,7 @@ public override bool Execute()
                 var executingProcess = currentProcess.ProcessName;
                 var processPath = currentProcess.MainModule?.FileName ?? "Unknown";
 
-                Log.LogMessage(MessageImportance.High, $"The task is executed in process: {executingProcess}");
+                Log.LogMessage(MessageImportance.High, $"The task is executed in process: {executingProcess} with id {currentProcess.Id}");
                 Log.LogMessage(MessageImportance.High, $"Process path: {processPath}");
 
                 string[] args = Environment.GetCommandLineArgs();

src/Build.UnitTests/TestAssets/TaskHostLifecycle/TaskHostLifecycleTestApp.csproj

@@ -0,0 +1,19 @@
+<Project Sdk="Microsoft.NET.Sdk">
+
+  <PropertyGroup>
+    <TargetFramework>net10.0</TargetFramework>
+    <TestProjectFolder>$([System.IO.Path]::GetFullPath('$([System.IO.Path]::Combine('$(AssemblyLocation)', '..'))'))</TestProjectFolder>
+    <ExampleTaskPath>$([System.IO.Path]::Combine('$(TestProjectFolder)', '$(TargetFramework)', 'ExampleTask.dll'))</ExampleTaskPath>
+  </PropertyGroup>
+
+  <UsingTask
+      TaskName="ExampleTask"
+      AssemblyFile="$(ExampleTaskPath)"
+      TaskFactory="$(TaskFactoryToUse)"
+      Runtime="$(RuntimeToUse)"/>
+
+  <Target Name="TestTask" BeforeTargets="Build">
+    <ExampleTask />
+  </Target>
+
+</Project>

src/Build/BackEnd/Components/Communications/NodeProviderOutOfProcBase.cs

@@ -216,6 +216,7 @@ protected IList<NodeContext> GetNodes(
                 }
             }
 
+            bool nodeReuseRequested = Handshake.IsHandshakeOptionEnabled(hostHandshake.HandshakeOptions, HandshakeOptions.NodeReuse);
             // Get all process of possible running node processes for reuse and put them into ConcurrentQueue.
             // Processes from this queue will be concurrently consumed by TryReusePossibleRunningNodes while
             //    trying to connect to them and reuse them. When queue is empty, no process to reuse left
@@ -224,7 +225,7 @@ protected IList<NodeContext> GetNodes(
             ConcurrentQueue<Process> possibleRunningNodes = null;
 #if FEATURE_NODE_REUSE
             // Try to connect to idle nodes if node reuse is enabled.
-            if (_componentHost.BuildParameters.EnableNodeReuse)
+            if (nodeReuseRequested)
             {
                 IList<Process> possibleRunningNodesList;
                 (expectedProcessName, possibleRunningNodesList) = GetPossibleRunningNodes(msbuildLocation);
@@ -236,14 +237,20 @@ protected IList<NodeContext> GetNodes(
                 }
             }
 #endif
+
             ConcurrentQueue<NodeContext> nodeContexts = new();
             ConcurrentQueue<Exception> exceptions = new();
             int currentProcessId = EnvironmentUtilities.CurrentProcessId;
             Parallel.For(nextNodeId, nextNodeId + numberOfNodesToCreate, (nodeId) =>
             {
                 try
                 {
-                    if (!TryReuseAnyFromPossibleRunningNodes(currentProcessId, nodeId) && !StartNewNode(nodeId))
+                    if (nodeReuseRequested && TryReuseAnyFromPossibleRunningNodes(currentProcessId, nodeId))
+                    {
+                        return;
+                    }
+
+                    if (!StartNewNode(nodeId))
                     {
                         // We were unable to reuse or launch a node.
                         CommunicationsUtilities.Trace("FAILED TO CONNECT TO A CHILD NODE");

src/Build/Instance/TaskFactories/AssemblyTaskFactory.cs

@@ -361,10 +361,14 @@ internal ITask CreateTaskInstance(
                 ErrorUtilities.VerifyThrowInternalNull(buildComponentHost);
 
                 mergedParameters = UpdateTaskHostParameters(mergedParameters);
-                (mergedParameters, bool isNetRuntime) = AddNetHostParamsIfNeeded(mergedParameters, getProperty);
+                mergedParameters = AddNetHostParamsIfNeeded(mergedParameters, getProperty);
 
-                bool useSidecarTaskHost = !(_factoryIdentityParameters.TaskHostFactoryExplicitlyRequested ?? false)
-                    || isNetRuntime;
+                // Sidecar here means that the task host is launched with /nodeReuse:true and doesn't terminate
+                // after the task execution. This improves performance for tasks that run multiple times in a build.
+                // If the task host factory is explicitly requested, do not act as a sidecar task host.
+                // This is important as customers use task host factories for short lived tasks to release
+                // potential locks.
+                bool useSidecarTaskHost = !(_factoryIdentityParameters.TaskHostFactoryExplicitlyRequested ?? false);
 
                 TaskHostTask task = new(
                     taskLocation,
@@ -631,33 +635,32 @@ private static TaskHostParameters MergeTaskFactoryParameterSets(
         /// Adds the properties necessary for .NET task host instantiation if the runtime is .NET.
         /// Returns a new TaskHostParameters with .NET host parameters added, or the original if not needed.
         /// </summary>
-        private static (TaskHostParameters TaskHostParams, bool isNetRuntime) AddNetHostParamsIfNeeded(
+        private static TaskHostParameters AddNetHostParamsIfNeeded(
             in TaskHostParameters currentParams,
             Func<string, ProjectPropertyInstance> getProperty)
         {
             // Only add .NET host parameters if runtime is .NET
             if (currentParams.Runtime == null ||
                 !currentParams.Runtime.Equals(XMakeAttributes.MSBuildRuntimeValues.net, StringComparison.OrdinalIgnoreCase))
             {
-                return (currentParams, isNetRuntime: false);
+                return currentParams;
             }
 
             string dotnetHostPath = getProperty(Constants.DotnetHostPathEnvVarName)?.EvaluatedValue;
             string ridGraphPath = getProperty(Constants.RuntimeIdentifierGraphPath)?.EvaluatedValue;
 
             if (string.IsNullOrEmpty(dotnetHostPath) || string.IsNullOrEmpty(ridGraphPath))
             {
-                return (currentParams, isNetRuntime: false);
+                return currentParams;
             }
 
             string msBuildAssemblyPath = Path.GetDirectoryName(ridGraphPath) ?? string.Empty;
 
-            return (new TaskHostParameters(
+            return new TaskHostParameters(
                 runtime: currentParams.Runtime,
                 architecture: currentParams.Architecture,
                 dotnetHostPath: dotnetHostPath,
-                msBuildAssemblyPath: msBuildAssemblyPath),
-                isNetRuntime: true);
+                msBuildAssemblyPath: msBuildAssemblyPath);
         }
 
         /// <summary>