Conversation
Test assemblies run concurrently as separate processes and all wrote MSBuild debug and crash files into the same machine-wide temp folder, which BuildFailureLogInvariant scans to detect out-of-process node crashes. A crash caused by one assembly could therefore be blamed on a test in another one. Set MSBUILDDEBUGPATH once per process in MSBuildTestPipelineStartup so each test assembly gets its own directory. Only the path is set - tracing stays gated on MSBuildDebugEngine and MSBUILDDEBUGCOMM. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
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.
What happens today
Test assemblies run concurrently as separate processes, because MSBuild builds the test projects in parallel (
xunit.runner.json'smaxParallelThreads/parallelizeTestCollectionsonly disable parallelism within an assembly). Each assembly already gets its own temp root:MSBuildTestPipelineStartupcallsTestEnvironment.SetTempPath(...), which setsTMP/TEMPbefore any test runs.The debug dump directory, though, is resolved independently per process. With
MSBUILDDEBUGPATHunset — which is the case in tests today —FrameworkDebugUtils.DebugPathis null, soDebugUtils.DebugDumpPathfalls back toFileUtilities.TempFileDirectory. That value comes fromFileUtilities.CreateFolderUnderTemp(), which isDirectory.CreateTempSubdirectory("MSBuildTemp")on .NET andPath.Combine(Path.GetTempPath(), $"MSBuildTemp{Guid.NewGuid():N}")on .NET Framework. Either way it is a fresh randomly-named subdirectory, created once per process.So a parent test process and the out-of-proc nodes it spawns resolve different dump directories, both nested one level below the temp root that
BuildFailureLogInvariantscans.Measured in a real test run (
Microsoft.Build.Engine.UnitTests, pid 4752), printed from inside a test:Path.GetTempPath()is the assembly's own temp root, not the machine-wide temp folder —SetTempPathhas already redirected it. The two locations the invariant scans are the parent's randomMSBuildTemp*subdirectory and its parent directory; a child node's ownMSBuildTemp*subdirectory is neither.Consequence: an out-of-process node crash dump can land in a directory the invariant never scans, so the crash is missed rather than attributed to the wrong test.
Evidential status
The parent-side paths above are measured and quoted verbatim. The child-node behaviour is inferred from
CreateFolderUnderTemp()plus those measured paths — no real out-of-proc node crash was reproduced for this PR. An earlier framing of this problem as "all assemblies share one machine-wide temp folder" was an assumption and is not what the measurements show; it has been dropped.The fix
Set
MSBUILDDEBUGPATHonce per assembly, inMSBuildTestPipelineStartupbefore any test runs, to a directory named after the assembly and process. Child nodes inherit the variable when they are spawned, so the parent and every node it starts resolve the same durable dump directory, andBuildFailureLogInvariantscans exactly that directory.Only the path is set. Tracing stays off — it is gated separately on
MSBuildDebugEngine/MSBUILDDEBUGENGINEandMSBUILDDEBUGCOMMviaTraits. Every debug-file writer (BuildRequestEngine.TraceEngine,Scheduler,CoordinatorServer.DefaultDebugOutput,CommunicationsUtilities.Trace, theXMakedeferred message) is gated on one of those flags, never onDebugPathbeing non-null.As a secondary benefit, dumps that are found can no longer have come from a concurrently running assembly. Attribution within an assembly is still approximate, because MSBuild reuses nodes across tests.
Implementation notes
Three details forced small deviations from the most direct version of this change.
1. The
DebugUtilsreset lives inMicrosoft.Build.UnitTests.Shared, not inTestAssemblyInfo.cs.Microsoft.Build.Shared.Debugging.DebugUtilsisinternaland compiled intoMicrosoft.Build,Microsoft.Build.Tasks,Microsoft.Build.UtilitiesandMSBuild.TestAssemblyInfo.csis compiled into every non-library test project, includingMicrosoft.Build.EndToEnd.Tests, which has noInternalsVisibleTogrant from any of them — only fromMicrosoft.Build.Framework. ReferencingDebugUtilsthere would not compile. The logic therefore sits in a newTestEnvironment.UseIsolatedDebugPathhelper, whichMicrosoft.Build.UnitTests.Sharedcan express (it has grants from bothFrameworkandBuild) and which every test project already references.2.
FrameworkDebugUtils.SetDebugPath()has to be called too.FrameworkDebugUtils.DebugPathis computed in a static constructor and is one of the locations the invariant scans. SettingMSBUILDDEBUGPATHwithout refreshing it would leaveDebugPathnull, so the new directory would never be scanned and the invariant would silently stop detecting crashes.3.
BuildFailureLogInvariantscans a startup-captured path. ThePath.GetTempPath()block is replaced by one scanningTestEnvironment.AssemblyDebugPath, a static captured once per process, rather than relying only onFrameworkDebugUtils.DebugPath. Several tests repointMSBUILDDEBUGPATHtemporarily, and at least one —BuildManager_Tests.MultiThreadedBuild_WithDebugSchedulerTracing_DoesNotDeadlock— never callsSetDebugPath()again on cleanup, leavingDebugPathpointing at a deleted folder for the rest of the assembly. A startup-captured path cannot be mutated by a test, so the invariant stays reliable regardless.TransientDebugEnginesetMSBUILDDEBUGPATHtoFileUtilities.TempFileDirectorywhen enabling, which would have overridden the isolation for its caller. It now sets it toFrameworkDebugUtils.DebugPath, falling back to the previous value when that is somehow unset. Its save/restore behaviour and itselsebranch are unchanged.The directory is created under the machine temp folder — deliberately not under the per-assembly temp folder, which is swapped out by per-test
TransientTempPathand deleted at teardown — and is removed best-effort inStopAsync.Validation
Planted-dump verification (strongest evidence here). A throwaway test wrote
MSBuild_pid-9999_deadbeef.failure.txtinto the resolved dump directory and returned, lettingBuildFailureLogInvariant.AssertInvariantrun. It failed, as intended:and named the file in the invariant's own output:
This exercises the whole chain end to end — directory resolution, the
MSBuild_*.txtglob,Except, and the assert — rather than assuming it. The test was removed afterwards; it is not part of this PR.A second throwaway test (also removed) confirmed the path wiring in a live run:
MSBUILDDEBUGPATH,FrameworkDebugUtils.DebugPathandDebugUtils.DebugDumpPathall resolve to the per-assembly directory,Traits.DebugEnginestaysfalse, and the directory is deleted at teardown.Other checks:
net11.0andnet472, including the assemblies with the narrowestInternalsVisibleTogrants (EndToEnd.Tests,Utilities.UnitTests,Tasks.UnitTests,MSBuild.UnitTests).DebugUtils_Tests6/6 andMSBuildServer_Tests.PropertyMSBuildStartupDirectoryOnServer— the onlyTransientDebugEnginecaller — pass. Every test inDebugUtils_tests.cssets and restoresMSBUILDDEBUGPATHexplicitly, so none depends on the ambient value;SetDebugPath_WhenUserNotSetDebugPathnulls it first.Microsoft.Build.Framework.UnitTestspasses in full (1103 passed, 37 platform skips).