Repository navigation
Isolate multithreaded build test from compiler-server mutex race - #56239
JeremyKuhne with Copilot wants to merge 2 commits into
Conversation
|
Azure Pipelines: 3 pipeline(s) were filtered out due to trigger conditions. There may be pipelines that require an authorized user to comment /azp run to run. |
Co-authored-by: JeremyKuhne <8184940+JeremyKuhne@users.noreply.github.com>
|
Azure Pipelines: Successfully started running 1 pipeline(s). 2 pipeline(s) were filtered out due to trigger conditions. There may be pipelines that require an authorized user to comment /azp run to run. |
There was a problem hiding this comment.
🟢 Approval recommended
The change is narrowly scoped; the remaining documentation note is non-blocking.
Pull request overview
Isolates a multithreaded build test from Roslyn’s shared-compilation mutex race while preserving its coverage.
Changes:
- Disables shared compilation for the child build.
- Retains multithreaded project-reference and single-process assertions.
File summaries
| File | Description |
|---|---|
test/dotnet.Tests/CommandTests/Run/RunFileTests_BuildOptions.cs |
Adds UseSharedCompilation=false to the targeted build test. |
Review details
Suppressed comments (1)
test/dotnet.Tests/CommandTests/Run/RunFileTests_BuildOptions.cs:896
- This is a test-isolation workaround for the compiler-server mutex race tracked by #56238. Please include the issue URL in the comment so future maintainers can tell when this opt-out can be revisited; otherwise the rationale is easy to lose when the test is updated.
// Shared compilation uses a named mutex, which is unrelated to testing MSBuild's in-process nodes and can fail independently.
- Files reviewed: 1/1 changed files
- Comments generated: 0
- Review effort level: Lite
|
|
||
| new DotnetCommand(Log, "build", "Program.cs", argument) | ||
| // Shared compilation uses a named mutex, which is unrelated to testing MSBuild's in-process nodes and can fail independently. | ||
| new DotnetCommand(Log, "build", "Program.cs", argument, "-p:UseSharedCompilation=false") |
There was a problem hiding this comment.
I would like to understand what the underlying issue here is. Are we just masking a real bug? Also, there are lot of other tests doing dotnet build Program.cs - do we need to add -p:UseSharedCompilation=false to all of them or is only this test flaky for some reason - and if so, what is the reason?
There was a problem hiding this comment.
You're right that this masks a real failure. This test is different from the other Program.cs builds because -mt plus two project references runs multiple newly multithreadable Csc tasks in one MSBuild process. In the failing Helix work item, the ordinary file-based builds passed, this -mt row failed, and the -mt:false row passed immediately afterward. The other tests therefore do not need UseSharedCompilation=false.
The exception comes from the Roslyn compiler-server client mutex and matches the failure family previously tracked by dotnet/runtime#53420 and dotnet/dotnet#4088. I opened dotnet/roslyn#85264 for the concurrent shared-compilation scenario and the missing thread-ID diagnostics.
I think disabling shared compilation is still appropriate here to isolate this test's MSBuild in-process-node assertion, but only as a local mitigation. We should not apply it broadly, and the underlying bug remains open in Roslyn.
Note
This response was drafted with GitHub Copilot assistance.
There was a problem hiding this comment.
Okay, thanks for filing the tracking issue. With that I'm fine with this workaround being merged. Consider linking the issue in a code comment near the workaround though.
MultiThreadedArgument_BuildsProjectReferencesInProcessintermittently failed on Linux when Roslyn shared compilation hit a named-mutex ownership error during concurrent builds.UseSharedCompilationfor this test’s child build.