Migrate CreateWindowsSdkKnownFrameworkReferences to IMultiThreadableTask#53951
Open
SimaTian wants to merge 2 commits intodotnet:mainfrom
Open
Migrate CreateWindowsSdkKnownFrameworkReferences to IMultiThreadableTask#53951SimaTian wants to merge 2 commits intodotnet:mainfrom
SimaTian wants to merge 2 commits intodotnet:mainfrom
Conversation
Task is pure in-memory with no file I/O or environment dependencies. Applied attribute-only migration pattern: - Implemented IMultiThreadableTask interface - Added TaskEnvironment nullable property (for API consistency) - Replaced metadata string literals with MetadataKeys constants Added comprehensive multi-threading tests: - Concurrent execution with varied inputs (8 scenarios) - Used start-gate pattern with ManualResetEventSlim + Task.Run to avoid Barrier+Parallel.For deadlock - Verified output correctness across different input modes - Tested error cases under concurrent execution - All metadata references use MetadataKeys constants Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- TaskEnvironment? -> TaskEnvironment under #nullable disable to fix CS8632 with TreatWarningsAsErrors - Replace Task.WaitAll+.Result with async/await Task.WhenAll (xUnit1031) - Fix scenario 4 expected count: 5 profiles per supported SDK version; two versions -> 10 items Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Migrates CreateWindowsSdkKnownFrameworkReferences to explicitly support MSBuild multi-threaded execution by implementing IMultiThreadableTask, and adds dedicated concurrency-focused unit tests to validate deterministic outputs and thread-safety.
Changes:
- Update
CreateWindowsSdkKnownFrameworkReferencesto implementIMultiThreadableTask(addsTaskEnvironmentproperty). - Replace several metadata string literals with
MetadataKeys.*constants. - Add a new unit test file covering concurrency scenarios and concurrent error-case behavior.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| src/Tasks/Microsoft.NET.Build.Tasks/CreateWindowsSdkKnownFrameworkReferences.cs | Implements IMultiThreadableTask and uses MetadataKeys constants for selected metadata. |
| src/Tasks/Microsoft.NET.Build.Tasks.UnitTests/GivenACreateWindowsSdkKnownFrameworkReferencesMultiThreading.cs | Adds dedicated behavioral + concurrent execution tests (including start-gate concurrency patterns). |
| const int concurrency = 8; | ||
| var taskInstances = new CreateWindowsSdkKnownFrameworkReferences[concurrency]; | ||
| var executeTasks = new Task<bool>[concurrency]; | ||
| var startGate = new ManualResetEventSlim(false); |
There was a problem hiding this comment.
ManualResetEventSlim implements IDisposable; consider declaring this start gate as using var (or disposing in a finally) to avoid leaking a wait handle if the test fails early.
Suggested change
| var startGate = new ManualResetEventSlim(false); | |
| using var startGate = new ManualResetEventSlim(false); |
| [Fact] | ||
| public async Task ErrorCasesAreConcurrentlySafe() | ||
| { | ||
| var startGate = new ManualResetEventSlim(false); |
There was a problem hiding this comment.
ManualResetEventSlim should be disposed. Using using var startGate = new ManualResetEventSlim(false); would ensure the wait handle is released even if assertions fail.
Suggested change
| var startGate = new ManualResetEventSlim(false); | |
| using var startGate = new ManualResetEventSlim(false); |
This was referenced Apr 17, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Migrates \CreateWindowsSdkKnownFrameworkReferences\ to support \IMultiThreadableTask.
Changes
Addresses review feedback from #53116
Supersedes
Part of the split of stuck merge-group PR #53116. This is the 2-of-5 split for \CreateWindowsSdkKnownFrameworkReferences.