Fix TimeoutException when DOTNET_CLI_USE_MSBUILD_SERVER=true on .NET 10.0.300

src/Build.UnitTests/BackEnd/MSBuildClient_Tests.cs

@@ -0,0 +1,50 @@
+// 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.IO;
+using System.Threading;
+using Microsoft.Build.Experimental;
+using Shouldly;
+using Xunit;
+
+namespace Microsoft.Build.Engine.UnitTests.BackEnd
+{
+    /// <summary>
+    /// Tests for the <see cref="MSBuildClient"/> fallback behaviour.
+    /// </summary>
+    public sealed class MSBuildClient_Tests
+    {
+        /// <summary>
+        /// When the configured msbuild executable does not exist, launching the server fails.
+        /// The client must report a recoverable exit type (LaunchError / UnableToConnect /
+        /// UnknownServerState / ServerBusy) rather than letting an exception escape.
+        /// </summary>
+        /// <remarks>
+        /// Regression coverage for the .NET 10.0.300 / Aspire timeout: when
+        /// <c>DOTNET_CLI_USE_MSBUILD_SERVER=true</c> is honoured but the server child cannot start
+        /// (e.g. the apphost can't find the .NET runtime), <see cref="MSBuildClient.Execute"/> must
+        /// not propagate a <see cref="System.TimeoutException"/> &#8212; it must return an exit type that
+        /// causes the host (<c>MSBuildClientApp</c>) to fall back to in-proc execution.
+        /// </remarks>
+        [Fact]
+        public void Execute_WithUnreachableServer_DoesNotPropagateException()
+        {
+            string[] commandLine = ["dummy.proj"];
+            string nonexistentMsBuild = Path.Combine(Path.GetTempPath(), "does-not-exist-" + Guid.NewGuid().ToString("N"), "MSBuild.dll");
+
+            MSBuildClient client = new MSBuildClient(commandLine, nonexistentMsBuild);
+
+            // The whole point of the regression fix: this must NOT throw. Any of the recoverable
+            // exit types is acceptable here &#8212; what matters is that MSBuildClientApp gets a chance
+            // to fall back to in-proc execution.
+            MSBuildClientExitResult result = client.Execute(CancellationToken.None);
+
+            result.MSBuildClientExitType.ShouldBeOneOf(
+                MSBuildClientExitType.LaunchError,
+                MSBuildClientExitType.UnableToConnect,
+                MSBuildClientExitType.UnknownServerState,
+                MSBuildClientExitType.ServerBusy);
+        }
+    }
+}

src/Build/BackEnd/Client/MSBuildClient.cs

@@ -101,6 +101,12 @@ public sealed class MSBuildClient
         /// </summary>
         private MSBuildClientPacketPump _packetPump = null!;
 
+        /// <summary>
+        /// PID of the server process this client launched (or null if no launch was attempted /
+        /// the server was already running). Used for diagnostics on connection failure.
+        /// </summary>
+        private int? _launchedServerPid;
+
         /// <summary>
         /// Public constructor with parameters.
         /// </summary>
@@ -459,8 +465,21 @@ private bool TryLaunchServer()
                 ];
                 NodeLauncher nodeLauncher = new NodeLauncher();
                 CommunicationsUtilities.Trace("Starting Server...");
