From 33f253f4bbb320226dc42bf54be4537ece3622d9 Mon Sep 17 00:00:00 2001 From: AR-May <67507805+AR-May@users.noreply.github.com> Date: Thu, 2 Jul 2026 11:35:11 +0200 Subject: [PATCH 1/3] Implement node reuse and server shutdown logic for multithreaded build. --- documentation/MSBuild-Server.md | 10 +++ .../BackEnd/ServerNodeBuildCommand_Tests.cs | 63 +++++++++++++++++++ src/Build/BackEnd/Client/MSBuildClient.cs | 25 +++++++- src/Build/BackEnd/Node/OutOfProcServerNode.cs | 4 +- .../BackEnd/Node/ServerNodeBuildCommand.cs | 11 +++- src/MSBuild.UnitTests/MSBuildServer_Tests.cs | 60 ++++++++++++++++++ src/MSBuild/MSBuildClientApp.cs | 11 +++- src/MSBuild/XMake.cs | 31 +++++++-- 8 files changed, 203 insertions(+), 12 deletions(-) create mode 100644 src/Build.UnitTests/BackEnd/ServerNodeBuildCommand_Tests.cs diff --git a/documentation/MSBuild-Server.md b/documentation/MSBuild-Server.md index dbcf95e56bb..2e19046f791 100644 --- a/documentation/MSBuild-Server.md +++ b/documentation/MSBuild-Server.md @@ -11,6 +11,16 @@ To re-enable MSBuild Server, remove the variable or set its value to `0`. When a build is multithreaded (`/mt`), the server node is launched with [Server GC](https://learn.microsoft.com/dotnet/standard/garbage-collection/workstation-server-gc) enabled. Under `/mt` the server runs all project work on threads in this single process, so Server GC's higher throughput is beneficial; without `/mt` the server only orchestrates and delegates project work to separate worker nodes, so it keeps the default Workstation GC. GC mode is fixed at CLR startup, so it is set via the `DOTNET_gcServer` environment variable in the server's launch environment (decided from the launching invocation's command line). An explicit user-set `DOTNET_gcServer` is honored (e.g. set `DOTNET_gcServer=0` to keep Workstation GC in a memory-constrained environment). This is scoped to the server process only: sidecar TaskHosts and worker nodes keep the default Workstation GC. +## Node reuse and server lifetime + +MSBuild Server is a form of node reuse: the whole point of the server is to stay resident between builds so later builds reuse its warmed-up process and caches. Consequently: + +- **Node reuse on (the default).** The server is eligible and, after a build, returns to listening so the next compatible client reuses it. +- **Node reuse off (`-nodeReuse:false` / `-nr:false`) without `/mt`.** Keeping a process resident contradicts the no-reuse intent, so the build does not use the server at all (it runs entirely in the launching process). See `ServerShouldNotRunWhenNodeReuseEqualsFalse`. +- **Node reuse off *with* `/mt`.** A `/mt` build needs the server for a different reason: multithreaded project execution runs inside the server process, which is where Server GC is applied (see [Garbage collection](#garbage-collection)). So a `/mt` build still engages the server even when node reuse is off - but it must honor the no-reuse request by **not** leaving the server resident afterwards. + +The client makes a single, response-file-aware determination and sets the `ShutdownAfterBuild` flag on the `ServerNodeBuildCommand` packet if server needs shutdown. + ## Communication protocol The server node uses same IPC approach as current worker nodes - named pipes. This solution allows to reuse existing code. When process starts, pipe with deterministic name is opened and waiting for commands. Client has following worfklow: diff --git a/src/Build.UnitTests/BackEnd/ServerNodeBuildCommand_Tests.cs b/src/Build.UnitTests/BackEnd/ServerNodeBuildCommand_Tests.cs new file mode 100644 index 00000000000..6bdc1c8937e --- /dev/null +++ b/src/Build.UnitTests/BackEnd/ServerNodeBuildCommand_Tests.cs @@ -0,0 +1,63 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System; +using System.Collections.Generic; +using System.Globalization; +using Microsoft.Build.BackEnd; +using Microsoft.Build.BackEnd.Logging; +using Shouldly; +using Xunit; + +namespace Microsoft.Build.UnitTests.BackEnd +{ + /// + /// Unit tests for the packet. + /// + public class ServerNodeBuildCommand_Tests + { + /// + /// Round-trips a through the binary translator and verifies that all + /// fields - including the flag that drives the + /// server's self-teardown - survive serialization for both values of the flag. + /// + [Theory] + [InlineData(true)] + [InlineData(false)] + public void RoundTripSerializationPreservesFields(bool shutdownAfterBuild) + { + string[] commandLine = ["msbuild.exe", "project.proj", "-mt", "-nr:false"]; + string startupDirectory = "C:\\some\\startup\\dir"; + Dictionary buildProcessEnvironment = new(StringComparer.OrdinalIgnoreCase) + { + ["VAR1"] = "value1", + ["VAR2"] = "value2", + }; + CultureInfo culture = new("en-US"); + CultureInfo uiCulture = new("en-GB"); + TargetConsoleConfiguration consoleConfiguration = new(bufferWidth: 80, acceptAnsiColorCodes: true, outputIsScreen: false, backgroundColor: ConsoleColor.Black); + + ServerNodeBuildCommand command = new( + commandLine, + startupDirectory, + buildProcessEnvironment, + culture, + uiCulture, + consoleConfiguration, + partialBuildTelemetry: null, + shutdownAfterBuild); + + ((INodePacket)command).Translate(TranslationHelpers.GetWriteTranslator()); + INodePacket packet = ServerNodeBuildCommand.FactoryForDeserialization(TranslationHelpers.GetReadTranslator()); + + ServerNodeBuildCommand deserialized = packet.ShouldBeOfType(); + + deserialized.ShutdownAfterBuild.ShouldBe(shutdownAfterBuild); + deserialized.CommandLine.ShouldBe(commandLine); + deserialized.StartupDirectory.ShouldBe(startupDirectory); + deserialized.BuildProcessEnvironment.ShouldBe(buildProcessEnvironment); + deserialized.Culture.ShouldBe(culture); + deserialized.UICulture.ShouldBe(uiCulture); + } + } +} diff --git a/src/Build/BackEnd/Client/MSBuildClient.cs b/src/Build/BackEnd/Client/MSBuildClient.cs index a8e7caef079..0525adf4962 100644 --- a/src/Build/BackEnd/Client/MSBuildClient.cs +++ b/src/Build/BackEnd/Client/MSBuildClient.cs @@ -113,6 +113,11 @@ public sealed class MSBuildClient /// private readonly bool _multiThreaded; + /// + /// Whether the server should shut itself down once this build completes instead of staying resident for reuse. + /// + private readonly bool _shutdownServerAfterBuild; + /// /// Public constructor with parameters. /// @@ -135,6 +140,22 @@ public MSBuildClient(string[] commandLine, string msbuildLocation) /// Whether this build is multithreaded (/mt). When true, the launched /// server process is started with Server GC. public MSBuildClient(string[] commandLine, string msbuildLocation, bool multiThreaded) + : this(commandLine, msbuildLocation, multiThreaded, shutdownServerAfterBuild: false) + { + } + + /// + /// Public constructor with parameters. + /// + /// The command line to process. The first argument + /// on the command line is assumed to be the name/path of the executable, and is ignored + /// Full path to current MSBuild.exe if executable is MSBuild.exe, + /// or to version of MSBuild.dll found to be associated with the current process. + /// Whether this build is multithreaded (/mt). When true, the launched + /// server process is started with Server GC. + /// Whether the server should shut itself down once this build + /// completes instead of staying resident for reuse (e.g. a /mt build with -nodeReuse:false). + public MSBuildClient(string[] commandLine, string msbuildLocation, bool multiThreaded, bool shutdownServerAfterBuild) { _serverEnvironmentVariables = new(); _exitResult = new(); @@ -143,6 +164,7 @@ public MSBuildClient(string[] commandLine, string msbuildLocation, bool multiThr _commandLine = commandLine; _msbuildLocation = msbuildLocation; _multiThreaded = multiThreaded; + _shutdownServerAfterBuild = shutdownServerAfterBuild; // Client <-> Server communication stream _handshake = GetHandshake(); @@ -587,7 +609,8 @@ private ServerNodeBuildCommand GetServerNodeBuildCommand() CultureInfo.CurrentCulture, CultureInfo.CurrentUICulture, _consoleConfiguration!, - partialBuildTelemetry); + partialBuildTelemetry, + _shutdownServerAfterBuild); } private ServerNodeHandshake GetHandshake() => new(CommunicationsUtilities.GetHandshakeOptions( diff --git a/src/Build/BackEnd/Node/OutOfProcServerNode.cs b/src/Build/BackEnd/Node/OutOfProcServerNode.cs index 43fe676e507..5379b0b13d8 100644 --- a/src/Build/BackEnd/Node/OutOfProcServerNode.cs +++ b/src/Build/BackEnd/Node/OutOfProcServerNode.cs @@ -456,8 +456,8 @@ private void HandleServerNodeBuildCommand(ServerNodeBuildCommand command) var response = new ServerNodeBuildResult(buildResult.exitCode, buildResult.exitType); SendPacket(response); - // Shutdown server if cancel was requested. This is consistent with nodes behavior. - _shutdownReason = _cancelRequested ? NodeEngineShutdownReason.BuildComplete : NodeEngineShutdownReason.BuildCompleteReuse; + // Shutdown server after this build if a cancel was requested, or if the client asked for no reuse. This is consistent with nodes behavior. + _shutdownReason = (_cancelRequested || command.ShutdownAfterBuild) ? NodeEngineShutdownReason.BuildComplete : NodeEngineShutdownReason.BuildCompleteReuse; _shutdownEvent.Set(); } diff --git a/src/Build/BackEnd/Node/ServerNodeBuildCommand.cs b/src/Build/BackEnd/Node/ServerNodeBuildCommand.cs index ed4e7b158d5..bfc6242096e 100644 --- a/src/Build/BackEnd/Node/ServerNodeBuildCommand.cs +++ b/src/Build/BackEnd/Node/ServerNodeBuildCommand.cs @@ -20,6 +20,7 @@ internal sealed class ServerNodeBuildCommand : INodePacket private CultureInfo _uiCulture = default!; private TargetConsoleConfiguration _consoleConfiguration = default!; private PartialBuildTelemetry? _partialBuildTelemetry = default; + private bool _shutdownAfterBuild; /// /// Retrieves the packet type. @@ -62,6 +63,11 @@ internal sealed class ServerNodeBuildCommand : INodePacket /// public PartialBuildTelemetry? PartialBuildTelemetry => _partialBuildTelemetry; + /// + /// Whether the server should shut itself down once this build completes instead of staying resident for reuse. + /// + public bool ShutdownAfterBuild => _shutdownAfterBuild; + /// /// Private constructor for deserialization /// @@ -75,7 +81,8 @@ public ServerNodeBuildCommand( Dictionary buildProcessEnvironment, CultureInfo culture, CultureInfo uiCulture, TargetConsoleConfiguration consoleConfiguration, - PartialBuildTelemetry? partialBuildTelemetry) + PartialBuildTelemetry? partialBuildTelemetry, + bool shutdownAfterBuild) { Assumed.NotNull(consoleConfiguration); @@ -86,6 +93,7 @@ public ServerNodeBuildCommand( _uiCulture = uiCulture; _consoleConfiguration = consoleConfiguration; _partialBuildTelemetry = partialBuildTelemetry; + _shutdownAfterBuild = shutdownAfterBuild; } /// @@ -101,6 +109,7 @@ public void Translate(ITranslator translator) translator.TranslateCulture(ref _uiCulture); translator.Translate(ref _consoleConfiguration, TargetConsoleConfiguration.FactoryForDeserialization); translator.Translate(ref _partialBuildTelemetry, PartialBuildTelemetry.FactoryForDeserialization); + translator.Translate(ref _shutdownAfterBuild); } /// diff --git a/src/MSBuild.UnitTests/MSBuildServer_Tests.cs b/src/MSBuild.UnitTests/MSBuildServer_Tests.cs index ec9016084f4..0b3d8534567 100644 --- a/src/MSBuild.UnitTests/MSBuildServer_Tests.cs +++ b/src/MSBuild.UnitTests/MSBuildServer_Tests.cs @@ -386,6 +386,66 @@ public void ServerStartsWhenMtInResponseFileEvenWithoutEnvVar() MSBuildClient.ShutdownServer(CancellationToken.None); } + /// + /// Disabling node reuse (e.g. -nr:false, as dotnet restore does) must NOT prevent a + /// multithreaded (/mt) build from using the server. Instead of skipping the server, the no-reuse intent is + /// honored by shutting the server down after the build. This test verifies both halves: the build runs in a + /// separate server process, and that process does not survive the build (so a subsequent build gets a fresh server). + /// + [Fact] + public void MultiThreadedServerIsUsedButShutDownWhenNodeReuseDisabled() + { + // Clear MSBUILDUSESERVER so we exercise the -mt-implies-server path, and isolate this test's server + // with a unique handshake salt and a clean environment. + PrepareIsolatedServerEnv(useServer: false); + TransientTestFile project = _env.CreateFile("mtNoReuseProbe.proj", GetServerGCProbeProjectContents(useTaskHostFactory: false)); + + // Make sure we start with no server running. + MSBuildClient.ShutdownServer(CancellationToken.None); + + // -mt forces the server on even though node reuse is disabled. + string output1 = RunnerUtilities.ExecMSBuild(BuildEnvironmentHelper.Instance.CurrentMSBuildExePath, $"{project.Path} -mt -nr:false", out bool success1, false, _output); + success1.ShouldBeTrue(); + int clientPid1 = ParseNumber(output1, "Process ID is "); + int serverPid1 = ParseNumber(output1, "TaskRanInPID="); + // Register cleanup before any assertion so the server does not leak if an assertion throws. + _env.WithTransientProcess(serverPid1); + + // The build ran in a separate server process: proof the server was engaged despite -nr:false. + serverPid1.ShouldNotBe(clientPid1, "Even with node reuse disabled, -mt must run the build in the server node, not the entry process."); + + // Because node reuse is disabled, the server must not persist past the build: its process should exit. + WaitForProcessExit(serverPid1).ShouldBeTrue($"Server process {serverPid1} should have been shut down after the build when node reuse is disabled."); + + // A second build cannot reuse the (now gone) server, so it must launch a fresh server process. + string output2 = RunnerUtilities.ExecMSBuild(BuildEnvironmentHelper.Instance.CurrentMSBuildExePath, $"{project.Path} -mt -nr:false", out bool success2, false, _output); + success2.ShouldBeTrue(); + int serverPid2 = ParseNumber(output2, "TaskRanInPID="); + _env.WithTransientProcess(serverPid2); + serverPid2.ShouldNotBe(serverPid1, "With node reuse disabled, each -mt build should get a fresh, non-persistent server process."); + + // Clean up the second server. + MSBuildClient.ShutdownServer(CancellationToken.None); + } + + /// + /// Waits up to for the process with the given PID to exit. Returns true if + /// the process exited (or was already gone), false if it was still running when the timeout elapsed. + /// + private static bool WaitForProcessExit(int pid, int timeoutMs = 10000) + { + try + { + using Process process = Process.GetProcessById(pid); + return process.WaitForExit(timeoutMs); + } + catch (ArgumentException) + { + // No process with that PID is running - it has already exited. + return true; + } + } + [Fact] public void PropertyMSBuildStartupDirectoryOnServer() { diff --git a/src/MSBuild/MSBuildClientApp.cs b/src/MSBuild/MSBuildClientApp.cs index 9818baf7ba4..c2041b1ee16 100644 --- a/src/MSBuild/MSBuildClientApp.cs +++ b/src/MSBuild/MSBuildClientApp.cs @@ -30,13 +30,15 @@ internal static class MSBuildClientApp /// on the command line is assumed to be the name/path of the executable, and /// is ignored. /// Whether this build is multithreaded (/mt). + /// Whether the server should shut itself down once the build + /// completes instead of staying resident for reuse. /// Cancellation token. /// A value of type that indicates whether the build succeeded, /// or the manner in which it failed. /// /// The locations of msbuild exe/dll and dotnet.exe would be automatically detected if called from dotnet or msbuild cli. Calling this function from other executables might not work. /// - public static MSBuildApp.ExitType Execute(string[] commandLineArgs, bool multiThreaded, CancellationToken cancellationToken) + public static MSBuildApp.ExitType Execute(string[] commandLineArgs, bool multiThreaded, bool shutdownServerAfterBuild, CancellationToken cancellationToken) { string msbuildLocation = BuildEnvironmentHelper.Instance.CurrentMSBuildExePath; @@ -44,6 +46,7 @@ public static MSBuildApp.ExitType Execute(string[] commandLineArgs, bool multiTh commandLineArgs, msbuildLocation, multiThreaded, + shutdownServerAfterBuild, cancellationToken); } @@ -56,12 +59,14 @@ public static MSBuildApp.ExitType Execute(string[] commandLineArgs, bool multiTh /// Full path to current MSBuild.exe if executable is MSBuild.exe, /// or to version of MSBuild.dll found to be associated with the current process. /// Whether this build is multithreaded (/mt). + /// Whether the server should shut itself down once the build + /// completes instead of staying resident for reuse. /// Cancellation token. /// A value of type that indicates whether the build succeeded, /// or the manner in which it failed. - public static MSBuildApp.ExitType Execute(string[] commandLineArgs, string msbuildLocation, bool multiThreaded, CancellationToken cancellationToken) + public static MSBuildApp.ExitType Execute(string[] commandLineArgs, string msbuildLocation, bool multiThreaded, bool shutdownServerAfterBuild, CancellationToken cancellationToken) { - MSBuildClient msbuildClient = new MSBuildClient(commandLineArgs, msbuildLocation, multiThreaded); + MSBuildClient msbuildClient = new MSBuildClient(commandLineArgs, msbuildLocation, multiThreaded, shutdownServerAfterBuild); MSBuildClientExitResult exitResult = msbuildClient.Execute(cancellationToken); if (exitResult.MSBuildClientExitType == MSBuildClientExitType.ServerBusy || diff --git a/src/MSBuild/XMake.cs b/src/MSBuild/XMake.cs index 1a8c47878e9..38269602dd0 100644 --- a/src/MSBuild/XMake.cs +++ b/src/MSBuild/XMake.cs @@ -310,13 +310,15 @@ public static int Main(string[] args) } // Perform the single authoritative command-line parse for this process. It yields: - // - canRunServer: whether switches (help/version/binlog/nodereuse/...) permit the server; + // - canRunServer: whether the command line is compatible with hosting the build on the server; // - multiThreaded: the response-file-aware /mt determination (includes the auto-response file, // any project Directory.Build.rsp, @response files, and MSBUILDFORCEMULTITHREADED); + // - shutdownServerAfterBuild: whether the server must tear itself down after this build; // - the gathered switches, which the in-proc build path below reuses so it does not re-parse. bool canRunServer = CanRunServerBasedOnCommandLineSwitches( args, out bool multiThreaded, + out bool shutdownServerAfterBuild, out CommandLineSwitches switchesFromAutoResponseFile, out CommandLineSwitches switchesNotFromAutoResponseFile); @@ -335,7 +337,8 @@ public static int Main(string[] args) // Hand the build off to the MSBuild Server client. The server (not this process) decides // Server GC for itself from the multiThreaded value we pass through. - exitCode = ((s_initialized && MSBuildClientApp.Execute(args, multiThreaded, s_buildCancellationSource.Token) == ExitType.Success) ? 0 : 1); + // When shutdownServerAfterBuild is set (a /mt build with node reuse off), the server tears itself down after this build. + exitCode = ((s_initialized && MSBuildClientApp.Execute(args, multiThreaded, shutdownServerAfterBuild, s_buildCancellationSource.Token) == ExitType.Success) ? 0 : 1); } else { @@ -368,6 +371,8 @@ public static int Main(string[] args) /// Set to whether this is a multithreaded (/mt) build, determined from /// the fully-parsed switches (which expand response files - including any project Directory.Build.rsp - /// and honor MSBUILDFORCEMULTITHREADED) using the same logic as the in-proc build path. + /// Set to when the server should tear itself + /// down after this build instead of staying resident for reuse. /// The gathered response-file switches (auto-response file plus any /// project Directory.Build.rsp), or if parsing failed. /// The gathered command-line/environment switches, or @@ -375,11 +380,13 @@ public static int Main(string[] args) private static bool CanRunServerBasedOnCommandLineSwitches( string[] commandLine, out bool multiThreaded, + out bool shutdownServerAfterBuild, out CommandLineSwitches switchesFromAutoResponseFile, out CommandLineSwitches switchesNotFromAutoResponseFile) { bool canRunServer = true; multiThreaded = false; + shutdownServerAfterBuild = false; switchesFromAutoResponseFile = null; switchesNotFromAutoResponseFile = null; bool switchesFullyGathered = false; @@ -406,12 +413,23 @@ private static bool CanRunServerBasedOnCommandLineSwitches( multiThreaded = IsMultiThreadedEnabled(commandLineSwitches); + bool nodeReuse = ProcessNodeReuseSwitch(commandLineSwitches[CommandLineSwitches.ParameterizedSwitch.NodeReuse]); + string projectFile = ProcessProjectSwitch(commandLineSwitches[CommandLineSwitches.ParameterizedSwitch.Project], commandLineSwitches[CommandLineSwitches.ParameterizedSwitch.IgnoreProjectExtensions], Directory.GetFiles); - if (commandLineSwitches[CommandLineSwitches.ParameterlessSwitch.Help] || + + // These switches are fundamentally incompatible with hosting the build in a separate server + // process (help/version produce immediate output, -nodeMode is itself a node, and a binary-log + // "project" is replayed, not built), so they always disqualify the server. + bool serverIncompatibleSwitch = + commandLineSwitches[CommandLineSwitches.ParameterlessSwitch.Help] || commandLineSwitches.IsParameterizedSwitchSet(CommandLineSwitches.ParameterizedSwitch.NodeMode) || commandLineSwitches[CommandLineSwitches.ParameterlessSwitch.Version] || - FileUtilities.IsBinaryLogFilename(projectFile) || - !ProcessNodeReuseSwitch(commandLineSwitches[CommandLineSwitches.ParameterizedSwitch.NodeReuse])) + FileUtilities.IsBinaryLogFilename(projectFile); + + // Node reuse being disabled normally disqualifies the server. The exception is a multithreaded (/mt) build: it requires the server purely to enable Server GC. + bool nodeReuseDisqualifies = !nodeReuse && !multiThreaded; + + if (serverIncompatibleSwitch || nodeReuseDisqualifies) { canRunServer = false; if (KnownTelemetry.PartialBuildTelemetry is not null) @@ -419,6 +437,9 @@ private static bool CanRunServerBasedOnCommandLineSwitches( KnownTelemetry.PartialBuildTelemetry.ServerFallbackReason = "Arguments"; } } + + // When /mt forced the server on despite node reuse being disabled, the server must not persist past this build. + shutdownServerAfterBuild = canRunServer && multiThreaded && !nodeReuse; } catch (Exception ex) { From 19832809fa0c01178eb788aa0765e21f25ae50f8 Mon Sep 17 00:00:00 2001 From: AR-May <67507805+AR-May@users.noreply.github.com> Date: Thu, 2 Jul 2026 11:59:55 +0200 Subject: [PATCH 2/3] Add conditional compilation for node reuse tests in multithreaded builds --- src/MSBuild.UnitTests/MSBuildServer_Tests.cs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/MSBuild.UnitTests/MSBuildServer_Tests.cs b/src/MSBuild.UnitTests/MSBuildServer_Tests.cs index 0b3d8534567..2d0a4c42a42 100644 --- a/src/MSBuild.UnitTests/MSBuildServer_Tests.cs +++ b/src/MSBuild.UnitTests/MSBuildServer_Tests.cs @@ -386,6 +386,7 @@ public void ServerStartsWhenMtInResponseFileEvenWithoutEnvVar() MSBuildClient.ShutdownServer(CancellationToken.None); } +#if NET /// /// Disabling node reuse (e.g. -nr:false, as dotnet restore does) must NOT prevent a /// multithreaded (/mt) build from using the server. Instead of skipping the server, the no-reuse intent is @@ -445,6 +446,7 @@ private static bool WaitForProcessExit(int pid, int timeoutMs = 10000) return true; } } +#endif [Fact] public void PropertyMSBuildStartupDirectoryOnServer() From 3c76456f0a2e21a1a576bd358c94bbf89deebad2 Mon Sep 17 00:00:00 2001 From: AR-May <67507805+AR-May@users.noreply.github.com> Date: Thu, 2 Jul 2026 12:12:47 +0200 Subject: [PATCH 3/3] Ensure server shutdown in multithreaded build tests even on assertion failure --- src/MSBuild.UnitTests/MSBuildServer_Tests.cs | 51 +++++++++++--------- 1 file changed, 28 insertions(+), 23 deletions(-) diff --git a/src/MSBuild.UnitTests/MSBuildServer_Tests.cs b/src/MSBuild.UnitTests/MSBuildServer_Tests.cs index 2d0a4c42a42..a17b5cb79f8 100644 --- a/src/MSBuild.UnitTests/MSBuildServer_Tests.cs +++ b/src/MSBuild.UnitTests/MSBuildServer_Tests.cs @@ -404,29 +404,34 @@ public void MultiThreadedServerIsUsedButShutDownWhenNodeReuseDisabled() // Make sure we start with no server running. MSBuildClient.ShutdownServer(CancellationToken.None); - // -mt forces the server on even though node reuse is disabled. - string output1 = RunnerUtilities.ExecMSBuild(BuildEnvironmentHelper.Instance.CurrentMSBuildExePath, $"{project.Path} -mt -nr:false", out bool success1, false, _output); - success1.ShouldBeTrue(); - int clientPid1 = ParseNumber(output1, "Process ID is "); - int serverPid1 = ParseNumber(output1, "TaskRanInPID="); - // Register cleanup before any assertion so the server does not leak if an assertion throws. - _env.WithTransientProcess(serverPid1); - - // The build ran in a separate server process: proof the server was engaged despite -nr:false. - serverPid1.ShouldNotBe(clientPid1, "Even with node reuse disabled, -mt must run the build in the server node, not the entry process."); - - // Because node reuse is disabled, the server must not persist past the build: its process should exit. - WaitForProcessExit(serverPid1).ShouldBeTrue($"Server process {serverPid1} should have been shut down after the build when node reuse is disabled."); - - // A second build cannot reuse the (now gone) server, so it must launch a fresh server process. - string output2 = RunnerUtilities.ExecMSBuild(BuildEnvironmentHelper.Instance.CurrentMSBuildExePath, $"{project.Path} -mt -nr:false", out bool success2, false, _output); - success2.ShouldBeTrue(); - int serverPid2 = ParseNumber(output2, "TaskRanInPID="); - _env.WithTransientProcess(serverPid2); - serverPid2.ShouldNotBe(serverPid1, "With node reuse disabled, each -mt build should get a fresh, non-persistent server process."); - - // Clean up the second server. - MSBuildClient.ShutdownServer(CancellationToken.None); + try + { + // -mt forces the server on even though node reuse is disabled. + string output1 = RunnerUtilities.ExecMSBuild(BuildEnvironmentHelper.Instance.CurrentMSBuildExePath, $"{project.Path} -mt -nr:false", out bool success1, false, _output); + success1.ShouldBeTrue(); + int clientPid1 = ParseNumber(output1, "Process ID is "); + int serverPid1 = ParseNumber(output1, "TaskRanInPID="); + // Register cleanup before any assertion so the server does not leak if an assertion throws. + _env.WithTransientProcess(serverPid1); + + // The build ran in a separate server process: proof the server was engaged despite -nr:false. + serverPid1.ShouldNotBe(clientPid1, "Even with node reuse disabled, -mt must run the build in the server node, not the entry process."); + + // Because node reuse is disabled, the server must not persist past the build: its process should exit. + WaitForProcessExit(serverPid1).ShouldBeTrue($"Server process {serverPid1} should have been shut down after the build when node reuse is disabled."); + + // A second build cannot reuse the (now gone) server, so it must launch a fresh server process. + string output2 = RunnerUtilities.ExecMSBuild(BuildEnvironmentHelper.Instance.CurrentMSBuildExePath, $"{project.Path} -mt -nr:false", out bool success2, false, _output); + success2.ShouldBeTrue(); + int serverPid2 = ParseNumber(output2, "TaskRanInPID="); + _env.WithTransientProcess(serverPid2); + serverPid2.ShouldNotBe(serverPid1, "With node reuse disabled, each -mt build should get a fresh, non-persistent server process."); + } + finally + { + // Ensure any server we spun up is torn down even if an assertion above fails. + MSBuildClient.ShutdownServer(CancellationToken.None); + } } ///