From 18fe1c52b7cb845423f02ad4c2c3a4a0987d01e8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jan=20Provazn=C3=ADk?= Date: Wed, 8 Oct 2025 12:43:48 +0200 Subject: [PATCH 01/13] taskroutingdecision --- .../RequestBuilder/TaskRoutingDecision.cs | 139 ++++++++++++++++++ .../TaskExecutionHost/TaskExecutionHost.cs | 7 +- .../TaskFactories/AssemblyTaskFactory.cs | 48 ++++++ src/Build/Microsoft.Build.csproj | 1 + 4 files changed, 192 insertions(+), 3 deletions(-) create mode 100644 src/Build/BackEnd/Components/RequestBuilder/TaskRoutingDecision.cs diff --git a/src/Build/BackEnd/Components/RequestBuilder/TaskRoutingDecision.cs b/src/Build/BackEnd/Components/RequestBuilder/TaskRoutingDecision.cs new file mode 100644 index 00000000000..92b3a965a56 --- /dev/null +++ b/src/Build/BackEnd/Components/RequestBuilder/TaskRoutingDecision.cs @@ -0,0 +1,139 @@ +// 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.Concurrent; +using Microsoft.Build.Framework; +using Microsoft.Build.Shared; + +#nullable disable + +namespace Microsoft.Build.BackEnd +{ + /// + /// Determines where a task should be executed based on its characteristics. + /// In multi-threaded execution mode, tasks implementing IMultiThreadableTask or marked with + /// MSBuildMultiThreadableTaskAttribute run in-process within thread nodes, while legacy tasks + /// are routed to sidecar TaskHost processes. + /// + internal static class TaskRoutingDecision + { + /// + /// Cache of task types to their multi-threadable capability status. + /// This avoids repeated reflection calls for the same task types. + /// + private static readonly ConcurrentDictionary s_multiThreadableTaskCache = new(); + + /// + /// Determines if a task should be executed out-of-process based on its characteristics + /// and the current execution mode. + /// + /// The type of the task to evaluate. + /// Whether the build is already running in an out-of-process node. + /// Whether multi-threaded execution mode is enabled. + /// Whether out-of-process execution was explicitly requested + /// (e.g., via TaskHostFactory or task parameters). + /// + /// True if the task should be executed in an out-of-process TaskHost; false if it should + /// run in-process within a thread node. + /// + /// + /// The routing decision follows this priority: + /// 1. If already out-of-process → stay out-of-process + /// 2. If explicitly requested out-of-process → go out-of-process + /// 3. If not in multi-threaded mode → use legacy in-process behavior + /// 4. If in multi-threaded mode: + /// - Tasks implementing IMultiThreadableTask OR marked with MSBuildMultiThreadableTaskAttribute → run in-process (thread node) + /// - Tasks NOT implementing interface or attribute → run out-of-process (sidecar TaskHost) + /// + public static bool ShouldExecuteOutOfProc( + Type taskType, + bool isAlreadyOutOfProc, + bool multiThreadedMode, + bool isOutOfProcExplicitlyRequested) + { + ErrorUtilities.VerifyThrowArgumentNull(taskType, nameof(taskType)); + + // Already out-of-process? Stay there. + if (isAlreadyOutOfProc) + { + return true; + } + + // Explicitly requested out-of-process? Honor it regardless of thread-safety indicators. + if (isOutOfProcExplicitlyRequested) + { + return true; + } + + // Not in multi-threaded mode? Use legacy behavior (in-process). + if (!multiThreadedMode) + { + return false; + } + + // Multi-threaded mode: route based on thread-safety indicators (interface or attribute). + // Tasks with thread-safety indicators can safely run in-process in a thread node. + // Tasks without them must run in a sidecar TaskHost for isolation. + return !IsMultiThreadableTask(taskType); + } + + /// + /// Checks if a task is multi-threadable via IMultiThreadableTask interface OR + /// MSBuildMultiThreadableTaskAttribute attribute. + /// Results are cached to avoid repeated reflection calls. + /// + /// The task type to check. + /// True if the task has thread-safety indicators; false otherwise. + private static bool IsMultiThreadableTask(Type taskType) + { + return s_multiThreadableTaskCache.GetOrAdd( + taskType, + static t => + ImplementsIMultiThreadableTask(t) || + HasMultiThreadableTaskAttribute(t)); + } + + /// + /// Checks if a task type implements the IMultiThreadableTask interface. + /// + /// The task type to check. + /// True if the task implements IMultiThreadableTask; false otherwise. + private static bool ImplementsIMultiThreadableTask(Type taskType) + { + return typeof(IMultiThreadableTask).IsAssignableFrom(taskType); + } + + /// + /// Checks if a task type is marked with MSBuildMultiThreadableTaskAttribute. + /// Detection is based on namespace and name only, ignoring the defining assembly, + /// which allows customers to define the attribute in their own assemblies. + /// + /// The task type to check. + /// True if the task has the attribute; false otherwise. + private static bool HasMultiThreadableTaskAttribute(Type taskType) + { + const string attributeFullName = "Microsoft.Build.Framework.MSBuildMultiThreadableTaskAttribute"; + + // Check for the attribute by full name, not by type identity + // This allows custom-defined attributes from different assemblies + foreach (object attr in taskType.GetCustomAttributes(inherit: false)) + { + if (attr.GetType().FullName == attributeFullName) + { + return true; + } + } + + return false; + } + + /// + /// Clears the thread-safety capability cache. Used primarily for testing. + /// + internal static void ClearCache() + { + s_multiThreadableTaskCache.Clear(); + } + } +} diff --git a/src/Build/BackEnd/TaskExecutionHost/TaskExecutionHost.cs b/src/Build/BackEnd/TaskExecutionHost/TaskExecutionHost.cs index 205ad622651..71a9f33c981 100644 --- a/src/Build/BackEnd/TaskExecutionHost/TaskExecutionHost.cs +++ b/src/Build/BackEnd/TaskExecutionHost/TaskExecutionHost.cs @@ -1003,7 +1003,7 @@ private ITask InstantiateTask(int scheduledNodeId, IDictionary t return null; } - task = CreateTaskHostTaskForOutOfProcFactory(taskIdentityParameters, taskFactoryEngineContext, outOfProcTaskFactory); + task = CreateTaskHostTaskForOutOfProcFactory(taskIdentityParameters, taskFactoryEngineContext, outOfProcTaskFactory, scheduledNodeId); isTaskHost = true; } else @@ -1740,8 +1740,9 @@ private void DisplayCancelWaitMessage() /// Task identity parameters. /// The engine context to use for the task. /// The out-of-process task factory instance. + /// The ID of the node where the task is scheduled to run. /// A TaskHostTask that will execute the inner task out of process, or null if task creation fails. - private ITask CreateTaskHostTaskForOutOfProcFactory(IDictionary taskIdentityParameters, TaskFactoryEngineContext taskFactoryEngineContext, IOutOfProcTaskFactory outOfProcTaskFactory) + private ITask CreateTaskHostTaskForOutOfProcFactory(IDictionary taskIdentityParameters, TaskFactoryEngineContext taskFactoryEngineContext, IOutOfProcTaskFactory outOfProcTaskFactory, int scheduledNodeId) { ITask innerTask; @@ -1802,7 +1803,7 @@ private ITask CreateTaskHostTaskForOutOfProcFactory(IDictionary #if FEATURE_APPDOMAIN , AppDomainSetup #endif - ); + , scheduledNodeId); #pragma warning restore SA1111, SA1009 // Closing parenthesis should be on line of last parameter } } diff --git a/src/Build/Instance/TaskFactories/AssemblyTaskFactory.cs b/src/Build/Instance/TaskFactories/AssemblyTaskFactory.cs index b02f17b639b..bb1d480c352 100644 --- a/src/Build/Instance/TaskFactories/AssemblyTaskFactory.cs +++ b/src/Build/Instance/TaskFactories/AssemblyTaskFactory.cs @@ -342,6 +342,36 @@ internal ITask CreateTaskInstance( bool isOutOfProc, int scheduledNodeId, Func getProperty) + { + return CreateTaskInstance( + taskLocation, + taskLoggingContext, + buildComponentHost, + taskIdentityParameters, +#if FEATURE_APPDOMAIN + appDomainSetup, +#endif + isOutOfProc, + scheduledNodeId, + getProperty, + taskEnvironment: null); + } + + /// + /// Create an instance of the wrapped ITask for a batch run of the task. + /// + internal ITask CreateTaskInstance( + ElementLocation taskLocation, + TaskLoggingContext taskLoggingContext, + IBuildComponentHost buildComponentHost, + IDictionary taskIdentityParameters, +#if FEATURE_APPDOMAIN + AppDomainSetup appDomainSetup, +#endif + bool isOutOfProc, + int scheduledNodeId, + Func getProperty, + TaskEnvironment taskEnvironment) { bool useTaskFactory = false; Dictionary mergedParameters = null; @@ -365,6 +395,24 @@ internal ITask CreateTaskInstance( // as the task factory. useTaskFactory = _taskHostFactoryExplicitlyRequested; } + + // Apply multi-threaded routing decision if not already determined by other factors + // This routes tasks implementing IMultiThreadableTask to run in-process (thread nodes), + // while legacy tasks are routed to out-of-process sidecar TaskHost for isolation. + if (!useTaskFactory && _loadedType?.Type != null && buildComponentHost?.BuildParameters != null) + { + bool shouldRouteOutOfProc = TaskRoutingDecision.ShouldExecuteOutOfProc( + _loadedType.Type, + isOutOfProc, + buildComponentHost.BuildParameters.MultiThreaded, + _taskHostFactoryExplicitlyRequested); + + // If routing says to go out-of-proc, override the in-proc decision + if (shouldRouteOutOfProc) + { + useTaskFactory = true; + } + } _taskLoggingContext?.TargetLoggingContext?.ProjectLoggingContext?.ProjectTelemetry?.AddTaskExecution(GetType().FullName, isTaskHost: useTaskFactory); diff --git a/src/Build/Microsoft.Build.csproj b/src/Build/Microsoft.Build.csproj index 6b6cdec68f3..6a30c138b09 100644 --- a/src/Build/Microsoft.Build.csproj +++ b/src/Build/Microsoft.Build.csproj @@ -345,6 +345,7 @@ + From 0aff17def9bb753ee59f0386382be69a5339ab83 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jan=20Provazn=C3=ADk?= Date: Tue, 14 Oct 2025 16:31:35 +0200 Subject: [PATCH 02/13] simplify --- .../TaskFactories/AssemblyTaskFactory.cs | 31 +------------------ 1 file changed, 1 insertion(+), 30 deletions(-) diff --git a/src/Build/Instance/TaskFactories/AssemblyTaskFactory.cs b/src/Build/Instance/TaskFactories/AssemblyTaskFactory.cs index bb1d480c352..fcac3fa7f33 100644 --- a/src/Build/Instance/TaskFactories/AssemblyTaskFactory.cs +++ b/src/Build/Instance/TaskFactories/AssemblyTaskFactory.cs @@ -328,35 +328,6 @@ internal LoadedType InitializeFactory( return _loadedType; } - /// - /// Create an instance of the wrapped ITask for a batch run of the task. - /// - internal ITask CreateTaskInstance( - ElementLocation taskLocation, - TaskLoggingContext taskLoggingContext, - IBuildComponentHost buildComponentHost, - IDictionary taskIdentityParameters, -#if FEATURE_APPDOMAIN - AppDomainSetup appDomainSetup, -#endif - bool isOutOfProc, - int scheduledNodeId, - Func getProperty) - { - return CreateTaskInstance( - taskLocation, - taskLoggingContext, - buildComponentHost, - taskIdentityParameters, -#if FEATURE_APPDOMAIN - appDomainSetup, -#endif - isOutOfProc, - scheduledNodeId, - getProperty, - taskEnvironment: null); - } - /// /// Create an instance of the wrapped ITask for a batch run of the task. /// @@ -371,7 +342,7 @@ internal ITask CreateTaskInstance( bool isOutOfProc, int scheduledNodeId, Func getProperty, - TaskEnvironment taskEnvironment) + TaskEnvironment taskEnvironment = null) // TODO: taskEnvironment logic was not implemented yet { bool useTaskFactory = false; Dictionary mergedParameters = null; From 457fedd14db6fbb8b63fb47ae8f565a221614494 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jan=20Provazn=C3=ADk?= Date: Tue, 14 Oct 2025 19:01:02 +0200 Subject: [PATCH 03/13] fix test --- .../Instance/TaskFactories/AssemblyTaskFactory.cs | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/src/Build/Instance/TaskFactories/AssemblyTaskFactory.cs b/src/Build/Instance/TaskFactories/AssemblyTaskFactory.cs index fcac3fa7f33..65038662b91 100644 --- a/src/Build/Instance/TaskFactories/AssemblyTaskFactory.cs +++ b/src/Build/Instance/TaskFactories/AssemblyTaskFactory.cs @@ -366,18 +366,20 @@ internal ITask CreateTaskInstance( // as the task factory. useTaskFactory = _taskHostFactoryExplicitlyRequested; } - - // Apply multi-threaded routing decision if not already determined by other factors + + // Apply multi-threaded routing decision if not already determined by other factors. // This routes tasks implementing IMultiThreadableTask to run in-process (thread nodes), - // while legacy tasks are routed to out-of-process sidecar TaskHost for isolation. - if (!useTaskFactory && _loadedType?.Type != null && buildComponentHost?.BuildParameters != null) + // while non-enlightened tasks are routed to out-of-process sidecar TaskHost for isolation. + // Only apply this routing logic in multi-threaded mode - in traditional multi-proc builds, + // isOutOfProc indicates we're in an out-of-proc build node, not a TaskHost sidecar. + if (!useTaskFactory && _loadedType?.Type != null && buildComponentHost?.BuildParameters != null && buildComponentHost.BuildParameters.MultiThreaded) { bool shouldRouteOutOfProc = TaskRoutingDecision.ShouldExecuteOutOfProc( _loadedType.Type, isOutOfProc, buildComponentHost.BuildParameters.MultiThreaded, _taskHostFactoryExplicitlyRequested); - + // If routing says to go out-of-proc, override the in-proc decision if (shouldRouteOutOfProc) { From 4bf6646a690a4f32f329e9c326b807f2fdd64dbf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jan=20Provazn=C3=ADk?= Date: Wed, 15 Oct 2025 11:20:34 +0200 Subject: [PATCH 04/13] remove nullable disable --- .../BackEnd/Components/RequestBuilder/TaskRoutingDecision.cs | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/Build/BackEnd/Components/RequestBuilder/TaskRoutingDecision.cs b/src/Build/BackEnd/Components/RequestBuilder/TaskRoutingDecision.cs index 92b3a965a56..e5b3299db65 100644 --- a/src/Build/BackEnd/Components/RequestBuilder/TaskRoutingDecision.cs +++ b/src/Build/BackEnd/Components/RequestBuilder/TaskRoutingDecision.cs @@ -6,8 +6,6 @@ using Microsoft.Build.Framework; using Microsoft.Build.Shared; -#nullable disable - namespace Microsoft.Build.BackEnd { /// From f688183081d0dd4b06818f896380ecc84f570f64 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jan=20Provazn=C3=ADk?= Date: Wed, 15 Oct 2025 15:13:26 +0200 Subject: [PATCH 05/13] simplify --- .../RequestBuilder/TaskRoutingDecision.cs | 65 +++++++------------ .../TaskFactories/AssemblyTaskFactory.cs | 21 ++---- 2 files changed, 28 insertions(+), 58 deletions(-) diff --git a/src/Build/BackEnd/Components/RequestBuilder/TaskRoutingDecision.cs b/src/Build/BackEnd/Components/RequestBuilder/TaskRoutingDecision.cs index e5b3299db65..9c3a24ab961 100644 --- a/src/Build/BackEnd/Components/RequestBuilder/TaskRoutingDecision.cs +++ b/src/Build/BackEnd/Components/RequestBuilder/TaskRoutingDecision.cs @@ -9,11 +9,15 @@ namespace Microsoft.Build.BackEnd { /// - /// Determines where a task should be executed based on its characteristics. + /// Determines where a task should be executed in multi-threaded mode. /// In multi-threaded execution mode, tasks implementing IMultiThreadableTask or marked with /// MSBuildMultiThreadableTaskAttribute run in-process within thread nodes, while legacy tasks - /// are routed to sidecar TaskHost processes. + /// are routed to sidecar TaskHost processes for isolation. /// + /// + /// This class should only be used when in multi-threaded mode. Traditional multi-proc builds + /// have different semantics and should not use this routing logic. + /// internal static class TaskRoutingDecision { /// @@ -23,56 +27,31 @@ internal static class TaskRoutingDecision private static readonly ConcurrentDictionary s_multiThreadableTaskCache = new(); /// - /// Determines if a task should be executed out-of-process based on its characteristics - /// and the current execution mode. + /// Determines if a task needs to be routed to an out-of-process TaskHost sidecar + /// in multi-threaded mode based on its thread-safety characteristics. /// /// The type of the task to evaluate. - /// Whether the build is already running in an out-of-process node. - /// Whether multi-threaded execution mode is enabled. - /// Whether out-of-process execution was explicitly requested - /// (e.g., via TaskHostFactory or task parameters). /// - /// True if the task should be executed in an out-of-process TaskHost; false if it should - /// run in-process within a thread node. + /// True if the task should be executed in an out-of-process TaskHost sidecar; + /// false if it can safely run in-process within a thread node. /// /// - /// The routing decision follows this priority: - /// 1. If already out-of-process → stay out-of-process - /// 2. If explicitly requested out-of-process → go out-of-process - /// 3. If not in multi-threaded mode → use legacy in-process behavior - /// 4. If in multi-threaded mode: - /// - Tasks implementing IMultiThreadableTask OR marked with MSBuildMultiThreadableTaskAttribute → run in-process (thread node) - /// - Tasks NOT implementing interface or attribute → run out-of-process (sidecar TaskHost) + /// This method only considers the task's thread-safety indicators. + /// The caller is responsible for: + /// - Only calling this in multi-threaded mode + /// - Handling explicit out-of-proc requests (via TaskHostFactory or parameters) + /// - Handling the isAlreadyOutOfProc scenario + /// + /// In multi-threaded mode: + /// - Tasks implementing IMultiThreadableTask OR marked with MSBuildMultiThreadableTaskAttribute + /// are considered thread-safe and can run in-process (returns false) + /// - Tasks without these indicators must run in a sidecar TaskHost for isolation (returns true) /// - public static bool ShouldExecuteOutOfProc( - Type taskType, - bool isAlreadyOutOfProc, - bool multiThreadedMode, - bool isOutOfProcExplicitlyRequested) + public static bool NeedsTaskHostInMultiThreadedMode(Type taskType) { ErrorUtilities.VerifyThrowArgumentNull(taskType, nameof(taskType)); - // Already out-of-process? Stay there. - if (isAlreadyOutOfProc) - { - return true; - } - - // Explicitly requested out-of-process? Honor it regardless of thread-safety indicators. - if (isOutOfProcExplicitlyRequested) - { - return true; - } - - // Not in multi-threaded mode? Use legacy behavior (in-process). - if (!multiThreadedMode) - { - return false; - } - - // Multi-threaded mode: route based on thread-safety indicators (interface or attribute). - // Tasks with thread-safety indicators can safely run in-process in a thread node. - // Tasks without them must run in a sidecar TaskHost for isolation. + // Tasks without thread-safety indicators need isolation in a TaskHost sidecar return !IsMultiThreadableTask(taskType); } diff --git a/src/Build/Instance/TaskFactories/AssemblyTaskFactory.cs b/src/Build/Instance/TaskFactories/AssemblyTaskFactory.cs index ab559c3c22a..26e78eae169 100644 --- a/src/Build/Instance/TaskFactories/AssemblyTaskFactory.cs +++ b/src/Build/Instance/TaskFactories/AssemblyTaskFactory.cs @@ -367,21 +367,12 @@ internal ITask CreateTaskInstance( useTaskFactory = _taskHostFactoryExplicitlyRequested; } - // Apply multi-threaded routing decision if not already determined by other factors. - // This routes tasks implementing IMultiThreadableTask to run in-process (thread nodes), - // while non-enlightened tasks are routed to out-of-process sidecar TaskHost for isolation. - // Only apply this routing logic in multi-threaded mode - in traditional multi-proc builds, - // isOutOfProc indicates we're in an out-of-proc build node, not a TaskHost sidecar. - if (!useTaskFactory && _loadedType?.Type != null && buildComponentHost?.BuildParameters != null && buildComponentHost.BuildParameters.MultiThreaded) - { - bool shouldRouteOutOfProc = TaskRoutingDecision.ShouldExecuteOutOfProc( - _loadedType.Type, - isOutOfProc, - buildComponentHost.BuildParameters.MultiThreaded, - _taskHostFactoryExplicitlyRequested); - - // If routing says to go out-of-proc, override the in-proc decision - if (shouldRouteOutOfProc) + // Multi-threaded mode routing: Determine if non-thread-safe tasks need TaskHost isolation. + if (!useTaskFactory + && _loadedType?.Type != null + && buildComponentHost?.BuildParameters?.MultiThreaded == true) + { + if (TaskRoutingDecision.NeedsTaskHostInMultiThreadedMode(_loadedType.Type)) { useTaskFactory = true; } From 3047a3e6b47a7e691f70d6f4b6b862f613fcc9fc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jan=20Provazn=C3=ADk?= Date: Wed, 15 Oct 2025 15:37:20 +0200 Subject: [PATCH 06/13] integration tests --- .../BackEnd/TaskRouting_IntegrationTests.cs | 419 ++++++++++++++++++ 1 file changed, 419 insertions(+) create mode 100644 src/Build.UnitTests/BackEnd/TaskRouting_IntegrationTests.cs diff --git a/src/Build.UnitTests/BackEnd/TaskRouting_IntegrationTests.cs b/src/Build.UnitTests/BackEnd/TaskRouting_IntegrationTests.cs new file mode 100644 index 00000000000..b8750c993b5 --- /dev/null +++ b/src/Build.UnitTests/BackEnd/TaskRouting_IntegrationTests.cs @@ -0,0 +1,419 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System; +using System.Collections.Generic; +using System.IO; +using System.Reflection; +using Microsoft.Build.Execution; +using Microsoft.Build.Framework; +using Microsoft.Build.Shared; +using Microsoft.Build.UnitTests; +using Microsoft.Build.UnitTests.Shared; +using Microsoft.Build.Utilities; +using Shouldly; +using Xunit; +using Xunit.Abstractions; + +#nullable disable + +namespace Microsoft.Build.Engine.UnitTests.BackEnd +{ + /// + /// Integration tests for task routing in multi-threaded mode. + /// Tests verify that tasks with IMultiThreadableTask or MSBuildMultiThreadableTaskAttribute + /// run in-process, while tasks without these indicators run in TaskHost for isolation. + /// + public class TaskRouting_IntegrationTests : IDisposable + { + private readonly ITestOutputHelper _output; + private readonly TestEnvironment _env; + private readonly string _testProjectsDir; + + public TaskRouting_IntegrationTests(ITestOutputHelper output) + { + _output = output; + _env = TestEnvironment.Create(output); + + // Create directory for test projects + _testProjectsDir = _env.CreateFolder().Path; + } + + public void Dispose() + { + _env.Dispose(); + } + + /// + /// Verifies that a legacy task (no interface, no attribute) runs in TaskHost + /// when MultiThreaded mode is enabled. + /// + [Fact] + public void NonEnlightenedTask_RunsInTaskHost_InMultiThreadedMode() + { + // Arrange + string projectContent = CreateTestProject( + taskName: "LegacyTestTask", + taskClass: "NonEnlightenedTask"); + + string projectFile = Path.Combine(_testProjectsDir, "NonEnlightenedTaskProject.proj"); + File.WriteAllText(projectFile, projectContent); + + var logger = new MockLogger(_output); + var buildParameters = new BuildParameters + { + MultiThreaded = true, + Loggers = new[] { logger }, + DisableInProcNode = false, + EnableNodeReuse = false + }; + + var buildRequestData = new BuildRequestData( + projectFile, + new Dictionary(), + null, + new[] { "TestTarget" }, + null); + + // Act + var buildManager = BuildManager.DefaultBuildManager; + var result = buildManager.Build(buildParameters, buildRequestData); + + // Assert + result.OverallResult.ShouldBe(BuildResultCode.Success); + + // Verify task was launched in TaskHost + logger.FullLog.ShouldContain("Launching task \"LegacyTestTask\""); + logger.FullLog.ShouldContain("external task host"); + + // Verify task executed successfully + logger.FullLog.ShouldContain("NonEnlightenedTask executed"); + } + + /// + /// Verifies that a task with IMultiThreadableTask interface runs in-process + /// (not in TaskHost) when MultiThreaded mode is enabled. + /// + [Fact] + public void TaskWithInterface_RunsInProcess_InMultiThreadedMode() + { + // Arrange + string projectContent = CreateTestProject( + taskName: "InterfaceTestTask", + taskClass: "TaskWithInterface"); + + string projectFile = Path.Combine(_testProjectsDir, "InterfaceTaskProject.proj"); + File.WriteAllText(projectFile, projectContent); + + var logger = new MockLogger(_output); + var buildParameters = new BuildParameters + { + MultiThreaded = true, + Loggers = new[] { logger }, + DisableInProcNode = false, + EnableNodeReuse = false + }; + + var buildRequestData = new BuildRequestData( + projectFile, + new Dictionary(), + null, + new[] { "TestTarget" }, + null); + + // Act + var buildManager = BuildManager.DefaultBuildManager; + var result = buildManager.Build(buildParameters, buildRequestData); + + // Assert + result.OverallResult.ShouldBe(BuildResultCode.Success); + + // Verify task was NOT launched in TaskHost (runs in-process) + logger.FullLog.ShouldNotContain("Launching task \"InterfaceTestTask\""); + logger.FullLog.ShouldNotContain("external task host"); + + // Verify task executed successfully + logger.FullLog.ShouldContain("TaskWithInterface executed"); + } + + /// + /// Verifies that a task with MSBuildMultiThreadableTaskAttribute runs in-process + /// (not in TaskHost) when MultiThreaded mode is enabled. + /// + [Fact] + public void TaskWithAttribute_RunsInProcess_InMultiThreadedMode() + { + // Arrange + string projectContent = CreateTestProject( + taskName: "AttributeTestTask", + taskClass: "TaskWithAttribute"); + + string projectFile = Path.Combine(_testProjectsDir, "AttributeTaskProject.proj"); + File.WriteAllText(projectFile, projectContent); + + var logger = new MockLogger(_output); + var buildParameters = new BuildParameters + { + MultiThreaded = true, + Loggers = new[] { logger }, + DisableInProcNode = false, + EnableNodeReuse = false + }; + + var buildRequestData = new BuildRequestData( + projectFile, + new Dictionary(), + null, + new[] { "TestTarget" }, + null); + + // Act + var buildManager = BuildManager.DefaultBuildManager; + var result = buildManager.Build(buildParameters, buildRequestData); + + // Assert + result.OverallResult.ShouldBe(BuildResultCode.Success); + + // Verify task was NOT launched in TaskHost (runs in-process) + logger.FullLog.ShouldNotContain("Launching task \"AttributeTestTask\""); + logger.FullLog.ShouldNotContain("external task host"); + + // Verify task executed successfully + logger.FullLog.ShouldContain("TaskWithAttribute executed"); + } + + /// + /// Verifies that when MultiThreaded mode is disabled, even legacy tasks + /// run in-process and do not use TaskHost. + /// + [Fact] + public void NonEnlightenedTask_RunsInProcess_WhenMultiThreadedModeDisabled() + { + // Arrange + string projectContent = CreateTestProject( + taskName: "LegacyTestTask", + taskClass: "NonEnlightenedTask"); + + string projectFile = Path.Combine(_testProjectsDir, "NonEnlightenedTaskSingleThreaded.proj"); + File.WriteAllText(projectFile, projectContent); + + var logger = new MockLogger(_output); + var buildParameters = new BuildParameters + { + MultiThreaded = false, // Single-threaded mode + Loggers = new[] { logger }, + DisableInProcNode = false, + EnableNodeReuse = false + }; + + var buildRequestData = new BuildRequestData( + projectFile, + new Dictionary(), + null, + new[] { "TestTarget" }, + null); + + // Act + var buildManager = BuildManager.DefaultBuildManager; + var result = buildManager.Build(buildParameters, buildRequestData); + + // Assert + result.OverallResult.ShouldBe(BuildResultCode.Success); + + // Verify task was NOT launched in TaskHost (runs in-process even though it's legacy) + logger.FullLog.ShouldNotContain("Launching task \"LegacyTestTask\""); + logger.FullLog.ShouldNotContain("external task host"); + + // Verify task executed successfully + logger.FullLog.ShouldContain("NonEnlightenedTask executed"); + } + + /// + /// Verifies that a task with interface doesn't use TaskHost in single-threaded mode. + /// + [Fact] + public void TaskWithInterface_RunsInProcess_WhenMultiThreadedModeDisabled() + { + // Arrange + string projectContent = CreateTestProject( + taskName: "InterfaceTestTask", + taskClass: "TaskWithInterface"); + + string projectFile = Path.Combine(_testProjectsDir, "InterfaceTaskSingleThreaded.proj"); + File.WriteAllText(projectFile, projectContent); + + var logger = new MockLogger(_output); + var buildParameters = new BuildParameters + { + MultiThreaded = false, // Single-threaded mode + Loggers = new[] { logger }, + DisableInProcNode = false, + EnableNodeReuse = false + }; + + var buildRequestData = new BuildRequestData( + projectFile, + new Dictionary(), + null, + new[] { "TestTarget" }, + null); + + // Act + var buildManager = BuildManager.DefaultBuildManager; + var result = buildManager.Build(buildParameters, buildRequestData); + + // Assert + result.OverallResult.ShouldBe(BuildResultCode.Success); + + // Verify task was NOT launched in TaskHost + logger.FullLog.ShouldNotContain("Launching task \"InterfaceTestTask\""); + logger.FullLog.ShouldNotContain("external task host"); + + // Verify task executed successfully + logger.FullLog.ShouldContain("TaskWithInterface executed"); + } + + /// + /// Verifies that multiple task types in the same build are routed correctly + /// based on their characteristics in multi-threaded mode. + /// + [Fact] + public void MixedTasks_RouteCorrectly_InMultiThreadedMode() + { + // Arrange + string projectContent = $@" + + + + + + + + + + +"; + + string projectFile = Path.Combine(_testProjectsDir, "MixedTasksProject.proj"); + File.WriteAllText(projectFile, projectContent); + + var logger = new MockLogger(_output); + var buildParameters = new BuildParameters + { + MultiThreaded = true, + Loggers = new[] { logger }, + DisableInProcNode = false, + EnableNodeReuse = false + }; + + var buildRequestData = new BuildRequestData( + projectFile, + new Dictionary(), + null, + new[] { "TestTarget" }, + null); + + // Act + var buildManager = BuildManager.DefaultBuildManager; + var result = buildManager.Build(buildParameters, buildRequestData); + + // Assert + result.OverallResult.ShouldBe(BuildResultCode.Success); + + // NonEnlightenedTask should use TaskHost + logger.FullLog.ShouldContain("Launching task \"LegacyTestTask\""); + + // Interface and Attribute tasks should NOT use TaskHost + logger.FullLog.ShouldNotContain("Launching task \"InterfaceTestTask\""); + logger.FullLog.ShouldNotContain("Launching task \"AttributeTestTask\""); + + // All tasks should execute successfully + logger.FullLog.ShouldContain("NonEnlightenedTask executed"); + logger.FullLog.ShouldContain("TaskWithInterface executed"); + logger.FullLog.ShouldContain("TaskWithAttribute executed"); + } + + private string CreateTestProject(string taskName, string taskClass) + { + return $@" + + + + + <{taskName} /> + +"; + } + } + + #region Test Task Implementations + + /// + /// Legacy task without IMultiThreadableTask interface or MSBuildMultiThreadableTaskAttribute. + /// Should run in TaskHost in multi-threaded mode. + /// + public class LegacyTestTask : Task + { + public override bool Execute() + { + Log.LogMessage(MessageImportance.High, "NonEnlightenedTask executed"); + return true; + } + } + + /// + /// Task implementing IMultiThreadableTask interface. + /// Should run in-process in multi-threaded mode. + /// + public class InterfaceTestTask : Task, IMultiThreadableTask + { + public TaskEnvironment TaskEnvironment { get; set; } + + public override bool Execute() + { + Log.LogMessage(MessageImportance.High, "TaskWithInterface executed"); + return true; + } + } + + #endregion +} + +// Custom attribute definition in Microsoft.Build.Framework namespace to match what TaskRoutingDecision expects +// TaskRoutingDecision looks for attributes with FullName = "Microsoft.Build.Framework.MSBuildMultiThreadableTaskAttribute" +// Since the real attribute is internal in Framework, we define our own test version here +namespace Microsoft.Build.Framework +{ + /// + /// Test attribute to mark tasks as safe for multi-threaded execution. + /// This is a test copy in this test assembly that will be recognized + /// by name-based attribute detection in TaskRoutingDecision. + /// + [AttributeUsage(AttributeTargets.Class, Inherited = false, AllowMultiple = false)] + public sealed class MSBuildMultiThreadableTaskAttribute : Attribute + { + } +} + +namespace Microsoft.Build.Engine.UnitTests.BackEnd +{ + /// + /// Task marked with MSBuildMultiThreadableTaskAttribute. + /// Should run in-process in multi-threaded mode. + /// + /// + /// Uses the public test version of MSBuildMultiThreadableTaskAttribute defined in this file, + /// which shadows the internal Framework version intentionally for testing. + /// +#pragma warning disable CS0436 // Type conflicts with imported type - intentional for testing + [MSBuildMultiThreadableTask] +#pragma warning restore CS0436 + public class AttributeTestTask : Task + { + public override bool Execute() + { + Log.LogMessage(MessageImportance.High, "TaskWithAttribute executed"); + return true; + } + } +} From 0330a84198f7b994e6672ba6dd5981d2dfe86a6d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jan=20Provazn=C3=ADk?= Date: Wed, 15 Oct 2025 15:49:23 +0200 Subject: [PATCH 07/13] pr feedback --- .../BackEnd/TaskRouting_IntegrationTests.cs | 24 +++++++++---------- .../TaskFactories/AssemblyTaskFactory.cs | 3 +-- 2 files changed, 13 insertions(+), 14 deletions(-) diff --git a/src/Build.UnitTests/BackEnd/TaskRouting_IntegrationTests.cs b/src/Build.UnitTests/BackEnd/TaskRouting_IntegrationTests.cs index b8750c993b5..9f0b70cc7ec 100644 --- a/src/Build.UnitTests/BackEnd/TaskRouting_IntegrationTests.cs +++ b/src/Build.UnitTests/BackEnd/TaskRouting_IntegrationTests.cs @@ -45,7 +45,7 @@ public void Dispose() } /// - /// Verifies that a legacy task (no interface, no attribute) runs in TaskHost + /// Verifies that a NonEnlightened task (no interface, no attribute) runs in TaskHost /// when MultiThreaded mode is enabled. /// [Fact] @@ -53,7 +53,7 @@ public void NonEnlightenedTask_RunsInTaskHost_InMultiThreadedMode() { // Arrange string projectContent = CreateTestProject( - taskName: "LegacyTestTask", + taskName: "NonEnlightenedTestTask", taskClass: "NonEnlightenedTask"); string projectFile = Path.Combine(_testProjectsDir, "NonEnlightenedTaskProject.proj"); @@ -83,7 +83,7 @@ public void NonEnlightenedTask_RunsInTaskHost_InMultiThreadedMode() result.OverallResult.ShouldBe(BuildResultCode.Success); // Verify task was launched in TaskHost - logger.FullLog.ShouldContain("Launching task \"LegacyTestTask\""); + logger.FullLog.ShouldContain("Launching task \"NonEnlightenedTestTask\""); logger.FullLog.ShouldContain("external task host"); // Verify task executed successfully @@ -183,7 +183,7 @@ public void TaskWithAttribute_RunsInProcess_InMultiThreadedMode() } /// - /// Verifies that when MultiThreaded mode is disabled, even legacy tasks + /// Verifies that when MultiThreaded mode is disabled, even NonEnlightened tasks /// run in-process and do not use TaskHost. /// [Fact] @@ -191,7 +191,7 @@ public void NonEnlightenedTask_RunsInProcess_WhenMultiThreadedModeDisabled() { // Arrange string projectContent = CreateTestProject( - taskName: "LegacyTestTask", + taskName: "NonEnlightenedTestTask", taskClass: "NonEnlightenedTask"); string projectFile = Path.Combine(_testProjectsDir, "NonEnlightenedTaskSingleThreaded.proj"); @@ -220,8 +220,8 @@ public void NonEnlightenedTask_RunsInProcess_WhenMultiThreadedModeDisabled() // Assert result.OverallResult.ShouldBe(BuildResultCode.Success); - // Verify task was NOT launched in TaskHost (runs in-process even though it's legacy) - logger.FullLog.ShouldNotContain("Launching task \"LegacyTestTask\""); + // Verify task was NOT launched in TaskHost (runs in-process even though it's NonEnlightened) + logger.FullLog.ShouldNotContain("Launching task \"NonEnlightenedTestTask\""); logger.FullLog.ShouldNotContain("external task host"); // Verify task executed successfully @@ -283,12 +283,12 @@ public void MixedTasks_RouteCorrectly_InMultiThreadedMode() // Arrange string projectContent = $@" - + - + @@ -321,7 +321,7 @@ public void MixedTasks_RouteCorrectly_InMultiThreadedMode() result.OverallResult.ShouldBe(BuildResultCode.Success); // NonEnlightenedTask should use TaskHost - logger.FullLog.ShouldContain("Launching task \"LegacyTestTask\""); + logger.FullLog.ShouldContain("Launching task \"NonEnlightenedTestTask\""); // Interface and Attribute tasks should NOT use TaskHost logger.FullLog.ShouldNotContain("Launching task \"InterfaceTestTask\""); @@ -349,10 +349,10 @@ private string CreateTestProject(string taskName, string taskClass) #region Test Task Implementations /// - /// Legacy task without IMultiThreadableTask interface or MSBuildMultiThreadableTaskAttribute. + /// NonEnlightened task without IMultiThreadableTask interface or MSBuildMultiThreadableTaskAttribute. /// Should run in TaskHost in multi-threaded mode. /// - public class LegacyTestTask : Task + public class NonEnlightenedTestTask : Task { public override bool Execute() { diff --git a/src/Build/Instance/TaskFactories/AssemblyTaskFactory.cs b/src/Build/Instance/TaskFactories/AssemblyTaskFactory.cs index 26e78eae169..9faee184299 100644 --- a/src/Build/Instance/TaskFactories/AssemblyTaskFactory.cs +++ b/src/Build/Instance/TaskFactories/AssemblyTaskFactory.cs @@ -341,8 +341,7 @@ internal ITask CreateTaskInstance( #endif bool isOutOfProc, int scheduledNodeId, - Func getProperty, - TaskEnvironment taskEnvironment = null) // TODO: taskEnvironment logic was not implemented yet + Func getProperty) { bool useTaskFactory = false; Dictionary mergedParameters = null; From f78f681bd3868a7744773ed5625259c6ec06ae87 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jan=20Provazn=C3=ADk?= Date: Wed, 15 Oct 2025 16:00:57 +0200 Subject: [PATCH 08/13] override test --- .../BackEnd/TaskRouting_IntegrationTests.cs | 106 +++++++++++++++--- 1 file changed, 93 insertions(+), 13 deletions(-) diff --git a/src/Build.UnitTests/BackEnd/TaskRouting_IntegrationTests.cs b/src/Build.UnitTests/BackEnd/TaskRouting_IntegrationTests.cs index 9f0b70cc7ec..a32e8dbe203 100644 --- a/src/Build.UnitTests/BackEnd/TaskRouting_IntegrationTests.cs +++ b/src/Build.UnitTests/BackEnd/TaskRouting_IntegrationTests.cs @@ -83,8 +83,7 @@ public void NonEnlightenedTask_RunsInTaskHost_InMultiThreadedMode() result.OverallResult.ShouldBe(BuildResultCode.Success); // Verify task was launched in TaskHost - logger.FullLog.ShouldContain("Launching task \"NonEnlightenedTestTask\""); - logger.FullLog.ShouldContain("external task host"); + TaskRoutingTestHelper.AssertTaskUsedTaskHost(logger, "NonEnlightenedTestTask"); // Verify task executed successfully logger.FullLog.ShouldContain("NonEnlightenedTask executed"); @@ -129,8 +128,7 @@ public void TaskWithInterface_RunsInProcess_InMultiThreadedMode() result.OverallResult.ShouldBe(BuildResultCode.Success); // Verify task was NOT launched in TaskHost (runs in-process) - logger.FullLog.ShouldNotContain("Launching task \"InterfaceTestTask\""); - logger.FullLog.ShouldNotContain("external task host"); + TaskRoutingTestHelper.AssertTaskRanInProcess(logger, "InterfaceTestTask"); // Verify task executed successfully logger.FullLog.ShouldContain("TaskWithInterface executed"); @@ -175,8 +173,7 @@ public void TaskWithAttribute_RunsInProcess_InMultiThreadedMode() result.OverallResult.ShouldBe(BuildResultCode.Success); // Verify task was NOT launched in TaskHost (runs in-process) - logger.FullLog.ShouldNotContain("Launching task \"AttributeTestTask\""); - logger.FullLog.ShouldNotContain("external task host"); + TaskRoutingTestHelper.AssertTaskRanInProcess(logger, "AttributeTestTask"); // Verify task executed successfully logger.FullLog.ShouldContain("TaskWithAttribute executed"); @@ -221,8 +218,7 @@ public void NonEnlightenedTask_RunsInProcess_WhenMultiThreadedModeDisabled() result.OverallResult.ShouldBe(BuildResultCode.Success); // Verify task was NOT launched in TaskHost (runs in-process even though it's NonEnlightened) - logger.FullLog.ShouldNotContain("Launching task \"NonEnlightenedTestTask\""); - logger.FullLog.ShouldNotContain("external task host"); + TaskRoutingTestHelper.AssertTaskRanInProcess(logger, "NonEnlightenedTestTask"); // Verify task executed successfully logger.FullLog.ShouldContain("NonEnlightenedTask executed"); @@ -266,8 +262,7 @@ public void TaskWithInterface_RunsInProcess_WhenMultiThreadedModeDisabled() result.OverallResult.ShouldBe(BuildResultCode.Success); // Verify task was NOT launched in TaskHost - logger.FullLog.ShouldNotContain("Launching task \"InterfaceTestTask\""); - logger.FullLog.ShouldNotContain("external task host"); + TaskRoutingTestHelper.AssertTaskRanInProcess(logger, "InterfaceTestTask"); // Verify task executed successfully logger.FullLog.ShouldContain("TaskWithInterface executed"); @@ -321,11 +316,11 @@ public void MixedTasks_RouteCorrectly_InMultiThreadedMode() result.OverallResult.ShouldBe(BuildResultCode.Success); // NonEnlightenedTask should use TaskHost - logger.FullLog.ShouldContain("Launching task \"NonEnlightenedTestTask\""); + TaskRoutingTestHelper.AssertTaskUsedTaskHost(logger, "NonEnlightenedTestTask"); // Interface and Attribute tasks should NOT use TaskHost - logger.FullLog.ShouldNotContain("Launching task \"InterfaceTestTask\""); - logger.FullLog.ShouldNotContain("Launching task \"AttributeTestTask\""); + TaskRoutingTestHelper.AssertTaskRanInProcess(logger, "InterfaceTestTask"); + TaskRoutingTestHelper.AssertTaskRanInProcess(logger, "AttributeTestTask"); // All tasks should execute successfully logger.FullLog.ShouldContain("NonEnlightenedTask executed"); @@ -333,6 +328,59 @@ public void MixedTasks_RouteCorrectly_InMultiThreadedMode() logger.FullLog.ShouldContain("TaskWithAttribute executed"); } + /// + /// Verifies that explicit TaskHostFactory request overrides routing logic, + /// forcing tasks to run in TaskHost even if they would normally run in-process. + /// + [Fact] + public void ExplicitTaskHostFactory_OverridesRoutingLogic() + { + // Arrange - Use a thread-safe task but explicitly request TaskHostFactory + string projectContent = $@" + + + + + + +"; + + string projectFile = Path.Combine(_testProjectsDir, "ExplicitTaskHostFactory.proj"); + File.WriteAllText(projectFile, projectContent); + + var logger = new MockLogger(_output); + var buildParameters = new BuildParameters + { + MultiThreaded = true, + Loggers = new[] { logger }, + DisableInProcNode = false, + EnableNodeReuse = false + }; + + var buildRequestData = new BuildRequestData( + projectFile, + new Dictionary(), + null, + new[] { "TestTarget" }, + null); + + // Act + var buildManager = BuildManager.DefaultBuildManager; + var result = buildManager.Build(buildParameters, buildRequestData); + + // Assert + result.OverallResult.ShouldBe(BuildResultCode.Success); + + // Task should use TaskHost because TaskHostFactory was explicitly requested + // This overrides the normal routing logic which would run this in-process + TaskRoutingTestHelper.AssertTaskUsedTaskHost(logger, "InterfaceTestTask"); + + // Verify task executed successfully + logger.FullLog.ShouldContain("TaskWithInterface executed"); + } + private string CreateTestProject(string taskName, string taskClass) { return $@" @@ -346,6 +394,38 @@ private string CreateTestProject(string taskName, string taskClass) } } + /// + /// Helper utilities for testing task routing behavior. + /// Provides robust assertions that are less fragile than raw log string matching. + /// + internal static class TaskRoutingTestHelper + { + /// + /// Asserts that a task was launched in an external TaskHost process. + /// + /// The build logger containing execution logs. + /// The name of the task to verify. + public static void AssertTaskUsedTaskHost(MockLogger logger, string taskName) + { + // Look for the distinctive "Launching task" message that indicates TaskHost usage + string launchingMessage = $"Launching task \"{taskName}\""; + logger.FullLog.ShouldContain(launchingMessage); + logger.FullLog.ShouldContain("external task host"); + } + + /// + /// Asserts that a task ran in-process (not in TaskHost). + /// + /// The build logger containing execution logs. + /// The name of the task to verify. + public static void AssertTaskRanInProcess(MockLogger logger, string taskName) + { + // Verify the "Launching task" message does NOT appear for this task + string launchingMessage = $"Launching task \"{taskName}\""; + logger.FullLog.ShouldNotContain(launchingMessage); + } + } + #region Test Task Implementations /// From 6d81044a81dd2f75865f9d56b0bbdee1ab66b7b8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jan=20Provazn=C3=ADk?= Date: Thu, 16 Oct 2025 14:02:05 +0200 Subject: [PATCH 09/13] pr feedback --- ...ests.cs => TaskRouter_IntegrationTests.cs} | 159 +++++++++++++----- .../BackEnd/BuildManager/BuildManager.cs | 2 + .../{TaskRoutingDecision.cs => TaskRouter.cs} | 28 ++- .../TaskFactories/AssemblyTaskFactory.cs | 2 +- src/Build/Microsoft.Build.csproj | 2 +- 5 files changed, 144 insertions(+), 49 deletions(-) rename src/Build.UnitTests/BackEnd/{TaskRouting_IntegrationTests.cs => TaskRouter_IntegrationTests.cs} (80%) rename src/Build/BackEnd/Components/RequestBuilder/{TaskRoutingDecision.cs => TaskRouter.cs} (78%) diff --git a/src/Build.UnitTests/BackEnd/TaskRouting_IntegrationTests.cs b/src/Build.UnitTests/BackEnd/TaskRouter_IntegrationTests.cs similarity index 80% rename from src/Build.UnitTests/BackEnd/TaskRouting_IntegrationTests.cs rename to src/Build.UnitTests/BackEnd/TaskRouter_IntegrationTests.cs index a32e8dbe203..8dd9e1c7ce9 100644 --- a/src/Build.UnitTests/BackEnd/TaskRouting_IntegrationTests.cs +++ b/src/Build.UnitTests/BackEnd/TaskRouter_IntegrationTests.cs @@ -24,17 +24,17 @@ namespace Microsoft.Build.Engine.UnitTests.BackEnd /// Tests verify that tasks with IMultiThreadableTask or MSBuildMultiThreadableTaskAttribute /// run in-process, while tasks without these indicators run in TaskHost for isolation. /// - public class TaskRouting_IntegrationTests : IDisposable + public class TaskRouter_IntegrationTests : IDisposable { private readonly ITestOutputHelper _output; private readonly TestEnvironment _env; private readonly string _testProjectsDir; - public TaskRouting_IntegrationTests(ITestOutputHelper output) + public TaskRouter_IntegrationTests(ITestOutputHelper output) { _output = output; _env = TestEnvironment.Create(output); - + // Create directory for test projects _testProjectsDir = _env.CreateFolder().Path; } @@ -81,10 +81,10 @@ public void NonEnlightenedTask_RunsInTaskHost_InMultiThreadedMode() // Assert result.OverallResult.ShouldBe(BuildResultCode.Success); - + // Verify task was launched in TaskHost - TaskRoutingTestHelper.AssertTaskUsedTaskHost(logger, "NonEnlightenedTestTask"); - + TaskRouterTestHelper.AssertTaskUsedTaskHost(logger, "NonEnlightenedTestTask"); + // Verify task executed successfully logger.FullLog.ShouldContain("NonEnlightenedTask executed"); } @@ -126,10 +126,10 @@ public void TaskWithInterface_RunsInProcess_InMultiThreadedMode() // Assert result.OverallResult.ShouldBe(BuildResultCode.Success); - + // Verify task was NOT launched in TaskHost (runs in-process) - TaskRoutingTestHelper.AssertTaskRanInProcess(logger, "InterfaceTestTask"); - + TaskRouterTestHelper.AssertTaskRanInProcess(logger, "InterfaceTestTask"); + // Verify task executed successfully logger.FullLog.ShouldContain("TaskWithInterface executed"); } @@ -171,10 +171,10 @@ public void TaskWithAttribute_RunsInProcess_InMultiThreadedMode() // Assert result.OverallResult.ShouldBe(BuildResultCode.Success); - + // Verify task was NOT launched in TaskHost (runs in-process) - TaskRoutingTestHelper.AssertTaskRanInProcess(logger, "AttributeTestTask"); - + TaskRouterTestHelper.AssertTaskRanInProcess(logger, "AttributeTestTask"); + // Verify task executed successfully logger.FullLog.ShouldContain("TaskWithAttribute executed"); } @@ -216,10 +216,10 @@ public void NonEnlightenedTask_RunsInProcess_WhenMultiThreadedModeDisabled() // Assert result.OverallResult.ShouldBe(BuildResultCode.Success); - + // Verify task was NOT launched in TaskHost (runs in-process even though it's NonEnlightened) - TaskRoutingTestHelper.AssertTaskRanInProcess(logger, "NonEnlightenedTestTask"); - + TaskRouterTestHelper.AssertTaskRanInProcess(logger, "NonEnlightenedTestTask"); + // Verify task executed successfully logger.FullLog.ShouldContain("NonEnlightenedTask executed"); } @@ -260,10 +260,10 @@ public void TaskWithInterface_RunsInProcess_WhenMultiThreadedModeDisabled() // Assert result.OverallResult.ShouldBe(BuildResultCode.Success); - + // Verify task was NOT launched in TaskHost - TaskRoutingTestHelper.AssertTaskRanInProcess(logger, "InterfaceTestTask"); - + TaskRouterTestHelper.AssertTaskRanInProcess(logger, "InterfaceTestTask"); + // Verify task executed successfully logger.FullLog.ShouldContain("TaskWithInterface executed"); } @@ -314,14 +314,14 @@ public void MixedTasks_RouteCorrectly_InMultiThreadedMode() // Assert result.OverallResult.ShouldBe(BuildResultCode.Success); - + // NonEnlightenedTask should use TaskHost - TaskRoutingTestHelper.AssertTaskUsedTaskHost(logger, "NonEnlightenedTestTask"); - + TaskRouterTestHelper.AssertTaskUsedTaskHost(logger, "NonEnlightenedTestTask"); + // Interface and Attribute tasks should NOT use TaskHost - TaskRoutingTestHelper.AssertTaskRanInProcess(logger, "InterfaceTestTask"); - TaskRoutingTestHelper.AssertTaskRanInProcess(logger, "AttributeTestTask"); - + TaskRouterTestHelper.AssertTaskRanInProcess(logger, "InterfaceTestTask"); + TaskRouterTestHelper.AssertTaskRanInProcess(logger, "AttributeTestTask"); + // All tasks should execute successfully logger.FullLog.ShouldContain("NonEnlightenedTask executed"); logger.FullLog.ShouldContain("TaskWithInterface executed"); @@ -372,15 +372,62 @@ public void ExplicitTaskHostFactory_OverridesRoutingLogic() // Assert result.OverallResult.ShouldBe(BuildResultCode.Success); - + // Task should use TaskHost because TaskHostFactory was explicitly requested // This overrides the normal routing logic which would run this in-process - TaskRoutingTestHelper.AssertTaskUsedTaskHost(logger, "InterfaceTestTask"); - + TaskRouterTestHelper.AssertTaskUsedTaskHost(logger, "InterfaceTestTask"); + // Verify task executed successfully logger.FullLog.ShouldContain("TaskWithInterface executed"); } + /// + /// Verifies that tasks inheriting IMultiThreadableTask from a base class (without directly + /// implementing it) are routed to TaskHost, not in-process. Only tasks that directly + /// implement the interface should run in-process. + /// + [Fact] + public void InheritedInterface_RunsInTaskHost_InMultiThreadedMode() + { + // Arrange + string projectContent = CreateTestProject( + taskName: "InheritedInterfaceTestTask", + taskClass: "InheritedInterfaceTask"); + + string projectFile = Path.Combine(_testProjectsDir, "InheritedInterfaceProject.proj"); + File.WriteAllText(projectFile, projectContent); + + var logger = new MockLogger(_output); + var buildParameters = new BuildParameters + { + MultiThreaded = true, + Loggers = new[] { logger }, + DisableInProcNode = false, + EnableNodeReuse = false + }; + + var buildRequestData = new BuildRequestData( + projectFile, + new Dictionary(), + null, + new[] { "TestTarget" }, + null); + + // Act + var buildManager = BuildManager.DefaultBuildManager; + var result = buildManager.Build(buildParameters, buildRequestData); + + // Assert + result.OverallResult.ShouldBe(BuildResultCode.Success); + + // Task should use TaskHost because it only inherits IMultiThreadableTask, + // doesn't directly implement it + TaskRouterTestHelper.AssertTaskUsedTaskHost(logger, "InheritedInterfaceTestTask"); + + // Verify task executed successfully + logger.FullLog.ShouldContain("InheritedInterfaceTask executed"); + } + private string CreateTestProject(string taskName, string taskClass) { return $@" @@ -398,7 +445,7 @@ private string CreateTestProject(string taskName, string taskClass) /// Helper utilities for testing task routing behavior. /// Provides robust assertions that are less fragile than raw log string matching. /// - internal static class TaskRoutingTestHelper + internal static class TaskRouterTestHelper { /// /// Asserts that a task was launched in an external TaskHost process. @@ -456,27 +503,35 @@ public override bool Execute() } } - #endregion -} + /// + /// Base task class that implements IMultiThreadableTask. + /// This simulates a hypothetical MultiThreadableTask base class. + /// + public class BaseMultiThreadableTask : Task, IMultiThreadableTask + { + public TaskEnvironment TaskEnvironment { get; set; } + + public override bool Execute() + { + Log.LogMessage(MessageImportance.High, "BaseMultiThreadableTask executed"); + return true; + } + } -// Custom attribute definition in Microsoft.Build.Framework namespace to match what TaskRoutingDecision expects -// TaskRoutingDecision looks for attributes with FullName = "Microsoft.Build.Framework.MSBuildMultiThreadableTaskAttribute" -// Since the real attribute is internal in Framework, we define our own test version here -namespace Microsoft.Build.Framework -{ /// - /// Test attribute to mark tasks as safe for multi-threaded execution. - /// This is a test copy in this test assembly that will be recognized - /// by name-based attribute detection in TaskRoutingDecision. + /// Task that inherits from BaseMultiThreadableTask but doesn't directly implement IMultiThreadableTask. + /// Should run in TaskHost in multi-threaded mode (NOT in-process) because it only inherits + /// the interface, which is not enough information whether it opted in. /// - [AttributeUsage(AttributeTargets.Class, Inherited = false, AllowMultiple = false)] - public sealed class MSBuildMultiThreadableTaskAttribute : Attribute + public class InheritedInterfaceTestTask : BaseMultiThreadableTask { + public override bool Execute() + { + Log.LogMessage(MessageImportance.High, "InheritedInterfaceTask executed"); + return true; + } } -} -namespace Microsoft.Build.Engine.UnitTests.BackEnd -{ /// /// Task marked with MSBuildMultiThreadableTaskAttribute. /// Should run in-process in multi-threaded mode. @@ -496,4 +551,22 @@ public override bool Execute() return true; } } + + #endregion +} + +// Custom attribute definition in Microsoft.Build.Framework namespace to match what TaskRouter expects +// TaskRouter looks for attributes with FullName = "Microsoft.Build.Framework.MSBuildMultiThreadableTaskAttribute" +// Since the real attribute is internal in Framework, we define our own test version here +namespace Microsoft.Build.Framework +{ + /// + /// Test attribute to mark tasks as safe for multi-threaded execution. + /// This is a test copy in this test assembly that will be recognized + /// by name-based attribute detection in TaskRouter. + /// + [AttributeUsage(AttributeTargets.Class, Inherited = false, AllowMultiple = false)] + public sealed class MSBuildMultiThreadableTaskAttribute : Attribute + { + } } diff --git a/src/Build/BackEnd/BuildManager/BuildManager.cs b/src/Build/BackEnd/BuildManager/BuildManager.cs index ff7f67df8c1..3032b80f93a 100644 --- a/src/Build/BackEnd/BuildManager/BuildManager.cs +++ b/src/Build/BackEnd/BuildManager/BuildManager.cs @@ -1068,6 +1068,8 @@ public void EndBuild() { _resultsCache!.ClearResults(); } + + TaskRouter.ClearCache(); } catch (Exception e) { diff --git a/src/Build/BackEnd/Components/RequestBuilder/TaskRoutingDecision.cs b/src/Build/BackEnd/Components/RequestBuilder/TaskRouter.cs similarity index 78% rename from src/Build/BackEnd/Components/RequestBuilder/TaskRoutingDecision.cs rename to src/Build/BackEnd/Components/RequestBuilder/TaskRouter.cs index 9c3a24ab961..52a3cf59d2b 100644 --- a/src/Build/BackEnd/Components/RequestBuilder/TaskRoutingDecision.cs +++ b/src/Build/BackEnd/Components/RequestBuilder/TaskRouter.cs @@ -18,7 +18,7 @@ namespace Microsoft.Build.BackEnd /// This class should only be used when in multi-threaded mode. Traditional multi-proc builds /// have different semantics and should not use this routing logic. /// - internal static class TaskRoutingDecision + internal static class TaskRouter { /// /// Cache of task types to their multi-threadable capability status. @@ -72,13 +72,33 @@ private static bool IsMultiThreadableTask(Type taskType) } /// - /// Checks if a task type implements the IMultiThreadableTask interface. + /// Checks if a task type directly implements the IMultiThreadableTask interface. + /// This only returns true if the type itself declares the interface, not if it inherits it + /// from a base class. This ensures tasks must explicitly opt-in to thread-safe routing. /// /// The task type to check. - /// True if the task implements IMultiThreadableTask; false otherwise. + /// True if the task directly implements IMultiThreadableTask; false otherwise. + /// It's not possible to distinguish the case where it both inherits and declares the interface so that case falls under "inherited". private static bool ImplementsIMultiThreadableTask(Type taskType) { - return typeof(IMultiThreadableTask).IsAssignableFrom(taskType); + // Check if task is assignable to IMultiThreadableTask + if (!typeof(IMultiThreadableTask).IsAssignableFrom(taskType)) + { + return false; + } + + // Check if base type is also assignable to IMultiThreadableTask + Type? baseType = taskType.BaseType; + if (baseType != null && typeof(IMultiThreadableTask).IsAssignableFrom(baseType)) + { + // Base type implements the interface, so this task inherited it + // Return false to route to TaskHost + return false; + } + + // Task implements the interface and base type doesn't + // This means the task directly declared it + return true; } /// diff --git a/src/Build/Instance/TaskFactories/AssemblyTaskFactory.cs b/src/Build/Instance/TaskFactories/AssemblyTaskFactory.cs index 9faee184299..d73c55312a7 100644 --- a/src/Build/Instance/TaskFactories/AssemblyTaskFactory.cs +++ b/src/Build/Instance/TaskFactories/AssemblyTaskFactory.cs @@ -371,7 +371,7 @@ internal ITask CreateTaskInstance( && _loadedType?.Type != null && buildComponentHost?.BuildParameters?.MultiThreaded == true) { - if (TaskRoutingDecision.NeedsTaskHostInMultiThreadedMode(_loadedType.Type)) + if (TaskRouter.NeedsTaskHostInMultiThreadedMode(_loadedType.Type)) { useTaskFactory = true; } diff --git a/src/Build/Microsoft.Build.csproj b/src/Build/Microsoft.Build.csproj index 6a30c138b09..d11075fd13b 100644 --- a/src/Build/Microsoft.Build.csproj +++ b/src/Build/Microsoft.Build.csproj @@ -345,7 +345,7 @@ - + From 0addfe4b9cfed7e16975e85b2f37519372175208 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jan=20Provazn=C3=ADk?= Date: Fri, 17 Oct 2025 10:38:33 +0200 Subject: [PATCH 10/13] inheritable semantics --- .../BackEnd/TaskRouter_IntegrationTests.cs | 250 +++++++++++++++++- .../Components/RequestBuilder/TaskRouter.cs | 39 ++- src/Framework/InternalMSBuildTaskAttribute.cs | 17 ++ 3 files changed, 287 insertions(+), 19 deletions(-) create mode 100644 src/Framework/InternalMSBuildTaskAttribute.cs diff --git a/src/Build.UnitTests/BackEnd/TaskRouter_IntegrationTests.cs b/src/Build.UnitTests/BackEnd/TaskRouter_IntegrationTests.cs index 8dd9e1c7ce9..144c7aadbb7 100644 --- a/src/Build.UnitTests/BackEnd/TaskRouter_IntegrationTests.cs +++ b/src/Build.UnitTests/BackEnd/TaskRouter_IntegrationTests.cs @@ -382,12 +382,11 @@ public void ExplicitTaskHostFactory_OverridesRoutingLogic() } /// - /// Verifies that tasks inheriting IMultiThreadableTask from a base class (without directly - /// implementing it) are routed to TaskHost, not in-process. Only tasks that directly - /// implement the interface should run in-process. + /// Verifies that tasks inheriting IMultiThreadableTask from an unmarked external base class + /// run in-process (inheritable semantics by default). /// [Fact] - public void InheritedInterface_RunsInTaskHost_InMultiThreadedMode() + public void InheritedInterface_RunsInProcess_InMultiThreadedMode() { // Arrange string projectContent = CreateTestProject( @@ -420,14 +419,152 @@ public void InheritedInterface_RunsInTaskHost_InMultiThreadedMode() // Assert result.OverallResult.ShouldBe(BuildResultCode.Success); - // Task should use TaskHost because it only inherits IMultiThreadableTask, - // doesn't directly implement it - TaskRouterTestHelper.AssertTaskUsedTaskHost(logger, "InheritedInterfaceTestTask"); + // Task should run in-process because it inherits from unmarked external base class + // (inheritable semantics) + TaskRouterTestHelper.AssertTaskRanInProcess(logger, "InheritedInterfaceTestTask"); // Verify task executed successfully logger.FullLog.ShouldContain("InheritedInterfaceTask executed"); } + /// + /// Verifies that tasks inheriting IMultiThreadableTask from an InternalMSBuildTaskAttribute-marked + /// base class (MSBuild repo tasks) are routed to TaskHost for backward compatibility + /// (non-inheritable semantics). + /// + [Fact] + public void InheritedFromInternalBase_RunsInTaskHost_InMultiThreadedMode() + { + // Arrange + string projectContent = CreateTestProject( + taskName: "InheritedFromInternalBaseTestTask", + taskClass: "InheritedFromInternalBaseTask"); + + string projectFile = Path.Combine(_testProjectsDir, "InheritedFromInternalBaseProject.proj"); + File.WriteAllText(projectFile, projectContent); + + var logger = new MockLogger(_output); + var buildParameters = new BuildParameters + { + MultiThreaded = true, + Loggers = new[] { logger }, + DisableInProcNode = false, + EnableNodeReuse = false + }; + + var buildRequestData = new BuildRequestData( + projectFile, + new Dictionary(), + null, + new[] { "TestTarget" }, + null); + + // Act + var buildManager = BuildManager.DefaultBuildManager; + var result = buildManager.Build(buildParameters, buildRequestData); + + // Assert + result.OverallResult.ShouldBe(BuildResultCode.Success); + + // Task should use TaskHost because base class has InternalMSBuildTaskAttribute + // (non-inheritable semantics for backward compatibility) + TaskRouterTestHelper.AssertTaskUsedTaskHost(logger, "InheritedFromInternalBaseTestTask"); + + // Verify task executed successfully + logger.FullLog.ShouldContain("InheritedFromInternalBaseTask executed"); + } + + /// + /// Verifies that multi-level inheritance from unmarked external base class maintains + /// inheritable semantics - tasks should run in-process. + /// + [Fact] + public void MultiLevelInheritanceFromUnmarkedBase_RunsInProcess_InMultiThreadedMode() + { + // Arrange + string projectContent = CreateTestProject( + taskName: "MultiLevelUnmarkedTestTask", + taskClass: "MultiLevelUnmarkedTask"); + + string projectFile = Path.Combine(_testProjectsDir, "MultiLevelUnmarkedProject.proj"); + File.WriteAllText(projectFile, projectContent); + + var logger = new MockLogger(_output); + var buildParameters = new BuildParameters + { + MultiThreaded = true, + Loggers = new[] { logger }, + DisableInProcNode = false, + EnableNodeReuse = false + }; + + var buildRequestData = new BuildRequestData( + projectFile, + new Dictionary(), + null, + new[] { "TestTarget" }, + null); + + // Act + var buildManager = BuildManager.DefaultBuildManager; + var result = buildManager.Build(buildParameters, buildRequestData); + + // Assert + result.OverallResult.ShouldBe(BuildResultCode.Success); + + // Task should run in-process - inheritable semantics through multiple levels + TaskRouterTestHelper.AssertTaskRanInProcess(logger, "MultiLevelUnmarkedTestTask"); + + // Verify task executed successfully + logger.FullLog.ShouldContain("MultiLevelUnmarkedTask executed"); + } + + /// + /// Verifies that multi-level inheritance from InternalMSBuildTaskAttribute-marked base + /// applies non-inheritable semantics - tasks should run in TaskHost even if the attribute + /// is on a grandparent class. + /// + [Fact] + public void MultiLevelInheritanceFromMarkedBase_RunsInTaskHost_InMultiThreadedMode() + { + // Arrange + string projectContent = CreateTestProject( + taskName: "MultiLevelMarkedTestTask", + taskClass: "MultiLevelMarkedTask"); + + string projectFile = Path.Combine(_testProjectsDir, "MultiLevelMarkedProject.proj"); + File.WriteAllText(projectFile, projectContent); + + var logger = new MockLogger(_output); + var buildParameters = new BuildParameters + { + MultiThreaded = true, + Loggers = new[] { logger }, + DisableInProcNode = false, + EnableNodeReuse = false + }; + + var buildRequestData = new BuildRequestData( + projectFile, + new Dictionary(), + null, + new[] { "TestTarget" }, + null); + + // Act + var buildManager = BuildManager.DefaultBuildManager; + var result = buildManager.Build(buildParameters, buildRequestData); + + // Assert + result.OverallResult.ShouldBe(BuildResultCode.Success); + + // Task should use TaskHost - non-inheritable semantics from marked grandparent + TaskRouterTestHelper.AssertTaskUsedTaskHost(logger, "MultiLevelMarkedTestTask"); + + // Verify task executed successfully + logger.FullLog.ShouldContain("MultiLevelMarkedTask executed"); + } + private string CreateTestProject(string taskName, string taskClass) { return $@" @@ -505,7 +642,7 @@ public override bool Execute() /// /// Base task class that implements IMultiThreadableTask. - /// This simulates a hypothetical MultiThreadableTask base class. + /// This simulates an external base class (not from MSBuild repo). /// public class BaseMultiThreadableTask : Task, IMultiThreadableTask { @@ -519,9 +656,24 @@ public override bool Execute() } /// - /// Task that inherits from BaseMultiThreadableTask but doesn't directly implement IMultiThreadableTask. - /// Should run in TaskHost in multi-threaded mode (NOT in-process) because it only inherits - /// the interface, which is not enough information whether it opted in. + /// Base task class that implements IMultiThreadableTask and is marked with InternalMSBuildTaskAttribute. + /// This simulates a task from the MSBuild repository that needs non-inheritable semantics. + /// + [InternalMSBuildTask] + public class InternalBaseMultiThreadableTask : Task, IMultiThreadableTask + { + public TaskEnvironment TaskEnvironment { get; set; } + + public override bool Execute() + { + Log.LogMessage(MessageImportance.High, "InternalBaseMultiThreadableTask executed"); + return true; + } + } + + /// + /// Task that inherits from BaseMultiThreadableTask (unmarked external base). + /// Should run IN-PROCESS in multi-threaded mode because of inheritable semantics. /// public class InheritedInterfaceTestTask : BaseMultiThreadableTask { @@ -532,6 +684,72 @@ public override bool Execute() } } + /// + /// Task that inherits from InternalBaseMultiThreadableTask (marked MSBuild repo base). + /// Should run in TaskHost in multi-threaded mode because of non-inheritable semantics + /// for backward compatibility. + /// + public class InheritedFromInternalBaseTestTask : InternalBaseMultiThreadableTask + { + public override bool Execute() + { + Log.LogMessage(MessageImportance.High, "InheritedFromInternalBaseTask executed"); + return true; + } + } + + /// + /// Intermediate task class for multi-level inheritance testing (unmarked). + /// Inherits from BaseMultiThreadableTask. + /// + public class MiddleUnmarkedTask : BaseMultiThreadableTask + { + public override bool Execute() + { + Log.LogMessage(MessageImportance.High, "MiddleUnmarkedTask executed"); + return true; + } + } + + /// + /// Task that inherits from MiddleUnmarkedTask (multi-level from unmarked base). + /// Should run in-process because of inheritable semantics through the chain. + /// + public class MultiLevelUnmarkedTestTask : MiddleUnmarkedTask + { + public override bool Execute() + { + Log.LogMessage(MessageImportance.High, "MultiLevelUnmarkedTask executed"); + return true; + } + } + + /// + /// Intermediate task class for multi-level inheritance testing (from marked base). + /// Inherits from InternalBaseMultiThreadableTask. + /// + public class MiddleMarkedTask : InternalBaseMultiThreadableTask + { + public override bool Execute() + { + Log.LogMessage(MessageImportance.High, "MiddleMarkedTask executed"); + return true; + } + } + + /// + /// Task that inherits from MiddleMarkedTask (multi-level from marked base). + /// Should run in TaskHost because InternalMSBuildTaskAttribute appears in the hierarchy. + /// + public class MultiLevelMarkedTestTask : MiddleMarkedTask + { + public override bool Execute() + { + Log.LogMessage(MessageImportance.High, "MultiLevelMarkedTask executed"); + return true; + } + } + /// /// Task marked with MSBuildMultiThreadableTaskAttribute. /// Should run in-process in multi-threaded mode. @@ -569,4 +787,14 @@ namespace Microsoft.Build.Framework public sealed class MSBuildMultiThreadableTaskAttribute : Attribute { } + + /// + /// Test attribute to mark tasks that are part of the MSBuild repository. + /// This is a test copy in this test assembly that will be recognized + /// by name-based attribute detection in TaskRouter. + /// + [AttributeUsage(AttributeTargets.Class, Inherited = true, AllowMultiple = false)] + internal sealed class InternalMSBuildTaskAttribute : Attribute + { + } } diff --git a/src/Build/BackEnd/Components/RequestBuilder/TaskRouter.cs b/src/Build/BackEnd/Components/RequestBuilder/TaskRouter.cs index 52a3cf59d2b..7902662a5b8 100644 --- a/src/Build/BackEnd/Components/RequestBuilder/TaskRouter.cs +++ b/src/Build/BackEnd/Components/RequestBuilder/TaskRouter.cs @@ -72,13 +72,14 @@ private static bool IsMultiThreadableTask(Type taskType) } /// - /// Checks if a task type directly implements the IMultiThreadableTask interface. - /// This only returns true if the type itself declares the interface, not if it inherits it - /// from a base class. This ensures tasks must explicitly opt-in to thread-safe routing. + /// Checks if a task type implements the IMultiThreadableTask interface. + /// By default, the interface has inheritable semantics - if a task inherits from a base class + /// that implements IMultiThreadableTask, it is also considered thread-safe. + /// However, if the base class is marked with InternalMSBuildTaskAttribute (tasks from MSBuild repo), + /// non-inheritable semantics apply for backward compatibility. /// /// The task type to check. - /// True if the task directly implements IMultiThreadableTask; false otherwise. - /// It's not possible to distinguish the case where it both inherits and declares the interface so that case falls under "inherited". + /// True if the task should be considered multi-threadable; false otherwise. private static bool ImplementsIMultiThreadableTask(Type taskType) { // Check if task is assignable to IMultiThreadableTask @@ -91,9 +92,8 @@ private static bool ImplementsIMultiThreadableTask(Type taskType) Type? baseType = taskType.BaseType; if (baseType != null && typeof(IMultiThreadableTask).IsAssignableFrom(baseType)) { - // Base type implements the interface, so this task inherited it - // Return false to route to TaskHost - return false; + // Inherited from base type - non-inheritable if base is from MSBuild repo + return !HasInternalMSBuildTaskAttribute(baseType); } // Task implements the interface and base type doesn't @@ -101,6 +101,29 @@ private static bool ImplementsIMultiThreadableTask(Type taskType) return true; } + /// + /// Checks if a task type is marked with InternalMSBuildTaskAttribute. + /// This attribute marks tasks from the MSBuild repository that need non-inheritable + /// IMultiThreadableTask semantics for backward compatibility. + /// + /// The task type to check. + /// True if the task has the attribute; false otherwise. + private static bool HasInternalMSBuildTaskAttribute(Type taskType) + { + const string attributeFullName = "Microsoft.Build.Framework.InternalMSBuildTaskAttribute"; + + // Check for the attribute by full name + foreach (object attr in taskType.GetCustomAttributes(inherit: true)) + { + if (attr.GetType().FullName == attributeFullName) + { + return true; + } + } + + return false; + } + /// /// Checks if a task type is marked with MSBuildMultiThreadableTaskAttribute. /// Detection is based on namespace and name only, ignoring the defining assembly, diff --git a/src/Framework/InternalMSBuildTaskAttribute.cs b/src/Framework/InternalMSBuildTaskAttribute.cs new file mode 100644 index 00000000000..4a7f1a2e311 --- /dev/null +++ b/src/Framework/InternalMSBuildTaskAttribute.cs @@ -0,0 +1,17 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System; + +namespace Microsoft.Build.Framework +{ + /// + /// Internal marker attribute for tasks that are part of the MSBuild repository. + /// When a task inherits from a base class marked with this attribute, IMultiThreadableTask + /// is treated with non-inheritable semantics to maintain backward compatibility. + /// + [AttributeUsage(AttributeTargets.Class, Inherited = true, AllowMultiple = false)] + internal sealed class InternalMSBuildTaskAttribute : Attribute + { + } +} From d2f69ff660e804514ae37f8c2e45e1ec36ae80ed Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jan=20Provazn=C3=ADk?= Date: Fri, 17 Oct 2025 10:38:44 +0200 Subject: [PATCH 11/13] update docs --- .../multithreading/multithreaded-msbuild.md | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-) diff --git a/documentation/specs/multithreading/multithreaded-msbuild.md b/documentation/specs/multithreading/multithreaded-msbuild.md index 54e867a2b98..04200200111 100644 --- a/documentation/specs/multithreading/multithreaded-msbuild.md +++ b/documentation/specs/multithreading/multithreaded-msbuild.md @@ -217,14 +217,27 @@ With a sidecar TaskHost per node, tasks will get the same constraints and freedo ## Thread-safe tasks -To mark that a task is multithreaded-MSBuild-aware, we will introduce a new interface that tasks can implement. We will provide a new `TaskEnvironment` object with information about the task invocation, including the current environment and working directory, so that tasks can access the same information they would have in a single-threaded or out-of-process execution. +To mark that a task is multithreaded-MSBuild-aware, we will introduce a new interface (`IMultiThreadableTask`) and attribute (`MSBuildMultiThreadableTaskAttribute`) that tasks can implement or apply. We will provide a new `TaskEnvironment` object with information about the task invocation, including the current environment and working directory, so that tasks can access the same information they would have in a single-threaded or out-of-process execution. -To ease task authoring, we will provide a Roslyn analyzer that will check for known-bad API usage, like `System.Environment.GetEnvironmentVariable` or `System.IO.Directory.SetCurrentDirectory`, and suggest alternatives that use the object provided by the engine (such as `context.GetEnvironmentVariable`). +### Task Routing Strategy + +In multithreaded mode, MSBuild determines where each task should execute based on thread-safety indicators: + +* **Thread-safe tasks** (implementing `IMultiThreadableTask` or marked with `MSBuildMultiThreadableTaskAttribute`) run **in-process** within thread nodes +* **Non-enlightened tasks** (without thread-safety indicators) run in **sidecar TaskHost processes** to maintain isolation and compatibility + +#### Interface Inheritance Semantics + +The `IMultiThreadableTask` interface has **inheritable semantics by default**: if a task inherits from a base class that implements the interface, it is considered thread-safe and will run in-process. This allows task library authors to create thread-safe base classes that derived tasks can inherit from. + +However, to maintain backward compatibility with existing MSBuild repository tasks that were not designed with inheritance in mind, we apply **non-inheritable semantics** to tasks from the MSBuild repository. These tasks are marked with an internal `InternalMSBuildTaskAttribute`, and any tasks inheriting from them are routed to TaskHost processes unless they are marked with `MSBuildMultiThreadableTaskAttribute`. ## Tasks transition In the initial phase of development of multithreaded execution mode, all tasks will run in sidecar taskhosts. Over time, we will update tasks that are maintained by us and our partners (such as MSBuild, SDK, and NuGet) to implement and use the new thread-safe task interface. As these tasks become thread-safe, their execution would be moved into the entry process. Customers' tasks would be executed in the sidecar taskhosts unless they implement the new interface. +To ease task authoring, we will provide a Roslyn analyzer that will check for known-bad API usage, like `System.Environment.GetEnvironmentVariable` or `System.IO.Directory.SetCurrentDirectory`, and suggest alternatives that use the object provided by the engine (such as `context.GetEnvironmentVariable`). + ## Interaction with `DisableInProcNode` We need to ensure the support for multithreaded mode in Visual Studio builds. Currently, the entry node for MSBuild runs entirely within the devenv process, but the majority of the build operation are run in the MSBuild worker processes, because project systems set `BuildParameters.DisableInProcNode=true`. In multithreaded mode, all of the task execution must continue to be out of process. To address this, unlike the CLI scenario, we will move all thread nodes to the out-of-process MSBuild process, keeping only the scheduler in devenv. From aeb51511abadace2dc1c605e0b20ebff86adca53 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jan=20Provazn=C3=ADk?= Date: Fri, 17 Oct 2025 10:48:02 +0200 Subject: [PATCH 12/13] fix test compilation --- .../BackEnd/TaskRouter_IntegrationTests.cs | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/src/Build.UnitTests/BackEnd/TaskRouter_IntegrationTests.cs b/src/Build.UnitTests/BackEnd/TaskRouter_IntegrationTests.cs index 144c7aadbb7..faf02273060 100644 --- a/src/Build.UnitTests/BackEnd/TaskRouter_IntegrationTests.cs +++ b/src/Build.UnitTests/BackEnd/TaskRouter_IntegrationTests.cs @@ -787,14 +787,4 @@ namespace Microsoft.Build.Framework public sealed class MSBuildMultiThreadableTaskAttribute : Attribute { } - - /// - /// Test attribute to mark tasks that are part of the MSBuild repository. - /// This is a test copy in this test assembly that will be recognized - /// by name-based attribute detection in TaskRouter. - /// - [AttributeUsage(AttributeTargets.Class, Inherited = true, AllowMultiple = false)] - internal sealed class InternalMSBuildTaskAttribute : Attribute - { - } } From 65a324019f08f9db8d746bb7a6f0b25fc57aacd0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jan=20Provazn=C3=ADk?= Date: Mon, 20 Oct 2025 17:24:10 +0200 Subject: [PATCH 13/13] update behavior --- .../multithreading/multithreaded-msbuild.md | 22 +- .../BackEnd/TaskRouter_IntegrationTests.cs | 328 ++---------------- .../Components/RequestBuilder/TaskRouter.cs | 156 +++------ src/Framework/InternalMSBuildTaskAttribute.cs | 17 - .../MSBuildMultiThreadableTaskAttribute.cs | 14 +- 5 files changed, 85 insertions(+), 452 deletions(-) delete mode 100644 src/Framework/InternalMSBuildTaskAttribute.cs diff --git a/documentation/specs/multithreading/multithreaded-msbuild.md b/documentation/specs/multithreading/multithreaded-msbuild.md index 04200200111..cc224f4aa80 100644 --- a/documentation/specs/multithreading/multithreaded-msbuild.md +++ b/documentation/specs/multithreading/multithreaded-msbuild.md @@ -217,26 +217,30 @@ With a sidecar TaskHost per node, tasks will get the same constraints and freedo ## Thread-safe tasks -To mark that a task is multithreaded-MSBuild-aware, we will introduce a new interface (`IMultiThreadableTask`) and attribute (`MSBuildMultiThreadableTaskAttribute`) that tasks can implement or apply. We will provide a new `TaskEnvironment` object with information about the task invocation, including the current environment and working directory, so that tasks can access the same information they would have in a single-threaded or out-of-process execution. +To mark that a task is multithreaded-MSBuild-aware, we use the `MSBuildMultiThreadableTaskAttribute` attribute. Tasks may also implement the `IMultiThreadableTask` interface to gain access to the `TaskEnvironment` object with information about the task invocation, including the current environment and working directory. ### Task Routing Strategy -In multithreaded mode, MSBuild determines where each task should execute based on thread-safety indicators: +In multithreaded mode, MSBuild determines where each task should execute based on: -* **Thread-safe tasks** (implementing `IMultiThreadableTask` or marked with `MSBuildMultiThreadableTaskAttribute`) run **in-process** within thread nodes -* **Non-enlightened tasks** (without thread-safety indicators) run in **sidecar TaskHost processes** to maintain isolation and compatibility +* **Thread-safe tasks** (marked with `MSBuildMultiThreadableTaskAttribute`) run **in-process** within thread nodes +* **All other tasks** (without the attribute) run in **sidecar TaskHost processes** to maintain isolation and compatibility -#### Interface Inheritance Semantics +#### Attribute Semantics -The `IMultiThreadableTask` interface has **inheritable semantics by default**: if a task inherits from a base class that implements the interface, it is considered thread-safe and will run in-process. This allows task library authors to create thread-safe base classes that derived tasks can inherit from. +The `MSBuildMultiThreadableTaskAttribute` is **non-inheritable** (`Inherited = false`): each task class must explicitly declare its thread-safety capability. This ensures that: -However, to maintain backward compatibility with existing MSBuild repository tasks that were not designed with inheritance in mind, we apply **non-inheritable semantics** to tasks from the MSBuild repository. These tasks are marked with an internal `InternalMSBuildTaskAttribute`, and any tasks inheriting from them are routed to TaskHost processes unless they are marked with `MSBuildMultiThreadableTaskAttribute`. +* Task authors must consciously opt into multithreaded execution for each task class +* Derived classes cannot accidentally inherit thread-safety assumptions from base classes +* The routing decision is always explicit and visible in the task's source code + +Tasks may optionally implement `IMultiThreadableTask` to access `TaskEnvironment` APIs, but only the attribute determines routing behavior. ## Tasks transition -In the initial phase of development of multithreaded execution mode, all tasks will run in sidecar taskhosts. Over time, we will update tasks that are maintained by us and our partners (such as MSBuild, SDK, and NuGet) to implement and use the new thread-safe task interface. As these tasks become thread-safe, their execution would be moved into the entry process. Customers' tasks would be executed in the sidecar taskhosts unless they implement the new interface. +In the initial phase of development of multithreaded execution mode, all tasks will run in sidecar taskhosts. Over time, we will update tasks that are maintained by us and our partners (such as MSBuild, SDK, and NuGet) to add the `MSBuildMultiThreadableTaskAttribute` and ensure thread-safety. As these tasks are marked with the attribute, their execution would be moved into the entry process. Customers' tasks would be executed in the sidecar taskhosts unless they add the attribute to their task classes. -To ease task authoring, we will provide a Roslyn analyzer that will check for known-bad API usage, like `System.Environment.GetEnvironmentVariable` or `System.IO.Directory.SetCurrentDirectory`, and suggest alternatives that use the object provided by the engine (such as `context.GetEnvironmentVariable`). +To ease task authoring, we will provide a Roslyn analyzer that will check for known-bad API usage, like `System.Environment.GetEnvironmentVariable` or `System.IO.Directory.SetCurrentDirectory`, and suggest alternatives that use the `TaskEnvironment` object (for tasks that also implement `IMultiThreadableTask`). ## Interaction with `DisableInProcNode` diff --git a/src/Build.UnitTests/BackEnd/TaskRouter_IntegrationTests.cs b/src/Build.UnitTests/BackEnd/TaskRouter_IntegrationTests.cs index faf02273060..fa508369b3d 100644 --- a/src/Build.UnitTests/BackEnd/TaskRouter_IntegrationTests.cs +++ b/src/Build.UnitTests/BackEnd/TaskRouter_IntegrationTests.cs @@ -21,8 +21,9 @@ namespace Microsoft.Build.Engine.UnitTests.BackEnd { /// /// Integration tests for task routing in multi-threaded mode. - /// Tests verify that tasks with IMultiThreadableTask or MSBuildMultiThreadableTaskAttribute - /// run in-process, while tasks without these indicators run in TaskHost for isolation. + /// Tests verify that tasks with MSBuildMultiThreadableTaskAttribute (non-inheritable) + /// run in-process, while tasks without this attribute run in TaskHost for isolation. + /// Tasks may also implement IMultiThreadableTask to gain access to TaskEnvironment APIs. /// public class TaskRouter_IntegrationTests : IDisposable { @@ -90,11 +91,11 @@ public void NonEnlightenedTask_RunsInTaskHost_InMultiThreadedMode() } /// - /// Verifies that a task with IMultiThreadableTask interface runs in-process - /// (not in TaskHost) when MultiThreaded mode is enabled. + /// Verifies that a task with IMultiThreadableTask interface but without MSBuildMultiThreadableTaskAttribute + /// runs in TaskHost when MultiThreaded mode is enabled. Only the attribute determines routing. /// [Fact] - public void TaskWithInterface_RunsInProcess_InMultiThreadedMode() + public void TaskWithInterface_RunsInTaskHost_InMultiThreadedMode() { // Arrange string projectContent = CreateTestProject( @@ -127,8 +128,8 @@ public void TaskWithInterface_RunsInProcess_InMultiThreadedMode() // Assert result.OverallResult.ShouldBe(BuildResultCode.Success); - // Verify task was NOT launched in TaskHost (runs in-process) - TaskRouterTestHelper.AssertTaskRanInProcess(logger, "InterfaceTestTask"); + // Verify task was launched in TaskHost (interface alone is not sufficient) + TaskRouterTestHelper.AssertTaskUsedTaskHost(logger, "InterfaceTestTask"); // Verify task executed successfully logger.FullLog.ShouldContain("TaskWithInterface executed"); @@ -225,7 +226,7 @@ public void NonEnlightenedTask_RunsInProcess_WhenMultiThreadedModeDisabled() } /// - /// Verifies that a task with interface doesn't use TaskHost in single-threaded mode. + /// Verifies that all tasks run in-process in single-threaded mode regardless of attributes. /// [Fact] public void TaskWithInterface_RunsInProcess_WhenMultiThreadedModeDisabled() @@ -315,11 +316,11 @@ public void MixedTasks_RouteCorrectly_InMultiThreadedMode() // Assert result.OverallResult.ShouldBe(BuildResultCode.Success); - // NonEnlightenedTask should use TaskHost + // NonEnlightenedTask and InterfaceTask should use TaskHost TaskRouterTestHelper.AssertTaskUsedTaskHost(logger, "NonEnlightenedTestTask"); + TaskRouterTestHelper.AssertTaskUsedTaskHost(logger, "InterfaceTestTask"); - // Interface and Attribute tasks should NOT use TaskHost - TaskRouterTestHelper.AssertTaskRanInProcess(logger, "InterfaceTestTask"); + // Only Attribute task should NOT use TaskHost TaskRouterTestHelper.AssertTaskRanInProcess(logger, "AttributeTestTask"); // All tasks should execute successfully @@ -330,20 +331,20 @@ public void MixedTasks_RouteCorrectly_InMultiThreadedMode() /// /// Verifies that explicit TaskHostFactory request overrides routing logic, - /// forcing tasks to run in TaskHost even if they would normally run in-process. + /// forcing tasks to run in TaskHost even if they have the MSBuildMultiThreadableTaskAttribute. /// [Fact] public void ExplicitTaskHostFactory_OverridesRoutingLogic() { - // Arrange - Use a thread-safe task but explicitly request TaskHostFactory + // Arrange - Use a task with attribute but explicitly request TaskHostFactory string projectContent = $@" - - + "; @@ -374,196 +375,14 @@ public void ExplicitTaskHostFactory_OverridesRoutingLogic() result.OverallResult.ShouldBe(BuildResultCode.Success); // Task should use TaskHost because TaskHostFactory was explicitly requested - // This overrides the normal routing logic which would run this in-process - TaskRouterTestHelper.AssertTaskUsedTaskHost(logger, "InterfaceTestTask"); - - // Verify task executed successfully - logger.FullLog.ShouldContain("TaskWithInterface executed"); - } - - /// - /// Verifies that tasks inheriting IMultiThreadableTask from an unmarked external base class - /// run in-process (inheritable semantics by default). - /// - [Fact] - public void InheritedInterface_RunsInProcess_InMultiThreadedMode() - { - // Arrange - string projectContent = CreateTestProject( - taskName: "InheritedInterfaceTestTask", - taskClass: "InheritedInterfaceTask"); - - string projectFile = Path.Combine(_testProjectsDir, "InheritedInterfaceProject.proj"); - File.WriteAllText(projectFile, projectContent); - - var logger = new MockLogger(_output); - var buildParameters = new BuildParameters - { - MultiThreaded = true, - Loggers = new[] { logger }, - DisableInProcNode = false, - EnableNodeReuse = false - }; - - var buildRequestData = new BuildRequestData( - projectFile, - new Dictionary(), - null, - new[] { "TestTarget" }, - null); - - // Act - var buildManager = BuildManager.DefaultBuildManager; - var result = buildManager.Build(buildParameters, buildRequestData); - - // Assert - result.OverallResult.ShouldBe(BuildResultCode.Success); - - // Task should run in-process because it inherits from unmarked external base class - // (inheritable semantics) - TaskRouterTestHelper.AssertTaskRanInProcess(logger, "InheritedInterfaceTestTask"); - - // Verify task executed successfully - logger.FullLog.ShouldContain("InheritedInterfaceTask executed"); - } - - /// - /// Verifies that tasks inheriting IMultiThreadableTask from an InternalMSBuildTaskAttribute-marked - /// base class (MSBuild repo tasks) are routed to TaskHost for backward compatibility - /// (non-inheritable semantics). - /// - [Fact] - public void InheritedFromInternalBase_RunsInTaskHost_InMultiThreadedMode() - { - // Arrange - string projectContent = CreateTestProject( - taskName: "InheritedFromInternalBaseTestTask", - taskClass: "InheritedFromInternalBaseTask"); - - string projectFile = Path.Combine(_testProjectsDir, "InheritedFromInternalBaseProject.proj"); - File.WriteAllText(projectFile, projectContent); - - var logger = new MockLogger(_output); - var buildParameters = new BuildParameters - { - MultiThreaded = true, - Loggers = new[] { logger }, - DisableInProcNode = false, - EnableNodeReuse = false - }; - - var buildRequestData = new BuildRequestData( - projectFile, - new Dictionary(), - null, - new[] { "TestTarget" }, - null); - - // Act - var buildManager = BuildManager.DefaultBuildManager; - var result = buildManager.Build(buildParameters, buildRequestData); - - // Assert - result.OverallResult.ShouldBe(BuildResultCode.Success); - - // Task should use TaskHost because base class has InternalMSBuildTaskAttribute - // (non-inheritable semantics for backward compatibility) - TaskRouterTestHelper.AssertTaskUsedTaskHost(logger, "InheritedFromInternalBaseTestTask"); + // This overrides the normal routing logic which would run attribute tasks in-process + TaskRouterTestHelper.AssertTaskUsedTaskHost(logger, "AttributeTestTask"); // Verify task executed successfully - logger.FullLog.ShouldContain("InheritedFromInternalBaseTask executed"); - } - - /// - /// Verifies that multi-level inheritance from unmarked external base class maintains - /// inheritable semantics - tasks should run in-process. - /// - [Fact] - public void MultiLevelInheritanceFromUnmarkedBase_RunsInProcess_InMultiThreadedMode() - { - // Arrange - string projectContent = CreateTestProject( - taskName: "MultiLevelUnmarkedTestTask", - taskClass: "MultiLevelUnmarkedTask"); - - string projectFile = Path.Combine(_testProjectsDir, "MultiLevelUnmarkedProject.proj"); - File.WriteAllText(projectFile, projectContent); - - var logger = new MockLogger(_output); - var buildParameters = new BuildParameters - { - MultiThreaded = true, - Loggers = new[] { logger }, - DisableInProcNode = false, - EnableNodeReuse = false - }; - - var buildRequestData = new BuildRequestData( - projectFile, - new Dictionary(), - null, - new[] { "TestTarget" }, - null); - - // Act - var buildManager = BuildManager.DefaultBuildManager; - var result = buildManager.Build(buildParameters, buildRequestData); - - // Assert - result.OverallResult.ShouldBe(BuildResultCode.Success); - - // Task should run in-process - inheritable semantics through multiple levels - TaskRouterTestHelper.AssertTaskRanInProcess(logger, "MultiLevelUnmarkedTestTask"); - - // Verify task executed successfully - logger.FullLog.ShouldContain("MultiLevelUnmarkedTask executed"); + logger.FullLog.ShouldContain("TaskWithAttribute executed"); } - /// - /// Verifies that multi-level inheritance from InternalMSBuildTaskAttribute-marked base - /// applies non-inheritable semantics - tasks should run in TaskHost even if the attribute - /// is on a grandparent class. - /// - [Fact] - public void MultiLevelInheritanceFromMarkedBase_RunsInTaskHost_InMultiThreadedMode() - { - // Arrange - string projectContent = CreateTestProject( - taskName: "MultiLevelMarkedTestTask", - taskClass: "MultiLevelMarkedTask"); - string projectFile = Path.Combine(_testProjectsDir, "MultiLevelMarkedProject.proj"); - File.WriteAllText(projectFile, projectContent); - - var logger = new MockLogger(_output); - var buildParameters = new BuildParameters - { - MultiThreaded = true, - Loggers = new[] { logger }, - DisableInProcNode = false, - EnableNodeReuse = false - }; - - var buildRequestData = new BuildRequestData( - projectFile, - new Dictionary(), - null, - new[] { "TestTarget" }, - null); - - // Act - var buildManager = BuildManager.DefaultBuildManager; - var result = buildManager.Build(buildParameters, buildRequestData); - - // Assert - result.OverallResult.ShouldBe(BuildResultCode.Success); - - // Task should use TaskHost - non-inheritable semantics from marked grandparent - TaskRouterTestHelper.AssertTaskUsedTaskHost(logger, "MultiLevelMarkedTestTask"); - - // Verify task executed successfully - logger.FullLog.ShouldContain("MultiLevelMarkedTask executed"); - } private string CreateTestProject(string taskName, string taskClass) { @@ -640,115 +459,7 @@ public override bool Execute() } } - /// - /// Base task class that implements IMultiThreadableTask. - /// This simulates an external base class (not from MSBuild repo). - /// - public class BaseMultiThreadableTask : Task, IMultiThreadableTask - { - public TaskEnvironment TaskEnvironment { get; set; } - - public override bool Execute() - { - Log.LogMessage(MessageImportance.High, "BaseMultiThreadableTask executed"); - return true; - } - } - - /// - /// Base task class that implements IMultiThreadableTask and is marked with InternalMSBuildTaskAttribute. - /// This simulates a task from the MSBuild repository that needs non-inheritable semantics. - /// - [InternalMSBuildTask] - public class InternalBaseMultiThreadableTask : Task, IMultiThreadableTask - { - public TaskEnvironment TaskEnvironment { get; set; } - public override bool Execute() - { - Log.LogMessage(MessageImportance.High, "InternalBaseMultiThreadableTask executed"); - return true; - } - } - - /// - /// Task that inherits from BaseMultiThreadableTask (unmarked external base). - /// Should run IN-PROCESS in multi-threaded mode because of inheritable semantics. - /// - public class InheritedInterfaceTestTask : BaseMultiThreadableTask - { - public override bool Execute() - { - Log.LogMessage(MessageImportance.High, "InheritedInterfaceTask executed"); - return true; - } - } - - /// - /// Task that inherits from InternalBaseMultiThreadableTask (marked MSBuild repo base). - /// Should run in TaskHost in multi-threaded mode because of non-inheritable semantics - /// for backward compatibility. - /// - public class InheritedFromInternalBaseTestTask : InternalBaseMultiThreadableTask - { - public override bool Execute() - { - Log.LogMessage(MessageImportance.High, "InheritedFromInternalBaseTask executed"); - return true; - } - } - - /// - /// Intermediate task class for multi-level inheritance testing (unmarked). - /// Inherits from BaseMultiThreadableTask. - /// - public class MiddleUnmarkedTask : BaseMultiThreadableTask - { - public override bool Execute() - { - Log.LogMessage(MessageImportance.High, "MiddleUnmarkedTask executed"); - return true; - } - } - - /// - /// Task that inherits from MiddleUnmarkedTask (multi-level from unmarked base). - /// Should run in-process because of inheritable semantics through the chain. - /// - public class MultiLevelUnmarkedTestTask : MiddleUnmarkedTask - { - public override bool Execute() - { - Log.LogMessage(MessageImportance.High, "MultiLevelUnmarkedTask executed"); - return true; - } - } - - /// - /// Intermediate task class for multi-level inheritance testing (from marked base). - /// Inherits from InternalBaseMultiThreadableTask. - /// - public class MiddleMarkedTask : InternalBaseMultiThreadableTask - { - public override bool Execute() - { - Log.LogMessage(MessageImportance.High, "MiddleMarkedTask executed"); - return true; - } - } - - /// - /// Task that inherits from MiddleMarkedTask (multi-level from marked base). - /// Should run in TaskHost because InternalMSBuildTaskAttribute appears in the hierarchy. - /// - public class MultiLevelMarkedTestTask : MiddleMarkedTask - { - public override bool Execute() - { - Log.LogMessage(MessageImportance.High, "MultiLevelMarkedTask executed"); - return true; - } - } /// /// Task marked with MSBuildMultiThreadableTaskAttribute. @@ -782,6 +493,7 @@ namespace Microsoft.Build.Framework /// Test attribute to mark tasks as safe for multi-threaded execution. /// This is a test copy in this test assembly that will be recognized /// by name-based attribute detection in TaskRouter. + /// Must match the non-inheritable definition (Inherited = false). /// [AttributeUsage(AttributeTargets.Class, Inherited = false, AllowMultiple = false)] public sealed class MSBuildMultiThreadableTaskAttribute : Attribute diff --git a/src/Build/BackEnd/Components/RequestBuilder/TaskRouter.cs b/src/Build/BackEnd/Components/RequestBuilder/TaskRouter.cs index 7902662a5b8..3b9a6252e1a 100644 --- a/src/Build/BackEnd/Components/RequestBuilder/TaskRouter.cs +++ b/src/Build/BackEnd/Components/RequestBuilder/TaskRouter.cs @@ -3,7 +3,6 @@ using System; using System.Collections.Concurrent; -using Microsoft.Build.Framework; using Microsoft.Build.Shared; namespace Microsoft.Build.BackEnd @@ -21,135 +20,72 @@ namespace Microsoft.Build.BackEnd internal static class TaskRouter { /// - /// Cache of task types to their multi-threadable capability status. + /// Cache of task types to their multi-threadable attribute status. /// This avoids repeated reflection calls for the same task types. /// private static readonly ConcurrentDictionary s_multiThreadableTaskCache = new(); - /// - /// Determines if a task needs to be routed to an out-of-process TaskHost sidecar - /// in multi-threaded mode based on its thread-safety characteristics. - /// - /// The type of the task to evaluate. - /// - /// True if the task should be executed in an out-of-process TaskHost sidecar; - /// false if it can safely run in-process within a thread node. - /// - /// - /// This method only considers the task's thread-safety indicators. - /// The caller is responsible for: - /// - Only calling this in multi-threaded mode - /// - Handling explicit out-of-proc requests (via TaskHostFactory or parameters) - /// - Handling the isAlreadyOutOfProc scenario - /// - /// In multi-threaded mode: - /// - Tasks implementing IMultiThreadableTask OR marked with MSBuildMultiThreadableTaskAttribute - /// are considered thread-safe and can run in-process (returns false) - /// - Tasks without these indicators must run in a sidecar TaskHost for isolation (returns true) - /// - public static bool NeedsTaskHostInMultiThreadedMode(Type taskType) - { - ErrorUtilities.VerifyThrowArgumentNull(taskType, nameof(taskType)); - - // Tasks without thread-safety indicators need isolation in a TaskHost sidecar - return !IsMultiThreadableTask(taskType); - } - - /// - /// Checks if a task is multi-threadable via IMultiThreadableTask interface OR - /// MSBuildMultiThreadableTaskAttribute attribute. - /// Results are cached to avoid repeated reflection calls. - /// - /// The task type to check. - /// True if the task has thread-safety indicators; false otherwise. - private static bool IsMultiThreadableTask(Type taskType) - { - return s_multiThreadableTaskCache.GetOrAdd( - taskType, - static t => - ImplementsIMultiThreadableTask(t) || - HasMultiThreadableTaskAttribute(t)); - } - - /// - /// Checks if a task type implements the IMultiThreadableTask interface. - /// By default, the interface has inheritable semantics - if a task inherits from a base class - /// that implements IMultiThreadableTask, it is also considered thread-safe. - /// However, if the base class is marked with InternalMSBuildTaskAttribute (tasks from MSBuild repo), - /// non-inheritable semantics apply for backward compatibility. - /// - /// The task type to check. - /// True if the task should be considered multi-threadable; false otherwise. - private static bool ImplementsIMultiThreadableTask(Type taskType) - { - // Check if task is assignable to IMultiThreadableTask - if (!typeof(IMultiThreadableTask).IsAssignableFrom(taskType)) - { - return false; - } - - // Check if base type is also assignable to IMultiThreadableTask - Type? baseType = taskType.BaseType; - if (baseType != null && typeof(IMultiThreadableTask).IsAssignableFrom(baseType)) - { - // Inherited from base type - non-inheritable if base is from MSBuild repo - return !HasInternalMSBuildTaskAttribute(baseType); - } - - // Task implements the interface and base type doesn't - // This means the task directly declared it - return true; - } - - /// - /// Checks if a task type is marked with InternalMSBuildTaskAttribute. - /// This attribute marks tasks from the MSBuild repository that need non-inheritable - /// IMultiThreadableTask semantics for backward compatibility. - /// - /// The task type to check. - /// True if the task has the attribute; false otherwise. - private static bool HasInternalMSBuildTaskAttribute(Type taskType) - { - const string attributeFullName = "Microsoft.Build.Framework.InternalMSBuildTaskAttribute"; - - // Check for the attribute by full name - foreach (object attr in taskType.GetCustomAttributes(inherit: true)) - { - if (attr.GetType().FullName == attributeFullName) - { - return true; - } - } + /// + /// Determines if a task needs to be routed to an out-of-process TaskHost sidecar + /// in multi-threaded mode based on its thread-safety characteristics. + /// + /// The type of the task to evaluate. + /// + /// True if the task should be executed in an out-of-process TaskHost sidecar; + /// false if it can safely run in-process within a thread node. + /// + /// + /// This method only considers the task's thread-safety indicators. + /// The caller is responsible for: + /// - Only calling this in multi-threaded mode + /// - Handling explicit out-of-proc requests (via TaskHostFactory or parameters) + /// - Handling the isAlreadyOutOfProc scenario + /// + /// In multi-threaded mode: + /// - Tasks marked with MSBuildMultiThreadableTaskAttribute (non-inheritable) are considered + /// thread-safe and can run in-process (returns false) + /// - Tasks without the attribute must run in a sidecar TaskHost for isolation (returns true) + /// + public static bool NeedsTaskHostInMultiThreadedMode(Type taskType) + { + ErrorUtilities.VerifyThrowArgumentNull(taskType, nameof(taskType)); - return false; - } + // Tasks without the thread-safety attribute need isolation in a TaskHost sidecar + return !HasMultiThreadableTaskAttribute(taskType); + } /// /// Checks if a task type is marked with MSBuildMultiThreadableTaskAttribute. /// Detection is based on namespace and name only, ignoring the defining assembly, /// which allows customers to define the attribute in their own assemblies. + /// Results are cached to avoid repeated reflection calls. /// /// The task type to check. /// True if the task has the attribute; false otherwise. private static bool HasMultiThreadableTaskAttribute(Type taskType) { - const string attributeFullName = "Microsoft.Build.Framework.MSBuildMultiThreadableTaskAttribute"; - - // Check for the attribute by full name, not by type identity - // This allows custom-defined attributes from different assemblies - foreach (object attr in taskType.GetCustomAttributes(inherit: false)) - { - if (attr.GetType().FullName == attributeFullName) + return s_multiThreadableTaskCache.GetOrAdd( + taskType, + static t => { - return true; - } - } + const string attributeFullName = "Microsoft.Build.Framework.MSBuildMultiThreadableTaskAttribute"; + + // Check for the attribute by full name, not by type identity + // This allows custom-defined attributes from different assemblies + foreach (object attr in t.GetCustomAttributes(inherit: false)) + { + if (attr.GetType().FullName == attributeFullName) + { + return true; + } + } - return false; + return false; + }); } /// - /// Clears the thread-safety capability cache. Used primarily for testing. + /// Clears the thread-safety cache. Used primarily for testing. /// internal static void ClearCache() { diff --git a/src/Framework/InternalMSBuildTaskAttribute.cs b/src/Framework/InternalMSBuildTaskAttribute.cs deleted file mode 100644 index 4a7f1a2e311..00000000000 --- a/src/Framework/InternalMSBuildTaskAttribute.cs +++ /dev/null @@ -1,17 +0,0 @@ -// Licensed to the .NET Foundation under one or more agreements. -// The .NET Foundation licenses this file to you under the MIT license. - -using System; - -namespace Microsoft.Build.Framework -{ - /// - /// Internal marker attribute for tasks that are part of the MSBuild repository. - /// When a task inherits from a base class marked with this attribute, IMultiThreadableTask - /// is treated with non-inheritable semantics to maintain backward compatibility. - /// - [AttributeUsage(AttributeTargets.Class, Inherited = true, AllowMultiple = false)] - internal sealed class InternalMSBuildTaskAttribute : Attribute - { - } -} diff --git a/src/Framework/MSBuildMultiThreadableTaskAttribute.cs b/src/Framework/MSBuildMultiThreadableTaskAttribute.cs index ddad66ccc7c..7b13ad2bed1 100644 --- a/src/Framework/MSBuildMultiThreadableTaskAttribute.cs +++ b/src/Framework/MSBuildMultiThreadableTaskAttribute.cs @@ -12,8 +12,7 @@ namespace Microsoft.Build.Framework /// /// /// Task classes marked with this attribute indicate they can be safely executed in parallel - /// in the same process with other tasks. This is a compatibility bridge option for existing tasks - /// that do not have access to TaskEnvironment APIs. + /// in the same process with other tasks. /// /// Tasks using this attribute must satisfy strict requirements: /// - Must not modify global process state (environment variables, working directory, etc.) @@ -21,13 +20,12 @@ namespace Microsoft.Build.Framework /// /// MSBuild detects this attribute by its namespace and name only, ignoring the defining assembly. /// This allows customers to define the attribute in their own assemblies alongside their tasks. - /// Since MSBuild does not ship this attribute, task authors can copy this definition into their - /// own projects and mark their task classes with it. Customers using newer MSBuild versions - /// should prefer implementing IMultiThreadableTask which provides access to TaskEnvironment - /// for safe process state operations. + /// + /// When defining polyfilled versions of this attribute in customer assemblies, + /// they must also specify Inherited = false to ensure proper non-inheritable semantics. /// - [AttributeUsage(AttributeTargets.Class, AllowMultiple = false)] - internal class MSBuildMultiThreadableTaskAttribute : Attribute + [AttributeUsage(AttributeTargets.Class, AllowMultiple = false, Inherited = false)] + public class MSBuildMultiThreadableTaskAttribute : Attribute { /// /// Initializes a new instance of the MSBuildMultiThreadableTaskAttribute class.