-                using Process msbuildProcess = nodeLauncher.Start(new NodeLaunchData(_msbuildLocation, string.Join(" ", msBuildServerOptions)), nodeId: 0);
-                CommunicationsUtilities.Trace($"Server started with PID: {msbuildProcess?.Id}");
+
+                // Apply the same DOTNET_ROOT environment overrides we pass to worker nodes (see
+                // NodeProviderOutOfProc.CreateNode). When the parent MSBuild process is the
+                // apphost (native executable), the launched server child is also the apphost and
+                // needs DOTNET_ROOT to locate the runtime. Without these overrides the server
+                // process fails to start, the named pipe is never opened, and the client times
+                // out after 20s (the DOTNET_CLI_USE_MSBUILD_SERVER=true regression in 10.0.300).
+                NodeLaunchData launchData = new(
+                    MSBuildLocation: _msbuildLocation,
+                    CommandLineArgs: string.Join(" ", msBuildServerOptions),
+                    EnvironmentOverrides: DotnetHostEnvironmentHelper.CreateDotnetRootEnvironmentOverrides()!);
+
+                using Process msbuildProcess = nodeLauncher.Start(launchData, nodeId: 0);
+                _launchedServerPid = msbuildProcess?.Id;
+                CommunicationsUtilities.Trace($"Server started with PID: {_launchedServerPid}");
             }
             catch (Exception ex)
             {
@@ -605,9 +624,36 @@ private bool TryConnectToServer(int timeoutMilliseconds)
             {
                 tryAgain = false;
 
+                HandshakeResult result;
+                bool connected;
+                try
+                {
+                    connected = NodeProviderOutOfProcBase.TryConnectToPipeStream(
+                        _nodeStream, _pipeName, _handshake, Math.Max(1, timeoutMilliseconds - (int)sw.ElapsedMilliseconds), out result);
+                }
+                catch (TimeoutException)
+                {
+                    // The underlying NamedPipeClientStream.Connect throws TimeoutException when the
+                    // pipe never becomes available — typically because the server child process
+                    // failed to start (e.g. apphost couldn't locate the runtime). Treat this as a
+                    // recoverable connection failure so MSBuildClientApp can fall back to in-proc
+                    // execution rather than crashing the whole CLI.
+                    LogConnectFailureDiagnostics(timeoutMilliseconds, isTimeout: true, errorMessage: null);
+                    _exitResult.MSBuildClientExitType = MSBuildClientExitType.UnableToConnect;
+                    return false;
+                }
+                catch (Exception ex) when (!ExceptionHandling.IsCriticalException(ex))
+                {
+                    // Mirror the exception-tolerant behavior of NodeProviderOutOfProcBase.TryConnectToProcess
+                    // so any non-critical failure (UnauthorizedAccessException, IOException,
+                    // InvalidOperationException, etc.) routes through the standard fallback path
+                    // rather than escaping out of MSBuildClient.Execute.
+                    LogConnectFailureDiagnostics(timeoutMilliseconds, isTimeout: false, errorMessage: ex.Message);
+                    _exitResult.MSBuildClientExitType = MSBuildClientExitType.UnableToConnect;
+                    return false;
+                }
 
-                if (NodeProviderOutOfProcBase.TryConnectToPipeStream(
-                    _nodeStream, _pipeName, _handshake, Math.Max(1, timeoutMilliseconds - (int)sw.ElapsedMilliseconds), out HandshakeResult result))
+                if (connected)
                 {
                     return true;
                 }
@@ -623,7 +669,7 @@ private bool TryConnectToServer(int timeoutMilliseconds)
                     }
                     else
                     {
-                        CommunicationsUtilities.Trace($"Failed to connect to server: {result.ErrorMessage}");
+                        LogConnectFailureDiagnostics(timeoutMilliseconds, isTimeout: result.Status is HandshakeStatus.Timeout, errorMessage: result.ErrorMessage);
                         _exitResult.MSBuildClientExitType = MSBuildClientExitType.UnableToConnect;
                         return false;
                     }
@@ -633,6 +679,61 @@ private bool TryConnectToServer(int timeoutMilliseconds)
             return false;
         }
 
