From fccc27a352cfa1f2e05c42cd444d4bdf582dcf06 Mon Sep 17 00:00:00 2001 From: Jan Provaznik Date: Mon, 9 Mar 2026 19:59:27 +0100 Subject: [PATCH 01/13] Add TaskHostBuildRequest/Response packet pair for BuildProjectFile callbacks Introduce two new IPC packets for forwarding BuildProjectFile* calls from the OOP TaskHost to the worker node: - TaskHostBuildRequest (0x20): Carries all 6-param canonical form args (projectFileNames, targetNames, globalProperties, removeGlobalProperties, toolsVersions, returnTargetOutputs) - TaskHostBuildResponse (0x21): Carries bool success + target outputs using proven TaskParameter/TaskParameterTaskItem serialization Includes TranslateNullableStringArray helper for string arrays with null elements (e.g. toolsVersions), and IDictionary<->Dictionary conversion helpers matching TaskHostConfiguration precedent. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- src/Build/Microsoft.Build.csproj | 2 + src/MSBuild/MSBuild.csproj | 2 + src/Shared/TaskHostBuildRequest.cs | 217 ++++++++++++++++++++++++++++ src/Shared/TaskHostBuildResponse.cs | 158 ++++++++++++++++++++ 4 files changed, 379 insertions(+) create mode 100644 src/Shared/TaskHostBuildRequest.cs create mode 100644 src/Shared/TaskHostBuildResponse.cs diff --git a/src/Build/Microsoft.Build.csproj b/src/Build/Microsoft.Build.csproj index 5343eabdc97..8772bae1957 100644 --- a/src/Build/Microsoft.Build.csproj +++ b/src/Build/Microsoft.Build.csproj @@ -127,6 +127,8 @@ + + diff --git a/src/MSBuild/MSBuild.csproj b/src/MSBuild/MSBuild.csproj index cf205b2366d..cef3894fa5e 100644 --- a/src/MSBuild/MSBuild.csproj +++ b/src/MSBuild/MSBuild.csproj @@ -118,6 +118,8 @@ + + diff --git a/src/Shared/TaskHostBuildRequest.cs b/src/Shared/TaskHostBuildRequest.cs new file mode 100644 index 00000000000..dd3a24e26a0 --- /dev/null +++ b/src/Shared/TaskHostBuildRequest.cs @@ -0,0 +1,217 @@ +// 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; +using System.Collections.Generic; + +namespace Microsoft.Build.BackEnd +{ + /// + /// Packet sent from TaskHost to owning worker node to execute a BuildProjectFile* callback. + /// All four BuildProjectFile/BuildProjectFilesInParallel overloads normalize to the + /// IBuildEngine3 6-param canonical form carried by this packet. + /// + internal class TaskHostBuildRequest : INodePacket, ITaskHostCallbackPacket + { + private int _requestId; + private string[]? _projectFileNames; + private string[]? _targetNames; + private Dictionary?[]? _globalProperties; + private List?[]? _removeGlobalProperties; + private string[]? _toolsVersions; + private bool _returnTargetOutputs; + + public TaskHostBuildRequest() + { + } + + public TaskHostBuildRequest( + string[]? projectFileNames, + string[]? targetNames, + Dictionary?[]? globalProperties, + List?[]? removeGlobalProperties, + string[]? toolsVersions, + bool returnTargetOutputs) + { + _projectFileNames = projectFileNames; + _targetNames = targetNames; + _globalProperties = globalProperties; + _removeGlobalProperties = removeGlobalProperties; + _toolsVersions = toolsVersions; + _returnTargetOutputs = returnTargetOutputs; + } + + public NodePacketType Type => NodePacketType.TaskHostBuildRequest; + + public int RequestId + { + get => _requestId; + set => _requestId = value; + } + + public string[]? ProjectFileNames => _projectFileNames; + + public string[]? TargetNames => _targetNames; + + public Dictionary?[]? GlobalProperties => _globalProperties; + + public List?[]? RemoveGlobalProperties => _removeGlobalProperties; + + public string[]? ToolsVersions => _toolsVersions; + + public bool ReturnTargetOutputs => _returnTargetOutputs; + + /// + /// Converts non-generic IDictionary[] (as used by IBuildEngine interfaces) to + /// Dictionary<string, string>[] for serialization. + /// + internal static Dictionary?[]? ConvertGlobalProperties(IDictionary[]? globalProperties) + { + if (globalProperties is null) + { + return null; + } + + var result = new Dictionary?[globalProperties.Length]; + for (int i = 0; i < globalProperties.Length; i++) + { + if (globalProperties[i] is not null) + { + result[i] = new Dictionary(StringComparer.OrdinalIgnoreCase); + foreach (DictionaryEntry entry in globalProperties[i]) + { + result[i]![(string)entry.Key] = (string)entry.Value!; + } + } + } + + return result; + } + + /// + /// Converts IList<string>[] to List<string>[] for serialization. + /// + internal static List?[]? ConvertRemoveGlobalProperties(IList[]? removeGlobalProperties) + { + if (removeGlobalProperties is null) + { + return null; + } + + var result = new List?[removeGlobalProperties.Length]; + for (int i = 0; i < removeGlobalProperties.Length; i++) + { + if (removeGlobalProperties[i] is not null) + { + result[i] = new List(removeGlobalProperties[i]); + } + } + + return result; + } + + public void Translate(ITranslator translator) + { + translator.Translate(ref _requestId); + TranslateNullableStringArray(translator, ref _projectFileNames); + TranslateNullableStringArray(translator, ref _targetNames); + translator.Translate(ref _returnTargetOutputs); + TranslateNullableStringArray(translator, ref _toolsVersions); + TranslateGlobalPropertiesArray(translator); + TranslateRemoveGlobalPropertiesArray(translator); + } + + /// + /// Serializes a string array where individual elements may be null. + /// The standard translator.Translate(ref string[]) doesn't handle null elements. + /// + private static void TranslateNullableStringArray(ITranslator translator, ref string[]? array) + { + bool hasArray = array is not null; + translator.Translate(ref hasArray); + + if (!hasArray) + { + array = null; + return; + } + + int length = array?.Length ?? 0; + translator.Translate(ref length); + + if (translator.Mode == TranslationDirection.ReadFromStream) + { + array = new string[length]; + } + + for (int i = 0; i < length; i++) + { + string? element = array![i]; + translator.Translate(ref element); + array[i] = element!; + } + } + + private void TranslateGlobalPropertiesArray(ITranslator translator) + { + bool hasArray = _globalProperties is not null; + translator.Translate(ref hasArray); + + if (!hasArray) + { + _globalProperties = null; + return; + } + + int length = _globalProperties?.Length ?? 0; + translator.Translate(ref length); + + if (translator.Mode == TranslationDirection.ReadFromStream) + { + _globalProperties = new Dictionary?[length]; + } + + for (int i = 0; i < length; i++) + { + Dictionary? dict = _globalProperties![i]; + translator.TranslateDictionary(ref dict, StringComparer.OrdinalIgnoreCase); + _globalProperties[i] = dict; + } + } + + private void TranslateRemoveGlobalPropertiesArray(ITranslator translator) + { + bool hasArray = _removeGlobalProperties is not null; + translator.Translate(ref hasArray); + + if (!hasArray) + { + _removeGlobalProperties = null; + return; + } + + int length = _removeGlobalProperties?.Length ?? 0; + translator.Translate(ref length); + + if (translator.Mode == TranslationDirection.ReadFromStream) + { + _removeGlobalProperties = new List?[length]; + } + + for (int i = 0; i < length; i++) + { + List? list = _removeGlobalProperties![i]; + translator.Translate(ref list); + _removeGlobalProperties[i] = list; + } + } + + internal static INodePacket FactoryForDeserialization(ITranslator translator) + { + var packet = new TaskHostBuildRequest(); + packet.Translate(translator); + return packet; + } + } +} diff --git a/src/Shared/TaskHostBuildResponse.cs b/src/Shared/TaskHostBuildResponse.cs new file mode 100644 index 00000000000..b520bcfebc2 --- /dev/null +++ b/src/Shared/TaskHostBuildResponse.cs @@ -0,0 +1,158 @@ +// 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 Microsoft.Build.Framework; + +namespace Microsoft.Build.BackEnd +{ + /// + /// Response packet from owning worker node to TaskHost with BuildProjectFile* results. + /// Carries the build success/failure and target outputs per project. + /// + internal class TaskHostBuildResponse : INodePacket, ITaskHostCallbackPacket + { + private int _requestId; + private bool _success; + + /// + /// Target outputs per project. Each entry is a dictionary mapping target names to TaskParameter + /// wrapping ITaskItem[] outputs. Uses the same TaskParameter serialization as TaskHostTaskComplete. + /// + private List>? _targetOutputsPerProject; + + public TaskHostBuildResponse() + { + } + + public TaskHostBuildResponse(int requestId, bool success, List>? targetOutputsPerProject) + { + _requestId = requestId; + _success = success; + _targetOutputsPerProject = targetOutputsPerProject; + } + + public NodePacketType Type => NodePacketType.TaskHostBuildResponse; + + public int RequestId + { + get => _requestId; + set => _requestId = value; + } + + public bool Success => _success; + + public List>? TargetOutputsPerProject => _targetOutputsPerProject; + + /// + /// Reconstructs a from this response packet. + /// Converts values back to [] arrays. + /// + public BuildEngineResult ToBuildEngineResult() + { + List>? result = null; + + if (_targetOutputsPerProject is not null) + { + result = new List>(_targetOutputsPerProject.Count); + + foreach (Dictionary projectOutputs in _targetOutputsPerProject) + { + var dict = new Dictionary(StringComparer.OrdinalIgnoreCase); + + if (projectOutputs is not null) + { + foreach (KeyValuePair entry in projectOutputs) + { + dict[entry.Key] = (ITaskItem[]?)entry.Value?.WrappedParameter ?? []; + } + } + + result.Add(dict); + } + } + + return new BuildEngineResult(_success, result!); + } + + /// + /// Creates a response from a . + /// Wraps [] arrays in for serialization. + /// + internal static TaskHostBuildResponse FromBuildEngineResult(int requestId, BuildEngineResult engineResult) + { + List>? outputs = null; + + if (engineResult.TargetOutputsPerProject is not null && engineResult.TargetOutputsPerProject.Count > 0) + { + outputs = new List>(engineResult.TargetOutputsPerProject.Count); + + foreach (IDictionary projectOutputs in engineResult.TargetOutputsPerProject) + { + var dict = new Dictionary(StringComparer.OrdinalIgnoreCase); + + if (projectOutputs is not null) + { + foreach (KeyValuePair entry in projectOutputs) + { + dict[entry.Key] = new TaskParameter(entry.Value); + } + } + + outputs.Add(dict); + } + } + + return new TaskHostBuildResponse(requestId, engineResult.Result, outputs); + } + + public void Translate(ITranslator translator) + { + translator.Translate(ref _requestId); + translator.Translate(ref _success); + TranslateTargetOutputs(translator); + } + + private void TranslateTargetOutputs(ITranslator translator) + { + bool hasOutputs = _targetOutputsPerProject is not null; + translator.Translate(ref hasOutputs); + + if (!hasOutputs) + { + _targetOutputsPerProject = null; + return; + } + + int count = _targetOutputsPerProject?.Count ?? 0; + translator.Translate(ref count); + + if (translator.Mode == TranslationDirection.ReadFromStream) + { + _targetOutputsPerProject = new List>(count); + for (int i = 0; i < count; i++) + { + Dictionary? dict = null; + translator.TranslateDictionary(ref dict, StringComparer.OrdinalIgnoreCase, TaskParameter.FactoryForDeserialization); + _targetOutputsPerProject.Add(dict!); + } + } + else + { + for (int i = 0; i < count; i++) + { + Dictionary? dict = _targetOutputsPerProject![i]; + translator.TranslateDictionary(ref dict, StringComparer.OrdinalIgnoreCase, TaskParameter.FactoryForDeserialization); + } + } + } + + internal static INodePacket FactoryForDeserialization(ITranslator translator) + { + var packet = new TaskHostBuildResponse(); + packet.Translate(translator); + return packet; + } + } +} From 42cba207b0ab80c24dcc22dd94d295c97ce86bd2 Mon Sep 17 00:00:00 2001 From: Jan Provaznik Date: Mon, 9 Mar 2026 19:59:38 +0100 Subject: [PATCH 02/13] Implement BuildProjectFile* callbacks in OutOfProcTaskHostNode Replace all 4 BuildProjectFile* stubs (IBuildEngine 4-param, IBuildEngine2 5-param, IBuildEngine2 7-param, IBuildEngine3 6-param) with real callback implementations that forward to the worker node. All overloads normalize to the canonical 6-param form, send a TaskHostBuildRequest via SendCallbackRequestAndWaitForResponse, and unpack the TaskHostBuildResponse. Gated behind CallbacksSupported with MSB5022 fallback for older worker nodes. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- src/MSBuild/OutOfProcTaskHostNode.cs | 68 ++++++++++++++++++++++------ 1 file changed, 54 insertions(+), 14 deletions(-) diff --git a/src/MSBuild/OutOfProcTaskHostNode.cs b/src/MSBuild/OutOfProcTaskHostNode.cs index 75716b705a7..a0f7bac55d3 100644 --- a/src/MSBuild/OutOfProcTaskHostNode.cs +++ b/src/MSBuild/OutOfProcTaskHostNode.cs @@ -247,6 +247,7 @@ public OutOfProcTaskHostNode() #if !CLR2COMPATIBILITY thisINodePacketFactory.RegisterPacketHandler(NodePacketType.TaskHostIsRunningMultipleNodesResponse, TaskHostIsRunningMultipleNodesResponse.FactoryForDeserialization, this); thisINodePacketFactory.RegisterPacketHandler(NodePacketType.TaskHostCoresResponse, TaskHostCoresResponse.FactoryForDeserialization, this); + thisINodePacketFactory.RegisterPacketHandler(NodePacketType.TaskHostBuildResponse, TaskHostBuildResponse.FactoryForDeserialization, this); #endif #if !CLR2COMPATIBILITY @@ -415,13 +416,11 @@ public void LogCustomEvent(CustomBuildEventArgs e) } /// - /// Stub implementation of IBuildEngine.BuildProjectFile. The task host does not support IBuildEngine - /// callbacks for the purposes of building projects, so error. + /// Implementation of IBuildEngine.BuildProjectFile. Delegates to the 5-param overload. /// public bool BuildProjectFile(string projectFileName, string[] targetNames, IDictionary globalProperties, IDictionary targetOutputs) { - LogErrorFromResource("BuildEngineCallbacksInTaskHostUnsupported"); - return false; + return BuildProjectFile(projectFileName, targetNames, globalProperties, targetOutputs, null); } #endregion // IBuildEngine Implementation (Methods) @@ -429,23 +428,44 @@ public bool BuildProjectFile(string projectFileName, string[] targetNames, IDict #region IBuildEngine2 Implementation (Methods) /// - /// Stub implementation of IBuildEngine2.BuildProjectFile. The task host does not support IBuildEngine - /// callbacks for the purposes of building projects, so error. + /// Implementation of IBuildEngine2.BuildProjectFile. Delegates to the 7-param BuildProjectFilesInParallel. /// public bool BuildProjectFile(string projectFileName, string[] targetNames, IDictionary globalProperties, IDictionary targetOutputs, string toolsVersion) { - LogErrorFromResource("BuildEngineCallbacksInTaskHostUnsupported"); - return false; + return BuildProjectFilesInParallel( + [projectFileName], + targetNames, + [globalProperties], + [targetOutputs], + [toolsVersion], + true, + false); } /// - /// Stub implementation of IBuildEngine2.BuildProjectFilesInParallel. The task host does not support IBuildEngine - /// callbacks for the purposes of building projects, so error. + /// Implementation of IBuildEngine2.BuildProjectFilesInParallel. Delegates to the 6-param IBuildEngine3 overload. /// public bool BuildProjectFilesInParallel(string[] projectFileNames, string[] targetNames, IDictionary[] globalProperties, IDictionary[] targetOutputsPerProject, string[] toolsVersion, bool useResultsCache, bool unloadProjectsOnCompletion) { - LogErrorFromResource("BuildEngineCallbacksInTaskHostUnsupported"); - return false; + bool includeTargetOutputs = targetOutputsPerProject is not null; + + BuildEngineResult result = BuildProjectFilesInParallel(projectFileNames, targetNames, globalProperties, new List[projectFileNames.Length], toolsVersion, includeTargetOutputs); + + if (includeTargetOutputs) + { + for (int i = 0; i < targetOutputsPerProject.Length && i < result.TargetOutputsPerProject.Count; i++) + { + if (targetOutputsPerProject[i] is not null) + { + foreach (KeyValuePair output in result.TargetOutputsPerProject[i]) + { + targetOutputsPerProject[i].Add(output.Key, output.Value); + } + } + } + } + + return result.Result; } #endregion // IBuildEngine2 Implementation (Methods) @@ -453,13 +473,32 @@ public bool BuildProjectFilesInParallel(string[] projectFileNames, string[] targ #region IBuildEngine3 Implementation /// - /// Stub implementation of IBuildEngine3.BuildProjectFilesInParallel. The task host does not support IBuildEngine - /// callbacks for the purposes of building projects, so error. + /// Implementation of IBuildEngine3.BuildProjectFilesInParallel. This is the canonical form that + /// sends the request to the owning worker node and waits for the response. /// public BuildEngineResult BuildProjectFilesInParallel(string[] projectFileNames, string[] targetNames, IDictionary[] globalProperties, IList[] removeGlobalProperties, string[] toolsVersion, bool returnTargetOutputs) { +#if CLR2COMPATIBILITY LogErrorFromResource("BuildEngineCallbacksInTaskHostUnsupported"); return new BuildEngineResult(false, null); +#else + if (!CallbacksSupported) + { + LogErrorFromResource("BuildEngineCallbacksInTaskHostUnsupported"); + return new BuildEngineResult(false, null); + } + + var request = new TaskHostBuildRequest( + projectFileNames, + targetNames, + TaskHostBuildRequest.ConvertGlobalProperties(globalProperties), + TaskHostBuildRequest.ConvertRemoveGlobalProperties(removeGlobalProperties), + toolsVersion, + returnTargetOutputs); + + var response = SendCallbackRequestAndWaitForResponse(request); + return response.ToBuildEngineResult(); +#endif } /// @@ -816,6 +855,7 @@ private void HandlePacket(INodePacket packet) // Callback response packets - route to pending request case NodePacketType.TaskHostIsRunningMultipleNodesResponse: case NodePacketType.TaskHostCoresResponse: + case NodePacketType.TaskHostBuildResponse: HandleCallbackResponse(packet); break; #endif From 561e91437dfc39b961edfb6fdcc80865c65005aa Mon Sep 17 00:00:00 2001 From: Jan Provaznik Date: Mon, 9 Mar 2026 19:59:47 +0100 Subject: [PATCH 03/13] Handle BuildProjectFile requests in TaskHostTask (worker node side) Add HandleBuildRequest to TaskHostTask that receives TaskHostBuildRequest from the OOP TaskHost, forwards to IBuildEngine3.BuildProjectFilesInParallel, and sends back a TaskHostBuildResponse. Wraps entire handler in try/catch with ExceptionHandling.IsCriticalException filter to always send a failure response on exception, preventing the OOP task thread from blocking forever on TCS. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../Instance/TaskFactories/TaskHostTask.cs | 62 +++++++++++++++++++ 1 file changed, 62 insertions(+) diff --git a/src/Build/Instance/TaskFactories/TaskHostTask.cs b/src/Build/Instance/TaskFactories/TaskHostTask.cs index 4a9dfdd5180..895b196c910 100644 --- a/src/Build/Instance/TaskFactories/TaskHostTask.cs +++ b/src/Build/Instance/TaskFactories/TaskHostTask.cs @@ -207,6 +207,7 @@ public TaskHostTask( (this as INodePacketFactory).RegisterPacketHandler(NodePacketType.NodeShutdown, NodeShutdown.FactoryForDeserialization, this); (this as INodePacketFactory).RegisterPacketHandler(NodePacketType.TaskHostIsRunningMultipleNodesRequest, TaskHostIsRunningMultipleNodesRequest.FactoryForDeserialization, this); (this as INodePacketFactory).RegisterPacketHandler(NodePacketType.TaskHostCoresRequest, TaskHostCoresRequest.FactoryForDeserialization, this); + (this as INodePacketFactory).RegisterPacketHandler(NodePacketType.TaskHostBuildRequest, TaskHostBuildRequest.FactoryForDeserialization, this); _packetReceivedEvent = new AutoResetEvent(false); _receivedPackets = new ConcurrentQueue(); @@ -519,6 +520,9 @@ private void HandlePacket(INodePacket packet, out bool taskFinished) case NodePacketType.TaskHostCoresRequest: HandleCoresRequest(packet as TaskHostCoresRequest); break; + case NodePacketType.TaskHostBuildRequest: + HandleBuildRequest(packet as TaskHostBuildRequest); + break; default: ErrorUtilities.ThrowInternalErrorUnreachable(); break; @@ -696,6 +700,64 @@ private void HandleCoresRequest(TaskHostCoresRequest request) _taskHostProvider.SendData(_taskHostNodeKey, response); } + /// + /// Handle BuildProjectFile* request from the TaskHost. + /// Forwards the call to the in-process IBuildEngine3.BuildProjectFilesInParallel, + /// which handles project resolution, scheduler interaction, and target execution. + /// + private void HandleBuildRequest(TaskHostBuildRequest request) + { + TaskHostBuildResponse response; + try + { + if (_buildEngine is not IBuildEngine3 engine3) + { + response = new TaskHostBuildResponse(request.RequestId, false, null); + _taskHostProvider.SendData(_taskHostNodeKey, response); + return; + } + + // Reconstruct IDictionary[] from the serialized Dictionary[] + System.Collections.IDictionary[] globalProperties = null; + if (request.GlobalProperties is not null) + { + globalProperties = new System.Collections.IDictionary[request.GlobalProperties.Length]; + for (int i = 0; i < request.GlobalProperties.Length; i++) + { + globalProperties[i] = request.GlobalProperties[i]; + } + } + + // Reconstruct IList[] from List[] + IList[] removeGlobalProperties = null; + if (request.RemoveGlobalProperties is not null) + { + removeGlobalProperties = new IList[request.RemoveGlobalProperties.Length]; + for (int i = 0; i < request.RemoveGlobalProperties.Length; i++) + { + removeGlobalProperties[i] = request.RemoveGlobalProperties[i]; + } + } + + BuildEngineResult result = engine3.BuildProjectFilesInParallel( + request.ProjectFileNames, + request.TargetNames, + globalProperties, + removeGlobalProperties, + request.ToolsVersions, + request.ReturnTargetOutputs); + + response = TaskHostBuildResponse.FromBuildEngineResult(request.RequestId, result); + } + catch (Exception ex) when (!ExceptionHandling.IsCriticalException(ex)) + { + // Always send a response to prevent the OOP task from hanging. + response = new TaskHostBuildResponse(request.RequestId, false, null); + } + + _taskHostProvider.SendData(_taskHostNodeKey, response); + } + /// /// Since we log that we weren't able to connect to the task host in a couple of different places, /// extract it out into a separate method. From 29b3f59f8c35f40691c019c0feb782194b99837a Mon Sep 17 00:00:00 2001 From: Jan Provaznik Date: Mon, 9 Mar 2026 20:00:02 +0100 Subject: [PATCH 04/13] Add BuildProjectFileTask test task and E2E test assets - BuildProjectFileTask: configurable test task calling BuildEngine.BuildProjectFile with ProjectPath, Targets, GlobalProperties, and BuildSucceeded output - TestNetTaskBuildCallback: E2E .NET task project with ChildProject.proj - Link BuildProjectFileTask into ExampleTask.csproj for E2E use Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../BackEnd/BuildProjectFileTask.cs | 91 +++++++++++++++++++ .../ExampleTask/ExampleTask.csproj | 1 + .../ChildProject.proj | 11 +++ .../TestNetTaskBuildCallback.csproj | 30 ++++++ .../TestNetTaskBuildCallback/global.json | 2 + 5 files changed, 135 insertions(+) create mode 100644 src/Build.UnitTests/BackEnd/BuildProjectFileTask.cs create mode 100644 src/Build.UnitTests/TestAssets/ExampleNetTask/TestNetTaskBuildCallback/ChildProject.proj create mode 100644 src/Build.UnitTests/TestAssets/ExampleNetTask/TestNetTaskBuildCallback/TestNetTaskBuildCallback.csproj create mode 100644 src/Build.UnitTests/TestAssets/ExampleNetTask/TestNetTaskBuildCallback/global.json diff --git a/src/Build.UnitTests/BackEnd/BuildProjectFileTask.cs b/src/Build.UnitTests/BackEnd/BuildProjectFileTask.cs new file mode 100644 index 00000000000..ae4c0aeaaf0 --- /dev/null +++ b/src/Build.UnitTests/BackEnd/BuildProjectFileTask.cs @@ -0,0 +1,91 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System.Collections; +using System.Collections.Generic; +using Microsoft.Build.Framework; +using Microsoft.Build.Utilities; + +namespace Microsoft.Build.UnitTests.BackEnd +{ + /// + /// A test task that calls IBuildEngine.BuildProjectFile to build another project. + /// Used by TaskHostCallback_Tests (in-process) and NetTaskHost_E2E_Tests (cross-runtime). + /// The E2E project includes this file via linked compile to avoid duplication. + /// + public class BuildProjectFileTask : Task + { + /// + /// Path to the project file to build. + /// + [Required] + public string ProjectFile { get; set; } = string.Empty; + + /// + /// Semicolon-separated list of targets to build. If empty, builds default targets. + /// + public string Targets { get; set; } = string.Empty; + + /// + /// Semicolon-separated list of Property=Value pairs to pass as global properties. + /// + public string Properties { get; set; } = string.Empty; + + /// + /// Whether the child build succeeded. + /// + [Output] + public bool BuildSucceeded { get; set; } + + /// + /// Target output items from the child build (if any). + /// + [Output] + public ITaskItem[] OutputItems { get; set; } = []; + + public override bool Execute() + { + string[]? targetNames = null; + if (!string.IsNullOrEmpty(Targets)) + { + targetNames = Targets.Split(';'); + } + + Hashtable? globalProperties = null; + if (!string.IsNullOrEmpty(Properties)) + { + globalProperties = new Hashtable(); + foreach (string pair in Properties.Split(';')) + { + string[] parts = pair.Split('='); + if (parts.Length == 2) + { + globalProperties[parts[0].Trim()] = parts[1].Trim(); + } + } + } + + Hashtable targetOutputs = new(); + + BuildSucceeded = BuildEngine.BuildProjectFile(ProjectFile, targetNames, globalProperties, targetOutputs); + + // Extract first target's outputs as ITaskItem[] if available. + if (BuildSucceeded && targetOutputs.Count > 0) + { + var items = new List(); + foreach (DictionaryEntry entry in targetOutputs) + { + if (entry.Value is ITaskItem[] taskItems) + { + items.AddRange(taskItems); + } + } + + OutputItems = items.ToArray(); + } + + Log.LogMessage(MessageImportance.High, $"BuildProjectFile({ProjectFile}) = {BuildSucceeded}, OutputItems={OutputItems.Length}"); + return true; + } + } +} diff --git a/src/Build.UnitTests/TestAssets/ExampleNetTask/ExampleTask/ExampleTask.csproj b/src/Build.UnitTests/TestAssets/ExampleNetTask/ExampleTask/ExampleTask.csproj index b6ef11cdaec..633f102a57d 100644 --- a/src/Build.UnitTests/TestAssets/ExampleNetTask/ExampleTask/ExampleTask.csproj +++ b/src/Build.UnitTests/TestAssets/ExampleNetTask/ExampleTask/ExampleTask.csproj @@ -17,6 +17,7 @@ + diff --git a/src/Build.UnitTests/TestAssets/ExampleNetTask/TestNetTaskBuildCallback/ChildProject.proj b/src/Build.UnitTests/TestAssets/ExampleNetTask/TestNetTaskBuildCallback/ChildProject.proj new file mode 100644 index 00000000000..bf232282a27 --- /dev/null +++ b/src/Build.UnitTests/TestAssets/ExampleNetTask/TestNetTaskBuildCallback/ChildProject.proj @@ -0,0 +1,11 @@ + + + + MetadataValue + + + + + + + diff --git a/src/Build.UnitTests/TestAssets/ExampleNetTask/TestNetTaskBuildCallback/TestNetTaskBuildCallback.csproj b/src/Build.UnitTests/TestAssets/ExampleNetTask/TestNetTaskBuildCallback/TestNetTaskBuildCallback.csproj new file mode 100644 index 00000000000..bbd9bbb81eb --- /dev/null +++ b/src/Build.UnitTests/TestAssets/ExampleNetTask/TestNetTaskBuildCallback/TestNetTaskBuildCallback.csproj @@ -0,0 +1,30 @@ + + + + $(LatestDotNetCoreForMSBuild) + + + + $([System.IO.Path]::GetFullPath('$([System.IO.Path]::Combine('$(MSBuildThisFileDirectory)', '..', '..', '..', '..'))')) + $([System.IO.Path]::Combine('$(TestProjectFolder)', '$(TargetFramework)', 'ExampleTask.dll')) + $([System.IO.Path]::Combine('$(MSBuildThisFileDirectory)', 'ChildProject.proj')) + + + + + + + + + + + + + + diff --git a/src/Build.UnitTests/TestAssets/ExampleNetTask/TestNetTaskBuildCallback/global.json b/src/Build.UnitTests/TestAssets/ExampleNetTask/TestNetTaskBuildCallback/global.json new file mode 100644 index 00000000000..2c63c085104 --- /dev/null +++ b/src/Build.UnitTests/TestAssets/ExampleNetTask/TestNetTaskBuildCallback/global.json @@ -0,0 +1,2 @@ +{ +} From ac6f952c03a0ac79e61a75e1caeb3e2e6e43d2e9 Mon Sep 17 00:00:00 2001 From: Jan Provaznik Date: Mon, 9 Mar 2026 20:00:16 +0100 Subject: [PATCH 05/13] Add comprehensive tests for BuildProjectFile* callback forwarding Serialization tests (4): - TaskHostBuildRequest round-trip with all fields - TaskHostBuildRequest with null arrays (toolsVersions, globalProperties) - TaskHostBuildResponse success with target outputs + ITaskItem metadata - TaskHostBuildResponse failure with no outputs Integration tests (6): - BuildProjectFile with explicit TaskHostFactory - BuildProjectFile with global properties forwarding - BuildProjectFile with target output extraction - BuildProjectFile child project failure returns false - BuildProjectFile auto-ejected in multithreaded mode - BuildProjectFile MSB5022 fallback when callbacks not supported E2E test (1): - Cross-runtime BuildProjectFile via .NET TaskHost Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../BackEnd/TaskHostCallbackPacket_Tests.cs | 113 ++++++++ .../BackEnd/TaskHostCallback_Tests.cs | 258 ++++++++++++++++++ src/Build.UnitTests/NetTaskHost_E2E_Tests.cs | 23 ++ 3 files changed, 394 insertions(+) diff --git a/src/Build.UnitTests/BackEnd/TaskHostCallbackPacket_Tests.cs b/src/Build.UnitTests/BackEnd/TaskHostCallbackPacket_Tests.cs index 36de360aa8f..de49f06041a 100644 --- a/src/Build.UnitTests/BackEnd/TaskHostCallbackPacket_Tests.cs +++ b/src/Build.UnitTests/BackEnd/TaskHostCallbackPacket_Tests.cs @@ -1,7 +1,11 @@ // 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 Microsoft.Build.BackEnd; +using Microsoft.Build.Framework; +using Microsoft.Build.Execution; using Shouldly; using Xunit; @@ -84,5 +88,114 @@ public void TaskHostCoresResponse_RoundTrip_Serialization(int grantedCores) deserialized.GrantedCores.ShouldBe(grantedCores); deserialized.Type.ShouldBe(NodePacketType.TaskHostCoresResponse); } + + [Fact] + public void TaskHostBuildRequest_RoundTrip_Serialization() + { + var globalProps = new Dictionary[] { new(StringComparer.OrdinalIgnoreCase) { ["Configuration"] = "Release" }, null }; + var removeProps = new List[] { new() { "Platform" }, null }; + var request = new TaskHostBuildRequest( + ["proj1.csproj", "proj2.csproj"], + ["Build", "Test"], + globalProps, + removeProps, + ["17.0", null], + returnTargetOutputs: true); + request.RequestId = 55; + + ITranslator writeTranslator = TranslationHelpers.GetWriteTranslator(); + request.Translate(writeTranslator); + + ITranslator readTranslator = TranslationHelpers.GetReadTranslator(); + var deserialized = (TaskHostBuildRequest)TaskHostBuildRequest.FactoryForDeserialization(readTranslator); + + deserialized.RequestId.ShouldBe(55); + deserialized.Type.ShouldBe(NodePacketType.TaskHostBuildRequest); + deserialized.ProjectFileNames.ShouldBe(["proj1.csproj", "proj2.csproj"]); + deserialized.TargetNames.ShouldBe(["Build", "Test"]); + deserialized.ToolsVersions.ShouldBe(["17.0", null]); + deserialized.ReturnTargetOutputs.ShouldBeTrue(); + deserialized.GlobalProperties.Length.ShouldBe(2); + deserialized.GlobalProperties[0]["Configuration"].ShouldBe("Release"); + deserialized.GlobalProperties[1].ShouldBeNull(); + deserialized.RemoveGlobalProperties.Length.ShouldBe(2); + deserialized.RemoveGlobalProperties[0].ShouldBe(["Platform"]); + deserialized.RemoveGlobalProperties[1].ShouldBeNull(); + } + + [Fact] + public void TaskHostBuildRequest_NullArrays_RoundTrip_Serialization() + { + var request = new TaskHostBuildRequest( + null, null, null, null, null, returnTargetOutputs: false); + request.RequestId = 10; + + ITranslator writeTranslator = TranslationHelpers.GetWriteTranslator(); + request.Translate(writeTranslator); + + ITranslator readTranslator = TranslationHelpers.GetReadTranslator(); + var deserialized = (TaskHostBuildRequest)TaskHostBuildRequest.FactoryForDeserialization(readTranslator); + + deserialized.RequestId.ShouldBe(10); + deserialized.ProjectFileNames.ShouldBeNull(); + deserialized.TargetNames.ShouldBeNull(); + deserialized.GlobalProperties.ShouldBeNull(); + deserialized.RemoveGlobalProperties.ShouldBeNull(); + deserialized.ToolsVersions.ShouldBeNull(); + deserialized.ReturnTargetOutputs.ShouldBeFalse(); + } + + [Fact] + public void TaskHostBuildResponse_Success_WithOutputs_RoundTrip_Serialization() + { + var outputs = new List> + { + new(StringComparer.OrdinalIgnoreCase) + { + ["Build"] = new TaskParameter(new ITaskItem[] { new Utilities.TaskItem("item1.dll") }), + ["Test"] = new TaskParameter(new ITaskItem[] { new Utilities.TaskItem("result.trx") }) + } + }; + + var response = new TaskHostBuildResponse(88, true, outputs); + + ITranslator writeTranslator = TranslationHelpers.GetWriteTranslator(); + response.Translate(writeTranslator); + + ITranslator readTranslator = TranslationHelpers.GetReadTranslator(); + var deserialized = (TaskHostBuildResponse)TaskHostBuildResponse.FactoryForDeserialization(readTranslator); + + deserialized.RequestId.ShouldBe(88); + deserialized.Success.ShouldBeTrue(); + deserialized.Type.ShouldBe(NodePacketType.TaskHostBuildResponse); + deserialized.TargetOutputsPerProject.ShouldNotBeNull(); + deserialized.TargetOutputsPerProject.Count.ShouldBe(1); + deserialized.TargetOutputsPerProject[0].ContainsKey("Build").ShouldBeTrue(); + + var buildEngineResult = deserialized.ToBuildEngineResult(); + buildEngineResult.Result.ShouldBeTrue(); + buildEngineResult.TargetOutputsPerProject.Count.ShouldBe(1); + buildEngineResult.TargetOutputsPerProject[0]["Build"].Length.ShouldBe(1); + buildEngineResult.TargetOutputsPerProject[0]["Build"][0].ItemSpec.ShouldBe("item1.dll"); + } + + [Fact] + public void TaskHostBuildResponse_Failure_NoOutputs_RoundTrip_Serialization() + { + var response = new TaskHostBuildResponse(33, false, null); + + ITranslator writeTranslator = TranslationHelpers.GetWriteTranslator(); + response.Translate(writeTranslator); + + ITranslator readTranslator = TranslationHelpers.GetReadTranslator(); + var deserialized = (TaskHostBuildResponse)TaskHostBuildResponse.FactoryForDeserialization(readTranslator); + + deserialized.RequestId.ShouldBe(33); + deserialized.Success.ShouldBeFalse(); + deserialized.TargetOutputsPerProject.ShouldBeNull(); + + var buildEngineResult = deserialized.ToBuildEngineResult(); + buildEngineResult.Result.ShouldBeFalse(); + } } } diff --git a/src/Build.UnitTests/BackEnd/TaskHostCallback_Tests.cs b/src/Build.UnitTests/BackEnd/TaskHostCallback_Tests.cs index 301e8c57a52..f2964ac045c 100644 --- a/src/Build.UnitTests/BackEnd/TaskHostCallback_Tests.cs +++ b/src/Build.UnitTests/BackEnd/TaskHostCallback_Tests.cs @@ -278,5 +278,263 @@ public void RequestCores_LogsErrorWhenCallbacksNotSupported() logger.ErrorCount.ShouldBeGreaterThan(0); logger.FullLog.ShouldContain("MSB5022"); } + + /// + /// Verifies BuildProjectFile callback works when task is explicitly run in TaskHost via TaskHostFactory. + /// The child project should build and the task should return success. + /// + [Fact] + public void BuildProjectFile_WorksWithExplicitTaskHostFactory() + { + using TestEnvironment env = TestEnvironment.Create(_output); + env.SetEnvironmentVariable("MSBUILDENABLETASKHOSTCALLBACKS", "1"); + + string childProject = env.CreateFile("Child.proj", """ + + + + + + """).Path; + + string projectContents = $@" + + + + <{nameof(BuildProjectFileTask)} ProjectFile=""{childProject}"" Targets=""Build""> + + + +"; + + TransientTestProjectWithFiles project = env.CreateTestProjectWithFiles(projectContents); + ProjectInstance projectInstance = new(project.ProjectFile); + + var logger = new MockLogger(_output); + BuildResult buildResult = BuildManager.DefaultBuildManager.Build( + new BuildParameters { MaxNodeCount = 4, EnableNodeReuse = false, Loggers = [logger] }, + new BuildRequestData(projectInstance, targetsToBuild: ["Test"])); + + buildResult.OverallResult.ShouldBe(BuildResultCode.Success); + bool.Parse(projectInstance.GetPropertyValue("Result")).ShouldBeTrue(); + logger.FullLog.ShouldContain("ChildProjectBuilt"); + } + + /// + /// Verifies BuildProjectFile forwards global properties to the child build. + /// + [Fact] + public void BuildProjectFile_ForwardsGlobalProperties() + { + using TestEnvironment env = TestEnvironment.Create(_output); + env.SetEnvironmentVariable("MSBUILDENABLETASKHOSTCALLBACKS", "1"); + + string childProject = env.CreateFile("Child.proj", """ + + + + + + """).Path; + + string projectContents = $@" + + + + <{nameof(BuildProjectFileTask)} ProjectFile=""{childProject}"" Targets=""Build"" Properties=""Configuration=Release""> + + + +"; + + TransientTestProjectWithFiles project = env.CreateTestProjectWithFiles(projectContents); + ProjectInstance projectInstance = new(project.ProjectFile); + + var logger = new MockLogger(_output); + BuildResult buildResult = BuildManager.DefaultBuildManager.Build( + new BuildParameters { MaxNodeCount = 4, EnableNodeReuse = false, Loggers = [logger] }, + new BuildRequestData(projectInstance, targetsToBuild: ["Test"])); + + buildResult.OverallResult.ShouldBe(BuildResultCode.Success); + logger.FullLog.ShouldContain("Config=Release"); + } + + /// + /// Verifies BuildProjectFile returns ITaskItem[] target outputs through the TaskHost callback. + /// + [Fact] + public void BuildProjectFile_ReturnsTargetOutputs() + { + using TestEnvironment env = TestEnvironment.Create(_output); + env.SetEnvironmentVariable("MSBUILDENABLETASKHOSTCALLBACKS", "1"); + + string childProject = env.CreateFile("Child.proj", """ + + + + Value1 + + + + + + + + """).Path; + + string projectContents = $@" + + + + <{nameof(BuildProjectFileTask)} ProjectFile=""{childProject}"" Targets=""GetOutputs""> + + + + Count())"" Importance=""high"" /> + +"; + + TransientTestProjectWithFiles project = env.CreateTestProjectWithFiles(projectContents); + ProjectInstance projectInstance = new(project.ProjectFile); + + var logger = new MockLogger(_output); + BuildResult buildResult = BuildManager.DefaultBuildManager.Build( + new BuildParameters { MaxNodeCount = 4, EnableNodeReuse = false, Loggers = [logger] }, + new BuildRequestData(projectInstance, targetsToBuild: ["Test"])); + + buildResult.OverallResult.ShouldBe(BuildResultCode.Success); + bool.Parse(projectInstance.GetPropertyValue("Result")).ShouldBeTrue(); + logger.FullLog.ShouldContain("OutputItemCount=2"); + } + + /// + /// Verifies BuildProjectFile returns false when the child project fails. + /// + [Fact] + public void BuildProjectFile_ChildFailure_ReturnsFalse() + { + using TestEnvironment env = TestEnvironment.Create(_output); + env.SetEnvironmentVariable("MSBUILDENABLETASKHOSTCALLBACKS", "1"); + + string childProject = env.CreateFile("Child.proj", """ + + + + + + """).Path; + + string projectContents = $@" + + + + <{nameof(BuildProjectFileTask)} ProjectFile=""{childProject}"" Targets=""Build""> + + + + +"; + + TransientTestProjectWithFiles project = env.CreateTestProjectWithFiles(projectContents); + ProjectInstance projectInstance = new(project.ProjectFile); + + var logger = new MockLogger(_output); + BuildResult buildResult = BuildManager.DefaultBuildManager.Build( + new BuildParameters { MaxNodeCount = 4, EnableNodeReuse = false, Loggers = [logger] }, + new BuildRequestData(projectInstance, targetsToBuild: ["Test"])); + + buildResult.OverallResult.ShouldBe(BuildResultCode.Success); + logger.FullLog.ShouldContain("ChildResult=False"); + } + + /// + /// Verifies BuildProjectFile auto-ejection works in multithreaded mode. + /// + [Fact] + public void BuildProjectFile_WorksWhenAutoEjectedInMultiThreadedMode() + { + using TestEnvironment env = TestEnvironment.Create(_output); + env.SetEnvironmentVariable("MSBUILDENABLETASKHOSTCALLBACKS", "1"); + string testDir = env.CreateFolder().Path; + + string childProject = Path.Combine(testDir, "Child.proj"); + File.WriteAllText(childProject, """ + + + + + + """); + + string projectContents = $@" + + + + <{nameof(BuildProjectFileTask)} ProjectFile=""{childProject}"" Targets=""Build""> + + + +"; + + string projectFile = Path.Combine(testDir, "Test.proj"); + File.WriteAllText(projectFile, projectContents); + + var logger = new MockLogger(_output); + BuildResult buildResult = BuildManager.DefaultBuildManager.Build( + new BuildParameters + { + MultiThreaded = true, + MaxNodeCount = 4, + Loggers = [logger], + EnableNodeReuse = false + }, + new BuildRequestData(projectFile, new Dictionary(), null, ["Test"], null)); + + buildResult.OverallResult.ShouldBe(BuildResultCode.Success); + logger.FullLog.ShouldContain("external task host"); + logger.FullLog.ShouldContain("ChildBuiltInMT"); + } + + /// + /// Verifies that BuildProjectFile when callbacks are disabled logs error MSB5022. + /// + [Fact] + public void BuildProjectFile_LogsErrorWhenCallbacksNotSupported() + { + using TestEnvironment env = TestEnvironment.Create(_output); + + string childProject = env.CreateFile("Child.proj", """ + + + + + + """).Path; + + // Explicitly do NOT set MSBUILDENABLETASKHOSTCALLBACKS + string projectContents = $@" + + + + <{nameof(BuildProjectFileTask)} ProjectFile=""{childProject}"" Targets=""Build""> + + + +"; + + TransientTestProjectWithFiles project = env.CreateTestProjectWithFiles(projectContents); + ProjectInstance projectInstance = new(project.ProjectFile); + + var logger = new MockLogger(_output); + BuildResult buildResult = BuildManager.DefaultBuildManager.Build( + new BuildParameters { MaxNodeCount = 4, EnableNodeReuse = false, Loggers = [logger] }, + new BuildRequestData(projectInstance, targetsToBuild: ["Test"])); + + // MSB5022 error should be logged + logger.ErrorCount.ShouldBeGreaterThan(0); + logger.FullLog.ShouldContain("MSB5022"); + // Child should not have been built + logger.FullLog.ShouldNotContain("ShouldNotRun"); + } } } diff --git a/src/Build.UnitTests/NetTaskHost_E2E_Tests.cs b/src/Build.UnitTests/NetTaskHost_E2E_Tests.cs index 70a8d38733d..21bfb21be03 100644 --- a/src/Build.UnitTests/NetTaskHost_E2E_Tests.cs +++ b/src/Build.UnitTests/NetTaskHost_E2E_Tests.cs @@ -227,6 +227,29 @@ public void NetTaskHost_CallbackRequestCoresTest() testTaskOutput.ShouldContain("CallbackResult: RequestCores(2) ="); } + [WindowsFullFrameworkOnlyFact] + public void NetTaskHost_CallbackBuildProjectFileTest() + { + using TestEnvironment env = TestEnvironment.Create(_output); + env.SetEnvironmentVariable("MSBUILDENABLETASKHOSTCALLBACKS", "1"); + + var coreDirectory = Path.Combine(RunnerUtilities.BootstrapRootPath, "core"); + env.SetEnvironmentVariable("DOTNET_MSBUILD_SDK_RESOLVER_CLI_DIR", coreDirectory); + + string testProjectPath = Path.Combine(TestAssetsRootPath, "ExampleNetTask", "TestNetTaskBuildCallback", "TestNetTaskBuildCallback.csproj"); + + string testTaskOutput = RunnerUtilities.ExecBootstrapedMSBuild($"{testProjectPath} -v:n -p:LatestDotNetCoreForMSBuild={RunnerUtilities.LatestDotNetCoreForMSBuild} -t:TestTask", out bool successTestTask); + + if (!successTestTask) + { + _output.WriteLine(testTaskOutput); + } + + successTestTask.ShouldBeTrue(); + testTaskOutput.ShouldContain("CallbackResult: BuildProjectFile = True"); + testTaskOutput.ShouldContain("ChildProject: GetOutputs target executed"); + } + [WindowsFullFrameworkOnlyFact] // This test verifies the fallback behavior with implicit host parameters. public void NetTaskWithImplicitHostParamsTest_FallbackToDotnet() { From b42f8809fb793d9e16222a6d3a095799e2839de1 Mon Sep 17 00:00:00 2001 From: Jan Provaznik Date: Mon, 9 Mar 2026 20:12:06 +0100 Subject: [PATCH 06/13] Add defensive null check for TargetOutputsPerProject in 7-param overload Guard against null TargetOutputsPerProject in the result-copy loop. BuildEngineResult defensively converts null to empty list, but the explicit check makes intent clearer and prevents potential NRE if that invariant ever changes. Found via multi-model code review (Gemini). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- src/MSBuild/OutOfProcTaskHostNode.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/MSBuild/OutOfProcTaskHostNode.cs b/src/MSBuild/OutOfProcTaskHostNode.cs index a0f7bac55d3..00ebacc4245 100644 --- a/src/MSBuild/OutOfProcTaskHostNode.cs +++ b/src/MSBuild/OutOfProcTaskHostNode.cs @@ -451,7 +451,7 @@ public bool BuildProjectFilesInParallel(string[] projectFileNames, string[] targ BuildEngineResult result = BuildProjectFilesInParallel(projectFileNames, targetNames, globalProperties, new List[projectFileNames.Length], toolsVersion, includeTargetOutputs); - if (includeTargetOutputs) + if (includeTargetOutputs && result.TargetOutputsPerProject is not null) { for (int i = 0; i < targetOutputsPerProject.Length && i < result.TargetOutputsPerProject.Count; i++) { From 1d520915367af26740b23fcbcdb7a5133ed36cbc Mon Sep 17 00:00:00 2001 From: Jan Provaznik Date: Tue, 10 Mar 2026 11:04:33 +0100 Subject: [PATCH 07/13] Fix nullable warnings in TaskHostBuildRequest serialization test Use properly typed nullable arrays and null-forgiving operators to satisfy warnings-as-errors in full build. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../BackEnd/TaskHostCallbackPacket_Tests.cs | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/src/Build.UnitTests/BackEnd/TaskHostCallbackPacket_Tests.cs b/src/Build.UnitTests/BackEnd/TaskHostCallbackPacket_Tests.cs index de49f06041a..209e2cc1cc9 100644 --- a/src/Build.UnitTests/BackEnd/TaskHostCallbackPacket_Tests.cs +++ b/src/Build.UnitTests/BackEnd/TaskHostCallbackPacket_Tests.cs @@ -92,14 +92,15 @@ public void TaskHostCoresResponse_RoundTrip_Serialization(int grantedCores) [Fact] public void TaskHostBuildRequest_RoundTrip_Serialization() { - var globalProps = new Dictionary[] { new(StringComparer.OrdinalIgnoreCase) { ["Configuration"] = "Release" }, null }; - var removeProps = new List[] { new() { "Platform" }, null }; + Dictionary?[] globalProps = [new(StringComparer.OrdinalIgnoreCase) { ["Configuration"] = "Release" }, null]; + List?[] removeProps = [new() { "Platform" }, null]; + string?[] toolsVersions = ["17.0", null]; var request = new TaskHostBuildRequest( ["proj1.csproj", "proj2.csproj"], ["Build", "Test"], globalProps, removeProps, - ["17.0", null], + toolsVersions!, returnTargetOutputs: true); request.RequestId = 55; @@ -113,13 +114,13 @@ public void TaskHostBuildRequest_RoundTrip_Serialization() deserialized.Type.ShouldBe(NodePacketType.TaskHostBuildRequest); deserialized.ProjectFileNames.ShouldBe(["proj1.csproj", "proj2.csproj"]); deserialized.TargetNames.ShouldBe(["Build", "Test"]); - deserialized.ToolsVersions.ShouldBe(["17.0", null]); + deserialized.ToolsVersions.ShouldBe(toolsVersions!); deserialized.ReturnTargetOutputs.ShouldBeTrue(); - deserialized.GlobalProperties.Length.ShouldBe(2); - deserialized.GlobalProperties[0]["Configuration"].ShouldBe("Release"); + deserialized.GlobalProperties!.Length.ShouldBe(2); + deserialized.GlobalProperties![0]!["Configuration"].ShouldBe("Release"); deserialized.GlobalProperties[1].ShouldBeNull(); - deserialized.RemoveGlobalProperties.Length.ShouldBe(2); - deserialized.RemoveGlobalProperties[0].ShouldBe(["Platform"]); + deserialized.RemoveGlobalProperties!.Length.ShouldBe(2); + deserialized.RemoveGlobalProperties![0].ShouldBe(["Platform"]); deserialized.RemoveGlobalProperties[1].ShouldBeNull(); } From 623a7f86d857490de569514035a83f17ca24aeda Mon Sep 17 00:00:00 2001 From: Jan Provaznik Date: Tue, 10 Mar 2026 12:54:41 +0100 Subject: [PATCH 08/13] Add TaskHostYieldRequest/Response packets for Yield/Reacquire forwarding Add packet pair (0x26/0x27) to forward IBuildEngine3.Yield() and Reacquire() calls from OOP TaskHost to the owning worker node. TaskHostYieldRequest carries a YieldOperation enum (Yield or Reacquire). TaskHostYieldResponse is a simple acknowledgment for Reacquire (Yield is fire-and-forget). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- src/Build/Microsoft.Build.csproj | 2 + src/MSBuild/MSBuild.csproj | 2 + src/Shared/TaskHostYieldRequest.cs | 71 +++++++++++++++++++++++++++++ src/Shared/TaskHostYieldResponse.cs | 43 +++++++++++++++++ 4 files changed, 118 insertions(+) create mode 100644 src/Shared/TaskHostYieldRequest.cs create mode 100644 src/Shared/TaskHostYieldResponse.cs diff --git a/src/Build/Microsoft.Build.csproj b/src/Build/Microsoft.Build.csproj index 8772bae1957..0f3a19db581 100644 --- a/src/Build/Microsoft.Build.csproj +++ b/src/Build/Microsoft.Build.csproj @@ -129,6 +129,8 @@ + + diff --git a/src/MSBuild/MSBuild.csproj b/src/MSBuild/MSBuild.csproj index cef3894fa5e..6be3b42e468 100644 --- a/src/MSBuild/MSBuild.csproj +++ b/src/MSBuild/MSBuild.csproj @@ -120,6 +120,8 @@ + + diff --git a/src/Shared/TaskHostYieldRequest.cs b/src/Shared/TaskHostYieldRequest.cs new file mode 100644 index 00000000000..0b4b01b4469 --- /dev/null +++ b/src/Shared/TaskHostYieldRequest.cs @@ -0,0 +1,71 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +namespace Microsoft.Build.BackEnd +{ + /// + /// Specifies which yield operation is being requested. + /// + internal enum YieldOperation : byte + { + /// + /// The task is yielding the node, allowing other work to be scheduled. + /// Fire-and-forget: no response is sent. + /// + Yield = 0, + + /// + /// The task is reacquiring the node after a yield. + /// Blocking: the TaskHost waits for a before continuing. + /// + Reacquire = 1, + } + + /// + /// Packet sent from TaskHost to owning worker node for Yield/Reacquire operations. + /// + /// Yield is fire-and-forget (no response expected). + /// Reacquire blocks until a is received. + /// + /// + internal class TaskHostYieldRequest : INodePacket, ITaskHostCallbackPacket + { + private int _requestId; + private YieldOperation _operation; + + public TaskHostYieldRequest() + { + } + + public TaskHostYieldRequest(YieldOperation operation) + { + _operation = operation; + } + + public NodePacketType Type => NodePacketType.TaskHostYieldRequest; + + public int RequestId + { + get => _requestId; + set => _requestId = value; + } + + public YieldOperation Operation => _operation; + + public void Translate(ITranslator translator) + { + translator.Translate(ref _requestId); + + byte op = (byte)_operation; + translator.Translate(ref op); + _operation = (YieldOperation)op; + } + + internal static INodePacket FactoryForDeserialization(ITranslator translator) + { + var packet = new TaskHostYieldRequest(); + packet.Translate(translator); + return packet; + } + } +} diff --git a/src/Shared/TaskHostYieldResponse.cs b/src/Shared/TaskHostYieldResponse.cs new file mode 100644 index 00000000000..d4e6a4871fe --- /dev/null +++ b/src/Shared/TaskHostYieldResponse.cs @@ -0,0 +1,43 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +namespace Microsoft.Build.BackEnd +{ + /// + /// Response from worker node to TaskHost acknowledging a Reacquire request. + /// Sent only for ; Yield is fire-and-forget. + /// + internal class TaskHostYieldResponse : INodePacket, ITaskHostCallbackPacket + { + private int _requestId; + + public TaskHostYieldResponse() + { + } + + public TaskHostYieldResponse(int requestId) + { + _requestId = requestId; + } + + public NodePacketType Type => NodePacketType.TaskHostYieldResponse; + + public int RequestId + { + get => _requestId; + set => _requestId = value; + } + + public void Translate(ITranslator translator) + { + translator.Translate(ref _requestId); + } + + internal static INodePacket FactoryForDeserialization(ITranslator translator) + { + var packet = new TaskHostYieldResponse(); + packet.Translate(translator); + return packet; + } + } +} From 456531c622e93f4f99d0174e9a26dd3a1288230a Mon Sep 17 00:00:00 2001 From: Jan Provaznik Date: Tue, 10 Mar 2026 12:54:57 +0100 Subject: [PATCH 09/13] Implement yield-for-callback pattern and Yield/Reacquire forwarding MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit OOP TaskHost side (OutOfProcTaskHostNode): - Add _yieldedTaskCount for tracking yielded state during callbacks - YieldForCallback/ReacquireAfterCallback bracket BuildProjectFile calls - Real Yield() sends TaskHostYieldRequest(Yield) — fire-and-forget - Real Reacquire() sends TaskHostYieldRequest(Reacquire) — blocking - HandleTaskHostConfiguration allows new config when task is yielded (_isTaskExecuting && _yieldedTaskCount > 0) Worker side (TaskHostTask): - HandleYieldRequest forwards Yield as fire-and-forget to engine3.Yield() - HandleYieldRequest forwards Reacquire with blocking engine3.Reacquire() then sends TaskHostYieldResponse acknowledgment Note: Same-process TaskHost reuse (child task running on the same OOP process as the yielded parent) requires additional changes to NodeProviderOutOfProcTaskHost connection management. Currently, nested TaskHostFactory tasks spawn separate OOP processes because the parent TaskHostTask holds its connection via a shared TaskHostNodeKey. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../Instance/TaskFactories/TaskHostTask.cs | 37 ++++++++ src/MSBuild/OutOfProcTaskHostNode.cs | 84 +++++++++++++++++-- 2 files changed, 113 insertions(+), 8 deletions(-) diff --git a/src/Build/Instance/TaskFactories/TaskHostTask.cs b/src/Build/Instance/TaskFactories/TaskHostTask.cs index 895b196c910..3791fc529f6 100644 --- a/src/Build/Instance/TaskFactories/TaskHostTask.cs +++ b/src/Build/Instance/TaskFactories/TaskHostTask.cs @@ -208,6 +208,7 @@ public TaskHostTask( (this as INodePacketFactory).RegisterPacketHandler(NodePacketType.TaskHostIsRunningMultipleNodesRequest, TaskHostIsRunningMultipleNodesRequest.FactoryForDeserialization, this); (this as INodePacketFactory).RegisterPacketHandler(NodePacketType.TaskHostCoresRequest, TaskHostCoresRequest.FactoryForDeserialization, this); (this as INodePacketFactory).RegisterPacketHandler(NodePacketType.TaskHostBuildRequest, TaskHostBuildRequest.FactoryForDeserialization, this); + (this as INodePacketFactory).RegisterPacketHandler(NodePacketType.TaskHostYieldRequest, TaskHostYieldRequest.FactoryForDeserialization, this); _packetReceivedEvent = new AutoResetEvent(false); _receivedPackets = new ConcurrentQueue(); @@ -523,6 +524,9 @@ private void HandlePacket(INodePacket packet, out bool taskFinished) case NodePacketType.TaskHostBuildRequest: HandleBuildRequest(packet as TaskHostBuildRequest); break; + case NodePacketType.TaskHostYieldRequest: + HandleYieldRequest(packet as TaskHostYieldRequest); + break; default: ErrorUtilities.ThrowInternalErrorUnreachable(); break; @@ -758,6 +762,39 @@ private void HandleBuildRequest(TaskHostBuildRequest request) _taskHostProvider.SendData(_taskHostNodeKey, response); } + /// + /// Handles Yield/Reacquire requests from the TaskHost. + /// + /// Yield is fire-and-forget: forwards to , no response sent. + /// Reacquire is blocking: forwards to (which may block + /// on the scheduler), then sends to unblock the TaskHost. + /// + /// + private void HandleYieldRequest(TaskHostYieldRequest request) + { + switch (request.Operation) + { + case YieldOperation.Yield: + if (_buildEngine is IBuildEngine3 engine3Yield) + { + engine3Yield.Yield(); + } + + // No response — Yield is fire-and-forget. + break; + + case YieldOperation.Reacquire: + if (_buildEngine is IBuildEngine3 engine3Reacquire) + { + engine3Reacquire.Reacquire(); + } + + // Send acknowledgment to unblock the TaskHost. + _taskHostProvider.SendData(_taskHostNodeKey, new TaskHostYieldResponse(request.RequestId)); + break; + } + } + /// /// Since we log that we weren't able to connect to the task host in a couple of different places, /// extract it out into a separate method. diff --git a/src/MSBuild/OutOfProcTaskHostNode.cs b/src/MSBuild/OutOfProcTaskHostNode.cs index 00ebacc4245..14b0d22b76c 100644 --- a/src/MSBuild/OutOfProcTaskHostNode.cs +++ b/src/MSBuild/OutOfProcTaskHostNode.cs @@ -118,6 +118,14 @@ internal class OutOfProcTaskHostNode : /// private bool _isTaskExecuting; +#if !CLR2COMPATIBILITY + /// + /// Number of tasks currently yielded (blocked on a BuildProjectFile callback or explicit Yield). + /// A yielded task does NOT prevent new tasks from being scheduled to this TaskHost. + /// + private int _yieldedTaskCount; +#endif + /// /// The event which is set when a task has completed. /// @@ -248,6 +256,7 @@ public OutOfProcTaskHostNode() thisINodePacketFactory.RegisterPacketHandler(NodePacketType.TaskHostIsRunningMultipleNodesResponse, TaskHostIsRunningMultipleNodesResponse.FactoryForDeserialization, this); thisINodePacketFactory.RegisterPacketHandler(NodePacketType.TaskHostCoresResponse, TaskHostCoresResponse.FactoryForDeserialization, this); thisINodePacketFactory.RegisterPacketHandler(NodePacketType.TaskHostBuildResponse, TaskHostBuildResponse.FactoryForDeserialization, this); + thisINodePacketFactory.RegisterPacketHandler(NodePacketType.TaskHostYieldResponse, TaskHostYieldResponse.FactoryForDeserialization, this); #endif #if !CLR2COMPATIBILITY @@ -496,27 +505,59 @@ public BuildEngineResult BuildProjectFilesInParallel(string[] projectFileNames, toolsVersion, returnTargetOutputs); - var response = SendCallbackRequestAndWaitForResponse(request); - return response.ToBuildEngineResult(); + // Yield before the callback so the scheduler can reuse this TaskHost for nested tasks. + YieldForCallback(); + try + { + var response = SendCallbackRequestAndWaitForResponse(request); + return response.ToBuildEngineResult(); + } + finally + { + ReacquireAfterCallback(); + } #endif } /// - /// Stub implementation of IBuildEngine3.Yield. The task host does not support yielding, so just go ahead and silently - /// return, letting the task continue. + /// Implementation of IBuildEngine3.Yield. Forwards to the owning worker node + /// so the scheduler can assign other work to this node while the task waits. /// public void Yield() { - return; +#if !CLR2COMPATIBILITY + if (!CallbacksSupported) + { + return; + } + + YieldForCallback(); + + // Fire-and-forget: send yield notification to parent, no response expected. + var request = new TaskHostYieldRequest(YieldOperation.Yield); + _nodeEndpoint.SendData(request); +#endif } /// - /// Stub implementation of IBuildEngine3.Reacquire. The task host does not support yielding, so just go ahead and silently - /// return, letting the task continue. + /// Implementation of IBuildEngine3.Reacquire. Sends a reacquire request to the owning worker node + /// and blocks until the scheduler allows the task to continue. /// public void Reacquire() { - return; +#if !CLR2COMPATIBILITY + if (!CallbacksSupported) + { + return; + } + + // Send reacquire request and wait for response. + // The worker side calls engine3.Reacquire() which may block on the scheduler. + var request = new TaskHostYieldRequest(YieldOperation.Reacquire); + SendCallbackRequestAndWaitForResponse(request); + + ReacquireAfterCallback(); +#endif } #endregion // IBuildEngine3 Implementation @@ -856,6 +897,7 @@ private void HandlePacket(INodePacket packet) case NodePacketType.TaskHostIsRunningMultipleNodesResponse: case NodePacketType.TaskHostCoresResponse: case NodePacketType.TaskHostBuildResponse: + case NodePacketType.TaskHostYieldResponse: HandleCallbackResponse(packet); break; #endif @@ -941,6 +983,23 @@ private TResponse SendCallbackRequestAndWaitForResponse(ITaskHostCall _pendingCallbackRequests.TryRemove(requestId, out _); } } + + /// + /// Marks this TaskHost as yielded so the scheduler can reuse it for nested tasks. + /// Called before sending a BuildProjectFile callback or explicit Yield. + /// + private void YieldForCallback() + { + Interlocked.Increment(ref _yieldedTaskCount); + } + + /// + /// Marks this TaskHost as active again after a callback or Reacquire completes. + /// + private void ReacquireAfterCallback() + { + Interlocked.Decrement(ref _yieldedTaskCount); + } #endif /// @@ -949,7 +1008,16 @@ private TResponse SendCallbackRequestAndWaitForResponse(ITaskHostCall /// private void HandleTaskHostConfiguration(TaskHostConfiguration taskHostConfiguration) { +#if !CLR2COMPATIBILITY + // Allow new configuration when the existing task is yielded (blocked on BuildProjectFile callback + // or explicit Yield). This enables nested build scenarios where the child build's tasks can run + // on the same TaskHost process. + ErrorUtilities.VerifyThrow(!_isTaskExecuting || _yieldedTaskCount > 0, + "Why are we getting a TaskHostConfiguration packet while a task is actively executing? yieldedTaskCount={0}", + _yieldedTaskCount); +#else ErrorUtilities.VerifyThrow(!_isTaskExecuting, "Why are we getting a TaskHostConfiguration packet while we're still executing a task?"); +#endif _currentConfiguration = taskHostConfiguration; // Kick off the task running thread. From 3398df3fb507f7dd857071bd13db9f74463d2d71 Mon Sep 17 00:00:00 2001 From: Jan Provaznik Date: Tue, 10 Mar 2026 12:55:07 +0100 Subject: [PATCH 10/13] Add yield packet serialization tests Add Theory tests for TaskHostYieldRequest round-trip serialization (both Yield and Reacquire operations) and a Fact test for TaskHostYieldResponse round-trip serialization. Use byte parameter with cast to avoid CS0051 (internal YieldOperation enum used as public method parameter). Also remove blank line before closing brace (SA1508) in TaskHostCallback_Tests. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../BackEnd/TaskHostCallbackPacket_Tests.cs | 35 +++++++++++++++++++ 1 file changed, 35 insertions(+) diff --git a/src/Build.UnitTests/BackEnd/TaskHostCallbackPacket_Tests.cs b/src/Build.UnitTests/BackEnd/TaskHostCallbackPacket_Tests.cs index 209e2cc1cc9..0fbc2ce4496 100644 --- a/src/Build.UnitTests/BackEnd/TaskHostCallbackPacket_Tests.cs +++ b/src/Build.UnitTests/BackEnd/TaskHostCallbackPacket_Tests.cs @@ -198,5 +198,40 @@ public void TaskHostBuildResponse_Failure_NoOutputs_RoundTrip_Serialization() var buildEngineResult = deserialized.ToBuildEngineResult(); buildEngineResult.Result.ShouldBeFalse(); } + + [Theory] + [InlineData((byte)YieldOperation.Yield)] + [InlineData((byte)YieldOperation.Reacquire)] + public void TaskHostYieldRequest_RoundTrip_Serialization(byte operationByte) + { + YieldOperation operation = (YieldOperation)operationByte; + var request = new TaskHostYieldRequest(operation); + request.RequestId = 77; + + ITranslator writeTranslator = TranslationHelpers.GetWriteTranslator(); + request.Translate(writeTranslator); + + ITranslator readTranslator = TranslationHelpers.GetReadTranslator(); + var deserialized = (TaskHostYieldRequest)TaskHostYieldRequest.FactoryForDeserialization(readTranslator); + + deserialized.RequestId.ShouldBe(77); + deserialized.Operation.ShouldBe(operation); + deserialized.Type.ShouldBe(NodePacketType.TaskHostYieldRequest); + } + + [Fact] + public void TaskHostYieldResponse_RoundTrip_Serialization() + { + var response = new TaskHostYieldResponse(42); + + ITranslator writeTranslator = TranslationHelpers.GetWriteTranslator(); + response.Translate(writeTranslator); + + ITranslator readTranslator = TranslationHelpers.GetReadTranslator(); + var deserialized = (TaskHostYieldResponse)TaskHostYieldResponse.FactoryForDeserialization(readTranslator); + + deserialized.RequestId.ShouldBe(42); + deserialized.Type.ShouldBe(NodePacketType.TaskHostYieldResponse); + } } } From 44f445a6e886a9df9c3a665d526cdba40af0667a Mon Sep 17 00:00:00 2001 From: Jan Provaznik Date: Tue, 10 Mar 2026 14:24:57 +0100 Subject: [PATCH 11/13] Add TaskHost process reuse for nested BuildProjectFile callbacks Implement handler stack pattern and concurrent task execution support in the OOP TaskHost, enabling a yielded task to allow the scheduler to reuse the same TaskHost process for nested builds. Key changes: - NodeProviderOutOfProcTaskHost: Replace _nodeIdToPacketHandler with _nodeIdToPacketHandlerStack (Stack) for push/pop of handlers when tasks yield and new tasks arrive on same process. - OutOfProcTaskHostNode: Replace _isTaskExecuting with _activeTaskCount and _yieldedTaskCount for concurrent task tracking. - TaskExecutionContext: Per-task state (config, environment, pending callbacks) stored in ConcurrentDictionary + AsyncLocal for thread isolation. - EffectiveConfiguration property: Uses per-task context first, falling back to global _currentConfiguration. Fixes NRE when a nested task's CompleteTask clears _currentConfiguration while the yielded parent task still needs it for logging. - YieldForCallback/ReacquireAfterCallback: Save/restore operating environment (current directory, env vars) around callbacks. - E2E test verifying parent and child tasks report same PID when using TaskHostFactory with same Runtime. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../BuildProjectFileAndReportPidTask.cs | 73 +++++ src/Build.UnitTests/NetTaskHost_E2E_Tests.cs | 34 ++ .../ExampleTask/ExampleTask.csproj | 1 + .../TestNetTaskHostReuse/ChildProject.proj | 15 + .../TestNetTaskHostReuse.csproj | 34 ++ .../TestNetTaskHostReuse/global.json | 7 + .../NodeProviderOutOfProcTaskHost.cs | 133 +++++--- src/MSBuild/MSBuild.csproj | 1 + src/MSBuild/OutOfProcTaskHostNode.cs | 298 +++++++++++++++--- src/MSBuild/TaskExecutionContext.cs | 120 +++++++ 10 files changed, 638 insertions(+), 78 deletions(-) create mode 100644 src/Build.UnitTests/BackEnd/BuildProjectFileAndReportPidTask.cs create mode 100644 src/Build.UnitTests/TestAssets/ExampleNetTask/TestNetTaskHostReuse/ChildProject.proj create mode 100644 src/Build.UnitTests/TestAssets/ExampleNetTask/TestNetTaskHostReuse/TestNetTaskHostReuse.csproj create mode 100644 src/Build.UnitTests/TestAssets/ExampleNetTask/TestNetTaskHostReuse/global.json create mode 100644 src/MSBuild/TaskExecutionContext.cs diff --git a/src/Build.UnitTests/BackEnd/BuildProjectFileAndReportPidTask.cs b/src/Build.UnitTests/BackEnd/BuildProjectFileAndReportPidTask.cs new file mode 100644 index 00000000000..6e8dceabf31 --- /dev/null +++ b/src/Build.UnitTests/BackEnd/BuildProjectFileAndReportPidTask.cs @@ -0,0 +1,73 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System.Collections; +using System.Diagnostics; +using Microsoft.Build.Framework; +using Microsoft.Build.Utilities; + +namespace Microsoft.Build.UnitTests.BackEnd +{ + /// + /// A test task that reports its process ID and optionally calls BuildProjectFile + /// on a child project. Used by TaskHost reuse E2E tests to verify that a parent + /// task and a nested child task run in the same OOP TaskHost process. + /// + public class BuildProjectFileAndReportPidTask : Task + { + /// + /// Path to the project file to build. If empty, just reports PID without building. + /// + public string ProjectFile { get; set; } = string.Empty; + + /// + /// Semicolon-separated list of Property=Value pairs to pass as global properties. + /// + public string Properties { get; set; } = string.Empty; + + /// + /// The process ID of the TaskHost running this task. + /// + [Output] + public int ProcessId { get; set; } + + /// + /// Whether the child build succeeded (only meaningful when ProjectFile is set). + /// + [Output] + public bool BuildSucceeded { get; set; } + + public override bool Execute() + { + ProcessId = Process.GetCurrentProcess().Id; + Log.LogMessage(MessageImportance.High, $"TASKHOST_PID={ProcessId}"); + + if (!string.IsNullOrEmpty(ProjectFile)) + { + Hashtable globalProperties = null; + if (!string.IsNullOrEmpty(Properties)) + { + globalProperties = new Hashtable(); + foreach (string pair in Properties.Split(';')) + { + string[] parts = pair.Split(new[] { '=' }, 2); + if (parts.Length == 2) + { + globalProperties[parts[0].Trim()] = parts[1].Trim(); + } + } + } + + Hashtable targetOutputs = new(); + BuildSucceeded = BuildEngine.BuildProjectFile(ProjectFile, null, globalProperties, targetOutputs); + Log.LogMessage(MessageImportance.High, $"BuildProjectFile({ProjectFile}) = {BuildSucceeded}"); + } + else + { + BuildSucceeded = true; + } + + return true; + } + } +} diff --git a/src/Build.UnitTests/NetTaskHost_E2E_Tests.cs b/src/Build.UnitTests/NetTaskHost_E2E_Tests.cs index 21bfb21be03..0e03bdc99a8 100644 --- a/src/Build.UnitTests/NetTaskHost_E2E_Tests.cs +++ b/src/Build.UnitTests/NetTaskHost_E2E_Tests.cs @@ -297,5 +297,39 @@ public void NetTaskWithImplicitHostParamsTest_AppHost() testTaskOutput.ShouldContain("The task is executed in process: MSBuild"); testTaskOutput.ShouldContain("/nodereuse:True"); } + [WindowsFullFrameworkOnlyFact] + public void NetTaskHost_TaskHostProcessReuse_SameProcessForNestedBuild() + { + using TestEnvironment env = TestEnvironment.Create(_output); + env.SetEnvironmentVariable("MSBUILDENABLETASKHOSTCALLBACKS", "1"); + + var coreDirectory = Path.Combine(RunnerUtilities.BootstrapRootPath, "core"); + env.SetEnvironmentVariable("DOTNET_MSBUILD_SDK_RESOLVER_CLI_DIR", coreDirectory); + + string testProjectPath = Path.Combine(TestAssetsRootPath, "ExampleNetTask", "TestNetTaskHostReuse", "TestNetTaskHostReuse.csproj"); + + string testTaskOutput = RunnerUtilities.ExecBootstrapedMSBuild($"{testProjectPath} -v:n -p:LatestDotNetCoreForMSBuild={RunnerUtilities.LatestDotNetCoreForMSBuild} -t:TestTaskHostReuse", out bool successTestTask); + + _output.WriteLine(testTaskOutput); + + successTestTask.ShouldBeTrue(); + + // Both the parent task and child task should report TASKHOST_PID + testTaskOutput.ShouldContain("TASKHOST_PID="); + testTaskOutput.ShouldContain("PARENT_TASKHOST_PID="); + testTaskOutput.ShouldContain("CHILD_TASKHOST_PID="); + + // Extract PIDs from output and verify they match (same process reused) + var parentPidMatch = System.Text.RegularExpressions.Regex.Match(testTaskOutput, @"PARENT_TASKHOST_PID=(\d+)"); + var childPidMatch = System.Text.RegularExpressions.Regex.Match(testTaskOutput, @"CHILD_TASKHOST_PID=(\d+)"); + + parentPidMatch.Success.ShouldBeTrue("Should find PARENT_TASKHOST_PID in output"); + childPidMatch.Success.ShouldBeTrue("Should find CHILD_TASKHOST_PID in output"); + + string parentPid = parentPidMatch.Groups[1].Value; + string childPid = childPidMatch.Groups[1].Value; + + parentPid.ShouldBe(childPid, $"Parent PID ({parentPid}) and child PID ({childPid}) should match — same TaskHost process should be reused for nested build"); + } } } diff --git a/src/Build.UnitTests/TestAssets/ExampleNetTask/ExampleTask/ExampleTask.csproj b/src/Build.UnitTests/TestAssets/ExampleNetTask/ExampleTask/ExampleTask.csproj index 633f102a57d..b6bdb6cdd24 100644 --- a/src/Build.UnitTests/TestAssets/ExampleNetTask/ExampleTask/ExampleTask.csproj +++ b/src/Build.UnitTests/TestAssets/ExampleNetTask/ExampleTask/ExampleTask.csproj @@ -18,6 +18,7 @@ + diff --git a/src/Build.UnitTests/TestAssets/ExampleNetTask/TestNetTaskHostReuse/ChildProject.proj b/src/Build.UnitTests/TestAssets/ExampleNetTask/TestNetTaskHostReuse/ChildProject.proj new file mode 100644 index 00000000000..3519828bd15 --- /dev/null +++ b/src/Build.UnitTests/TestAssets/ExampleNetTask/TestNetTaskHostReuse/ChildProject.proj @@ -0,0 +1,15 @@ + + + + + + + + + + + diff --git a/src/Build.UnitTests/TestAssets/ExampleNetTask/TestNetTaskHostReuse/TestNetTaskHostReuse.csproj b/src/Build.UnitTests/TestAssets/ExampleNetTask/TestNetTaskHostReuse/TestNetTaskHostReuse.csproj new file mode 100644 index 00000000000..512ab717b31 --- /dev/null +++ b/src/Build.UnitTests/TestAssets/ExampleNetTask/TestNetTaskHostReuse/TestNetTaskHostReuse.csproj @@ -0,0 +1,34 @@ + + + + $(LatestDotNetCoreForMSBuild) + + + + $([System.IO.Path]::GetFullPath('$([System.IO.Path]::Combine('$(MSBuildThisFileDirectory)', '..', '..', '..', '..'))')) + $([System.IO.Path]::Combine('$(TestProjectFolder)', '$(TargetFramework)', 'ExampleTask.dll')) + $([System.IO.Path]::Combine('$(MSBuildThisFileDirectory)', 'ChildProject.proj')) + + + + + + + + + + + + + + + + diff --git a/src/Build.UnitTests/TestAssets/ExampleNetTask/TestNetTaskHostReuse/global.json b/src/Build.UnitTests/TestAssets/ExampleNetTask/TestNetTaskHostReuse/global.json new file mode 100644 index 00000000000..cd9fb530953 --- /dev/null +++ b/src/Build.UnitTests/TestAssets/ExampleNetTask/TestNetTaskHostReuse/global.json @@ -0,0 +1,7 @@ +{ + "sdk": { + "version": "10.0.100", + "allowPrerelease": true, + "rollForward": "latestMajor" + } +} diff --git a/src/Build/BackEnd/Components/Communications/NodeProviderOutOfProcTaskHost.cs b/src/Build/BackEnd/Components/Communications/NodeProviderOutOfProcTaskHost.cs index a7987f124f7..2a05eef0bd3 100644 --- a/src/Build/BackEnd/Components/Communications/NodeProviderOutOfProcTaskHost.cs +++ b/src/Build/BackEnd/Components/Communications/NodeProviderOutOfProcTaskHost.cs @@ -102,11 +102,12 @@ internal class NodeProviderOutOfProcTaskHost : NodeProviderOutOfProcBase, INodeP /// /// A mapping of all of the INodePacketHandlers wrapped by this provider. + /// When multiple tasks use the same node (nested BuildProjectFile), handlers + /// are stacked. The most recent handler receives packets. When it disconnects, + /// the previous handler is restored. /// Keyed by the communication node ID (NodeContext.NodeId) for O(1) packet routing. - /// Thread-safe to support parallel taskhost creation in /mt mode where multiple thread nodes - /// can simultaneously create their own taskhosts. /// - private ConcurrentDictionary _nodeIdToPacketHandler; + private ConcurrentDictionary> _nodeIdToPacketHandlerStack; /// /// Keeps track of the set of node IDs for which we have not yet received shutdown notification. @@ -241,7 +242,7 @@ public void InitializeComponent(IBuildComponentHost host) _nodeContexts = new ConcurrentDictionary(); _nodeIdToNodeKey = new ConcurrentDictionary(); _nodeIdToPacketFactory = new ConcurrentDictionary(); - _nodeIdToPacketHandler = new ConcurrentDictionary(); + _nodeIdToPacketHandlerStack = new ConcurrentDictionary>(); _activeNodes = []; _nextNodeId = 0; @@ -251,6 +252,14 @@ public void InitializeComponent(IBuildComponentHost host) (this as INodePacketFactory).RegisterPacketHandler(NodePacketType.LogMessage, LogMessagePacket.FactoryForDeserialization, this); (this as INodePacketFactory).RegisterPacketHandler(NodePacketType.TaskHostTaskComplete, TaskHostTaskComplete.FactoryForDeserialization, this); (this as INodePacketFactory).RegisterPacketHandler(NodePacketType.NodeShutdown, NodeShutdown.FactoryForDeserialization, this); + + // Register callback request packet types so we can deserialize them when + // they arrive from TaskHost processes. These are forwarded to the current + // TaskHostTask handler via the handler stack. + (this as INodePacketFactory).RegisterPacketHandler(NodePacketType.TaskHostIsRunningMultipleNodesRequest, TaskHostIsRunningMultipleNodesRequest.FactoryForDeserialization, this); + (this as INodePacketFactory).RegisterPacketHandler(NodePacketType.TaskHostCoresRequest, TaskHostCoresRequest.FactoryForDeserialization, this); + (this as INodePacketFactory).RegisterPacketHandler(NodePacketType.TaskHostBuildRequest, TaskHostBuildRequest.FactoryForDeserialization, this); + (this as INodePacketFactory).RegisterPacketHandler(NodePacketType.TaskHostYieldRequest, TaskHostYieldRequest.FactoryForDeserialization, this); } /// @@ -286,20 +295,17 @@ public void UnregisterPacketHandler(NodePacketType packetType) /// /// Takes a serializer, deserializes the packet and routes it to the appropriate handler. + /// Always uses the local packet factory for deserialization, which routes through + /// our PacketReceived method (using the handler stack). /// /// The node from which the packet was received. /// The packet type. /// The translator containing the data from which the packet should be reconstructed. public void DeserializeAndRoutePacket(int nodeId, NodePacketType packetType, ITranslator translator) { - if (_nodeIdToPacketFactory.TryGetValue(nodeId, out INodePacketFactory nodePacketFactory)) - { - nodePacketFactory.DeserializeAndRoutePacket(nodeId, packetType, translator); - } - else - { - _localPacketFactory.DeserializeAndRoutePacket(nodeId, packetType, translator); - } + // Always route through our local factory which handles deserialization + // and routes to our PacketReceived, which uses the handler stack. + _localPacketFactory.DeserializeAndRoutePacket(nodeId, packetType, translator); } /// @@ -313,20 +319,13 @@ public INodePacket DeserializePacket(NodePacketType packetType, ITranslator tran } /// - /// Routes the specified packet + /// Routes the specified packet through our PacketReceived method (handler stack). /// /// The node from which the packet was received. /// The packet to route. public void RoutePacket(int nodeId, INodePacket packet) { - if (_nodeIdToPacketFactory.TryGetValue(nodeId, out INodePacketFactory nodePacketFactory)) - { - nodePacketFactory.RoutePacket(nodeId, packet); - } - else - { - _localPacketFactory.RoutePacket(nodeId, packet); - } + _localPacketFactory.RoutePacket(nodeId, packet); } #endregion @@ -341,27 +340,46 @@ public void RoutePacket(int nodeId, INodePacket packet) /// The packet. public void PacketReceived(int node, INodePacket packet) { - if (_nodeIdToPacketHandler.TryGetValue(node, out INodePacketHandler packetHandler)) + // Try to find the handler stack for this node and forward the packet. + // Lock on the stack to synchronize with DisconnectFromHost to prevent race conditions + // where we get the handler right before it's popped from the stack. + if (_nodeIdToPacketHandlerStack.TryGetValue(node, out Stack handlerStack)) { - packetHandler.PacketReceived(node, packet); - } - else - { - ErrorUtilities.VerifyThrow(packet.Type == NodePacketType.NodeShutdown, "We should only ever handle packets of type NodeShutdown -- everything else should only come in when there's an active task"); - - // May also be removed by unnatural termination, so don't assume it's there - lock (_activeNodes) + lock (handlerStack) { - if (_activeNodes.Contains(node)) + if (handlerStack.Count > 0) { - _activeNodes.Remove(node); + INodePacketHandler packetHandler = handlerStack.Peek(); + packetHandler.PacketReceived(node, packet); + return; } + } + } - if (_activeNodes.Count == 0) + // No handler on the stack. This can happen in nested BuildProjectFile scenarios + // where late-arriving packets arrive after the handler has disconnected. + switch (packet.Type) + { + case NodePacketType.NodeShutdown: + // Expected - node is shutting down and no handler needed + lock (_activeNodes) { - _noNodesActiveEvent.Set(); + if (_activeNodes.Contains(node)) + { + _activeNodes.Remove(node); + } + + if (_activeNodes.Count == 0) + { + _noNodesActiveEvent.Set(); + } } - } + + break; + default: + // Late-arriving packets (log messages, completion) after handler disconnected + // can be safely ignored. + break; } } @@ -613,9 +631,25 @@ internal bool AcquireAndSetUpHost( if (nodeCreationSucceeded) { NodeContext context = _nodeContexts[nodeKey]; - // Map the transport ID directly to the handlers for O(1) packet routing - _nodeIdToPacketFactory[context.NodeId] = factory; - _nodeIdToPacketHandler[context.NodeId] = handler; + + // Only register the factory for the first task on this node. + // For nested tasks (BuildProjectFile callbacks), we reuse the existing + // factory setup and just push a new handler onto the stack. + // This ensures all packets are routed through NodeProviderOutOfProcTaskHost.PacketReceived + // which uses the handler stack, rather than going directly to individual TaskHostTask handlers. + if (!_nodeIdToPacketFactory.ContainsKey(context.NodeId)) + { + _nodeIdToPacketFactory[context.NodeId] = this; + } + + // Push the new handler onto the stack. This supports nested tasks + // (e.g., BuildProjectFile callbacks) where multiple TaskHostTask instances + // share the same TaskHost process. + Stack handlerStack = _nodeIdToPacketHandlerStack.GetOrAdd(context.NodeId, _ => new Stack()); + lock (handlerStack) + { + handlerStack.Push(handler); + } // Configure the node. context.SendData(configuration); @@ -641,10 +675,27 @@ internal void DisconnectFromHost(TaskHostNodeKey nodeKey) return; } - bool successRemoveFactory = _nodeIdToPacketFactory.TryRemove(context.NodeId, out _); - bool successRemoveHandler = _nodeIdToPacketHandler.TryRemove(context.NodeId, out _); + int nodeId = context.NodeId; - ErrorUtilities.VerifyThrow(successRemoveFactory && successRemoveHandler, "Why are we trying to disconnect from a context that we already disconnected from? Did we call DisconnectFromHost twice?"); + // Pop the handler from the stack. If there are still handlers remaining, + // the previous handler becomes active again (supporting nested tasks). + if (_nodeIdToPacketHandlerStack.TryGetValue(nodeId, out Stack handlerStack)) + { + lock (handlerStack) + { + if (handlerStack.Count > 0) + { + handlerStack.Pop(); + } + + // Only fully disconnect when all handlers are done + if (handlerStack.Count == 0) + { + _nodeIdToPacketFactory.TryRemove(nodeId, out _); + _nodeIdToPacketHandlerStack.TryRemove(nodeId, out _); + } + } + } } /// diff --git a/src/MSBuild/MSBuild.csproj b/src/MSBuild/MSBuild.csproj index 6be3b42e468..723871cfc99 100644 --- a/src/MSBuild/MSBuild.csproj +++ b/src/MSBuild/MSBuild.csproj @@ -151,6 +151,7 @@ + diff --git a/src/MSBuild/OutOfProcTaskHostNode.cs b/src/MSBuild/OutOfProcTaskHostNode.cs index 14b0d22b76c..38e7b801e89 100644 --- a/src/MSBuild/OutOfProcTaskHostNode.cs +++ b/src/MSBuild/OutOfProcTaskHostNode.cs @@ -114,9 +114,11 @@ internal class OutOfProcTaskHostNode : private NodeEngineShutdownReason _shutdownReason; /// - /// We set this flag to track a currently executing task + /// Count of tasks that are actively executing (not yielded). + /// When a task yields, this decrements. When it reacquires, this increments. + /// Used to determine if we can accept new TaskHostConfiguration packets. /// - private bool _isTaskExecuting; + private int _activeTaskCount; #if !CLR2COMPATIBILITY /// @@ -204,9 +206,29 @@ internal class OutOfProcTaskHostNode : /// /// Pending callback requests awaiting responses from the owning worker node. /// Key is the request ID, value is the TaskCompletionSource to signal when response arrives. + /// This is the fallback for backward compatibility when task context is not available. /// private readonly ConcurrentDictionary> _pendingCallbackRequests = new(); + /// + /// All active task execution contexts, keyed by task ID. + /// Supports concurrent task execution when tasks yield or await callbacks. + /// + private readonly ConcurrentDictionary _taskContexts + = new ConcurrentDictionary(); + + /// + /// The currently active task context for the calling thread. + /// Uses AsyncLocal to support concurrent task threads with proper context isolation. + /// + private readonly AsyncLocal _currentTaskContext + = new AsyncLocal(); + + /// + /// Counter for generating task IDs when configuration doesn't provide one. + /// + private int _nextLocalTaskId; + /// /// The packet version negotiated with the owning worker node. /// Used to determine if the worker node supports callback packets. @@ -226,6 +248,18 @@ internal class OutOfProcTaskHostNode : private bool CallbacksSupported => _parentPacketVersion >= CallbacksMinPacketVersion || Traits.Instance.EnableTaskHostCallbacks; #endif + /// + /// Gets the effective configuration for the current task thread. + /// In concurrent-task mode (non-CLR2), uses the per-task context first. + /// Falls back to . + /// + private TaskHostConfiguration EffectiveConfiguration => +#if !CLR2COMPATIBILITY + GetCurrentConfiguration(); +#else + _currentConfiguration; +#endif + /// /// Constructor. /// @@ -273,8 +307,8 @@ public bool ContinueOnError { get { - ErrorUtilities.VerifyThrow(_currentConfiguration != null, "We should never have a null configuration during a BuildEngine callback!"); - return _currentConfiguration.ContinueOnError; + ErrorUtilities.VerifyThrow(EffectiveConfiguration != null, "We should never have a null configuration during a BuildEngine callback!"); + return EffectiveConfiguration.ContinueOnError; } } @@ -285,8 +319,8 @@ public int LineNumberOfTaskNode { get { - ErrorUtilities.VerifyThrow(_currentConfiguration != null, "We should never have a null configuration during a BuildEngine callback!"); - return _currentConfiguration.LineNumberOfTask; + ErrorUtilities.VerifyThrow(EffectiveConfiguration != null, "We should never have a null configuration during a BuildEngine callback!"); + return EffectiveConfiguration.LineNumberOfTask; } } @@ -297,8 +331,8 @@ public int ColumnNumberOfTaskNode { get { - ErrorUtilities.VerifyThrow(_currentConfiguration != null, "We should never have a null configuration during a BuildEngine callback!"); - return _currentConfiguration.ColumnNumberOfTask; + ErrorUtilities.VerifyThrow(EffectiveConfiguration != null, "We should never have a null configuration during a BuildEngine callback!"); + return EffectiveConfiguration.ColumnNumberOfTask; } } @@ -309,8 +343,8 @@ public string ProjectFileOfTaskNode { get { - ErrorUtilities.VerifyThrow(_currentConfiguration != null, "We should never have a null configuration during a BuildEngine callback!"); - return _currentConfiguration.ProjectFileOfTask; + ErrorUtilities.VerifyThrow(EffectiveConfiguration != null, "We should never have a null configuration during a BuildEngine callback!"); + return EffectiveConfiguration.ProjectFileOfTask; } } @@ -635,7 +669,7 @@ public void LogTelemetry(string eventName, IDictionary propertie /// An containing the global properties of the current project. public IReadOnlyDictionary GetGlobalProperties() { - return new Dictionary(_currentConfiguration.GlobalProperties); + return new Dictionary(EffectiveConfiguration.GlobalProperties); } #endregion @@ -705,8 +739,8 @@ public override bool IsTaskInputLoggingEnabled { get { - ErrorUtilities.VerifyThrow(_taskHost._currentConfiguration != null, "We should never have a null configuration during a BuildEngine callback!"); - return _taskHost._currentConfiguration.IsTaskInputLoggingEnabled; + ErrorUtilities.VerifyThrow(_taskHost.EffectiveConfiguration != null, "We should never have a null configuration during a BuildEngine callback!"); + return _taskHost.EffectiveConfiguration.IsTaskInputLoggingEnabled; } } @@ -917,8 +951,16 @@ private void HandleCallbackResponse(INodePacket packet) return; } - // Request ID not found is expected if the connection was lost and the task thread - // already cleaned up via the finally block in SendCallbackRequestAndWaitForResponse. + // Try per-task context pending requests first, then fall back to global + foreach (var kvp in _taskContexts) + { + if (kvp.Value.PendingCallbackRequests.TryRemove(callbackPacket.RequestId, out TaskCompletionSource tcsFromContext)) + { + tcsFromContext.TrySetResult(packet); + return; + } + } + if (_pendingCallbackRequests.TryRemove(callbackPacket.RequestId, out TaskCompletionSource tcs)) { tcs.TrySetResult(packet); @@ -954,14 +996,24 @@ private void HandleCallbackResponse(INodePacket packet) private TResponse SendCallbackRequestAndWaitForResponse(ITaskHostCallbackPacket request) where TResponse : class, INodePacket { + // Get context - use per-task pending requests if available + var context = GetCurrentTaskContext(); + var pendingRequests = context?.PendingCallbackRequests ?? _pendingCallbackRequests; + + // IMPORTANT: Request IDs must be globally unique across all task contexts int requestId = Interlocked.Increment(ref _nextCallbackRequestId); request.RequestId = requestId; var tcs = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); - _pendingCallbackRequests[requestId] = tcs; + pendingRequests[requestId] = tcs; try { + if (context is not null) + { + context.State = TaskExecutionState.BlockedOnCallback; + } + // Send the request packet to the owning worker node _nodeEndpoint.SendData(request); @@ -980,25 +1032,131 @@ private TResponse SendCallbackRequestAndWaitForResponse(ITaskHostCall } finally { - _pendingCallbackRequests.TryRemove(requestId, out _); + pendingRequests.TryRemove(requestId, out _); + + if (context is not null) + { + context.State = TaskExecutionState.Executing; + } } } /// /// Marks this TaskHost as yielded so the scheduler can reuse it for nested tasks. - /// Called before sending a BuildProjectFile callback or explicit Yield. + /// Saves the current operating environment before yielding. /// private void YieldForCallback() { + // Save environment state before yielding + var context = GetCurrentTaskContext(); + if (context is not null) + { + SaveOperatingEnvironment(context); + } + + // Decrement active count to allow the parent to send a new TaskHostConfiguration + // for the nested build. The task is still running but is now "yielded" waiting + // for the callback response. + Interlocked.Decrement(ref _activeTaskCount); Interlocked.Increment(ref _yieldedTaskCount); } /// /// Marks this TaskHost as active again after a callback or Reacquire completes. + /// Restores the previously saved operating environment. /// private void ReacquireAfterCallback() { Interlocked.Decrement(ref _yieldedTaskCount); + Interlocked.Increment(ref _activeTaskCount); + + // Restore environment state after reacquiring + var context = GetCurrentTaskContext(); + if (context is not null) + { + RestoreOperatingEnvironment(context); + } + } + + /// + /// Gets the task execution context for the current thread. + /// + private TaskExecutionContext GetCurrentTaskContext() + { + return _currentTaskContext.Value; + } + + /// + /// Gets the configuration for the currently executing task on this thread. + /// Uses the thread-local task context if available, falling back to the global configuration. + /// + private TaskHostConfiguration GetCurrentConfiguration() + { + var context = GetCurrentTaskContext(); + return context?.Configuration ?? _currentConfiguration; + } + + /// + /// Creates a new task execution context for the given configuration. + /// + private TaskExecutionContext CreateTaskContext(TaskHostConfiguration configuration) + { + int taskId = Interlocked.Increment(ref _nextLocalTaskId); + + var context = new TaskExecutionContext(taskId, configuration); + + if (!_taskContexts.TryAdd(taskId, context)) + { + context.Dispose(); + throw new InvalidOperationException( + $"Task ID {taskId} already exists in TaskHost."); + } + + return context; + } + + /// + /// Removes and disposes a task execution context. + /// + private void RemoveTaskContext(int taskId) + { + if (_taskContexts.TryRemove(taskId, out var context)) + { + context.Dispose(); + } + } + + /// + /// Saves the current operating environment to the task context. + /// Called before yielding or blocking on a callback that allows other tasks to run. + /// + internal void SaveOperatingEnvironment(TaskExecutionContext context) + { + ErrorUtilities.VerifyThrowArgumentNull(context, nameof(context)); + + context.SavedCurrentDirectory = NativeMethodsShared.GetCurrentDirectory(); + context.SavedEnvironment = new Dictionary( + CommunicationsUtilities.GetEnvironmentVariables(), + StringComparer.OrdinalIgnoreCase); + } + + /// + /// Restores the previously saved operating environment from the task context. + /// + internal void RestoreOperatingEnvironment(TaskExecutionContext context) + { + ErrorUtilities.VerifyThrowArgumentNull(context, nameof(context)); + + if (context.SavedCurrentDirectory is null || context.SavedEnvironment is null) + { + return; + } + + CommunicationsUtilities.SetEnvironment(context.SavedEnvironment); + NativeMethodsShared.SetCurrentDirectory(context.SavedCurrentDirectory); + + context.SavedCurrentDirectory = null; + context.SavedEnvironment = null; } #endif @@ -1009,20 +1167,31 @@ private void ReacquireAfterCallback() private void HandleTaskHostConfiguration(TaskHostConfiguration taskHostConfiguration) { #if !CLR2COMPATIBILITY - // Allow new configuration when the existing task is yielded (blocked on BuildProjectFile callback - // or explicit Yield). This enables nested build scenarios where the child build's tasks can run - // on the same TaskHost process. - ErrorUtilities.VerifyThrow(!_isTaskExecuting || _yieldedTaskCount > 0, - "Why are we getting a TaskHostConfiguration packet while a task is actively executing? yieldedTaskCount={0}", - _yieldedTaskCount); + // Allow new configuration when no task is actively executing. + // A task that is yielded (blocked on BuildProjectFile callback) does NOT prevent + // a new task from starting - this enables nested build scenarios. + ErrorUtilities.VerifyThrow(_activeTaskCount == 0, + "Why are we getting a TaskHostConfiguration packet while a task is actively executing? activeTaskCount={0}", + _activeTaskCount); #else - ErrorUtilities.VerifyThrow(!_isTaskExecuting, "Why are we getting a TaskHostConfiguration packet while we're still executing a task?"); + ErrorUtilities.VerifyThrow(_activeTaskCount == 0, "Why are we getting a TaskHostConfiguration packet while we're still executing a task?"); #endif _currentConfiguration = taskHostConfiguration; +#if !CLR2COMPATIBILITY + // Create task execution context for this task + var context = CreateTaskContext(taskHostConfiguration); + context.State = TaskExecutionState.Executing; +#endif + // Kick off the task running thread. _taskRunnerThread = new Thread(new ParameterizedThreadStart(RunTask)); _taskRunnerThread.Name = "Task runner for task " + taskHostConfiguration.TaskName; + +#if !CLR2COMPATIBILITY + context.ExecutingThread = _taskRunnerThread; +#endif + _taskRunnerThread.Start(taskHostConfiguration); } @@ -1031,7 +1200,10 @@ private void HandleTaskHostConfiguration(TaskHostConfiguration taskHostConfigura /// private void CompleteTask() { - ErrorUtilities.VerifyThrow(!_isTaskExecuting, "The task should be done executing before CompleteTask."); + // With multiple concurrent tasks (nested BuildProjectFile), we cannot assert + // that no task is executing here. The task that completed set _taskCompletePacket + // and signaled _taskCompleteEvent, but another task may have reacquired from yield + // by the time this method runs. if (_nodeEndpoint.LinkStatus == LinkStatus.Active) { TaskHostTaskComplete taskCompletePacketToSend; @@ -1046,7 +1218,16 @@ private void CompleteTask() _nodeEndpoint.SendData(taskCompletePacketToSend); } +#if !CLR2COMPATIBILITY + // Only clear _currentConfiguration when no tasks remain (active or yielded). + // A yielded task still needs the config for logging via EffectiveConfiguration. + if (_activeTaskCount == 0 && _yieldedTaskCount == 0) + { + _currentConfiguration = null; + } +#else _currentConfiguration = null; +#endif // If the task has been canceled, the event will still be set. // If so, now that we've completed the task, we want to shut down @@ -1076,7 +1257,7 @@ private void CancelTask() { // Don't bother aborting the task if it has passed the actual user task Execute() // It means we're already in the process of shutting down - Wait for the taskCompleteEvent to be set instead. - if (_isTaskExecuting) + if (_activeTaskCount > 0) { #if FEATURE_THREAD_ABORT // The thread will be terminated crudely so our environment may be trashed but it's ok since we are @@ -1093,7 +1274,7 @@ private void CancelTask() /// private void HandleNodeBuildComplete(NodeBuildComplete buildComplete) { - ErrorUtilities.VerifyThrow(!_isTaskExecuting, "We should never have a task in the process of executing when we receive NodeBuildComplete."); + ErrorUtilities.VerifyThrow(_activeTaskCount == 0, "We should never have a task in the process of executing when we receive NodeBuildComplete."); // Sidecar TaskHost will persist after the build is done. if (_nodeReuse) @@ -1190,6 +1371,19 @@ private void OnLinkStatusChanged(INodeEndpoint endpoint, LinkStatus status) "TaskHost lost connection to owning worker node during callback.")); } } + + // Also fail per-task context pending requests + foreach (var contextKvp in _taskContexts) + { + foreach (var reqKvp in contextKvp.Value.PendingCallbackRequests) + { + if (contextKvp.Value.PendingCallbackRequests.TryRemove(reqKvp.Key, out TaskCompletionSource ctxTcs)) + { + ctxTcs.TrySetException(new InvalidOperationException( + "TaskHost lost connection to owning worker node during callback.")); + } + } + } #endif _shutdownEvent.Set(); @@ -1208,9 +1402,28 @@ private void OnLinkStatusChanged(INodeEndpoint endpoint, LinkStatus status) /// private void RunTask(object state) { - _isTaskExecuting = true; + Interlocked.Increment(ref _activeTaskCount); OutOfProcTaskHostTaskResult taskResult = null; TaskHostConfiguration taskConfiguration = state as TaskHostConfiguration; + +#if !CLR2COMPATIBILITY + // Set the current task context for this thread + TaskExecutionContext taskContext = null; + foreach (var kvp in _taskContexts) + { + if (kvp.Value.Configuration == taskConfiguration) + { + taskContext = kvp.Value; + break; + } + } + + if (taskContext is not null) + { + _currentTaskContext.Value = taskContext; + } +#endif + IDictionary taskParams = taskConfiguration.TaskParameters; // We only really know the values of these variables for sure once we see what we received from the owning worker node @@ -1281,7 +1494,7 @@ private void RunTask(object state) { try { - _isTaskExecuting = false; + Interlocked.Decrement(ref _activeTaskCount); IDictionary currentEnvironment = CommunicationsUtilities.GetEnvironmentVariables(); currentEnvironment = UpdateEnvironmentForMainNode(currentEnvironment); @@ -1331,6 +1544,17 @@ private void RunTask(object state) // Call CleanupTask to unload any domains and other necessary cleanup in the taskWrapper _taskWrapper.CleanupTask(); +#if !CLR2COMPATIBILITY + // Mark context as completed and clean up + if (taskContext is not null) + { + taskContext.State = TaskExecutionState.Completed; + taskContext.CompletedEvent.Set(); + _currentTaskContext.Value = null; + RemoveTaskContext(taskContext.TaskId); + } +#endif + // The task has now fully completed executing _taskCompleteEvent.Set(); } @@ -1525,7 +1749,7 @@ private void SendBuildEvent(BuildEventArgs e) return; } - LogMessagePacketBase logMessage = new(new KeyValuePair(_currentConfiguration.NodeId, e)); + LogMessagePacketBase logMessage = new(new KeyValuePair(EffectiveConfiguration.NodeId, e)); _nodeEndpoint.SendData(logMessage); } } @@ -1535,13 +1759,13 @@ private void SendBuildEvent(BuildEventArgs e) /// private void LogMessageFromResource(MessageImportance importance, string messageResource, params object[] messageArgs) { - ErrorUtilities.VerifyThrow(_currentConfiguration != null, "We should never have a null configuration when we're trying to log messages!"); + ErrorUtilities.VerifyThrow(EffectiveConfiguration != null, "We should never have a null configuration when we're trying to log messages!"); // Using the CLR 2 build event because this class is shared between MSBuildTaskHost.exe (CLR2) and MSBuild.exe (CLR4+) BuildMessageEventArgs message = new BuildMessageEventArgs( ResourceUtilities.FormatString(AssemblyResources.GetString(messageResource), messageArgs), null, - _currentConfiguration.TaskName, + EffectiveConfiguration.TaskName, importance); LogMessageEvent(message); @@ -1552,7 +1776,7 @@ private void LogMessageFromResource(MessageImportance importance, string message /// private void LogWarningFromResource(string messageResource, params object[] messageArgs) { - ErrorUtilities.VerifyThrow(_currentConfiguration != null, "We should never have a null configuration when we're trying to log warnings!"); + ErrorUtilities.VerifyThrow(EffectiveConfiguration != null, "We should never have a null configuration when we're trying to log warnings!"); // Using the CLR 2 build event because this class is shared between MSBuildTaskHost.exe (CLR2) and MSBuild.exe (CLR4+) BuildWarningEventArgs warning = new BuildWarningEventArgs( @@ -1565,7 +1789,7 @@ private void LogWarningFromResource(string messageResource, params object[] mess 0, ResourceUtilities.FormatString(AssemblyResources.GetString(messageResource), messageArgs), null, - _currentConfiguration.TaskName); + EffectiveConfiguration.TaskName); LogWarningEvent(warning); } @@ -1575,7 +1799,7 @@ private void LogWarningFromResource(string messageResource, params object[] mess /// private void LogErrorFromResource(string messageResource) { - ErrorUtilities.VerifyThrow(_currentConfiguration != null, "We should never have a null configuration when we're trying to log errors!"); + ErrorUtilities.VerifyThrow(EffectiveConfiguration != null, "We should never have a null configuration when we're trying to log errors!"); // Using the CLR 2 build event because this class is shared between MSBuildTaskHost.exe (CLR2) and MSBuild.exe (CLR4+) BuildErrorEventArgs error = new BuildErrorEventArgs( @@ -1588,7 +1812,7 @@ private void LogErrorFromResource(string messageResource) 0, AssemblyResources.GetString(messageResource), null, - _currentConfiguration.TaskName); + EffectiveConfiguration.TaskName); LogErrorEvent(error); } diff --git a/src/MSBuild/TaskExecutionContext.cs b/src/MSBuild/TaskExecutionContext.cs new file mode 100644 index 00000000000..06502f3ff3f --- /dev/null +++ b/src/MSBuild/TaskExecutionContext.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. + +#if !CLR2COMPATIBILITY + +using System; +using System.Collections.Concurrent; +using System.Collections.Generic; +using System.Threading; +using System.Threading.Tasks; +using Microsoft.Build.BackEnd; + +#nullable disable + +namespace Microsoft.Build.CommandLine +{ + /// + /// Represents the execution context for a single task running in the TaskHost. + /// Multiple contexts may exist concurrently when tasks yield or await callbacks + /// like BuildProjectFile. + /// + /// + /// When Task A calls BuildProjectFile and blocks, Task B may be dispatched + /// to the same TaskHost process. Each task needs its own: + /// - Configuration and parameters + /// - Pending callback requests (for request/response correlation) + /// - Saved environment (for context switching on yield) + /// - Completion signaling + /// + internal sealed class TaskExecutionContext : IDisposable + { + /// + /// Unique identifier for this task execution. + /// + public int TaskId { get; } + + /// + /// The configuration packet that initiated this task execution. + /// + public TaskHostConfiguration Configuration { get; } + + /// + /// The thread executing this task, or null if not yet started. + /// + public Thread ExecutingThread { get; set; } + + /// + /// Current execution state of this task. + /// + public TaskExecutionState State { get; set; } + + /// + /// Saved current directory when task yields or awaits a blocking callback. + /// + public string SavedCurrentDirectory { get; set; } + + /// + /// Saved environment variables when task yields or awaits a blocking callback. + /// + public IDictionary SavedEnvironment { get; set; } + + /// + /// Pending callback requests for THIS task, keyed by request ID. + /// Each task has isolated pending requests to prevent cross-contamination + /// when multiple tasks are blocked on callbacks simultaneously. + /// + public ConcurrentDictionary> PendingCallbackRequests { get; } + = new ConcurrentDictionary>(); + + /// + /// Event signaled when this task completes execution. + /// + public ManualResetEvent CompletedEvent { get; } = new ManualResetEvent(false); + + /// + /// Event signaled when this specific task is cancelled. + /// + public ManualResetEvent CancelledEvent { get; } = new ManualResetEvent(false); + + /// + /// Creates a new task execution context. + /// + public TaskExecutionContext(int taskId, TaskHostConfiguration configuration) + { + TaskId = taskId; + Configuration = configuration ?? throw new ArgumentNullException(nameof(configuration)); + State = TaskExecutionState.Pending; + } + + /// + /// Disposes resources held by this context. + /// + public void Dispose() + { + CompletedEvent?.Dispose(); + CancelledEvent?.Dispose(); + } + } + + /// + /// Execution states for a task running in the TaskHost. + /// + internal enum TaskExecutionState + { + /// Task context created but not yet started. + Pending, + /// Task is actively executing on its thread. + Executing, + /// Task has called Yield and is waiting for Reacquire. + Yielded, + /// Task is blocked waiting for a callback response. + BlockedOnCallback, + /// Task has finished execution. + Completed, + /// Task was cancelled. + Cancelled, + } +} + +#endif From cd69cb9339e9abe0494c68be8bc870295e0456c6 Mon Sep 17 00:00:00 2001 From: Jan Provaznik Date: Tue, 10 Mar 2026 14:58:39 +0100 Subject: [PATCH 12/13] Fix 6 concurrency issues found by multi-model review Fixes identified by parallel review with Sonnet, Opus, Gemini, GPT: 1. _taskWrapper overwrite: Capture in local variable before ExecuteTask so nested Task B cannot overwrite it. Prevents leaked wrappers and double-cleanup. 2. _taskRunnerThread overwrite: HandleShutdown now joins all active task threads from _taskContexts (not just the last one), and fails their pending callbacks so they can unblock before WaitHandle disposal. 3. _taskCompletePacket single-slot race: Replace with ConcurrentQueue and drain in CompleteTask. Prevents lost completion packets when multiple tasks finish concurrently. 4. Non-atomic active/yielded transition: Reverse Interlocked order in YieldForCallback (increment yielded BEFORE decrementing active) and ReacquireAfterCallback (increment active BEFORE decrementing yielded). Ensures the sum is never zero during transition, preventing CompleteTask from prematurely nulling _currentConfiguration. 5. Shared warning fields overwrite: Save/restore WarningsAsErrors, WarningsNotAsErrors, WarningsAsMessages in TaskExecutionContext alongside environment variables. 6. Yield/Reacquire counter imbalance: RunTask finally block checks TaskExecutionState.Yielded to determine whether _activeTaskCount was already decremented by YieldForCallback, preventing counters from going negative if a task exits after Yield without Reacquire. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- src/MSBuild/OutOfProcTaskHostNode.cs | 113 ++++++++++++++++++++++++--- src/MSBuild/TaskExecutionContext.cs | 12 +++ 2 files changed, 114 insertions(+), 11 deletions(-) diff --git a/src/MSBuild/OutOfProcTaskHostNode.cs b/src/MSBuild/OutOfProcTaskHostNode.cs index 38e7b801e89..cd30b3007ad 100644 --- a/src/MSBuild/OutOfProcTaskHostNode.cs +++ b/src/MSBuild/OutOfProcTaskHostNode.cs @@ -134,15 +134,22 @@ internal class OutOfProcTaskHostNode : private AutoResetEvent _taskCompleteEvent; /// - /// Packet containing all the information relating to the - /// completed state of the task. + /// Queue of completed task packets waiting to be sent by the main thread. + /// Using a queue instead of a single slot prevents lost completions when + /// multiple tasks finish concurrently (e.g., nested task reuse scenarios). /// +#if !CLR2COMPATIBILITY + private readonly ConcurrentQueue _taskCompleteQueue = new(); +#else private TaskHostTaskComplete _taskCompletePacket; +#endif /// - /// Object used to synchronize access to taskCompletePacket + /// Object used to synchronize access to taskCompletePacket (CLR2 only) /// +#if CLR2COMPATIBILITY private LockType _taskCompleteLock = new(); +#endif /// /// The event which is set when a task is cancelled @@ -1052,13 +1059,15 @@ private void YieldForCallback() if (context is not null) { SaveOperatingEnvironment(context); + context.State = TaskExecutionState.Yielded; } - // Decrement active count to allow the parent to send a new TaskHostConfiguration - // for the nested build. The task is still running but is now "yielded" waiting - // for the callback response. - Interlocked.Decrement(ref _activeTaskCount); + // Transition from "active" to "yielded" state. + // Order matters: increment yielded BEFORE decrementing active so the sum + // (_activeTaskCount + _yieldedTaskCount) is never zero during the transition. + // This prevents CompleteTask from prematurely nulling _currentConfiguration. Interlocked.Increment(ref _yieldedTaskCount); + Interlocked.Decrement(ref _activeTaskCount); } /// @@ -1067,8 +1076,11 @@ private void YieldForCallback() /// private void ReacquireAfterCallback() { - Interlocked.Decrement(ref _yieldedTaskCount); + // Transition from "yielded" back to "active" state. + // Order matters: increment active BEFORE decrementing yielded so the sum + // (_activeTaskCount + _yieldedTaskCount) is never zero during the transition. Interlocked.Increment(ref _activeTaskCount); + Interlocked.Decrement(ref _yieldedTaskCount); // Restore environment state after reacquiring var context = GetCurrentTaskContext(); @@ -1138,6 +1150,11 @@ internal void SaveOperatingEnvironment(TaskExecutionContext context) context.SavedEnvironment = new Dictionary( CommunicationsUtilities.GetEnvironmentVariables(), StringComparer.OrdinalIgnoreCase); + + // Save warning settings that may be overwritten by a nested task + context.SavedWarningsAsErrors = WarningsAsErrors; + context.SavedWarningsNotAsErrors = WarningsNotAsErrors; + context.SavedWarningsAsMessages = WarningsAsMessages; } /// @@ -1155,6 +1172,11 @@ internal void RestoreOperatingEnvironment(TaskExecutionContext context) CommunicationsUtilities.SetEnvironment(context.SavedEnvironment); NativeMethodsShared.SetCurrentDirectory(context.SavedCurrentDirectory); + // Restore warning settings + WarningsAsErrors = context.SavedWarningsAsErrors; + WarningsNotAsErrors = context.SavedWarningsNotAsErrors; + WarningsAsMessages = context.SavedWarningsAsMessages; + context.SavedCurrentDirectory = null; context.SavedEnvironment = null; } @@ -1206,6 +1228,14 @@ private void CompleteTask() // by the time this method runs. if (_nodeEndpoint.LinkStatus == LinkStatus.Active) { +#if !CLR2COMPATIBILITY + // Drain all queued completion packets — multiple tasks may have completed + // before the main thread serviced the AutoResetEvent. + while (_taskCompleteQueue.TryDequeue(out TaskHostTaskComplete packet)) + { + _nodeEndpoint.SendData(packet); + } +#else TaskHostTaskComplete taskCompletePacketToSend; lock (_taskCompleteLock) @@ -1216,6 +1246,7 @@ private void CompleteTask() } _nodeEndpoint.SendData(taskCompletePacketToSend); +#endif } #if !CLR2COMPATIBILITY @@ -1297,6 +1328,28 @@ private NodeEngineShutdownReason HandleShutdown() // Wait for the RunTask task runner thread before shutting down so that we can cleanly dispose all WaitHandles. _taskRunnerThread?.Join(); +#if !CLR2COMPATIBILITY + // Also join any other task threads that may be blocked (e.g., a yielded task waiting on TCS). + // Fail their pending callback requests first so they can unblock. + foreach (var kvp in _taskContexts) + { + Thread thread = kvp.Value.ExecutingThread; + if (thread is not null && thread != _taskRunnerThread && thread.IsAlive) + { + // Fail any pending callbacks so the thread can unblock + foreach (var reqKvp in kvp.Value.PendingCallbackRequests) + { + if (kvp.Value.PendingCallbackRequests.TryRemove(reqKvp.Key, out var tcs)) + { + tcs.TrySetException(new InvalidOperationException("TaskHost shutting down.")); + } + } + + thread.Join(TimeSpan.FromSeconds(5)); + } + } +#endif + using StreamWriter debugWriter = _debugCommunications ? File.CreateText(string.Format(CultureInfo.CurrentCulture, Path.Combine(FileUtilities.TempFileDirectory, @"MSBuild_NodeShutdown_{0}.txt"), EnvironmentUtilities.CurrentProcessId)) : null; @@ -1435,6 +1488,7 @@ private void RunTask(object state) WarningsAsErrors = taskConfiguration.WarningsAsErrors; WarningsNotAsErrors = taskConfiguration.WarningsNotAsErrors; WarningsAsMessages = taskConfiguration.WarningsAsMessages; + OutOfProcTaskAppDomainWrapper taskWrapper = null; try { // Change to the startup directory @@ -1462,9 +1516,10 @@ private void RunTask(object state) #endif // We will not create an appdomain now because of a bug // As a fix, we will create the class directly without wrapping it in a domain - _taskWrapper = new OutOfProcTaskAppDomainWrapper(); + taskWrapper = new OutOfProcTaskAppDomainWrapper(); + _taskWrapper = taskWrapper; - taskResult = _taskWrapper.ExecuteTask( + taskResult = taskWrapper.ExecuteTask( this as IBuildEngine, taskName, taskLocation, @@ -1494,13 +1549,37 @@ private void RunTask(object state) { try { +#if !CLR2COMPATIBILITY + // Reconcile counters: if the task is still in "yielded" state (e.g., it called Yield() + // without a matching Reacquire() before exiting), fix up the counters. + // The yielded count should be decremented and active count should NOT be decremented again + // since YieldForCallback already did that. + if (taskContext is not null && taskContext.State == TaskExecutionState.Yielded) + { + Interlocked.Decrement(ref _yieldedTaskCount); + // Don't decrement _activeTaskCount — it was already decremented by YieldForCallback + } + else + { + Interlocked.Decrement(ref _activeTaskCount); + } +#else Interlocked.Decrement(ref _activeTaskCount); +#endif IDictionary currentEnvironment = CommunicationsUtilities.GetEnvironmentVariables(); currentEnvironment = UpdateEnvironmentForMainNode(currentEnvironment); taskResult ??= new OutOfProcTaskHostTaskResult(TaskCompleteType.Failure); +#if !CLR2COMPATIBILITY + _taskCompleteQueue.Enqueue(new TaskHostTaskComplete( + taskResult, +#if FEATURE_REPORTFILEACCESSES + _fileAccessData, +#endif + currentEnvironment)); +#else lock (_taskCompleteLock) { _taskCompletePacket = new TaskHostTaskComplete( @@ -1510,6 +1589,7 @@ private void RunTask(object state) #endif currentEnvironment); } +#endif #if FEATURE_APPDOMAIN foreach (TaskParameter param in taskParams.Values) @@ -1524,6 +1604,15 @@ private void RunTask(object state) } catch (Exception e) { +#if !CLR2COMPATIBILITY + // Create a minimal taskCompletePacket to carry the exception so that the TaskHostTask does not hang while waiting + _taskCompleteQueue.Enqueue(new TaskHostTaskComplete( + new OutOfProcTaskHostTaskResult(TaskCompleteType.CrashedAfterExecution, e), +#if FEATURE_REPORTFILEACCESSES + _fileAccessData, +#endif + null)); +#else lock (_taskCompleteLock) { // Create a minimal taskCompletePacket to carry the exception so that the TaskHostTask does not hang while waiting @@ -1534,6 +1623,7 @@ private void RunTask(object state) #endif null); } +#endif } finally { @@ -1542,7 +1632,8 @@ private void RunTask(object state) #endif // Call CleanupTask to unload any domains and other necessary cleanup in the taskWrapper - _taskWrapper.CleanupTask(); + // Use local variable — _taskWrapper may have been overwritten by a nested task. + taskWrapper?.CleanupTask(); #if !CLR2COMPATIBILITY // Mark context as completed and clean up diff --git a/src/MSBuild/TaskExecutionContext.cs b/src/MSBuild/TaskExecutionContext.cs index 06502f3ff3f..abcb13b333d 100644 --- a/src/MSBuild/TaskExecutionContext.cs +++ b/src/MSBuild/TaskExecutionContext.cs @@ -59,6 +59,18 @@ internal sealed class TaskExecutionContext : IDisposable /// public IDictionary SavedEnvironment { get; set; } + /// + /// Saved per-task warning/debug settings, captured from OutOfProcTaskHostNode shared fields + /// before yielding so they can be restored when the task resumes. + /// + public ICollection SavedWarningsAsErrors { get; set; } + + /// Saved WarningsNotAsErrors. + public ICollection SavedWarningsNotAsErrors { get; set; } + + /// Saved WarningsAsMessages. + public ICollection SavedWarningsAsMessages { get; set; } + /// /// Pending callback requests for THIS task, keyed by request ID. /// Each task has isolated pending requests to prevent cross-contamination From 62eef28afacaf8d0a0b9873d48c5c898e67ae010 Mon Sep 17 00:00:00 2001 From: Jan Provaznik Date: Tue, 10 Mar 2026 16:46:00 +0100 Subject: [PATCH 13/13] Add external repo validation script and fix nullable warning - scripts/test-external-repos.ps1: Reusable script to build Roslyn and WPF projects with bootstrap MSBuild. Enables callbacks (MSBUILDENABLETASKHOSTCALLBACKS=1) and -mt flag. Supports -SkipClone, -CleanBuild, -Repos parameters. - Fix CS8600 nullable warning in BuildProjectFileAndReportPidTask.cs Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- scripts/test-external-repos.ps1 | 237 ++++++++++++++++++ .../BuildProjectFileAndReportPidTask.cs | 2 +- 2 files changed, 238 insertions(+), 1 deletion(-) create mode 100644 scripts/test-external-repos.ps1 diff --git a/scripts/test-external-repos.ps1 b/scripts/test-external-repos.ps1 new file mode 100644 index 00000000000..ae572150167 --- /dev/null +++ b/scripts/test-external-repos.ps1 @@ -0,0 +1,237 @@ +<# +.SYNOPSIS + Builds external repos and projects using bootstrap MSBuild to validate callback changes. + +.DESCRIPTION + Clones dotnet/roslyn to Q:\, creates a WPF test project, then builds them using the + locally-built bootstrap MSBuild with TaskHost callbacks enabled and multithreaded mode (-mt). + Re-run after rebuilding bootstrap to validate new changes. + + WPF repo itself requires SDK 11.0 so we test with 'dotnet new wpf' instead. + +.PARAMETER Repos + Which to build. Default: both. Options: 'roslyn', 'wpf', 'both' + +.PARAMETER SkipClone + Skip cloning if repos already exist at Q:\roslyn and Q:\wpf. + +.PARAMETER CleanBuild + Delete artifacts/bin before building to force a clean build. + +.EXAMPLE + # First run: clone + build both + .\scripts\test-external-repos.ps1 + + # Re-run after rebuilding bootstrap (skip clone, just rebuild) + .\scripts\test-external-repos.ps1 -SkipClone + + # Build only roslyn + .\scripts\test-external-repos.ps1 -Repos roslyn -SkipClone +#> +param( + [ValidateSet('roslyn', 'wpf', 'both')] + [string]$Repos = 'both', + + [switch]$SkipClone, + + [switch]$CleanBuild +) + +$ErrorActionPreference = 'Continue' + +# --- Paths --- +$msbuildRoot = "C:\Users\janprovaznik\dev\msbuild" +$bootstrapRoot = "$msbuildRoot\artifacts\bin\bootstrap" +$bootstrapDotnet = "$bootstrapRoot\core\dotnet.exe" +$bootstrapMSBuildExe = "$bootstrapRoot\net472\MSBuild\Current\Bin\MSBuild.exe" + +# Find the SDK version in bootstrap +$sdkDir = Get-ChildItem "$bootstrapRoot\core\sdk" -Directory | Sort-Object Name -Descending | Select-Object -First 1 +$bootstrapSdkPath = $sdkDir.FullName + +Write-Host "============================================" -ForegroundColor Cyan +Write-Host " External Repo Build Validation" -ForegroundColor Cyan +Write-Host "============================================" -ForegroundColor Cyan +Write-Host "" +Write-Host "Bootstrap dotnet : $bootstrapDotnet" +Write-Host "Bootstrap SDK : $bootstrapSdkPath" +Write-Host "Bootstrap MSBuild: $bootstrapMSBuildExe" +Write-Host "" + +# Verify bootstrap exists +if (-not (Test-Path $bootstrapDotnet)) { + Write-Error "Bootstrap dotnet not found at $bootstrapDotnet. Run 'build.cmd' first." + exit 1 +} + +# --- Environment: enable callbacks + multithreaded --- +$env:MSBUILDENABLETASKHOSTCALLBACKS = "1" +$env:DOTNET_ROOT = "$bootstrapRoot\core" +$env:DOTNET_MSBUILD_SDK_RESOLVER_CLI_DIR = "$bootstrapRoot\core" +# Use bootstrap dotnet for SDK resolution +$env:PATH = "$bootstrapRoot\core;$env:PATH" + +Write-Host "Environment:" -ForegroundColor Yellow +Write-Host " MSBUILDENABLETASKHOSTCALLBACKS = 1" +Write-Host " DOTNET_ROOT = $env:DOTNET_ROOT" +Write-Host "" + +# --- Helper: Clone or update repo --- +function Ensure-Repo { + param([string]$Org, [string]$Name, [string]$TargetDir) + + if (Test-Path $TargetDir) { + if ($SkipClone) { + Write-Host "[$Name] Using existing clone at $TargetDir" -ForegroundColor Green + return + } + Write-Host "[$Name] Removing existing clone..." -ForegroundColor Yellow + Remove-Item -Recurse -Force $TargetDir + } + + Write-Host "[$Name] Cloning $Org/$Name to $TargetDir (shallow)..." -ForegroundColor Yellow + git clone --depth 1 "https://github.com/$Org/$Name.git" $TargetDir + if ($LASTEXITCODE -ne 0) { + Write-Error "Failed to clone $Org/$Name" + return $false + } + return $true +} + +# --- Helper: Build a repo --- +function Build-Repo { + param( + [string]$Name, + [string]$RepoDir, + [string]$BuildCommand + ) + + Write-Host "" + Write-Host "============================================" -ForegroundColor Cyan + Write-Host " Building $Name" -ForegroundColor Cyan + Write-Host "============================================" -ForegroundColor Cyan + + Push-Location $RepoDir + try { + if ($CleanBuild -and (Test-Path "artifacts\bin")) { + Write-Host "[$Name] Cleaning artifacts..." -ForegroundColor Yellow + Remove-Item -Recurse -Force "artifacts\bin" -ErrorAction SilentlyContinue + } + + $logFile = "Q:\${Name}-build.binlog" + Write-Host "[$Name] Build command: $BuildCommand" -ForegroundColor Yellow + Write-Host "[$Name] Binlog: $logFile" -ForegroundColor Yellow + Write-Host "" + + $sw = [System.Diagnostics.Stopwatch]::StartNew() + Invoke-Expression $BuildCommand | Out-Host + $exitCode = $LASTEXITCODE + $sw.Stop() + + if ($exitCode -eq 0) { + Write-Host "" + Write-Host "[$Name] BUILD SUCCEEDED in $($sw.Elapsed.ToString('mm\:ss'))" -ForegroundColor Green + } else { + Write-Host "" + Write-Host "[$Name] BUILD FAILED (exit code $exitCode) after $($sw.Elapsed.ToString('mm\:ss'))" -ForegroundColor Red + Write-Host "[$Name] Check binlog: $logFile" -ForegroundColor Red + } + + return $exitCode + } finally { + Pop-Location + } +} + +# --- Ensure Q: exists --- +if (-not (Test-Path "Q:\")) { + Write-Error "Q:\ drive does not exist. Please create or mount it first." + exit 1 +} + +$results = @{} + +# --- Roslyn --- +if ($Repos -eq 'roslyn' -or $Repos -eq 'both') { + $roslynDir = "Q:\roslyn" + Ensure-Repo -Org "dotnet" -Name "roslyn" -TargetDir $roslynDir + + if (Test-Path $roslynDir) { + # Patch global.json to allow rollForward from our bootstrap SDK + $globalJsonPath = "$roslynDir\global.json" + $globalJson = Get-Content $globalJsonPath -Raw | ConvertFrom-Json + $globalJson.sdk.rollForward = "latestFeature" + $globalJson | ConvertTo-Json -Depth 10 | Set-Content $globalJsonPath + + # Build Roslyn Compilers solution filter (smaller, faster than full Roslyn.slnx) + # -mt enables multithreaded mode which ejects non-thread-safe tasks to TaskHost. + $roslynBuildCmd = "& `"$bootstrapDotnet`" build `"$roslynDir\Compilers.slnf`" -c Debug -mt /bl:`"Q:\roslyn-build.binlog`" -v:m --no-restore" + + # Restore with bootstrap dotnet + Write-Host "[roslyn] Restoring Compilers.slnf..." -ForegroundColor Yellow + Push-Location $roslynDir + & $bootstrapDotnet restore Compilers.slnf -v:m 2>&1 | Select-Object -Last 10 + $restoreExit = $LASTEXITCODE + Pop-Location + + if ($restoreExit -ne 0) { + Write-Host "[roslyn] RESTORE FAILED (exit code $restoreExit)" -ForegroundColor Red + $results['roslyn'] = $restoreExit + } else { + $results['roslyn'] = Build-Repo -Name "roslyn" -RepoDir $roslynDir -BuildCommand $roslynBuildCmd + } + } +} + +# --- WPF (dotnet new wpf) --- +if ($Repos -eq 'wpf' -or $Repos -eq 'both') { + $wpfDir = "Q:\wpf-testproject" + + if (-not (Test-Path $wpfDir) -or -not $SkipClone) { + Write-Host "[wpf] Creating new WPF project at $wpfDir..." -ForegroundColor Yellow + if (Test-Path $wpfDir) { Remove-Item -Recurse -Force $wpfDir } + New-Item -ItemType Directory -Path $wpfDir -Force | Out-Null + + # Use system dotnet for scaffolding — temporarily unset bootstrap env to avoid conflicts + $savedDotnetRoot = $env:DOTNET_ROOT + $savedResolverDir = $env:DOTNET_MSBUILD_SDK_RESOLVER_CLI_DIR + $env:DOTNET_ROOT = $null + $env:DOTNET_MSBUILD_SDK_RESOLVER_CLI_DIR = $null + + Push-Location $wpfDir + & "C:\Program Files\dotnet\dotnet.exe" new wpf -n WpfTestApp --force 2>&1 | Write-Host + Pop-Location + + # Restore bootstrap env + $env:DOTNET_ROOT = $savedDotnetRoot + $env:DOTNET_MSBUILD_SDK_RESOLVER_CLI_DIR = $savedResolverDir + } else { + Write-Host "[wpf] Using existing WPF project at $wpfDir" -ForegroundColor Green + } + + if (Test-Path "$wpfDir\WpfTestApp") { + $wpfBuildCmd = "& `"$bootstrapDotnet`" build `"$wpfDir\WpfTestApp\WpfTestApp.csproj`" -c Debug -mt /bl:`"Q:\wpf-build.binlog`" -v:m" + + $results['wpf'] = Build-Repo -Name "wpf" -RepoDir "$wpfDir\WpfTestApp" -BuildCommand $wpfBuildCmd + } else { + Write-Host "[wpf] Project creation failed." -ForegroundColor Red + $results['wpf'] = 1 + } +} + +# --- Summary --- +Write-Host "" +Write-Host "============================================" -ForegroundColor Cyan +Write-Host " Summary" -ForegroundColor Cyan +Write-Host "============================================" -ForegroundColor Cyan + +foreach ($repo in $results.Keys) { + $code = $results[$repo] + $status = if ($code -eq 0) { "PASS" } else { "FAIL ($code)" } + $color = if ($code -eq 0) { "Green" } else { "Red" } + Write-Host " $repo : $status" -ForegroundColor $color +} + +Write-Host "" +Write-Host "Binlogs at Q:\*-build.binlog" -ForegroundColor Yellow +Write-Host "Re-run with -SkipClone after rebuilding bootstrap." -ForegroundColor Yellow diff --git a/src/Build.UnitTests/BackEnd/BuildProjectFileAndReportPidTask.cs b/src/Build.UnitTests/BackEnd/BuildProjectFileAndReportPidTask.cs index 6e8dceabf31..864a85362b1 100644 --- a/src/Build.UnitTests/BackEnd/BuildProjectFileAndReportPidTask.cs +++ b/src/Build.UnitTests/BackEnd/BuildProjectFileAndReportPidTask.cs @@ -44,7 +44,7 @@ public override bool Execute() if (!string.IsNullOrEmpty(ProjectFile)) { - Hashtable globalProperties = null; + Hashtable? globalProperties = null; if (!string.IsNullOrEmpty(Properties)) { globalProperties = new Hashtable();