Migrate FindItemsFromPackages to IMultiThreadableTask by SimaTian · Pull Request #53952 · dotnet/sdk

SimaTian · 2026-04-17T09:55:50Z

Migrates \FindItemsFromPackages\ to support \IMultiThreadableTask.

Changes

Pattern A (pure in-memory): attribute + interface + stub TaskEnvironment. Task only reads \NuGetPackageId/\NuGetPackageVersion\ metadata via pure helper \ItemUtilities.GetPackageIdentity().
Uses \MetadataKeys.NuGetPackageId\ / \MetadataKeys.NuGetPackageVersion\ constants.
Adds 7 behavioral tests in a dedicated file using start-gate pattern.

Addresses review feedback from #53116

✅ Start-gate pattern replaces Barrier + Parallel.For.
✅ Dedicated per-task test file.
✅ MetadataKeys constants.

Supersedes

Part of the split of stuck merge-group PR #53116. This is the 3-of-5 split for \FindItemsFromPackages.

Copilot

Pull request overview

Migrates FindItemsFromPackages to support MSBuild multithreaded execution by implementing IMultiThreadableTask and adding dedicated concurrency-focused unit tests.

Changes:

Update FindItemsFromPackages to implement IMultiThreadableTask and expose a TaskEnvironment property.
Add a new unit test suite validating interface implementation plus concurrent execution behavior via a start-gate pattern.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 7 comments.

File	Description
`src/Tasks/Microsoft.NET.Build.Tasks/FindItemsFromPackages.cs`	Implements `IMultiThreadableTask` and adds `TaskEnvironment` plumbing for multithreaded execution.
`src/Tasks/Microsoft.NET.Build.Tasks.UnitTests/GivenAFindItemsFromPackagesMultiThreading.cs`	Adds behavioral + concurrency tests to validate correctness under parallel execution.

Copilot · 2026-04-17T10:02:18Z

+
+            var multiThreadable = task as IMultiThreadableTask;
+            multiThreadable.Should().NotBeNull();
+
+            var property = typeof(FindItemsFromPackages).GetProperty(nameof(IMultiThreadableTask.TaskEnvironment));
+            property.Should().NotBeNull();
+            property.CanRead.Should().BeTrue();
+            property.CanWrite.Should().BeTrue();


This test name says the TaskEnvironment property "can be set", but the test only checks via reflection that the property exists and is readable/writable. Consider actually setting TaskEnvironment on the task instance and asserting that the getter returns the same value (or that it can be accessed without throwing).

Suggested change

var multiThreadable = task as IMultiThreadableTask;

multiThreadable.Should().NotBeNull();

var property = typeof(FindItemsFromPackages).GetProperty(nameof(IMultiThreadableTask.TaskEnvironment));

property.Should().NotBeNull();

property.CanRead.Should().BeTrue();

property.CanWrite.Should().BeTrue();

var multiThreadable = task as IMultiThreadableTask;

multiThreadable.Should().NotBeNull();

var property = typeof(FindItemsFromPackages).GetProperty(nameof(IMultiThreadableTask.TaskEnvironment));

property.Should().NotBeNull();

property.CanRead.Should().BeTrue();

property.CanWrite.Should().BeTrue();

var taskEnvironment = Activator.CreateInstance(property.PropertyType);

taskEnvironment.Should().NotBeNull();

property.SetValue(task, taskEnvironment);

property.GetValue(task).Should().BeSameAs(taskEnvironment);

Copilot · 2026-04-17T10:02:19Z

+            // Start-gate pattern to ensure maximum concurrency
+            var startGate = new ManualResetEventSlim(false);
+            var taskList = new List<Task>();
+
+            for (int i = 0; i < threadCount; i++)
+            {
+                int taskIndex = i;
+                taskList.Add(Task.Run(() =>
+                {
+                    // Wait for start signal
+                    startGate.Wait();
+
+                    try
+                    {
+                        bool success = tasks[taskIndex].Execute();
+                        success.Should().BeTrue($"Task {taskIndex} should succeed");
+                        results.Add(tasks[taskIndex].ItemsFromPackages);
+                    }
+                    catch (Exception ex)
+                    {
+                        exceptions.Add(ex);
+                    }
+                }));
+            }
+
+            // Release all threads simultaneously
+            startGate.Set();
+
+            // Wait for all tasks to complete
+            Task.WaitAll(taskList.ToArray());
+


These concurrency tests use startGate.Wait() and Task.WaitAll() without any timeout. If a task deadlocks or a thread never starts, this can hang the test run indefinitely. Consider adding timeouts (and failing the test with a clear message) and disposing the ManualResetEventSlim (e.g., via using) to avoid CI hangs/leaks.

