Fix ShutdownAllNodes to find AppHost-launched worker nodes

src/Build.UnitTests/BackEnd/NodeProcessNameResolution_Tests.cs

@@ -0,0 +1,88 @@
+// 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.Linq;
+using Microsoft.Build.BackEnd;
+using Microsoft.Build.Shared;
+using Shouldly;
+using Xunit;
+using Constants = Microsoft.Build.Framework.Constants;
+
+namespace Microsoft.Build.Engine.UnitTests.BackEnd
+{
+    /// <summary>
+    /// Tests for <see cref="NodeProviderOutOfProcBase.ResolveProcessNamesToSearch"/>, the resolver
+    /// changed by the fix for https://github.com/dotnet/msbuild/issues/13508.
+    /// </summary>
+    public class NodeProcessNameResolution_Tests
+    {
+        private const string AppHostName = "MSBuild";  // Constants.MSBuildAppName
+        private static readonly string AppHostExeName = Constants.MSBuildExecutableName;
+
+        private static void ShouldContainIgnoreCase(string[] names, string expected) =>
+            names.Any(n => string.Equals(n, expected, StringComparison.OrdinalIgnoreCase))
+                .ShouldBeTrue($"Expected names [{string.Join(", ", names)}] to contain '{expected}' (case-insensitive).");
+
+        [Fact]
+        public void ReuseBranch_AppHostPath_ReturnsAppHostName()
+        {
+            string[] names = NodeProviderOutOfProcBase.ResolveProcessNamesToSearch(
+                msbuildLocation: Path.Combine("c:", "tools", AppHostExeName),
+                configuredNodeExeLocation: null);
+
+            names.ShouldBe([AppHostName]);
+        }
+
+        [Fact]
+        public void ReuseBranch_DllPath_ReturnsHostName()
+        {
+            // For a managed-assembly path the launcher uses the current host (e.g. "dotnet" on .NET Core).
+            string[] names = NodeProviderOutOfProcBase.ResolveProcessNamesToSearch(
+                msbuildLocation: Path.Combine("c:", "tools", Constants.MSBuildAssemblyName),
+                configuredNodeExeLocation: null);
+
+            names.Length.ShouldBe(1);
+            names[0].ShouldNotBeNullOrEmpty();
+        }
+
+        // Regression for https://github.com/dotnet/msbuild/issues/13508.
+        [Fact]
+        public void ShutdownBranch_NoConfiguredLocation_AlwaysIncludesAppHostName()
+        {
+            string[] names = NodeProviderOutOfProcBase.ResolveProcessNamesToSearch(
+                msbuildLocation: null,
+                configuredNodeExeLocation: null);
+
+            ShouldContainIgnoreCase(names, AppHostName);
+        }
+
+        [Fact]
+        public void ShutdownBranch_ConfiguredAppHostLocation_IncludesBothNames()
+        {
+            string[] names = NodeProviderOutOfProcBase.ResolveProcessNamesToSearch(
+                msbuildLocation: null,
+                configuredNodeExeLocation: Path.Combine("c:", "tools", AppHostExeName));
+
+            ShouldContainIgnoreCase(names, AppHostName);
+#if NET
+            // On .NET Core the alternate host is the current host (e.g. "dotnet").
+            names.Length.ShouldBe(2);
+#endif
+        }
+
+#if NET
+        [Fact]
+        public void ShutdownBranch_NetCore_ConfiguredDllLocation_IncludesAppHostFallback()
+        {
+            string[] names = NodeProviderOutOfProcBase.ResolveProcessNamesToSearch(
+                msbuildLocation: null,
+                configuredNodeExeLocation: Path.Combine("c:", "tools", Constants.MSBuildAssemblyName));
+
+            ShouldContainIgnoreCase(names, AppHostName);
+            names.Length.ShouldBe(2);
+        }
+#endif
+    }
+}

src/Build/BackEnd/Components/Communications/NodeProviderOutOfProcBase.cs

@@ -239,19 +239,9 @@
             // worker nodes are launched via dotnet.exe, matching the parent process.
             // This only applies to regular out-of-proc worker nodes (nodemode:1), not task host nodes
             // (nodemode:2) which may need the AppHost for COM host object support.