+        /// <summary>
+        /// Emits a single diagnostic trace entry describing why connection to the MSBuild server
+        /// failed, including the launched server PID (if any) and its current state. This makes
+        /// the otherwise-opaque 20s timeout actionable when MSBUILDDEBUGCOMM tracing is enabled.
+        /// Also populates <see cref="MSBuildClientExitResult.ServerProcessExitCode"/> when the
+        /// launched server child has already exited, so the host can surface that fact to the
+        /// user-visible "falling back to in-proc" message instead of a generic timeout.
+        /// </summary>
+        private void LogConnectFailureDiagnostics(int timeoutMilliseconds, bool isTimeout, string? errorMessage)
+        {
+            string serverState;
+            if (_launchedServerPid is int pid)
+            {
+                try
+                {
+                    using Process? launched = Process.GetProcessById(pid);
+                    if (launched is null)
+                    {
+                        serverState = $"PID {pid} (no longer present)";
+                    }
+                    else if (launched.HasExited)
+                    {
+                        _exitResult.ServerProcessExitCode = launched.ExitCode;
+                        serverState = $"PID {pid} (already exited with code {launched.ExitCode})";
+                    }
+                    else
+                    {
+                        serverState = $"PID {pid} (still running)";
+                    }
+                }
+                catch (ArgumentException)
+                {
+                    // Process already terminated and was reaped before we could query it.
+                    serverState = $"PID {pid} (already exited)";
+                }
+                catch (InvalidOperationException)
+                {
+                    serverState = $"PID {pid} (state unavailable)";
+                }
+            }
+            else
+            {
+                serverState = "no launch attempted (server reported as already running)";
+            }
+
+            string reason = isTimeout
+                ? $"timed out after {timeoutMilliseconds} ms waiting for the named pipe"
+                : $"connection error: {errorMessage}";
+
+            CommunicationsUtilities.Trace(
+                $"MSBuild server connection failed ({reason}). Launched server: {serverState}. " +
+                "Falling back to in-proc build. " +
+                "If the server child process exited immediately, ensure DOTNET_ROOT is set correctly so the apphost can locate the .NET runtime.");
+        }
+
         private void WritePacket(Stream nodeStream, INodePacket packet)
         {
             MemoryStream memoryStream = _packetMemoryStream;

src/Build/BackEnd/Client/MSBuildClientExitResult.cs

@@ -20,5 +20,14 @@ public sealed class MSBuildClientExitResult
         /// This field is null if MSBuild client execution was not successful.
         /// </summary>
         public string? MSBuildAppExitTypeString { get; set; }
+
+        /// <summary>
+        /// When this client launched a server child process and that process had already exited
+        /// by the time we observed the connection failure, this is its exit code. <c>null</c>
+        /// otherwise (server still running, never launched, or its state could not be queried).
+        /// Hosts use this to surface "server crashed immediately on launch" to the user instead
+        /// of a generic timeout message.
+        /// </summary>
+        public int? ServerProcessExitCode { get; set; }
     }
 }

src/MSBuild/MSBuildClientApp.cs

@@ -2,6 +2,7 @@
 // The .NET Foundation licenses this file to you under the MIT license.
 
 using System;
+using System.Globalization;
 using System.Threading;
 using Microsoft.Build.Experimental;
 using Microsoft.Build.Framework.Telemetry;
@@ -70,7 +71,18 @@ public static MSBuildApp.ExitType Execute(string[] commandLineArgs, string msbui
                     KnownTelemetry.PartialBuildTelemetry.ServerFallbackReason = exitResult.MSBuildClientExitType.ToString();
                 }
 
-                // Server is busy, fallback to old behavior.
+                // Surface a single user-visible message on stderr when the failure is something
+                // other than the well-understood "another client is racing us for the launch
+                // mutex" case. Without this the user sees no indication that MSBuild Server was
+                // requested but unavailable; previously a connection timeout would even crash
+                // the process (the DOTNET_CLI_USE_MSBUILD_SERVER=true regression in 10.0.300).
+                if (exitResult.MSBuildClientExitType != MSBuildClientExitType.ServerBusy)
+                {
+                    string detail = GetServerFallbackDetail(exitResult);
+                    Console.Error.WriteLine(ResourceUtilities.FormatResourceStringIgnoreCodeAndKeyword("MSBuildServerUnavailable", detail));
+                }
+
+                // Server is busy / unavailable, fallback to old behavior.
                 return MSBuildApp.Execute(commandLineArgs);
             }
 
@@ -84,5 +96,25 @@ public static MSBuildApp.ExitType Execute(string[] commandLineArgs, string msbui
 
             return MSBuildApp.ExitType.MSBuildClientFailure;
         }
+
+        /// <summary>
+        /// Picks the most specific localized "why MSBuild server was unavailable" sub-message for
+        /// the user-visible fallback notice. Prefers the "server crashed immediately on launch"
+        /// detail over a generic timeout when the launched server's exit code is known.
+        /// </summary>
+        private static string GetServerFallbackDetail(MSBuildClientExitResult exitResult)
+        {
+            return exitResult.MSBuildClientExitType switch
+            {
+                MSBuildClientExitType.LaunchError => AssemblyResources.GetString("MSBuildServerLaunchError"),
+                MSBuildClientExitType.UnknownServerState => AssemblyResources.GetString("MSBuildServerStateUnknown"),
+                MSBuildClientExitType.UnableToConnect when exitResult.ServerProcessExitCode is int code =>
+                    ResourceUtilities.FormatResourceStringIgnoreCodeAndKeyword(
+                        "MSBuildServerCrashedOnLaunch",
+                        code.ToString(CultureInfo.InvariantCulture)),
+                MSBuildClientExitType.UnableToConnect => AssemblyResources.GetString("MSBuildServerLaunchTimeout"),
+                _ => AssemblyResources.GetString("MSBuildServerConnectFailed"),
+            };
+        }
     }
 }

src/MSBuild/Resources/Strings.resx

@@ -1736,6 +1736,33 @@
   </data>
   <!-- **** TaskHost strings end **** -->
 
+  <!-- **** MSBuild Server fallback strings begin **** -->
+  <data name="MSBuildServerUnavailable" xml:space="preserve">
+    <value>MSBuild server unavailable: {0} Falling back to in-proc build.</value>
+    <comment>LOCALIZATION: {0} is a sub-message describing why the MSBuild server could not be used (e.g. that it crashed or did not respond). It is one of MSBuildServerCrashedOnLaunch / MSBuildServerLaunchTimeout / MSBuildServerLaunchError / MSBuildServerStateUnknown / MSBuildServerConnectFailed.</comment>
+  </data>
+  <data name="MSBuildServerCrashedOnLaunch" xml:space="preserve">
+    <value>the server process exited with code {0} immediately after launch (ensure DOTNET_ROOT is set so the apphost can locate the .NET runtime; set MSBUILDDEBUGCOMM=1 for diagnostics).</value>
+    <comment>LOCALIZATION: {0} is the integer process exit code.</comment>
+  </data>
+  <data name="MSBuildServerLaunchTimeout" xml:space="preserve">
+    <value>the server process did not respond in time and may have failed to start (ensure DOTNET_ROOT is set so the apphost can locate the .NET runtime; set MSBUILDDEBUGCOMM=1 for diagnostics).</value>
+    <comment>LOCALIZATION: No format arguments.</comment>
+  </data>
+  <data name="MSBuildServerLaunchError" xml:space="preserve">
+    <value>the server process could not be launched (set MSBUILDDEBUGCOMM=1 for diagnostics).</value>
+    <comment>LOCALIZATION: No format arguments.</comment>
+  </data>
+  <data name="MSBuildServerStateUnknown" xml:space="preserve">
+    <value>the current server state could not be determined (set MSBUILDDEBUGCOMM=1 for diagnostics).</value>
+    <comment>LOCALIZATION: No format arguments.</comment>
+  </data>
+  <data name="MSBuildServerConnectFailed" xml:space="preserve">
+    <value>could not connect to the server (set MSBUILDDEBUGCOMM=1 for diagnostics).</value>
+    <comment>LOCALIZATION: No format arguments. Used when no specific cause has been identified for a connection failure.</comment>
+  </data>
+  <!-- **** MSBuild Server fallback strings end **** -->
+
     <!--
         The command line message bucket is: MSB1001 - MSB1999