Copilot · 2026-04-17T10:02:19Z

+            var startGate = new ManualResetEventSlim(false);
+            var taskList = new List<Task>();
+
+            for (int i = 0; i < threadCount; i++)
+            {
+                int taskIndex = i;
+                taskList.Add(Task.Run(() =>
+                {
+                    startGate.Wait();
+
+                    bool success = tasks[taskIndex].Execute();
+                    success.Should().BeTrue();
+                    results[taskIndex] = tasks[taskIndex].ItemsFromPackages;
+                }));
+            }
+
+            startGate.Set();
+            Task.WaitAll(taskList.ToArray());
+


This test uses startGate.Wait() and Task.WaitAll() without a timeout. To avoid rare but costly CI hangs, consider adding timeouts (with a clear assertion message) and disposing startGate (ManualResetEventSlim implements IDisposable).

Copilot · 2026-04-17T10:02:20Z

+            var startGate = new ManualResetEventSlim(false);
+            var taskList = new List<Task>();
+            var results = new ConcurrentBag<ITaskItem[]>();
+
+            for (int i = 0; i < threadCount; i++)
+            {
+                int taskIndex = i;
+                taskList.Add(Task.Run(() =>
+                {
+                    startGate.Wait();
+
+                    bool success = tasks[taskIndex].Execute();
+                    success.Should().BeTrue();
+                    results.Add(tasks[taskIndex].ItemsFromPackages);
+                }));
+            }
+
+            startGate.Set();
+            Task.WaitAll(taskList.ToArray());
+


This test uses startGate.Wait() and Task.WaitAll() without any timeout. Adding timeouts (and disposing the ManualResetEventSlim) would prevent indefinite hangs if something goes wrong during concurrent execution.

Copilot · 2026-04-17T10:02:20Z

+            var startGate = new ManualResetEventSlim(false);
+            var taskList = new List<Task>();
+
+            for (int i = 0; i < threadCount; i++)
+            {
+                int taskIndex = i;
+                taskList.Add(Task.Run(() =>
+                {
+                    startGate.Wait();
+
+                    bool success = tasks[taskIndex].Execute();
+                    success.Should().BeTrue();
+                    results.Add(tasks[taskIndex].ItemsFromPackages);
+                }));
+            }
+
+            startGate.Set();
+            Task.WaitAll(taskList.ToArray());
+


This test uses startGate.Wait() and Task.WaitAll() without a timeout. Consider adding timeouts (and disposing ManualResetEventSlim) to avoid indefinite hangs in CI if a task never completes.

Copilot · 2026-04-17T10:02:20Z

+            var startGate = new ManualResetEventSlim(false);
+            var taskList = new List<Task>();
+
+            for (int i = 0; i < threadCount; i++)
+            {
+                int taskIndex = i;
+                taskList.Add(Task.Run(() =>
+                {
+                    startGate.Wait();
+
+                    bool success = tasks[taskIndex].Execute();
+                    success.Should().BeTrue();
+                    results.Add(tasks[taskIndex].ItemsFromPackages);
+                }));
+            }
+
+            startGate.Set();
+            Task.WaitAll(taskList.ToArray());
+


This test uses startGate.Wait() and Task.WaitAll() without any timeout. Adding timeouts (and disposing the ManualResetEventSlim) would help prevent the test suite from hanging indefinitely under failure conditions.

Copilot · 2026-04-17T10:02:20Z

+            get => _taskEnvironment ??= new TaskEnvironment(new ProcessTaskEnvironmentDriver(Directory.GetCurrentDirectory()));
+            set => _taskEnvironment = value;
+        }


On NETFRAMEWORK, the TaskEnvironment getter lazily initializes using Directory.GetCurrentDirectory(), which is process-global and may not match the project directory when tasks are executed concurrently for multiple projects. Since TaskEnvironment is intended to provide the per-project execution context, consider removing this CWD-based fallback and relying on MSBuild to set TaskEnvironment, or derive the directory from BuildEngine.ProjectFileOfTaskNode (when available) instead of Directory.GetCurrentDirectory().

Suggested change

get => _taskEnvironment ??= new TaskEnvironment(new ProcessTaskEnvironmentDriver(Directory.GetCurrentDirectory()));

set => _taskEnvironment = value;

}

get => _taskEnvironment ??= CreateTaskEnvironment();

set => _taskEnvironment = value;

}

private TaskEnvironment CreateTaskEnvironment()

