Multithreading migration: Group 6 — 3 attribute-only + 2 Pattern B tasks by SimaTian · Pull Request #53117 · dotnet/sdk

SimaTian · 2026-02-22T20:00:56Z

Summary

Migrate 5 tasks to support multithreaded MSBuild execution. Three tasks are attribute-only (Pattern A), two require full interface-based migration (Pattern B) with TaskEnvironment path absolutization.

Tasks Migrated

Task	Pattern	Key Changes
GetDefaultPlatformTargetForNetFramework	A	Attribute already on main; tests added
GetEmbeddedApphostPaths	A	Attribute already on main; tests added
GetNuGetShortFolderName	A	Attribute already on main; tests added
ProduceContentAssets	B	Added attribute, `IMultiThreadableTask`, `TaskEnvironment`; absolutized `ContentPreprocessorOutputDirectory` and `contentFile.ItemSpec` before file I/O
ResolveCopyLocalAssets	B	Added attribute, `IMultiThreadableTask`, `TaskEnvironment`; absolutized `AssetsFilePath` before `LockFileCache.GetLockFile()`

Pattern B Details

ProduceContentAssets: Delegates to NugetContentAssetPreprocessor which uses File.Exists, File.OpenRead, File.WriteAllText, Directory.CreateDirectory. Both ContentPreprocessorOutputDirectory and contentFile.ItemSpec are absolutized via TaskEnvironment.GetAbsolutePath() before passing to the preprocessor.

ResolveCopyLocalAssets: Calls LockFileCache.GetLockFile(AssetsFilePath) which enforces Path.IsPathRooted() and throws if not. AssetsFilePath is absolutized before the call.

Tests Added

GivenAttributeOnlyTasksGroup6.cs — attribute-presence and behavioral tests for all 5 tasks, concurrent execution tests
GivenAProduceContentAssetsMultiThreading.cs — CWD-independence parity test
GivenAResolveCopyLocalAssetsMultiThreading.cs — CWD-independence parity test

Files Changed

src/Tasks/Microsoft.NET.Build.Tasks/ProduceContentAssets.cs
src/Tasks/Microsoft.NET.Build.Tasks/ResolveCopyLocalAssets.cs
src/Tasks/Microsoft.NET.Build.Tasks.UnitTests/GivenAttributeOnlyTasksGroup6.cs (new)
src/Tasks/Microsoft.NET.Build.Tasks.UnitTests/GivenAProduceContentAssetsMultiThreading.cs (new)
src/Tasks/Microsoft.NET.Build.Tasks.UnitTests/GivenAResolveCopyLocalAssetsMultiThreading.cs (new)

Tasks: GetDefaultPlatformTargetForNetFramework, GetEmbeddedApphostPaths, GetNuGetShortFolderName, ProduceContentAssets, ResolveCopyLocalAssets. These tasks already have [MSBuildMultiThreadableTask] attribute. This commit adds TDD tests verifying attribute presence. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

…(Group 6) Both tasks perform file I/O and were incorrectly treated as attribute-only. ProduceContentAssets: Add [MSBuildMultiThreadableTask], IMultiThreadableTask, TaskEnvironment. Absolutize ContentPreprocessorOutputDirectory via TaskEnvironment.GetAbsolutePath(). ResolveCopyLocalAssets: Add [MSBuildMultiThreadableTask], IMultiThreadableTask, TaskEnvironment. Absolutize AssetsFilePath via TaskEnvironment.GetAbsolutePath(). Add attribute-presence tests for all 5 Group 6 tasks. Add parity test files for the two migrated tasks. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

…up 6) - Add TaskEnvironment to ResolveCopyLocalAssets_WithMinimalAssetsFile_Succeeds test - Absolutize contentFile.ItemSpec before passing to AssetPreprocessor.Process() Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot

Pull request overview

Migrates a set of MSBuild tasks in Microsoft.NET.Build.Tasks to be safe under multithreaded MSBuild execution by marking them multi-threadable and (for Pattern B) resolving file-system paths via TaskEnvironment instead of process-wide CWD assumptions.

Changes:

Mark ProduceContentAssets and ResolveCopyLocalAssets as [MSBuildMultiThreadableTask] and implement IMultiThreadableTask, using TaskEnvironment.GetAbsolutePath(...) before file I/O.
Add a new consolidated unit test file covering attribute presence, behavior, and parallel execution smoke tests for the 5 tasks.
Add CWD-independence parity tests for the two Pattern B tasks.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 6 comments.

