diff --git a/src/Build.UnitTests/BackEnd/NodeProcessNameResolution_Tests.cs b/src/Build.UnitTests/BackEnd/NodeProcessNameResolution_Tests.cs new file mode 100644 index 00000000000..b1ba00a1a3e --- /dev/null +++ b/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 +{ + /// + /// Tests for , the resolver + /// changed by the fix for https://github.com/dotnet/msbuild/issues/13508. + /// + 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 + } +} diff --git a/src/Build/BackEnd/Components/Communications/NodeProviderOutOfProcBase.cs b/src/Build/BackEnd/Components/Communications/NodeProviderOutOfProcBase.cs index d30dc85a5e2..fdcf73a40d2 100644 --- a/src/Build/BackEnd/Components/Communications/NodeProviderOutOfProcBase.cs +++ b/src/Build/BackEnd/Components/Communications/NodeProviderOutOfProcBase.cs @@ -239,19 +239,9 @@ protected IList GetNodes( // 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 @@ void CreateNodeContext(int nodeId, Process nodeToReuse, Stream nodeStream, byte 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 processes = new(); + foreach (string name in processNamesToSearch) { - // Process enumeration can fail due to permissions or transient OS errors. - return (expectedProcessName, Array.Empty()); + 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 @@ void CreateNodeContext(int nodeId, Process nodeToReuse, Stream nodeStream, byte 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); } + /// + /// Returns the candidate process names to search for when locating worker nodes. + /// On the reuse path ( non-null) returns the single name + /// that location would produce. On the shutdown path ( null) + /// derives the name from or the current MSBuild + /// path — mirroring — and adds the alternate host as a defensive + /// fallback for idle nodes started by an earlier build under a different host kind. + /// is a parameter (rather than instance state) + /// so the resolver can be unit-tested in isolation. + /// + 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 + /// + /// When the current process is dotnet and points + /// at the MSBuild AppHost, returns the sibling MSBuild.dll (so workers launch via + /// dotnet MSBuild.dll instead of the slower AppHost). Otherwise returns the input + /// unchanged. Shared by and . + /// + 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 + /// /// 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. /// - private static IList FilterProcessesByNodeMode(Process[] processes, NodeMode expectedNodeMode, string expectedProcessName) + private static IList FilterProcessesByNodeMode(List processes, NodeMode expectedNodeMode, string expectedProcessName) { CommunicationsUtilities.Trace($"Filtering {processes.Length} candidate processes by NodeMode {expectedNodeMode} for process name '{expectedProcessName}'"); - List filtered = new(capacity: processes.Length); + List filtered = new(capacity: processes.Count); foreach (Process process in processes) {