{

string projectFile = BuildEngine?.ProjectFileOfTaskNode;

if (string.IsNullOrEmpty(projectFile))

{

throw new InvalidOperationException("TaskEnvironment was not set and the project file path is unavailable from the build engine.");

}

string projectDirectory = System.IO.Path.GetDirectoryName(projectFile);

if (string.IsNullOrEmpty(projectDirectory))

{

throw new InvalidOperationException("TaskEnvironment was not set and the project directory could not be determined from the build engine project file path.");

}

return new TaskEnvironment(new ProcessTaskEnvironmentDriver(projectDirectory));

}

- Add IMultiThreadableTask interface implementation - Add TaskEnvironment property stub (pure in-memory task) - Create comprehensive multithreading test suite: * Interface implementation verification * Concurrent execution with start-gate pattern * Independent package filtering per thread * Empty input handling * Metadata preservation * Version matching correctness - All 7 tests passed This task is pure in-memory (no file system or environment access), so only attribute + stub property are needed per Pattern A. Co-authored-by: Copilot <[email protected]>

Co-authored-by: Copilot <[email protected]>

AR-May · 2026-05-05T09:09:44Z

The task is already enlightened, the tests looks to me having very limited value.

Copilot AI review requested due to automatic review settings April 17, 2026 09:55

Copilot started reviewing on behalf of SimaTian April 17, 2026 09:56 View session

SimaTian mentioned this pull request Apr 17, 2026

Multithreading migration: Group 5 — 5 attribute-only tasks #53116

Closed

Copilot AI reviewed Apr 17, 2026

View reviewed changes

This was referenced Apr 17, 2026

[Known Build Error] RunFileTests csc-only mode fails expecting binary log warning that is not emitted #53869

Closed

[Known Build Error] Invalid digest failures in Containers.UnitTests #53918

Closed

SimaTian marked this pull request as draft April 28, 2026 08:16

SimaTian and others added 2 commits April 29, 2026 13:53

Address FindItemsFromPackages multithreading feedback

0d26a88

Co-authored-by: Copilot <[email protected]>

SimaTian force-pushed the migrate-find-items-from-packages branch from daec033 to 0d26a88 Compare April 29, 2026 12:30

This was referenced Apr 29, 2026

slow macOS - "##[error]The job running on agent Azure Pipelines 9 ran longer than the maximum time of 60 minutes." dotnet/dnceng#1883

Open

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

Open

jankratochvilcz requested a review from JanProvaznik May 5, 2026 08:49

jankratochvilcz assigned AR-May May 5, 2026

AR-May closed this May 5, 2026

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Migrate FindItemsFromPackages to IMultiThreadableTask#53952

Migrate FindItemsFromPackages to IMultiThreadableTask#53952
SimaTian wants to merge 2 commits intodotnet:mainfrom
SimaTian:migrate-find-items-from-packages

SimaTian commented Apr 17, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Apr 17, 2026

Uh oh!

Copilot AI Apr 17, 2026

Uh oh!

Copilot AI Apr 17, 2026

Uh oh!

Copilot AI Apr 17, 2026

Uh oh!

Copilot AI Apr 17, 2026

Uh oh!

Copilot AI Apr 17, 2026

Uh oh!

Copilot AI Apr 17, 2026

Uh oh!

AR-May commented May 5, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

-            get => _taskEnvironment ??= new TaskEnvironment(new ProcessTaskEnvironmentDriver(Directory.GetCurrentDirectory()));
-            set => _taskEnvironment = value;
-        }
+            get => _taskEnvironment ??= CreateTaskEnvironment();
+            set => _taskEnvironment = value;
+        }
+        private TaskEnvironment CreateTaskEnvironment()
+        {
+            string projectFile = BuildEngine?.ProjectFileOfTaskNode;
+            if (string.IsNullOrEmpty(projectFile))
+            {
+                throw new InvalidOperationException("TaskEnvironment was not set and the project file path is unavailable from the build engine.");
+            }
+            string projectDirectory = System.IO.Path.GetDirectoryName(projectFile);
+            if (string.IsNullOrEmpty(projectDirectory))
+            {
+                throw new InvalidOperationException("TaskEnvironment was not set and the project directory could not be determined from the build engine project file path.");
+            }
+            return new TaskEnvironment(new ProcessTaskEnvironmentDriver(projectDirectory));
+        }

Conversation

SimaTian commented Apr 17, 2026

Changes

Addresses review feedback from #53116

Supersedes

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Apr 17, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 17, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 17, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 17, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 17, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 17, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 17, 2026

Choose a reason for hiding this comment

Uh oh!

AR-May commented May 5, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants