diff --git a/.github/instructions/tasks.instructions.md b/.github/instructions/tasks.instructions.md index 2045910acc7..08ebfb794c3 100644 --- a/.github/instructions/tasks.instructions.md +++ b/.github/instructions/tasks.instructions.md @@ -32,6 +32,8 @@ Built-in tasks ship with MSBuild and cannot be independently versioned. * Support UNC paths, long paths (> 260 chars), and cross-platform separators. ## Multithreaded Task Migration + +* All built-in tasks implement `IMultiThreadableTask` with a default `TaskEnvironment` backed by `MultiProcessTaskEnvironmentDriver.Instance`. * Shared static state is a concurrency hazard in multi-process builds. ## Related Documentation diff --git a/.github/skills/multithreaded-task-migration/SKILL.md b/.github/skills/multithreaded-task-migration/SKILL.md index b098e2c49ce..2a64b6ecc8b 100644 --- a/.github/skills/multithreaded-task-migration/SKILL.md +++ b/.github/skills/multithreaded-task-migration/SKILL.md @@ -18,7 +18,7 @@ b. Implement `IMultiThreadableTask` **only if** the task needs `TaskEnvironment` [MSBuildMultiThreadableTask] public class MyTask : Task, IMultiThreadableTask { - public TaskEnvironment TaskEnvironment { get; set; } + public TaskEnvironment TaskEnvironment { get; set; } = new TaskEnvironment(MultiProcessTaskEnvironmentDriver.Instance); ... } ``` @@ -61,7 +61,7 @@ The [`AbsolutePath`](https://github.com/dotnet/msbuild/blob/main/src/Framework/P ## Updating Unit Tests -Every test creating a task instance must set `TaskEnvironment = TaskEnvironmentHelper.CreateForTest()`. +Built-in MSBuild tasks now initialize `TaskEnvironment` with a `MultiProcessTaskEnvironmentDriver`-backed default. Tests creating instances of built-in tasks no longer need manual `TaskEnvironment` setup. For custom or third-party tasks that implement `IMultiThreadableTask` without a default initializer, set `TaskEnvironment = TaskEnvironmentHelper.CreateForTest()`. ## APIs to Avoid @@ -276,7 +276,7 @@ Assertions: Execute() return value, [Output] exact string, error message content ## Sign-Off Checklist - [ ] `[MSBuildMultiThreadableTask]` on every concrete class (not just base — `Inherited=false`) -- [ ] `IMultiThreadableTask` only on classes that use `TaskEnvironment` APIs +- [ ] `IMultiThreadableTask` on classes that use `TaskEnvironment` APIs, with default initializer `= new TaskEnvironment(MultiProcessTaskEnvironmentDriver.Instance)` - [ ] Every `[Output]` property: exact string value matches pre-migration - [ ] Every `Log.LogError`/`LogWarning`: path in message matches pre-migration (use `OriginalValue`) - [ ] Every `GetAbsolutePath` call: null/empty exception behavior matches old code path @@ -285,7 +285,7 @@ Assertions: Execute() return value, [Output] exact string, error message content - [ ] Every `??` or `?.` added: verified it doesn't swallow a previously-thrown exception - [ ] No `AbsolutePath` leaks into user-visible strings unintentionally - [ ] Helper methods traced for internal File API usage with non-absolutized paths -- [ ] All tests set `TaskEnvironment = TaskEnvironmentHelper.CreateForTest()` +- [ ] Tests for custom tasks set `TaskEnvironment = TaskEnvironmentHelper.CreateForTest()` (built-in tasks have a default) - [ ] Cross-framework: tested on both net472 and net10.0 - [ ] Concurrent execution: two tasks with different project directories produce correct results - [ ] No forbidden APIs (`Environment.Exit`, `Console.*`, etc.) diff --git a/documentation/specs/multithreading/multithreaded-msbuild.md b/documentation/specs/multithreading/multithreaded-msbuild.md index cc224f4aa80..6b9f510f30f 100644 --- a/documentation/specs/multithreading/multithreaded-msbuild.md +++ b/documentation/specs/multithreading/multithreaded-msbuild.md @@ -234,7 +234,7 @@ The `MSBuildMultiThreadableTaskAttribute` is **non-inheritable** (`Inherited = f * Derived classes cannot accidentally inherit thread-safety assumptions from base classes * The routing decision is always explicit and visible in the task's source code -Tasks may optionally implement `IMultiThreadableTask` to access `TaskEnvironment` APIs, but only the attribute determines routing behavior. +Tasks may optionally implement `IMultiThreadableTask` to access `TaskEnvironment` APIs, but only the attribute determines routing behavior. If task implements `IMultiThreadableTask`, `TaskEnvironment` should be backed by `MultiProcessTaskEnvironmentDriver.Instance`, which acts as a fallback for explicit instantiation and task host scenarios. ## Tasks transition diff --git a/documentation/specs/multithreading/taskhost-threading.md b/documentation/specs/multithreading/taskhost-threading.md index 37116f3de74..c04d53f366b 100644 --- a/documentation/specs/multithreading/taskhost-threading.md +++ b/documentation/specs/multithreading/taskhost-threading.md @@ -26,7 +26,7 @@ When the main thread receives a `TaskHostConfiguration` packet, it spawns the ta 1. Sets up the environment (working directory, env vars, culture) 2. Loads the task assembly and instantiates the task 3. Sets task parameters via reflection -4. Calls `task.Execute()` +4. Calls `task.Execute()` — for tasks implementing `IMultiThreadableTask`, the default `TaskEnvironment` (backed by `MultiProcessTaskEnvironmentDriver`) is available, providing safe access to the task host process's working directory and environment variables 5. Collects output parameters 6. Packages the result into `TaskHostTaskComplete` and signals `_taskCompleteEvent` diff --git a/documentation/specs/multithreading/thread-safe-tasks.md b/documentation/specs/multithreading/thread-safe-tasks.md index a983fda8ef7..3ef98a4b215 100644 --- a/documentation/specs/multithreading/thread-safe-tasks.md +++ b/documentation/specs/multithreading/thread-safe-tasks.md @@ -39,10 +39,12 @@ Similar to how MSBuild provides the abstract `Task` class with default implement namespace Microsoft.Build.Utilities; public abstract class MultiThreadableTask : Task, IMultiThreadableTask { - public TaskEnvironment TaskEnvironment{ get; set; } + public TaskEnvironment TaskEnvironment{ get; set; } = new TaskEnvironment(MultiProcessTaskEnvironmentDriver.Instance); } ``` +Built-in MSBuild tasks initialize `TaskEnvironment` with a `MultiProcessTaskEnvironmentDriver`-backed default. This ensures tasks have a usable `TaskEnvironment` even when explicitly instantiated outside the engine (e.g., `new Copy()`) or run in the out-of-proc task host. The engine's in-proc path (`TaskExecutionHost.InitializeForBatch`) overwrites the default with the appropriate driver before `Execute()` is called. + Task authors who want to support older MSBuild versions need to: - Maintain both thread-safe and legacy implementations. - Use conditional task declarations based on MSBuild version to select which assembly to load the task from. diff --git a/src/Build.UnitTests/BackEnd/TaskHost_MultiThreadableTask_Tests.cs b/src/Build.UnitTests/BackEnd/TaskHost_MultiThreadableTask_Tests.cs new file mode 100644 index 00000000000..f1e59ba557b --- /dev/null +++ b/src/Build.UnitTests/BackEnd/TaskHost_MultiThreadableTask_Tests.cs @@ -0,0 +1,122 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System; +using System.Collections.Generic; +using System.IO; +using System.Reflection; +using Microsoft.Build.Execution; +using Microsoft.Build.Framework; +using Microsoft.Build.Tasks; +using Microsoft.Build.UnitTests; +using Microsoft.Build.Utilities; +using Shouldly; +using Xunit; +using Xunit.Abstractions; + +namespace Microsoft.Build.Engine.UnitTests.BackEnd +{ + /// + /// Tests that IMultiThreadableTask implementations always have a usable TaskEnvironment, + /// even when explicitly instantiated or run in the out-of-proc task host. + /// + public class TaskHost_MultiThreadableTask_Tests : IDisposable + { + private readonly ITestOutputHelper _output; + private readonly TestEnvironment _env; + private readonly string _testProjectsDir; + + public TaskHost_MultiThreadableTask_Tests(ITestOutputHelper output) + { + _output = output; + _env = TestEnvironment.Create(output); + _testProjectsDir = _env.CreateFolder().Path; + } + + public void Dispose() + { + _env.Dispose(); + } + + /// + /// A subclass of a built-in IMultiThreadableTask (MakeDir) should inherit the + /// non-null default TaskEnvironment. This covers the scenario where someone + /// derives from a built-in task and explicitly instantiates it. + /// + [Fact] + public void ExplicitlyInstantiated_InheritedTask_HasNonNullTaskEnvironment() + { + var task = new DerivedMakeDirTask(); + IMultiThreadableTask multiThreadable = task; + + multiThreadable.TaskEnvironment.ShouldNotBeNull(); + } + + /// + /// When a task that inherits from a built-in IMultiThreadableTask (MakeDir) runs in + /// the out-of-proc task host (via TaskHostFactory), the inherited default TaskEnvironment + /// should be usable. The task accesses TaskEnvironment.ProjectDirectory in Execute() — + /// without the default it would NRE. + /// + [Fact] + public void InheritedTask_InTaskHost_HasUsableTaskEnvironment() + { + string projectContent = $""" + + + + + + + + """; + + string projectFile = Path.Combine(_testProjectsDir, "TaskEnvTest.proj"); + File.WriteAllText(projectFile, projectContent); + + var logger = new MockLogger(_output); + var buildParameters = new BuildParameters + { + Loggers = [logger], + DisableInProcNode = false, + EnableNodeReuse = false, + }; + + var buildRequestData = new BuildRequestData( + projectFile, + new Dictionary(), + null, + ["TestTarget"], + null); + + var result = BuildManager.DefaultBuildManager.Build(buildParameters, buildRequestData); + + _output.WriteLine(logger.FullLog); + + result.OverallResult.ShouldBe(BuildResultCode.Success); + + // Verify the task actually ran in the task host + TaskRouterTestHelper.AssertTaskUsedTaskHost(logger, "DerivedMakeDirTask"); + + // Verify the task was able to read ProjectDirectory without NRE + logger.FullLog.ShouldContain("TaskEnvironment.ProjectDirectory="); + } + } + + /// + /// Task that inherits from the built-in MakeDir (which implements IMultiThreadableTask). + /// Does NOT declare its own TaskEnvironment — it relies on the inherited default from MakeDir. + /// Overrides Execute() to log TaskEnvironment.ProjectDirectory, proving the default works. + /// + public class DerivedMakeDirTask : MakeDir + { + public override bool Execute() + { + string projectDir = TaskEnvironment.ProjectDirectory; + Log.LogMessage(MessageImportance.High, $"TaskEnvironment.ProjectDirectory={projectDir}"); + return true; + } + } +} diff --git a/src/Tasks/AssignTargetPath.cs b/src/Tasks/AssignTargetPath.cs index f8f42c9945d..43e39efd7a2 100644 --- a/src/Tasks/AssignTargetPath.cs +++ b/src/Tasks/AssignTargetPath.cs @@ -23,7 +23,7 @@ public class AssignTargetPath : TaskExtension, IMultiThreadableTask /// /// Gets or sets the task execution environment for thread-safe path resolution. /// - public TaskEnvironment TaskEnvironment { get; set; } + public TaskEnvironment TaskEnvironment { get; set; } = new TaskEnvironment(MultiProcessTaskEnvironmentDriver.Instance); /// diff --git a/src/Tasks/Copy.cs b/src/Tasks/Copy.cs index 36d2a3d20f9..58ff7cfc6ae 100644 --- a/src/Tasks/Copy.cs +++ b/src/Tasks/Copy.cs @@ -187,7 +187,7 @@ public Copy() public bool FailIfNotIncremental { get; set; } /// - public TaskEnvironment TaskEnvironment { get; set; } + public TaskEnvironment TaskEnvironment { get; set; } = new TaskEnvironment(MultiProcessTaskEnvironmentDriver.Instance); #endregion diff --git a/src/Tasks/Delete.cs b/src/Tasks/Delete.cs index d687b9c0006..b928bf858af 100644 --- a/src/Tasks/Delete.cs +++ b/src/Tasks/Delete.cs @@ -65,7 +65,7 @@ public ITaskItem[] Files public bool FailIfNotIncremental { get; set; } /// - public TaskEnvironment TaskEnvironment { get; set; } + public TaskEnvironment TaskEnvironment { get; set; } = new TaskEnvironment(MultiProcessTaskEnvironmentDriver.Instance); /// /// Verify that the inputs are correct. diff --git a/src/Tasks/DownloadFile.cs b/src/Tasks/DownloadFile.cs index 5da52626eed..2386d482d8c 100644 --- a/src/Tasks/DownloadFile.cs +++ b/src/Tasks/DownloadFile.cs @@ -69,7 +69,7 @@ public sealed class DownloadFile : TaskExtension, ICancelableTask, IIncrementalT public bool FailIfNotIncremental { get; set; } /// - public TaskEnvironment TaskEnvironment { get; set; } + public TaskEnvironment TaskEnvironment { get; set; } = new TaskEnvironment(MultiProcessTaskEnvironmentDriver.Instance); /// /// Gets or sets a to use. This is used by unit tests to mock a connection to a remote server. diff --git a/src/Tasks/FileIO/GetFileHash.cs b/src/Tasks/FileIO/GetFileHash.cs index 655ff3a6de8..9aa91f99ad0 100644 --- a/src/Tasks/FileIO/GetFileHash.cs +++ b/src/Tasks/FileIO/GetFileHash.cs @@ -66,7 +66,7 @@ internal static readonly Dictionary> SupportedAlgori public ITaskItem[] Items { get; set; } /// - public TaskEnvironment TaskEnvironment { get; set; } + public TaskEnvironment TaskEnvironment { get; set; } = new TaskEnvironment(MultiProcessTaskEnvironmentDriver.Instance); public override bool Execute() { diff --git a/src/Tasks/FileIO/ReadLinesFromFile.cs b/src/Tasks/FileIO/ReadLinesFromFile.cs index 1b80db354aa..552db96d7ed 100644 --- a/src/Tasks/FileIO/ReadLinesFromFile.cs +++ b/src/Tasks/FileIO/ReadLinesFromFile.cs @@ -25,7 +25,7 @@ public class ReadLinesFromFile : TaskExtension, IMultiThreadableTask public ITaskItem File { get; set; } /// - public TaskEnvironment TaskEnvironment { get; set; } + public TaskEnvironment TaskEnvironment { get; set; } = new TaskEnvironment(MultiProcessTaskEnvironmentDriver.Instance); /// /// Receives lines from file. diff --git a/src/Tasks/FileIO/VerifyFileHash.cs b/src/Tasks/FileIO/VerifyFileHash.cs index 8dab7a858b2..9b6dd12d3d0 100644 --- a/src/Tasks/FileIO/VerifyFileHash.cs +++ b/src/Tasks/FileIO/VerifyFileHash.cs @@ -17,7 +17,7 @@ namespace Microsoft.Build.Tasks public sealed class VerifyFileHash : TaskExtension, ICancelableTask, IMultiThreadableTask { /// - public TaskEnvironment TaskEnvironment { get; set; } + public TaskEnvironment TaskEnvironment { get; set; } = new TaskEnvironment(MultiProcessTaskEnvironmentDriver.Instance); /// /// The file path. diff --git a/src/Tasks/FileIO/WriteLinesToFile.cs b/src/Tasks/FileIO/WriteLinesToFile.cs index 82c6517eb93..0cced7ad137 100644 --- a/src/Tasks/FileIO/WriteLinesToFile.cs +++ b/src/Tasks/FileIO/WriteLinesToFile.cs @@ -23,7 +23,7 @@ public class WriteLinesToFile : TaskExtension, IIncrementalTask, IMultiThreadabl private static readonly Encoding s_defaultEncoding = new UTF8Encoding(encoderShouldEmitUTF8Identifier: false, throwOnInvalidBytes: true); /// - public TaskEnvironment TaskEnvironment { get; set; } + public TaskEnvironment TaskEnvironment { get; set; } = new TaskEnvironment(MultiProcessTaskEnvironmentDriver.Instance); /// /// File to write lines to. diff --git a/src/Tasks/ListOperators/FindUnderPath.cs b/src/Tasks/ListOperators/FindUnderPath.cs index 46d280dd332..1347cbd3528 100644 --- a/src/Tasks/ListOperators/FindUnderPath.cs +++ b/src/Tasks/ListOperators/FindUnderPath.cs @@ -21,7 +21,7 @@ public class FindUnderPath : TaskExtension, IMultiThreadableTask /// /// Gets or sets the task execution environment for thread-safe path resolution. /// - public TaskEnvironment TaskEnvironment { get; set; } + public TaskEnvironment TaskEnvironment { get; set; } = new TaskEnvironment(MultiProcessTaskEnvironmentDriver.Instance); /// /// Filter based on whether items fall under this path or not. diff --git a/src/Tasks/MakeDir.cs b/src/Tasks/MakeDir.cs index b16b810d5b2..cc3f4a33ccf 100644 --- a/src/Tasks/MakeDir.cs +++ b/src/Tasks/MakeDir.cs @@ -35,7 +35,7 @@ public ITaskItem[] Directories public bool FailIfNotIncremental { get; set; } /// - public TaskEnvironment TaskEnvironment { get; set; } + public TaskEnvironment TaskEnvironment { get; set; } = new TaskEnvironment(MultiProcessTaskEnvironmentDriver.Instance); private ITaskItem[] _directories; diff --git a/src/Tasks/Move.cs b/src/Tasks/Move.cs index 5e0a13b3742..e764d89cfe1 100644 --- a/src/Tasks/Move.cs +++ b/src/Tasks/Move.cs @@ -76,7 +76,7 @@ public class Move : TaskExtension, ICancelableTask, IIncrementalTask, IMultiThre public bool FailIfNotIncremental { get; set; } /// - public TaskEnvironment TaskEnvironment { get; set; } + public TaskEnvironment TaskEnvironment { get; set; } = new TaskEnvironment(MultiProcessTaskEnvironmentDriver.Instance); /// /// Stop and return (in an undefined state) as soon as possible. diff --git a/src/Tasks/RemoveDir.cs b/src/Tasks/RemoveDir.cs index 5eeda7f53d4..83b4569c8d2 100644 --- a/src/Tasks/RemoveDir.cs +++ b/src/Tasks/RemoveDir.cs @@ -25,7 +25,7 @@ public class RemoveDir : TaskExtension, IIncrementalTask, IMultiThreadableTask private ITaskItem[] _directories; /// - public TaskEnvironment TaskEnvironment { get; set; } + public TaskEnvironment TaskEnvironment { get; set; } = new TaskEnvironment(MultiProcessTaskEnvironmentDriver.Instance); [Required] public ITaskItem[] Directories diff --git a/src/Tasks/Touch.cs b/src/Tasks/Touch.cs index 3ca82c7c951..b524a842d16 100644 --- a/src/Tasks/Touch.cs +++ b/src/Tasks/Touch.cs @@ -50,7 +50,7 @@ public class Touch : TaskExtension, IIncrementalTask, IMultiThreadableTask public ITaskItem[] TouchedFiles { get; set; } /// - public TaskEnvironment TaskEnvironment { get; set; } + public TaskEnvironment TaskEnvironment { get; set; } = new TaskEnvironment(MultiProcessTaskEnvironmentDriver.Instance); /// /// Importance: high, normal, low (default normal) diff --git a/src/Tasks/Unzip.cs b/src/Tasks/Unzip.cs index fd262b1007c..e1a0dfa0de3 100644 --- a/src/Tasks/Unzip.cs +++ b/src/Tasks/Unzip.cs @@ -77,7 +77,7 @@ public sealed class Unzip : TaskExtension, ICancelableTask, IIncrementalTask, IM public bool FailIfNotIncremental { get; set; } /// - public TaskEnvironment TaskEnvironment { get; set; } + public TaskEnvironment TaskEnvironment { get; set; } = new TaskEnvironment(MultiProcessTaskEnvironmentDriver.Instance); /// public void Cancel() diff --git a/src/Tasks/WriteCodeFragment.cs b/src/Tasks/WriteCodeFragment.cs index c0f58314d0f..7fd8815cbdd 100644 --- a/src/Tasks/WriteCodeFragment.cs +++ b/src/Tasks/WriteCodeFragment.cs @@ -34,7 +34,7 @@ namespace Microsoft.Build.Tasks public class WriteCodeFragment : TaskExtension, IMultiThreadableTask { /// - public TaskEnvironment TaskEnvironment { get; set; } + public TaskEnvironment TaskEnvironment { get; set; } = new TaskEnvironment(MultiProcessTaskEnvironmentDriver.Instance); private const string TypeNameSuffix = "_TypeName"; private const string IsLiteralSuffix = "_IsLiteral"; private static readonly string[] NamespaceImports = ["System", "System.Reflection"]; diff --git a/src/Tasks/XmlPeek.cs b/src/Tasks/XmlPeek.cs index b156df8b0a5..026be8fcf65 100644 --- a/src/Tasks/XmlPeek.cs +++ b/src/Tasks/XmlPeek.cs @@ -25,7 +25,7 @@ public class XmlPeek : TaskExtension, IMultiThreadableTask #region Properties /// - public TaskEnvironment TaskEnvironment { get; set; } + public TaskEnvironment TaskEnvironment { get; set; } = new TaskEnvironment(MultiProcessTaskEnvironmentDriver.Instance); /// /// The XPath Query. diff --git a/src/Tasks/XmlPoke.cs b/src/Tasks/XmlPoke.cs index 59b368c6dcc..1ca7f718c91 100644 --- a/src/Tasks/XmlPoke.cs +++ b/src/Tasks/XmlPoke.cs @@ -24,7 +24,7 @@ public class XmlPoke : TaskExtension, IMultiThreadableTask #region Properties /// - public TaskEnvironment TaskEnvironment { get; set; } + public TaskEnvironment TaskEnvironment { get; set; } = new TaskEnvironment(MultiProcessTaskEnvironmentDriver.Instance); /// /// The XML input as file path. diff --git a/src/Tasks/XslTransformation.cs b/src/Tasks/XslTransformation.cs index bacc8c910ac..1d760bb619b 100644 --- a/src/Tasks/XslTransformation.cs +++ b/src/Tasks/XslTransformation.cs @@ -29,7 +29,7 @@ public class XslTransformation : TaskExtension, IMultiThreadableTask #region Members /// - public TaskEnvironment TaskEnvironment { get; set; } + public TaskEnvironment TaskEnvironment { get; set; } = new TaskEnvironment(MultiProcessTaskEnvironmentDriver.Instance); /// /// The output files. diff --git a/src/Tasks/ZipDirectory.cs b/src/Tasks/ZipDirectory.cs index a93097933e8..30ed730be31 100644 --- a/src/Tasks/ZipDirectory.cs +++ b/src/Tasks/ZipDirectory.cs @@ -59,7 +59,7 @@ public sealed class ZipDirectory : TaskExtension, IIncrementalTask, IMultiThread public string? CompressionLevel { get; set; } /// - public TaskEnvironment TaskEnvironment { get; set; } = null!; + public TaskEnvironment TaskEnvironment { get; set; } = new TaskEnvironment(MultiProcessTaskEnvironmentDriver.Instance); public override bool Execute() {