-            if (expectedNodeMode == NodeMode.OutOfProcNode
-                && !String.IsNullOrEmpty(msbuildLocation)
-                && Path.GetFileName(msbuildLocation).Equals(Constants.MSBuildExecutableName, StringComparison.OrdinalIgnoreCase))
+            if (expectedNodeMode == NodeMode.OutOfProcNode)
             {
-                string currentProcessName = Path.GetFileName(EnvironmentUtilities.ProcessPath);
-                if (currentProcessName?.Equals(Constants.DotnetProcessName, StringComparison.OrdinalIgnoreCase) == true)
-                {
-                    string dllPath = Path.Combine(Path.GetDirectoryName(msbuildLocation), Constants.MSBuildAssemblyName);
-                    if (File.Exists(dllPath))
-                    {
-                        msbuildLocation = dllPath;
-                    }
-                }
+                msbuildLocation = RemapAppHostToManagedDllIfHostedByDotnet(msbuildLocation);
             }
 #endif
 
@@ -469,24 +459,27 @@
             string msbuildLocation = null,
             NodeMode? expectedNodeMode = null)
         {
-            bool isNativeHost = msbuildLocation != null && Path.GetFileName(msbuildLocation).Equals(Constants.MSBuildExecutableName, StringComparison.OrdinalIgnoreCase);
-            string expectedProcessName = Path.GetFileNameWithoutExtension(isNativeHost ? msbuildLocation : (CurrentHost.GetCurrentHost() ?? msbuildLocation));
-
-#if NETFRAMEWORK
-            // Fall back to the standard executable name for most nodes
-            // on .NET Framework, to function in `ShutdownAllNodes()`
-            expectedProcessName ??= Constants.MSBuildAppName;
-#endif
-
-            Process[] processes;
-            try
-            {
-                processes = Process.GetProcessesByName(expectedProcessName);
-            }
-            catch
+            string[] processNamesToSearch = ResolveProcessNamesToSearch(
+                msbuildLocation,
+                _componentHost?.BuildParameters?.NodeExeLocation);
+
+            ErrorUtilities.VerifyThrow(processNamesToSearch.Length > 0, "Expected at least one process name to search for.");
+            string expectedProcessName = processNamesToSearch.Length == 1
+                ? processNamesToSearch[0]
+                : string.Join(", ", processNamesToSearch);
+
+            // Enumerate all candidate processes matching any of the target names.
+            List<Process> processes = new();
+            foreach (string name in processNamesToSearch)
             {
-                // Process enumeration can fail due to permissions or transient OS errors.
-                return (expectedProcessName, Array.Empty<Process>());
+                try
+                {
+                    processes.AddRange(Process.GetProcessesByName(name));
+                }
+                catch
+                {
+                    // Process enumeration can fail due to permissions or transient OS errors.
+                }
             }
 
             bool shouldFilterByNodeMode = expectedNodeMode.HasValue && ChangeWaves.AreFeaturesEnabled(ChangeWaves.Wave18_5);
@@ -495,21 +488,90 @@
                 return (expectedProcessName, FilterProcessesByNodeMode(processes, expectedNodeMode.Value, expectedProcessName));
             }
 
-            Array.Sort(processes, static (left, right) => left.Id.CompareTo(right.Id));
+            processes.Sort(static (left, right) => left.Id.CompareTo(right.Id));
 
             return (expectedProcessName, processes);
         }
 
+        /// <summary>
+        /// Returns the candidate process names to search for when locating worker nodes.
+        /// On the reuse path (<paramref name="msbuildLocation"/> non-null) returns the single name
+        /// that location would produce. On the shutdown path (<paramref name="msbuildLocation"/> null)
+        /// derives the name from <paramref name="configuredNodeExeLocation"/> or the current MSBuild
+        /// path — mirroring <see cref="GetNodes"/> — and adds the alternate host as a defensive
+        /// fallback for idle nodes started by an earlier build under a different host kind.
+        /// <paramref name="configuredNodeExeLocation"/> is a parameter (rather than instance state)
+        /// so the resolver can be unit-tested in isolation.
+        /// </summary>
+        internal static string[] ResolveProcessNamesToSearch(string msbuildLocation, string configuredNodeExeLocation)
+        {
+            if (msbuildLocation != null)
+            {
+                return [GetProcessNameForLocation(msbuildLocation)];
+            }
+
+            string wouldLaunchPath = !string.IsNullOrEmpty(configuredNodeExeLocation)
+                ? configuredNodeExeLocation
+                : BuildEnvironmentHelper.Instance.CurrentMSBuildExePath;
+
+#if RUNTIME_TYPE_NETCORE
+            wouldLaunchPath = RemapAppHostToManagedDllIfHostedByDotnet(wouldLaunchPath);
+#endif
+
+            string primary = !string.IsNullOrEmpty(wouldLaunchPath)
+                ? GetProcessNameForLocation(wouldLaunchPath)
+                : (Path.GetFileNameWithoutExtension(CurrentHost.GetCurrentHost()) ?? Constants.MSBuildAppName);
+
+            string alternate = string.Equals(primary, Constants.MSBuildAppName, StringComparison.OrdinalIgnoreCase)
+                ? Path.GetFileNameWithoutExtension(CurrentHost.GetCurrentHost())
+                : Constants.MSBuildAppName;
+
+            return string.IsNullOrEmpty(alternate) || string.Equals(primary, alternate, StringComparison.OrdinalIgnoreCase)
+                ? [primary]
+                : [primary, alternate];
+
+            // AppHost path -> "MSBuild"; managed DLL path -> current host name (e.g. "dotnet").
+            static string GetProcessNameForLocation(string location)
+            {
+                bool isAppHost = Path.GetFileName(location)
+                    .Equals(Constants.MSBuildExecutableName, StringComparison.OrdinalIgnoreCase);
+
+                return Path.GetFileNameWithoutExtension(
+                    isAppHost ? location : (CurrentHost.GetCurrentHost() ?? location));
+            }
+        }
+
+#if RUNTIME_TYPE_NETCORE
+        /// <summary>
+        /// When the current process is <c>dotnet</c> and <paramref name="msbuildLocation"/> points
+        /// at the MSBuild AppHost, returns the sibling <c>MSBuild.dll</c> (so workers launch via
+        /// <c>dotnet MSBuild.dll</c> instead of the slower AppHost). Otherwise returns the input
+        /// unchanged. Shared by <see cref="GetNodes"/> and <see cref="ResolveProcessNamesToSearch"/>.
+        /// </summary>
+        private static string RemapAppHostToManagedDllIfHostedByDotnet(string msbuildLocation)
+        {
+            if (string.IsNullOrEmpty(msbuildLocation)
+                || !Path.GetFileName(msbuildLocation).Equals(Constants.MSBuildExecutableName, StringComparison.OrdinalIgnoreCase)
+                || !string.Equals(Path.GetFileName(EnvironmentUtilities.ProcessPath), Constants.DotnetProcessName, StringComparison.OrdinalIgnoreCase))
+            {
+                return msbuildLocation;
+            }
+
+            string dllPath = Path.Combine(Path.GetDirectoryName(msbuildLocation), Constants.MSBuildAssemblyName);
+            return File.Exists(dllPath) ? dllPath : msbuildLocation;
+        }
+#endif
+
         /// <summary>
         /// Filters candidate processes whose command-line NodeMode argument matches the expected value.
         /// Processes whose command line cannot be retrieved (unsupported platform) are included
         /// unconditionally to preserve node reuse on those platforms.
         /// </summary>
-        private static IList<Process> FilterProcessesByNodeMode(Process[] processes, NodeMode expectedNodeMode, string expectedProcessName)
+        private static IList<Process> FilterProcessesByNodeMode(List<Process> processes, NodeMode expectedNodeMode, string expectedProcessName)
         {
             CommunicationsUtilities.Trace($"Filtering {processes.Length} candidate processes by NodeMode {expectedNodeMode} for process name '{expectedProcessName}'");
 
-            List<Process> filtered = new(capacity: processes.Length);
+            List<Process> filtered = new(capacity: processes.Count);
 
             foreach (Process process in processes)
             {