Show a summary per file

File	Description
src/Tasks/Microsoft.NET.Build.Tasks/ProduceContentAssets.cs	Implements Pattern B migration and uses `TaskEnvironment` to absolutize output directory and content file paths.
src/Tasks/Microsoft.NET.Build.Tasks/ResolveCopyLocalAssets.cs	Implements Pattern B migration and uses `TaskEnvironment` to absolutize `AssetsFilePath` before lock file load.
src/Tasks/Microsoft.NET.Build.Tasks.UnitTests/GivenAttributeOnlyTasksGroup6.cs	Adds attribute presence, behavioral coverage, and parallel execution smoke tests for the group.
src/Tasks/Microsoft.NET.Build.Tasks.UnitTests/GivenAProduceContentAssetsMultiThreading.cs	Adds CWD-independence test coverage for `ProduceContentAssets`.
src/Tasks/Microsoft.NET.Build.Tasks.UnitTests/GivenAResolveCopyLocalAssetsMultiThreading.cs	Adds CWD-independence test coverage for `ResolveCopyLocalAssets`.

Comments suppressed due to low confidence (1)

src/Tasks/Microsoft.NET.Build.Tasks.UnitTests/GivenAttributeOnlyTasksGroup6.cs:521

The Barrier.SignalAndWait() is inside a try. If any iteration throws before reaching the barrier, the remaining iterations can block indefinitely and hang the test run. Consider ensuring the barrier is always signaled (e.g., via finally/timeout) or using a coordination primitive that won’t deadlock the runner.

            var barrier = new Barrier(parallelism);

            Parallel.For(0, parallelism, new ParallelOptions { MaxDegreeOfParallelism = parallelism }, i =>
            {
                try

Copilot · 2026-02-22T20:05:57Z

        {
            var lockFileCache = new LockFileCache(this);
-            LockFile lockFile = lockFileCache.GetLockFile(AssetsFilePath);
+            LockFile lockFile = lockFileCache.GetLockFile(TaskEnvironment.GetAbsolutePath(AssetsFilePath));
            HashSet<PackageIdentity> packagestoBeFiltered = null;


TaskEnvironment can be null when this task is executed under MSBuild versions that don't populate IMultiThreadableTask.TaskEnvironment (or if invoked directly in-process). Calling TaskEnvironment.GetAbsolutePath(AssetsFilePath) will then throw NullReferenceException, and it can also change the error behavior for empty/null AssetsFilePath (throwing ArgumentException before LockFileCache produces the intended BuildErrorException). Consider falling back to the original AssetsFilePath when TaskEnvironment is null and only absolutizing when it’s non-null/non-empty so existing validation/messages remain consistent.

Copilot · 2026-02-22T20:05:58Z

                }

-                AssetPreprocessor.ConfigurePreprocessor(ContentPreprocessorOutputDirectory, preprocessorValues);
+                AssetPreprocessor.ConfigurePreprocessor(TaskEnvironment.GetAbsolutePath(ContentPreprocessorOutputDirectory), preprocessorValues);


TaskEnvironment may be null at runtime (e.g., older MSBuild that doesn't set it). Unconditionally calling TaskEnvironment.GetAbsolutePath(ContentPreprocessorOutputDirectory) can cause a NullReferenceException and break existing behavior where relative paths were resolved against the process CWD. Consider guarding on TaskEnvironment != null (and leaving the original string when it’s null) to preserve backwards compatibility.

Suggested change

AssetPreprocessor.ConfigurePreprocessor(TaskEnvironment.GetAbsolutePath(ContentPreprocessorOutputDirectory), preprocessorValues);

AssetPreprocessor.ConfigurePreprocessor(

TaskEnvironment != null

? TaskEnvironment.GetAbsolutePath(ContentPreprocessorOutputDirectory)

: ContentPreprocessorOutputDirectory,

preprocessorValues);

Copilot · 2026-02-22T20:05:58Z

        private void ProduceContentAsset(ITaskItem contentFile)
        {
-            string resolvedPath = contentFile.ItemSpec;
+            string resolvedPath = TaskEnvironment.GetAbsolutePath(contentFile.ItemSpec);


TaskEnvironment.GetAbsolutePath(contentFile.ItemSpec) will throw if TaskEnvironment is null (older MSBuild) and also changes behavior compared to the previous code path that used ItemSpec directly (resolved relative to process CWD). To keep compatibility, consider using contentFile.ItemSpec when TaskEnvironment is null and only absolutizing when it’s available.

Suggested change

string resolvedPath = TaskEnvironment.GetAbsolutePath(contentFile.ItemSpec);

string resolvedPath = TaskEnvironment != null

? TaskEnvironment.GetAbsolutePath(contentFile.ItemSpec)

: contentFile.ItemSpec;

Copilot · 2026-02-22T20:05:58Z

+                var task = new ResolveCopyLocalAssets
+                {
+                    BuildEngine = new MockBuildEngine(),
+                    AssetsFilePath = "obj\\project.assets.json",


This test uses a Windows-only path separator ("obj\\project.assets.json"). On non-Windows platforms, backslash is a valid filename character rather than a directory separator, so the task won’t find the file and the test will fail. Use Path.Combine("obj", "project.assets.json") (or Path.DirectorySeparatorChar) to build the relative path.

Suggested change

AssetsFilePath = "obj\\project.assets.json",

AssetsFilePath = Path.Combine("obj", "project.assets.json"),

Copilot · 2026-02-22T20:05:58Z

+                // --- Multiprocess mode: CWD == projectDir ---
+                Directory.SetCurrentDirectory(projectDir);
+                var engine1 = new MockBuildEngine();
+                var task1 = new ProduceContentAssets
+                {


This test mutates the process-wide current directory via Directory.SetCurrentDirectory(...). Because xUnit runs tests in parallel by default, this can race with other tests in the same assembly and cause flakiness. Consider isolating this test in a non-parallelized collection or restructuring to avoid changing CWD.

Copilot · 2026-02-22T20:05:59Z

+                // --- Multiprocess mode: CWD == projectDir ---
+                Directory.SetCurrentDirectory(projectDir);
+                var engine1 = new MockBuildEngine();
+                var task1 = new ResolveCopyLocalAssets
+                {


This test changes the process-wide current directory (Directory.SetCurrentDirectory). With xUnit’s default parallel execution, this can interfere with unrelated tests and introduce flakiness. Consider putting this test into a non-parallelized collection or avoiding global CWD mutation.

- Replace Barrier+Parallel.For with ManualResetEventSlim+Task.Run - Use async test methods with Task.WhenAll - Replace Windows path separator with Path.Combine Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

ProduceContentAssets now uses TaskEnvironment.GetAbsolutePath() in ProduceContentAsset(). Tests must provide TaskEnvironment. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

ProduceContentAssets now uses TaskEnvironment.GetAbsolutePath() in ProduceContentAsset(). All 4 behavioral tests must provide TaskEnvironment. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

- Add TaskEnvironmentDefaults.cs for NETFRAMEWORK lazy-init fallback - Apply lazy-init pattern to ProduceContentAssets, ResolveCopyLocalAssets Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

The combining constructor AbsolutePath(string, AbsolutePath) used Path.Combine without Path.GetFullPath, leaving '..' segments unresolved. This caused output paths like 'dir\..\ClassLib\...' instead of 'ClassLib\...', breaking string-based path comparisons in downstream MSBuild targets and tests. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

On .NET Framework, ProcessStartInfo defaults to UseShellExecute=true, which prevents EnvironmentVariables from being applied. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Normalization is caller's responsibility, matching real MSBuild polyfill. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

- Removed 5 attribute-presence tests from GivenAttributeOnlyTasksGroup6.cs (redundant per skill guidelines) - Fixed 8 NullReferenceExceptions in GivenAProduceContentsAssetsTask.cs: existing tests weren't setting TaskEnvironment after ProduceContentAssets migration - Added TaskEnvironment + BuildEngine to all 8 test methods - Made ContentOutputDirectory/PackageRootDirectory absolute to match GetAbsolutePath() output Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

…tems in parity tests - Concurrent tests now validate per-thread output values (not just exception-free) - Parity tests compare output item counts (ProcessedContentItems, CopyLocalItems, FileWrites, ResolvedAssets) - ProduceContentAssets_EmptyDependencies_Succeeds now sets TaskEnvironment Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

SimaTian · 2026-04-17T08:57:40Z

Temporarily closing due to review-queue bottleneck — one big grouped PR has turned out harder to review than 5 individual task PRs (see #52936 thread).

Strategy update: tasks in this PR have been recorded in our internal postponed-tasks backlog and will be re-opened as individual per-task PRs once the current merge-group-2 and merge-group-5 splits are cleared. Note that some tasks in this group (ValidateExecutableReferences, ProcessFrameworkReferences, ResolveTargetingPackAssets) have already been delivered via individual PRs based on OrchardCore profiling priority — see #53942, #53943, #53944, #53945, #53946.

SimaTian and others added 4 commits February 22, 2026 16:27

Add concurrent execution tests for Group 6 tasks

974db65

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot AI review requested due to automatic review settings February 22, 2026 20:00

Copilot started reviewing on behalf of SimaTian February 22, 2026 20:01 View session

Copilot AI reviewed Feb 22, 2026

View reviewed changes

Address PR review: ManualResetEventSlim + Path.Combine

0784270

- Replace Barrier+Parallel.For with ManualResetEventSlim+Task.Run - Use async test methods with Task.WhenAll - Replace Windows path separator with Path.Combine Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

SimaTian self-assigned this Feb 23, 2026

SimaTian requested a review from JanProvaznik February 23, 2026 14:13

SimaTian and others added 2 commits February 23, 2026 16:39

Add TaskEnvironment to ProduceContentAssets tests

9e0fc69

ProduceContentAssets now uses TaskEnvironment.GetAbsolutePath() in ProduceContentAsset(). Tests must provide TaskEnvironment. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Add TaskEnvironment to ProduceContentAssets tests

ffd3e45

ProduceContentAssets now uses TaskEnvironment.GetAbsolutePath() in ProduceContentAsset(). All 4 behavioral tests must provide TaskEnvironment. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

SimaTian force-pushed the merge-group-6 branch from 51e3e98 to ffd3e45 Compare February 23, 2026 15:49

SimaTian and others added 3 commits February 23, 2026 17:12

Fix CWD race: add [Collection] to serialize CWD-mutating tests

c19ea45

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Add TaskEnvironment lazy-init for merge-group-6 tasks

78a2816

- Add TaskEnvironmentDefaults.cs for NETFRAMEWORK lazy-init fallback - Apply lazy-init pattern to ProduceContentAssets, ResolveCopyLocalAssets Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

SimaTian marked this pull request as draft February 24, 2026 17:54

This was referenced Feb 24, 2026

The Operation will be canceled. The next steps may not contain expected logs. dotnet/dnceng#3008

Open

[android-arm64] The Operation will be canceled. The next steps may not contain expected logs. dotnet/dnceng#6408

Open

SimaTian and others added 4 commits February 25, 2026 13:20

Add UseShellExecute=false in ProcessTaskEnvironmentDriver

166837f

On .NET Framework, ProcessStartInfo defaults to UseShellExecute=true, which prevents EnvironmentVariables from being applied. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Revert AbsolutePath: remove Path.GetFullPath wrapping

49ccdc3

Normalization is caller's responsibility, matching real MSBuild polyfill. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

SimaTian force-pushed the merge-group-6 branch from 205d6b1 to 6a5059d Compare March 26, 2026 15:59

SimaTian closed this Apr 17, 2026

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Multithreading migration: Group 6 — 3 attribute-only + 2 Pattern B tasks#53117

Multithreading migration: Group 6 — 3 attribute-only + 2 Pattern B tasks#53117
SimaTian wants to merge 14 commits intodotnet:mainfrom
SimaTian:merge-group-6

SimaTian commented Feb 22, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Feb 22, 2026

Uh oh!

Copilot AI Feb 22, 2026

Uh oh!

Copilot AI Feb 22, 2026

Uh oh!

Copilot AI Feb 22, 2026

Uh oh!

Copilot AI Feb 22, 2026

Uh oh!

Copilot AI Feb 22, 2026

Uh oh!

SimaTian commented Apr 17, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

-            string resolvedPath = TaskEnvironment.GetAbsolutePath(contentFile.ItemSpec);
+            string resolvedPath = TaskEnvironment != null
+                ? TaskEnvironment.GetAbsolutePath(contentFile.ItemSpec)
+                : contentFile.ItemSpec;

	AssetsFilePath = "obj\\project.assets.json",
	AssetsFilePath = Path.Combine("obj", "project.assets.json"),

Conversation

SimaTian commented Feb 22, 2026

Summary

Tasks Migrated

Pattern B Details

Tests Added

Files Changed

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Feb 22, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 22, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 22, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 22, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 22, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 22, 2026

Choose a reason for hiding this comment

Uh oh!

SimaTian commented Apr 17, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants