From 6ed2049446a6c8f72050d64208254e7d328b44d4 Mon Sep 17 00:00:00 2001 From: Tom Deseyn Date: Thu, 21 Jan 2021 09:43:33 +0100 Subject: [PATCH 01/17] When sharing the terminal with child nodes, wait for the children to terminate before exiting ourselves. This avoids issues where the child changes terminal configuration after our own exit. --- .../NodeProviderOutOfProcBase.cs | 43 ++++++++++++++++++- 1 file changed, 41 insertions(+), 2 deletions(-) diff --git a/src/Build/BackEnd/Components/Communications/NodeProviderOutOfProcBase.cs b/src/Build/BackEnd/Components/Communications/NodeProviderOutOfProcBase.cs index e65f614e08e..0ed9e7fcee9 100644 --- a/src/Build/BackEnd/Components/Communications/NodeProviderOutOfProcBase.cs +++ b/src/Build/BackEnd/Components/Communications/NodeProviderOutOfProcBase.cs @@ -96,9 +96,23 @@ protected void ShutdownConnectedNodes(List contextsToShutDown, bool // Send the build completion message to the nodes, causing them to shutdown or reset. _processesToIgnore.Clear(); + // We wait for child nodes to exit to avoid them changing the terminal + // after this process terminates. + bool waitForExit = !enableReuse && + !Console.IsInputRedirected && + Traits.Instance.EscapeHatches.EnsureStdOutForChildNodesIsPrimaryStdout; + foreach (NodeContext nodeContext in contextsToShutDown) { - nodeContext?.SendData(new NodeBuildComplete(enableReuse)); + if (nodeContext is null) + { + continue; + } + nodeContext.SendData(new NodeBuildComplete(enableReuse)); + if (waitForExit) + { + nodeContext.WaitForExit(); + } } } @@ -792,8 +806,9 @@ public void SendData(INodePacket packet) /// /// Closes the node's context, disconnecting it from the node. /// - public void Close() + private void Close() { + _processId = -1; _clientToServerStream.Dispose(); if (!object.ReferenceEquals(_clientToServerStream, _serverToClientStream)) { @@ -802,6 +817,30 @@ public void Close() _terminateDelegate(_nodeId); } + /// + /// Waits for the child node process to exit. + /// + public void WaitForExit() + { + int processId = _processId; + if (processId != -1) + { + Process childProcess; + try + { + childProcess = Process.GetProcessById(processId); + } + catch (System.ArgumentException) + { + // The process has terminated already. + return; + } + // Wait for the process to terminate. + CommunicationsUtilities.Trace("Waiting for node with pid = {0} to terminate", processId); + childProcess.WaitForExit(); + } + } + #if FEATURE_APM /// /// Completes the asynchronous packet write to the node. From 412452e2667762fad9cb928d9ba250d87874c160 Mon Sep 17 00:00:00 2001 From: Tom Deseyn Date: Mon, 25 Jan 2021 14:34:12 +0100 Subject: [PATCH 02/17] Kill node after timeout. Only wait when close sent. --- .../NodeProviderOutOfProcBase.cs | 51 ++++++++++++++++--- 1 file changed, 44 insertions(+), 7 deletions(-) diff --git a/src/Build/BackEnd/Components/Communications/NodeProviderOutOfProcBase.cs b/src/Build/BackEnd/Components/Communications/NodeProviderOutOfProcBase.cs index 0ed9e7fcee9..676c89573d0 100644 --- a/src/Build/BackEnd/Components/Communications/NodeProviderOutOfProcBase.cs +++ b/src/Build/BackEnd/Components/Communications/NodeProviderOutOfProcBase.cs @@ -50,6 +50,11 @@ internal abstract class NodeProviderOutOfProcBase /// private const int TimeoutForNewNodeCreation = 30000; + /// + /// The amount of time to wait for an out-of-proc node to exit. + /// + private const int TimeoutForWaitForExit = 30000; + /// /// The build component host. /// @@ -102,6 +107,8 @@ protected void ShutdownConnectedNodes(List contextsToShutDown, bool !Console.IsInputRedirected && Traits.Instance.EscapeHatches.EnsureStdOutForChildNodesIsPrimaryStdout; + Task[] waitForExitTasks = waitForExit && contextsToShutDown.Count > 0? new Task[contextsToShutDown.Count] : null; + int i = 0; foreach (NodeContext nodeContext in contextsToShutDown) { if (nodeContext is null) @@ -111,9 +118,13 @@ protected void ShutdownConnectedNodes(List contextsToShutDown, bool nodeContext.SendData(new NodeBuildComplete(enableReuse)); if (waitForExit) { - nodeContext.WaitForExit(); + waitForExitTasks[i++] = nodeContext.WaitForExitAsync(); } } + if (waitForExitTasks != null) + { + Task.WaitAll(waitForExitTasks); + } } /// @@ -627,14 +638,14 @@ internal class NodeContext private byte[] _smallReadBuffer; /// - /// Event indicating the node has terminated. + /// Delegate called when the context terminates. /// - private ManualResetEvent _nodeTerminated; + private NodeContextTerminateDelegate _terminateDelegate; /// - /// Delegate called when the context terminates. + /// Node was requested to terminate. /// - private NodeContextTerminateDelegate _terminateDelegate; + private bool _closeSent; /// /// Per node read buffers @@ -655,7 +666,6 @@ internal class NodeContext _packetFactory = factory; _headerByte = new byte[5]; // 1 for the packet type, 4 for the body length _smallReadBuffer = new byte[1000]; // 1000 was just an average seen on one profile run. - _nodeTerminated = new ManualResetEvent(false); _terminateDelegate = terminateDelegate; _sharedReadBuffer = InterningBinaryReader.CreateSharedBuffer(); } @@ -791,6 +801,7 @@ public void SendData(INodePacket packet) #endif } } + _closeSent = packet is NodeBuildComplete buildCompletePacket && !buildCompletePacket.PrepareForReuse; } catch (IOException e) { @@ -820,7 +831,7 @@ private void Close() /// /// Waits for the child node process to exit. /// - public void WaitForExit() + public async Task WaitForExitAsync() { int processId = _processId; if (processId != -1) @@ -835,8 +846,34 @@ public void WaitForExit() // The process has terminated already. return; } + // Wait for the process to terminate. CommunicationsUtilities.Trace("Waiting for node with pid = {0} to terminate", processId); + + if (_closeSent) + { + // .NET 5 introduces a real WaitForExitAsyc. + // This is a poor man's implementation that uses polling. + int timeout = TimeoutForWaitForExit; + int delay = 5; + while (timeout > 0) + { + bool exited = childProcess.WaitForExit(milliseconds: 0); + if (exited) + { + return; + } + timeout -= delay; + await Task.Delay(delay).ConfigureAwait(false); + + // Double delay up to 500ms. + delay = Math.Min(delay * 2, 500); + } + } + + // Kill the child and do a blocking wait. + CommunicationsUtilities.Trace("Killing node with pid = {0}", processId); + childProcess.Kill(); childProcess.WaitForExit(); } } From 812a0c9151e6717aecb1e11f606c17454719e663 Mon Sep 17 00:00:00 2001 From: Tom Deseyn Date: Mon, 25 Jan 2021 15:11:47 +0100 Subject: [PATCH 03/17] Deploy-MSBuild.ps1: fix filename casing --- scripts/Deploy-MSBuild.ps1 | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/scripts/Deploy-MSBuild.ps1 b/scripts/Deploy-MSBuild.ps1 index 125e9447ca5..235f29c5259 100644 --- a/scripts/Deploy-MSBuild.ps1 +++ b/scripts/Deploy-MSBuild.ps1 @@ -73,11 +73,11 @@ $filesToCopyToBin = @( FileToCopy "$bootstrapBinDirectory\Microsoft.Managed.targets" FileToCopy "$bootstrapBinDirectory\Microsoft.Managed.Before.targets" FileToCopy "$bootstrapBinDirectory\Microsoft.Managed.After.targets" - FileToCopy "$bootstrapBinDirectory\Microsoft.Net.props" - FileToCopy "$bootstrapBinDirectory\Microsoft.NetFramework.CurrentVersion.props" - FileToCopy "$bootstrapBinDirectory\Microsoft.NetFramework.CurrentVersion.targets" - FileToCopy "$bootstrapBinDirectory\Microsoft.NetFramework.props" - FileToCopy "$bootstrapBinDirectory\Microsoft.NetFramework.targets" + FileToCopy "$bootstrapBinDirectory\Microsoft.NET.props" + FileToCopy "$bootstrapBinDirectory\Microsoft.NETFramework.CurrentVersion.props" + FileToCopy "$bootstrapBinDirectory\Microsoft.NETFramework.CurrentVersion.targets" + FileToCopy "$bootstrapBinDirectory\Microsoft.NETFramework.props" + FileToCopy "$bootstrapBinDirectory\Microsoft.NETFramework.targets" FileToCopy "$bootstrapBinDirectory\Microsoft.VisualBasic.CrossTargeting.targets" FileToCopy "$bootstrapBinDirectory\Microsoft.VisualBasic.CurrentVersion.targets" FileToCopy "$bootstrapBinDirectory\Microsoft.VisualBasic.targets" From c6e5870605e8bfadb0b30999a91911d3b0523165 Mon Sep 17 00:00:00 2001 From: Tom Deseyn Date: Tue, 26 Jan 2021 09:24:52 +0100 Subject: [PATCH 04/17] Keep a usable Process instance in the NodeContext --- .../NodeProviderOutOfProcBase.cs | 101 ++++++++---------- .../TaskExecutionHost/TaskExecutionHost.cs | 1 + src/Build/Microsoft.Build.csproj | 1 + src/Utilities/ProcessExtensions.cs | 16 ++- 4 files changed, 55 insertions(+), 64 deletions(-) diff --git a/src/Build/BackEnd/Components/Communications/NodeProviderOutOfProcBase.cs b/src/Build/BackEnd/Components/Communications/NodeProviderOutOfProcBase.cs index 676c89573d0..27b9e28457c 100644 --- a/src/Build/BackEnd/Components/Communications/NodeProviderOutOfProcBase.cs +++ b/src/Build/BackEnd/Components/Communications/NodeProviderOutOfProcBase.cs @@ -26,6 +26,7 @@ using Microsoft.Build.Utilities; using BackendNativeMethods = Microsoft.Build.BackEnd.NativeMethods; +using Task = System.Threading.Tasks.Task; namespace Microsoft.Build.BackEnd { @@ -164,7 +165,7 @@ protected void ShutdownAllNodes(bool nodeReuse, NodeContextTerminateDelegate ter { // If we're able to connect to such a process, send a packet requesting its termination CommunicationsUtilities.Trace("Shutting down node with pid = {0}", nodeProcess.Id); - NodeContext nodeContext = new NodeContext(0, nodeProcess.Id, nodeStream, factory, terminateNode); + NodeContext nodeContext = new NodeContext(0, nodeProcess, nodeStream, factory, terminateNode); nodeContext.SendData(new NodeBuildComplete(false /* no node reuse */)); nodeStream.Dispose(); } @@ -230,7 +231,7 @@ protected NodeContext GetNode(string msbuildLocation, string commandLineArgs, in { // Connection successful, use this node. CommunicationsUtilities.Trace("Successfully connected to existed node {0} which is PID {1}", nodeId, nodeProcess.Id); - return new NodeContext(nodeId, nodeProcess.Id, nodeStream, factory, terminateNode); + return new NodeContext(nodeId, nodeProcess, nodeStream, factory, terminateNode); } } } @@ -268,20 +269,20 @@ protected NodeContext GetNode(string msbuildLocation, string commandLineArgs, in #endif // Create the node process - int msbuildProcessId = LaunchNode(msbuildLocation, commandLineArgs); - _processesToIgnore.Add(GetProcessesToIgnoreKey(hostHandshake, msbuildProcessId)); + Process msbuildProcess = LaunchNode(msbuildLocation, commandLineArgs); + _processesToIgnore.Add(GetProcessesToIgnoreKey(hostHandshake, msbuildProcess.Id)); // Note, when running under IMAGEFILEEXECUTIONOPTIONS registry key to debug, the process ID // gotten back from CreateProcess is that of the debugger, which causes this to try to connect // to the debugger process. Instead, use MSBUILDDEBUGONSTART=1 // Now try to connect to it. - Stream nodeStream = TryConnectToProcess(msbuildProcessId, TimeoutForNewNodeCreation, hostHandshake); + Stream nodeStream = TryConnectToProcess(msbuildProcess.Id, TimeoutForNewNodeCreation, hostHandshake); if (nodeStream != null) { // Connection successful, use this node. - CommunicationsUtilities.Trace("Successfully connected to created node {0} which is PID {1}", nodeId, msbuildProcessId); - return new NodeContext(nodeId, msbuildProcessId, nodeStream, factory, terminateNode); + CommunicationsUtilities.Trace("Successfully connected to created node {0} which is PID {1}", nodeId, msbuildProcess.Id); + return new NodeContext(nodeId, msbuildProcess, nodeStream, factory, terminateNode); } } @@ -417,7 +418,7 @@ private Stream TryConnectToProcess(int nodeProcessId, int timeout, Handshake han /// /// Creates a new MSBuild process /// - private int LaunchNode(string msbuildLocation, string commandLineArgs) + private Process LaunchNode(string msbuildLocation, string commandLineArgs) { // Should always have been set already. ErrorUtilities.VerifyThrowInternalLength(msbuildLocation, nameof(msbuildLocation)); @@ -516,7 +517,7 @@ private int LaunchNode(string msbuildLocation, string commandLineArgs) } CommunicationsUtilities.Trace("Successfully launched {1} node with PID {0}", process.Id, exeName); - return process.Id; + return process; } else { @@ -574,7 +575,7 @@ out processInfo } CommunicationsUtilities.Trace("Successfully launched {1} node with PID {0}", childProcessId, exeName); - return childProcessId; + return Process.GetProcessById(childProcessId); } } @@ -623,9 +624,9 @@ internal class NodeContext private int _nodeId; /// - /// The process id + /// The node process. /// - private int _processId; + private readonly Process _process; /// /// An array used to store the header byte for each packet when read. @@ -645,7 +646,7 @@ internal class NodeContext /// /// Node was requested to terminate. /// - private bool _closeSent; + private bool _isExiting; /// /// Per node read buffers @@ -655,12 +656,12 @@ internal class NodeContext /// /// Constructor. /// - public NodeContext(int nodeId, int processId, + public NodeContext(int nodeId, Process process, Stream nodePipe, INodePacketFactory factory, NodeContextTerminateDelegate terminateDelegate) { _nodeId = nodeId; - _processId = processId; + _process = process; _clientToServerStream = nodePipe; _serverToClientStream = nodePipe; _packetFactory = factory; @@ -785,7 +786,7 @@ public void SendData(INodePacket packet) #else _serverToClientStream.WriteAsync(writeStreamBuffer, i, lengthToWrite); #endif - return; + break; } else { @@ -801,7 +802,7 @@ public void SendData(INodePacket packet) #endif } } - _closeSent = packet is NodeBuildComplete buildCompletePacket && !buildCompletePacket.PrepareForReuse; + _isExiting = packet is NodeBuildComplete buildCompletePacket && !buildCompletePacket.PrepareForReuse; } catch (IOException e) { @@ -819,7 +820,6 @@ public void SendData(INodePacket packet) /// private void Close() { - _processId = -1; _clientToServerStream.Dispose(); if (!object.ReferenceEquals(_clientToServerStream, _serverToClientStream)) { @@ -833,49 +833,33 @@ private void Close() /// public async Task WaitForExitAsync() { - int processId = _processId; - if (processId != -1) + // Wait for the process to exit. + if (_isExiting) { - Process childProcess; - try - { - childProcess = Process.GetProcessById(processId); - } - catch (System.ArgumentException) - { - // The process has terminated already. - return; - } - - // Wait for the process to terminate. - CommunicationsUtilities.Trace("Waiting for node with pid = {0} to terminate", processId); + CommunicationsUtilities.Trace("Waiting for node with pid = {0} to exit", _process.Id); - if (_closeSent) + // .NET 5 introduces a real WaitForExitAsyc. + // This is a poor man's implementation that uses polling. + int timeout = TimeoutForWaitForExit; + int delay = 5; + while (timeout > 0) { - // .NET 5 introduces a real WaitForExitAsyc. - // This is a poor man's implementation that uses polling. - int timeout = TimeoutForWaitForExit; - int delay = 5; - while (timeout > 0) + bool exited = _process.WaitForExit(milliseconds: 0); + if (exited) { - bool exited = childProcess.WaitForExit(milliseconds: 0); - if (exited) - { - return; - } - timeout -= delay; - await Task.Delay(delay).ConfigureAwait(false); - - // Double delay up to 500ms. - delay = Math.Min(delay * 2, 500); + return; } - } + timeout -= delay; + await Task.Delay(delay).ConfigureAwait(false); - // Kill the child and do a blocking wait. - CommunicationsUtilities.Trace("Killing node with pid = {0}", processId); - childProcess.Kill(); - childProcess.WaitForExit(); + // Double delay up to 500ms. + delay = Math.Min(delay * 2, 500); + } } + + // Kill the child and do a blocking wait. + CommunicationsUtilities.Trace("Killing node with pid = {0}", _process.Id); + _process.KillTree(timeout: -1); } #if FEATURE_APM @@ -899,17 +883,16 @@ private bool ProcessHeaderBytesRead(int bytesRead) { if (bytesRead != _headerByte.Length) { - CommunicationsUtilities.Trace(_nodeId, "COMMUNICATIONS ERROR (HRC) Node: {0} Process: {1} Bytes Read: {2} Expected: {3}", _nodeId, _processId, bytesRead, _headerByte.Length); + CommunicationsUtilities.Trace(_nodeId, "COMMUNICATIONS ERROR (HRC) Node: {0} Process: {1} Bytes Read: {2} Expected: {3}", _nodeId, _process.Id, bytesRead, _headerByte.Length); try { - Process childProcess = Process.GetProcessById(_processId); - if (childProcess?.HasExited != false) + if (_process.HasExited) { - CommunicationsUtilities.Trace(_nodeId, " Child Process {0} has exited.", _processId); + CommunicationsUtilities.Trace(_nodeId, " Child Process {0} has exited.", _process.Id); } else { - CommunicationsUtilities.Trace(_nodeId, " Child Process {0} is still running.", _processId); + CommunicationsUtilities.Trace(_nodeId, " Child Process {0} is still running.", _process.Id); } } catch (Exception e) when (!ExceptionHandling.IsCriticalException(e)) diff --git a/src/Build/BackEnd/TaskExecutionHost/TaskExecutionHost.cs b/src/Build/BackEnd/TaskExecutionHost/TaskExecutionHost.cs index 2a98b2fdf32..37a0bb91438 100644 --- a/src/Build/BackEnd/TaskExecutionHost/TaskExecutionHost.cs +++ b/src/Build/BackEnd/TaskExecutionHost/TaskExecutionHost.cs @@ -24,6 +24,7 @@ using Microsoft.Build.Utilities; using TaskItem = Microsoft.Build.Execution.ProjectItemInstance.TaskItem; +using Task = System.Threading.Tasks.Task; namespace Microsoft.Build.BackEnd { diff --git a/src/Build/Microsoft.Build.csproj b/src/Build/Microsoft.Build.csproj index b14c97d1565..ba6fe6ae510 100644 --- a/src/Build/Microsoft.Build.csproj +++ b/src/Build/Microsoft.Build.csproj @@ -28,6 +28,7 @@ + diff --git a/src/Utilities/ProcessExtensions.cs b/src/Utilities/ProcessExtensions.cs index 04d6afb3f36..886b069eb4f 100644 --- a/src/Utilities/ProcessExtensions.cs +++ b/src/Utilities/ProcessExtensions.cs @@ -9,7 +9,7 @@ namespace Microsoft.Build.Utilities { - internal static class ProcessExtensions + public static class ProcessExtensions { public static void KillTree(this Process process, int timeout) { @@ -77,10 +77,16 @@ private static void GetAllChildIdsUnix(int parentId, ISet children) private static void KillProcessUnix(int processId) { - RunProcessAndWaitForExit( - "kill", - $"-TERM {processId}", - out string _); + try + { + using Process process = Process.GetProcessById(processId); + process.Kill(); + } + catch (ArgumentException) + { + // Process already terminated. + return; + } } private static int RunProcessAndWaitForExit(string fileName, string arguments, out string stdout) From d0091f40c0d923ab2fdfd0536f16e239b105ddef Mon Sep 17 00:00:00 2001 From: Tom Deseyn Date: Tue, 26 Jan 2021 09:25:12 +0100 Subject: [PATCH 05/17] Deploy-MSBuild: copy from net5.0 --- scripts/Deploy-MSBuild.ps1 | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/Deploy-MSBuild.ps1 b/scripts/Deploy-MSBuild.ps1 index 235f29c5259..90bee4b3905 100644 --- a/scripts/Deploy-MSBuild.ps1 +++ b/scripts/Deploy-MSBuild.ps1 @@ -48,7 +48,7 @@ Write-Host "Existing MSBuild assemblies backed up to $BackupFolder" if ($runtime -eq "Desktop") { $targetFramework = "net472" } else { - $targetFramework = "netcoreapp2.1" + $targetFramework = "net5.0" } $bootstrapBinDirectory = "artifacts\bin\MSBuild.Bootstrap\$configuration\$targetFramework" From cdefa6cce63a1e4863f664fe3d47a9e09c5c5641 Mon Sep 17 00:00:00 2001 From: Tom Deseyn Date: Tue, 26 Jan 2021 09:31:29 +0100 Subject: [PATCH 06/17] style nit --- .../Components/Communications/NodeProviderOutOfProcBase.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Build/BackEnd/Components/Communications/NodeProviderOutOfProcBase.cs b/src/Build/BackEnd/Components/Communications/NodeProviderOutOfProcBase.cs index 27b9e28457c..7c5292e6cde 100644 --- a/src/Build/BackEnd/Components/Communications/NodeProviderOutOfProcBase.cs +++ b/src/Build/BackEnd/Components/Communications/NodeProviderOutOfProcBase.cs @@ -108,7 +108,7 @@ protected void ShutdownConnectedNodes(List contextsToShutDown, bool !Console.IsInputRedirected && Traits.Instance.EscapeHatches.EnsureStdOutForChildNodesIsPrimaryStdout; - Task[] waitForExitTasks = waitForExit && contextsToShutDown.Count > 0? new Task[contextsToShutDown.Count] : null; + Task[] waitForExitTasks = waitForExit && contextsToShutDown.Count > 0 ? new Task[contextsToShutDown.Count] : null; int i = 0; foreach (NodeContext nodeContext in contextsToShutDown) { From f2c3945a94bbf485b0538af8fd47557f4ca3368c Mon Sep 17 00:00:00 2001 From: Tom Deseyn Date: Tue, 26 Jan 2021 10:53:10 +0100 Subject: [PATCH 07/17] Exec_Tests: update expected exit codes for using SIGKILL --- src/Tasks.UnitTests/Exec_Tests.cs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/Tasks.UnitTests/Exec_Tests.cs b/src/Tasks.UnitTests/Exec_Tests.cs index df8ac22edce..4d6a8089424 100644 --- a/src/Tasks.UnitTests/Exec_Tests.cs +++ b/src/Tasks.UnitTests/Exec_Tests.cs @@ -103,8 +103,8 @@ public void ExitCodeCausesFailure() [Fact] public void Timeout() { - // On non-Windows the exit code of a killed process is SIGTERM (143) - int expectedExitCode = NativeMethodsShared.IsWindows ? -1 : 143; + // On non-Windows the exit code of a killed process is SIGKILL (137) + int expectedExitCode = NativeMethodsShared.IsWindows ? -1 : 137; Exec exec = PrepareExec(NativeMethodsShared.IsWindows ? ":foo \n goto foo" : "while true; do sleep 1; done"); exec.Timeout = 5; @@ -141,13 +141,13 @@ public void TimeoutFailsEvenWhenExitCodeIsIgnored() // The standard check for SIGTERM fails intermittently on macOS Mono // https://github.com/dotnet/msbuild/issues/5506 // To avoid test flakiness, allow 259 even though I can't justify it. - exec.ExitCode.ShouldBeOneOf(143, 259); + exec.ExitCode.ShouldBeOneOf(137, 259); } else { - // On non-Windows the exit code of a killed process is generally 128 + SIGTERM = 143 + // On non-Windows the exit code of a killed process is generally 128 + SIGKILL = 137 // though this isn't 100% guaranteed, see https://unix.stackexchange.com/a/99134 - exec.ExitCode.ShouldBe(NativeMethodsShared.IsWindows ? -1 : 143); + exec.ExitCode.ShouldBe(NativeMethodsShared.IsWindows ? -1 : 137); } } From 83524bf1a2149b92cfc74ddb9aa41495d21bac0c Mon Sep 17 00:00:00 2001 From: Tom Deseyn Date: Tue, 26 Jan 2021 15:33:45 +0100 Subject: [PATCH 08/17] Fix Windows build --- .../Components/Communications/NodeProviderOutOfProcBase.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/Build/BackEnd/Components/Communications/NodeProviderOutOfProcBase.cs b/src/Build/BackEnd/Components/Communications/NodeProviderOutOfProcBase.cs index 7c5292e6cde..3f4d2a1e54f 100644 --- a/src/Build/BackEnd/Components/Communications/NodeProviderOutOfProcBase.cs +++ b/src/Build/BackEnd/Components/Communications/NodeProviderOutOfProcBase.cs @@ -27,6 +27,7 @@ using BackendNativeMethods = Microsoft.Build.BackEnd.NativeMethods; using Task = System.Threading.Tasks.Task; +using DotNetFrameworkArchitecture = Microsoft.Build.Shared.DotNetFrameworkArchitecture; namespace Microsoft.Build.BackEnd { From 2cc72534b97a59d3d561199834123e19cf4bb294 Mon Sep 17 00:00:00 2001 From: Tom Deseyn Date: Thu, 28 Jan 2021 09:02:50 +0100 Subject: [PATCH 09/17] PR feedback --- src/Build/BackEnd/BuildManager/BuildManager.cs | 6 +++--- src/Build/Microsoft.Build.csproj | 2 +- src/{Utilities => Shared}/ProcessExtensions.cs | 18 ++++++++++++++---- src/Utilities/Microsoft.Build.Utilities.csproj | 3 +++ 4 files changed, 21 insertions(+), 8 deletions(-) rename src/{Utilities => Shared}/ProcessExtensions.cs (90%) diff --git a/src/Build/BackEnd/BuildManager/BuildManager.cs b/src/Build/BackEnd/BuildManager/BuildManager.cs index 6327090ecca..9e94c5bd41d 100644 --- a/src/Build/BackEnd/BuildManager/BuildManager.cs +++ b/src/Build/BackEnd/BuildManager/BuildManager.cs @@ -629,7 +629,7 @@ void Callback(object state) } } - ShutdownConnectedNodesAsync(true /* abort */); + ShutdownConnectedNodes(true /* abort */); CheckForActiveNodesAndCleanUpSubmissions(); } } @@ -774,7 +774,7 @@ public void EndBuild() try { _noActiveSubmissionsEvent.WaitOne(); - ShutdownConnectedNodesAsync(false /* normal termination */); + ShutdownConnectedNodes(false /* normal termination */); _noNodesActiveEvent.WaitOne(); // Wait for all of the actions in the work queue to drain. Wait() could throw here if there was an unhandled exception @@ -1937,7 +1937,7 @@ public void Dispose() /// Asks the nodeManager to tell the currently connected nodes to shut down and sets a flag preventing all non-shutdown-related packets from /// being processed. /// - private void ShutdownConnectedNodesAsync(bool abort) + private void ShutdownConnectedNodes(bool abort) { _shuttingDown = true; diff --git a/src/Build/Microsoft.Build.csproj b/src/Build/Microsoft.Build.csproj index 416a83b69e8..1a851c22044 100644 --- a/src/Build/Microsoft.Build.csproj +++ b/src/Build/Microsoft.Build.csproj @@ -29,7 +29,6 @@ - @@ -129,6 +128,7 @@ + BackEnd\Components\RequestBuilder\IntrinsicTasks\TaskLoggingHelper.cs True diff --git a/src/Utilities/ProcessExtensions.cs b/src/Shared/ProcessExtensions.cs similarity index 90% rename from src/Utilities/ProcessExtensions.cs rename to src/Shared/ProcessExtensions.cs index 886b069eb4f..7f5c5d019c7 100644 --- a/src/Utilities/ProcessExtensions.cs +++ b/src/Shared/ProcessExtensions.cs @@ -7,9 +7,9 @@ using System.IO; using Microsoft.Build.Shared; -namespace Microsoft.Build.Utilities +namespace Microsoft.Build.Shared { - public static class ProcessExtensions + internal static class ProcessExtensions { public static void KillTree(this Process process, int timeout) { @@ -87,6 +87,11 @@ private static void KillProcessUnix(int processId) // Process already terminated. return; } + catch (InvalidOperationException) + { + // Process already terminated. + return; + } } private static int RunProcessAndWaitForExit(string fileName, string arguments, out string stdout) @@ -108,8 +113,13 @@ private static int RunProcessAndWaitForExit(string fileName, string arguments, o } else { - process.Kill(); - + try + { + process.Kill(); + } + catch (InvalidOperationException) + { } + // Kill is asynchronous so we should still wait a little // process.WaitForExit((int) TimeSpan.FromSeconds(1).TotalMilliseconds); diff --git a/src/Utilities/Microsoft.Build.Utilities.csproj b/src/Utilities/Microsoft.Build.Utilities.csproj index 2fdd06afdd6..f42d087cc9f 100644 --- a/src/Utilities/Microsoft.Build.Utilities.csproj +++ b/src/Utilities/Microsoft.Build.Utilities.csproj @@ -125,6 +125,9 @@ Shared\InprocTrackingNativeMethods.cs + + Shared\ProcessExtensions.cs + Shared\ReadOnlyEmptyCollection.cs From 8d4ecbc21967f40d1303b3731b92fe7b208ff42e Mon Sep 17 00:00:00 2001 From: Tom Deseyn Date: Tue, 2 Feb 2021 09:43:56 +0100 Subject: [PATCH 10/17] Remove timeouts on WaitForExit which can cause unexpected behavior. --- .../NodeProviderOutOfProcBase.cs | 2 +- src/Shared/NativeMethodsShared.cs | 4 +-- src/Shared/ProcessExtensions.cs | 33 ++++--------------- src/Utilities/ToolTask.cs | 12 +------ 4 files changed, 10 insertions(+), 41 deletions(-) diff --git a/src/Build/BackEnd/Components/Communications/NodeProviderOutOfProcBase.cs b/src/Build/BackEnd/Components/Communications/NodeProviderOutOfProcBase.cs index 3f4d2a1e54f..bf901e00015 100644 --- a/src/Build/BackEnd/Components/Communications/NodeProviderOutOfProcBase.cs +++ b/src/Build/BackEnd/Components/Communications/NodeProviderOutOfProcBase.cs @@ -860,7 +860,7 @@ public async Task WaitForExitAsync() // Kill the child and do a blocking wait. CommunicationsUtilities.Trace("Killing node with pid = {0}", _process.Id); - _process.KillTree(timeout: -1); + _process.KillTree(); } #if FEATURE_APM diff --git a/src/Shared/NativeMethodsShared.cs b/src/Shared/NativeMethodsShared.cs index 4818d7eda9c..42e8a3ead07 100644 --- a/src/Shared/NativeMethodsShared.cs +++ b/src/Shared/NativeMethodsShared.cs @@ -465,10 +465,8 @@ public SystemInformationData() string arch = null; if (proc != null) { - // Since uname -m simply returns kernel property, it should be quick. - // 1 second is the best guess for a safe timeout. - proc.WaitForExit(1000); arch = proc.StandardOutput.ReadLine(); + proc.WaitForExit(); } if (!string.IsNullOrEmpty(arch)) diff --git a/src/Shared/ProcessExtensions.cs b/src/Shared/ProcessExtensions.cs index 7f5c5d019c7..86b84a9a88e 100644 --- a/src/Shared/ProcessExtensions.cs +++ b/src/Shared/ProcessExtensions.cs @@ -11,7 +11,7 @@ namespace Microsoft.Build.Shared { internal static class ProcessExtensions { - public static void KillTree(this Process process, int timeout) + public static void KillTree(this Process process) { if (NativeMethodsShared.IsWindows) { @@ -41,17 +41,17 @@ public static void KillTree(this Process process, int timeout) // wait until the process finishes exiting/getting killed. // We don't want to wait forever here because the task is already supposed to be dieing, we just want to give it long enough // to try and flush what it can and stop. If it cannot do that in a reasonable time frame then we will just ignore it. - process.WaitForExit(timeout); + process.WaitForExit(); } private static void GetAllChildIdsUnix(int parentId, ISet children) { - var exitCode = RunProcessAndWaitForExit( + RunProcessAndWaitForExit( "pgrep", $"-P {parentId}", out string stdout); - if (exitCode == 0 && !string.IsNullOrEmpty(stdout)) + if (!string.IsNullOrEmpty(stdout)) { using (var reader = new StringReader(stdout)) { @@ -94,7 +94,7 @@ private static void KillProcessUnix(int processId) } } - private static int RunProcessAndWaitForExit(string fileName, string arguments, out string stdout) + private static void RunProcessAndWaitForExit(string fileName, string arguments, out string stdout) { var startInfo = new ProcessStartInfo { @@ -105,27 +105,8 @@ private static int RunProcessAndWaitForExit(string fileName, string arguments, o }; var process = Process.Start(startInfo); - - stdout = null; - if (process.WaitForExit((int) TimeSpan.FromSeconds(30).TotalMilliseconds)) - { - stdout = process.StandardOutput.ReadToEnd(); - } - else - { - try - { - process.Kill(); - } - catch (InvalidOperationException) - { } - - // Kill is asynchronous so we should still wait a little - // - process.WaitForExit((int) TimeSpan.FromSeconds(1).TotalMilliseconds); - } - - return process.HasExited ? process.ExitCode : -1; + stdout = process.StandardOutput.ReadToEnd(); + process.WaitForExit(); } } } diff --git a/src/Utilities/ToolTask.cs b/src/Utilities/ToolTask.cs index 23f7abc7e67..604f1633c77 100644 --- a/src/Utilities/ToolTask.cs +++ b/src/Utilities/ToolTask.cs @@ -940,17 +940,7 @@ private void KillToolProcessOnTimeout(Process proc, bool isBeingCancelled) LogShared.LogWarningWithCodeFromResources("Shared.KillingProcessByCancellation", processName); } - int timeout = 5000; - string timeoutFromEnvironment = Environment.GetEnvironmentVariable("MSBUILDTOOLTASKCANCELPROCESSWAITTIMEOUT"); - if (timeoutFromEnvironment != null) - { - if (int.TryParse(timeoutFromEnvironment, out int result) && result >= 0) - { - timeout = result; - } - } - - proc.KillTree(timeout); + proc.KillTree(); } } From f14d6a229889132a96f65d91b27fb54bed9f31e6 Mon Sep 17 00:00:00 2001 From: Tom Deseyn Date: Tue, 2 Feb 2021 10:36:30 +0100 Subject: [PATCH 11/17] TimeoutFailsEvenWhenExitCodeIsIgnored: on mono we expect 137 due to removing WaitForExit timeout --- src/Tasks.UnitTests/Exec_Tests.cs | 15 ++------------- 1 file changed, 2 insertions(+), 13 deletions(-) diff --git a/src/Tasks.UnitTests/Exec_Tests.cs b/src/Tasks.UnitTests/Exec_Tests.cs index 4d6a8089424..736f1aaf598 100644 --- a/src/Tasks.UnitTests/Exec_Tests.cs +++ b/src/Tasks.UnitTests/Exec_Tests.cs @@ -136,19 +136,8 @@ public void TimeoutFailsEvenWhenExitCodeIsIgnored() // ToolTask does not log an error on timeout. mockEngine.Errors.ShouldBe(0); - if (NativeMethodsShared.IsMono) - { - // The standard check for SIGTERM fails intermittently on macOS Mono - // https://github.com/dotnet/msbuild/issues/5506 - // To avoid test flakiness, allow 259 even though I can't justify it. - exec.ExitCode.ShouldBeOneOf(137, 259); - } - else - { - // On non-Windows the exit code of a killed process is generally 128 + SIGKILL = 137 - // though this isn't 100% guaranteed, see https://unix.stackexchange.com/a/99134 - exec.ExitCode.ShouldBe(NativeMethodsShared.IsWindows ? -1 : 137); - } + // On non-Windows the exit code of a killed process is 128 + SIGKILL = 137 + exec.ExitCode.ShouldBe(NativeMethodsShared.IsWindows ? -1 : 137); } [Fact] From 888068ab50dd37bb58f0bd23e5dfb59064bb7477 Mon Sep 17 00:00:00 2001 From: Tom Deseyn Date: Tue, 2 Feb 2021 10:36:51 +0100 Subject: [PATCH 12/17] Add KillTree test --- .../ProcessExtensions_Tests.cs | 30 +++++++++++++++++++ 1 file changed, 30 insertions(+) create mode 100644 src/Utilities.UnitTests/ProcessExtensions_Tests.cs diff --git a/src/Utilities.UnitTests/ProcessExtensions_Tests.cs b/src/Utilities.UnitTests/ProcessExtensions_Tests.cs new file mode 100644 index 00000000000..d9f1a1e9f47 --- /dev/null +++ b/src/Utilities.UnitTests/ProcessExtensions_Tests.cs @@ -0,0 +1,30 @@ +// Copyright (c) Microsoft. All rights reserved. +// Licensed under the MIT license. See LICENSE file in the project root for full license information. + +using Shouldly; +using Xunit; + +using Microsoft.Build.Shared; +using System.Diagnostics; +using System.Threading.Tasks; + +namespace Microsoft.Build.UnitTests +{ + public class ProcessExtensions_Tests + { + [Fact] + public async Task KillTree() + { + Process p = Process.Start("sleep", "600"); // sleep 10m. + + // Verify the process is running. + await Task.Delay(500); + p.HasExited.ShouldBe(false); + + // Kill the process. + p.KillTree(); + p.HasExited.ShouldBe(true); + p.ExitCode.ShouldNotBe(0); + } + } +} From 12d14e71e09e46559c5a80ee87faa9d6a25f433d Mon Sep 17 00:00:00 2001 From: Tom Deseyn Date: Fri, 5 Feb 2021 13:31:20 +0100 Subject: [PATCH 13/17] Log a warning when a node is killed --- .../Communications/NodeProviderOutOfProcBase.cs | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/src/Build/BackEnd/Components/Communications/NodeProviderOutOfProcBase.cs b/src/Build/BackEnd/Components/Communications/NodeProviderOutOfProcBase.cs index bf901e00015..50fe2a95302 100644 --- a/src/Build/BackEnd/Components/Communications/NodeProviderOutOfProcBase.cs +++ b/src/Build/BackEnd/Components/Communications/NodeProviderOutOfProcBase.cs @@ -28,6 +28,8 @@ using BackendNativeMethods = Microsoft.Build.BackEnd.NativeMethods; using Task = System.Threading.Tasks.Task; using DotNetFrameworkArchitecture = Microsoft.Build.Shared.DotNetFrameworkArchitecture; +using Microsoft.Build.Framework; +using Microsoft.Build.BackEnd.Logging; namespace Microsoft.Build.BackEnd { @@ -111,6 +113,7 @@ protected void ShutdownConnectedNodes(List contextsToShutDown, bool Task[] waitForExitTasks = waitForExit && contextsToShutDown.Count > 0 ? new Task[contextsToShutDown.Count] : null; int i = 0; + var loggingService = _componentHost.LoggingService; foreach (NodeContext nodeContext in contextsToShutDown) { if (nodeContext is null) @@ -120,7 +123,7 @@ protected void ShutdownConnectedNodes(List contextsToShutDown, bool nodeContext.SendData(new NodeBuildComplete(enableReuse)); if (waitForExit) { - waitForExitTasks[i++] = nodeContext.WaitForExitAsync(); + waitForExitTasks[i++] = nodeContext.WaitForExitAsync(loggingService); } } if (waitForExitTasks != null) @@ -832,7 +835,7 @@ private void Close() /// /// Waits for the child node process to exit. /// - public async Task WaitForExitAsync() + public async Task WaitForExitAsync(ILoggingService loggingService) { // Wait for the process to exit. if (_isExiting) @@ -859,7 +862,15 @@ public async Task WaitForExitAsync() } // Kill the child and do a blocking wait. + loggingService?.LogWarningFromText( + BuildEventContext.Invalid, + null, + null, + null, + BuildEventFileInfo.Empty, + $"Killing node with pid = {_process.Id}"); CommunicationsUtilities.Trace("Killing node with pid = {0}", _process.Id); + _process.KillTree(); } From 0ce5d234dfa29ca3ba03898f7ba03191c3c114d4 Mon Sep 17 00:00:00 2001 From: Tom Deseyn Date: Tue, 9 Feb 2021 18:03:04 +0100 Subject: [PATCH 14/17] Add back timeouts --- .../Communications/NodeProviderOutOfProcBase.cs | 2 +- src/Shared/ProcessExtensions.cs | 4 ++-- src/Utilities.UnitTests/ProcessExtensions_Tests.cs | 2 +- src/Utilities/ToolTask.cs | 11 ++++++++++- 4 files changed, 14 insertions(+), 5 deletions(-) diff --git a/src/Build/BackEnd/Components/Communications/NodeProviderOutOfProcBase.cs b/src/Build/BackEnd/Components/Communications/NodeProviderOutOfProcBase.cs index 50fe2a95302..8db5fafe25b 100644 --- a/src/Build/BackEnd/Components/Communications/NodeProviderOutOfProcBase.cs +++ b/src/Build/BackEnd/Components/Communications/NodeProviderOutOfProcBase.cs @@ -871,7 +871,7 @@ public async Task WaitForExitAsync(ILoggingService loggingService) $"Killing node with pid = {_process.Id}"); CommunicationsUtilities.Trace("Killing node with pid = {0}", _process.Id); - _process.KillTree(); + _process.KillTree(timeout: 5000); } #if FEATURE_APM diff --git a/src/Shared/ProcessExtensions.cs b/src/Shared/ProcessExtensions.cs index 86b84a9a88e..9504440d124 100644 --- a/src/Shared/ProcessExtensions.cs +++ b/src/Shared/ProcessExtensions.cs @@ -11,7 +11,7 @@ namespace Microsoft.Build.Shared { internal static class ProcessExtensions { - public static void KillTree(this Process process) + public static void KillTree(this Process process, int timeout) { if (NativeMethodsShared.IsWindows) { @@ -41,7 +41,7 @@ public static void KillTree(this Process process) // wait until the process finishes exiting/getting killed. // We don't want to wait forever here because the task is already supposed to be dieing, we just want to give it long enough // to try and flush what it can and stop. If it cannot do that in a reasonable time frame then we will just ignore it. - process.WaitForExit(); + process.WaitForExit(timeout); } private static void GetAllChildIdsUnix(int parentId, ISet children) diff --git a/src/Utilities.UnitTests/ProcessExtensions_Tests.cs b/src/Utilities.UnitTests/ProcessExtensions_Tests.cs index d9f1a1e9f47..e24dca74ec4 100644 --- a/src/Utilities.UnitTests/ProcessExtensions_Tests.cs +++ b/src/Utilities.UnitTests/ProcessExtensions_Tests.cs @@ -22,7 +22,7 @@ public async Task KillTree() p.HasExited.ShouldBe(false); // Kill the process. - p.KillTree(); + p.KillTree(timeout: 5000); p.HasExited.ShouldBe(true); p.ExitCode.ShouldNotBe(0); } diff --git a/src/Utilities/ToolTask.cs b/src/Utilities/ToolTask.cs index 604f1633c77..5ccd30763e2 100644 --- a/src/Utilities/ToolTask.cs +++ b/src/Utilities/ToolTask.cs @@ -940,7 +940,16 @@ private void KillToolProcessOnTimeout(Process proc, bool isBeingCancelled) LogShared.LogWarningWithCodeFromResources("Shared.KillingProcessByCancellation", processName); } - proc.KillTree(); + int timeout = 5000; + string timeoutFromEnvironment = Environment.GetEnvironmentVariable("MSBUILDTOOLTASKCANCELPROCESSWAITTIMEOUT"); + if (timeoutFromEnvironment != null) + { + if (int.TryParse(timeoutFromEnvironment, out int result) && result >= 0) + { + timeout = result; + } + } + proc.KillTree(timeout); } } From f863718d8ced59aea68795b84479e93a076a29ff Mon Sep 17 00:00:00 2001 From: Tom Deseyn Date: Thu, 11 Feb 2021 15:08:32 +0100 Subject: [PATCH 15/17] Update exit packet tracking --- .../NodeProviderOutOfProcBase.cs | 34 ++++++++++++++++--- 1 file changed, 29 insertions(+), 5 deletions(-) diff --git a/src/Build/BackEnd/Components/Communications/NodeProviderOutOfProcBase.cs b/src/Build/BackEnd/Components/Communications/NodeProviderOutOfProcBase.cs index 21c371f67c2..c1daef25827 100644 --- a/src/Build/BackEnd/Components/Communications/NodeProviderOutOfProcBase.cs +++ b/src/Build/BackEnd/Components/Communications/NodeProviderOutOfProcBase.cs @@ -612,6 +612,13 @@ private static string GetCurrentHost() /// internal class NodeContext { + enum ExitPacketState + { + None, + ExitPacketQueued, + ExitPacketSent + } + // The pipe(s) used to communicate with the node. private Stream _clientToServerStream; private Stream _serverToClientStream; @@ -666,9 +673,9 @@ internal class NodeContext private NodeContextTerminateDelegate _terminateDelegate; /// - /// Node was requested to terminate. + /// Tracks the state of the packet sent to terminate the node. /// - private bool _isExiting; + private ExitPacketState _exitPacketState; /// /// Per node read buffers @@ -777,6 +784,10 @@ public async Task RunPacketReadLoopAsync() /// The packet to send. public void SendData(INodePacket packet) { + if (IsExitPacket(packet)) + { + _exitPacketState = ExitPacketState.ExitPacketQueued; + } _packetWriteQueue.Add(packet); DrainPacketQueue(); } @@ -844,7 +855,10 @@ private void SendDataCore(INodePacket packet) int lengthToWrite = Math.Min(writeStreamLength - i, MaxPacketWriteSize); _serverToClientStream.Write(writeStreamBuffer, i, lengthToWrite); } - _isExiting = packet is NodeBuildComplete buildCompletePacket && !buildCompletePacket.PrepareForReuse; + if (IsExitPacket(packet)) + { + _exitPacketState = ExitPacketState.ExitPacketSent; + } } catch (IOException e) { @@ -857,6 +871,11 @@ private void SendDataCore(INodePacket packet) } } + private static bool IsExitPacket(INodePacket packet) + { + return packet is NodeBuildComplete buildCompletePacket && !buildCompletePacket.PrepareForReuse; + } + /// /// Avoid having a BinaryWriter just to write a 4-byte int /// @@ -886,8 +905,13 @@ private void Close() /// public async Task WaitForExitAsync(ILoggingService loggingService) { - // Wait for the process to exit. - if (_isExiting) + if (_exitPacketState == ExitPacketState.ExitPacketQueued) + { + // Wait up to 100ms until all remaining packets are sent. + // We don't need to wait long, just long enough for the Task to start running on the ThreadPool. + await Task.WhenAny(_packetWriteDrainTask, Task.Delay(100)); + } + if (_exitPacketState == ExitPacketState.ExitPacketSent) { CommunicationsUtilities.Trace("Waiting for node with pid = {0} to exit", _process.Id); From 1b0177ce40e30b9187e156b91c30b00f7160fefb Mon Sep 17 00:00:00 2001 From: Tom Deseyn Date: Thu, 11 Feb 2021 15:28:03 +0100 Subject: [PATCH 16/17] Use resource string for logging kill --- .../Components/Communications/NodeProviderOutOfProcBase.cs | 7 +++---- src/Build/Resources/Strings.resx | 3 +++ src/Build/Resources/xlf/Strings.cs.xlf | 5 +++++ src/Build/Resources/xlf/Strings.de.xlf | 5 +++++ src/Build/Resources/xlf/Strings.en.xlf | 5 +++++ src/Build/Resources/xlf/Strings.es.xlf | 5 +++++ src/Build/Resources/xlf/Strings.fr.xlf | 5 +++++ src/Build/Resources/xlf/Strings.it.xlf | 5 +++++ src/Build/Resources/xlf/Strings.ja.xlf | 5 +++++ src/Build/Resources/xlf/Strings.ko.xlf | 5 +++++ src/Build/Resources/xlf/Strings.pl.xlf | 5 +++++ src/Build/Resources/xlf/Strings.pt-BR.xlf | 5 +++++ src/Build/Resources/xlf/Strings.ru.xlf | 5 +++++ src/Build/Resources/xlf/Strings.tr.xlf | 5 +++++ src/Build/Resources/xlf/Strings.zh-Hans.xlf | 5 +++++ src/Build/Resources/xlf/Strings.zh-Hant.xlf | 5 +++++ 16 files changed, 76 insertions(+), 4 deletions(-) diff --git a/src/Build/BackEnd/Components/Communications/NodeProviderOutOfProcBase.cs b/src/Build/BackEnd/Components/Communications/NodeProviderOutOfProcBase.cs index c1daef25827..0889994b493 100644 --- a/src/Build/BackEnd/Components/Communications/NodeProviderOutOfProcBase.cs +++ b/src/Build/BackEnd/Components/Communications/NodeProviderOutOfProcBase.cs @@ -935,13 +935,12 @@ public async Task WaitForExitAsync(ILoggingService loggingService) } // Kill the child and do a blocking wait. - loggingService?.LogWarningFromText( + loggingService?.LogWarning( BuildEventContext.Invalid, null, - null, - null, BuildEventFileInfo.Empty, - $"Killing node with pid = {_process.Id}"); + "KillingProcessWithPid", + _process.Id); CommunicationsUtilities.Trace("Killing node with pid = {0}", _process.Id); _process.KillTree(timeout: 5000); diff --git a/src/Build/Resources/Strings.resx b/src/Build/Resources/Strings.resx index 5b737e6d5f0..9946e751fed 100644 --- a/src/Build/Resources/Strings.resx +++ b/src/Build/Resources/Strings.resx @@ -1876,4 +1876,7 @@ Utilization: {0} Average Utilization: {1:###.0} MSB4270: No project cache plugins found in assembly "{0}". Expected one. + + Killing process with pid = {0}. + diff --git a/src/Build/Resources/xlf/Strings.cs.xlf b/src/Build/Resources/xlf/Strings.cs.xlf index 157443afc16..2153f0e22a1 100644 --- a/src/Build/Resources/xlf/Strings.cs.xlf +++ b/src/Build/Resources/xlf/Strings.cs.xlf @@ -132,6 +132,11 @@ Objekty EvaluationContext vytvořené pomocí SharingPolicy.Isolated nepodporují předávání souborového systému MSBuildFileSystemBase. + + Killing process with pid = {0}. + Killing process with pid = {0}. + + "Loading the following project cache plugin: {0}" diff --git a/src/Build/Resources/xlf/Strings.de.xlf b/src/Build/Resources/xlf/Strings.de.xlf index 895d6839645..d864f0a20f2 100644 --- a/src/Build/Resources/xlf/Strings.de.xlf +++ b/src/Build/Resources/xlf/Strings.de.xlf @@ -132,6 +132,11 @@ "Die Übergabe eines MSBuildFileSystemBase-Dateisystems wird von EvaluationContext-Objekten, die mit SharingPolicy.Isolated erstellt wurden, nicht unterstützt." + + Killing process with pid = {0}. + Killing process with pid = {0}. + + "Loading the following project cache plugin: {0}" diff --git a/src/Build/Resources/xlf/Strings.en.xlf b/src/Build/Resources/xlf/Strings.en.xlf index 444ca3a5542..1254d414831 100644 --- a/src/Build/Resources/xlf/Strings.en.xlf +++ b/src/Build/Resources/xlf/Strings.en.xlf @@ -132,6 +132,11 @@ EvaluationContext objects created with SharingPolicy.Isolated do not support being passed an MSBuildFileSystemBase file system. + + Killing process with pid = {0}. + Killing process with pid = {0}. + + "Loading the following project cache plugin: {0}" diff --git a/src/Build/Resources/xlf/Strings.es.xlf b/src/Build/Resources/xlf/Strings.es.xlf index 36a2729aa8b..99441e2fb4f 100644 --- a/src/Build/Resources/xlf/Strings.es.xlf +++ b/src/Build/Resources/xlf/Strings.es.xlf @@ -132,6 +132,11 @@ "Los objetos EvaluationContext creados con SharingPolicy.Isolated no admiten que se les pase un sistema de archivos MSBuildFileSystemBase". + + Killing process with pid = {0}. + Killing process with pid = {0}. + + "Loading the following project cache plugin: {0}" diff --git a/src/Build/Resources/xlf/Strings.fr.xlf b/src/Build/Resources/xlf/Strings.fr.xlf index 26c0b149b57..a19de6cd10b 100644 --- a/src/Build/Resources/xlf/Strings.fr.xlf +++ b/src/Build/Resources/xlf/Strings.fr.xlf @@ -132,6 +132,11 @@ "Les objets EvaluationContext créés avec SharingPolicy.Isolated ne prennent pas en charge le passage d'un système de fichiers MSBuildFileSystemBase." + + Killing process with pid = {0}. + Killing process with pid = {0}. + + "Loading the following project cache plugin: {0}" diff --git a/src/Build/Resources/xlf/Strings.it.xlf b/src/Build/Resources/xlf/Strings.it.xlf index 86a00974a5f..eddf911a5ef 100644 --- a/src/Build/Resources/xlf/Strings.it.xlf +++ b/src/Build/Resources/xlf/Strings.it.xlf @@ -132,6 +132,11 @@ "Agli oggetti EvaluationContext creati con SharingPolicy.Isolated non è possibile passare un file system MSBuildFileSystemBase." + + Killing process with pid = {0}. + Killing process with pid = {0}. + + "Loading the following project cache plugin: {0}" diff --git a/src/Build/Resources/xlf/Strings.ja.xlf b/src/Build/Resources/xlf/Strings.ja.xlf index 27c3d3f6130..ee8252b9dcd 100644 --- a/src/Build/Resources/xlf/Strings.ja.xlf +++ b/src/Build/Resources/xlf/Strings.ja.xlf @@ -132,6 +132,11 @@ "SharingPolicy.Isolated を指定して作成された EvaluationContext オブジェクトに MSBuildFileSystemBase ファイル システムを渡すことはサポートされていません。" + + Killing process with pid = {0}. + Killing process with pid = {0}. + + "Loading the following project cache plugin: {0}" diff --git a/src/Build/Resources/xlf/Strings.ko.xlf b/src/Build/Resources/xlf/Strings.ko.xlf index 367cf54895e..14e55b6f38f 100644 --- a/src/Build/Resources/xlf/Strings.ko.xlf +++ b/src/Build/Resources/xlf/Strings.ko.xlf @@ -132,6 +132,11 @@ "SharingPolicy.Isolated로 만든 EvaluationContext 개체는 MSBuildFileSystemBase 파일 시스템 전달을 지원하지 않습니다." + + Killing process with pid = {0}. + Killing process with pid = {0}. + + "Loading the following project cache plugin: {0}" diff --git a/src/Build/Resources/xlf/Strings.pl.xlf b/src/Build/Resources/xlf/Strings.pl.xlf index b13eb7e424f..4b42a435adb 100644 --- a/src/Build/Resources/xlf/Strings.pl.xlf +++ b/src/Build/Resources/xlf/Strings.pl.xlf @@ -132,6 +132,11 @@ „Obiekty EvaluationContext utworzone za pomocą elementu SharingPolicy.Isolated nie obsługują przekazywania za pomocą systemu plików MSBuildFileSystemBase.” + + Killing process with pid = {0}. + Killing process with pid = {0}. + + "Loading the following project cache plugin: {0}" diff --git a/src/Build/Resources/xlf/Strings.pt-BR.xlf b/src/Build/Resources/xlf/Strings.pt-BR.xlf index 188df5f9cf3..edf8687b3cf 100644 --- a/src/Build/Resources/xlf/Strings.pt-BR.xlf +++ b/src/Build/Resources/xlf/Strings.pt-BR.xlf @@ -132,6 +132,11 @@ "Os objetos EvaluationContext criados com SharingPolicy.Isolable não são compatíveis com o recebimento de um sistema de arquivos MSBuildFileSystemBase." + + Killing process with pid = {0}. + Killing process with pid = {0}. + + "Loading the following project cache plugin: {0}" diff --git a/src/Build/Resources/xlf/Strings.ru.xlf b/src/Build/Resources/xlf/Strings.ru.xlf index 508d9a08b28..5e51ed494ea 100644 --- a/src/Build/Resources/xlf/Strings.ru.xlf +++ b/src/Build/Resources/xlf/Strings.ru.xlf @@ -132,6 +132,11 @@ "Объекты EvaluationContext, созданные с помощью SharingPolicy.Isolated, не поддерживают передачу в файловую систему MSBuildFileSystemBase." + + Killing process with pid = {0}. + Killing process with pid = {0}. + + "Loading the following project cache plugin: {0}" diff --git a/src/Build/Resources/xlf/Strings.tr.xlf b/src/Build/Resources/xlf/Strings.tr.xlf index 9bcd9bde59b..9e3d3c7c50c 100644 --- a/src/Build/Resources/xlf/Strings.tr.xlf +++ b/src/Build/Resources/xlf/Strings.tr.xlf @@ -132,6 +132,11 @@ "SharingPolicy.Isolated ile oluşturulan EvaluationContext nesneleri bir MSBuildFileSystemBase dosya sisteminin geçirilmesini desteklemez." + + Killing process with pid = {0}. + Killing process with pid = {0}. + + "Loading the following project cache plugin: {0}" diff --git a/src/Build/Resources/xlf/Strings.zh-Hans.xlf b/src/Build/Resources/xlf/Strings.zh-Hans.xlf index 942ffe8d692..9a69ab3846e 100644 --- a/src/Build/Resources/xlf/Strings.zh-Hans.xlf +++ b/src/Build/Resources/xlf/Strings.zh-Hans.xlf @@ -132,6 +132,11 @@ “使用 SharingPolicy.Isolated 创建的 EvaluationContext 对象不支持通过 MSBuildFileSystemBase 文件系统传递。” + + Killing process with pid = {0}. + Killing process with pid = {0}. + + "Loading the following project cache plugin: {0}" diff --git a/src/Build/Resources/xlf/Strings.zh-Hant.xlf b/src/Build/Resources/xlf/Strings.zh-Hant.xlf index eadd2f2e6cc..e6a19c0a9c3 100644 --- a/src/Build/Resources/xlf/Strings.zh-Hant.xlf +++ b/src/Build/Resources/xlf/Strings.zh-Hant.xlf @@ -132,6 +132,11 @@ "使用 SharingPolicy.Isolated 建立的 EvaluationContext 物件不支援以 MSBuildFileSystemBase 檔案系統傳遞。" + + Killing process with pid = {0}. + Killing process with pid = {0}. + + "Loading the following project cache plugin: {0}" From 6f67a094c58c5c3990692740fe557cd57ba605ff Mon Sep 17 00:00:00 2001 From: Tom Deseyn Date: Thu, 11 Feb 2021 16:43:47 +0100 Subject: [PATCH 17/17] TimeoutFailsEvenWhenExitCodeIsIgnored: add back STILL_ACTIVE --- src/Tasks.UnitTests/Exec_Tests.cs | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/src/Tasks.UnitTests/Exec_Tests.cs b/src/Tasks.UnitTests/Exec_Tests.cs index 736f1aaf598..3bc0657cab2 100644 --- a/src/Tasks.UnitTests/Exec_Tests.cs +++ b/src/Tasks.UnitTests/Exec_Tests.cs @@ -122,7 +122,6 @@ public void Timeout() [Fact] public void TimeoutFailsEvenWhenExitCodeIsIgnored() { - Exec exec = PrepareExec(NativeMethodsShared.IsWindows ? ":foo \n goto foo" : "while true; do sleep 1; done"); exec.Timeout = 5; exec.IgnoreExitCode = true; @@ -136,8 +135,16 @@ public void TimeoutFailsEvenWhenExitCodeIsIgnored() // ToolTask does not log an error on timeout. mockEngine.Errors.ShouldBe(0); - // On non-Windows the exit code of a killed process is 128 + SIGKILL = 137 - exec.ExitCode.ShouldBe(NativeMethodsShared.IsWindows ? -1 : 137); + if (NativeMethodsShared.IsMono) + { + const int STILL_ACTIVE = 259; // When Process.WaitForExit times out. + exec.ExitCode.ShouldBeOneOf(137, STILL_ACTIVE); + } + else + { + // On non-Windows the exit code of a killed process is 128 + SIGKILL = 137 + exec.ExitCode.ShouldBe(NativeMethodsShared.IsWindows ? -1 : 137); + } } [Fact]