From bb5db45c4b15f642d24292b56f93f34c41c21644 Mon Sep 17 00:00:00 2001 From: Jonathan Hart Date: Thu, 16 Nov 2017 00:01:55 +0000 Subject: [PATCH 1/2] Start-Process: add parameter 'Timeout' [feature] If the timeout is reached and the process hasn't finished, the process is stopped. --- .../commands/management/Process.cs | 167 +++++++++++++++++- .../resources/ProcessResources.resx | 9 + .../Start-Process.Tests.ps1 | 46 +++++ 3 files changed, 215 insertions(+), 7 deletions(-) diff --git a/src/Microsoft.PowerShell.Commands.Management/commands/management/Process.cs b/src/Microsoft.PowerShell.Commands.Management/commands/management/Process.cs index 30ef0e096ac..9de7612efae 100644 --- a/src/Microsoft.PowerShell.Commands.Management/commands/management/Process.cs +++ b/src/Microsoft.PowerShell.Commands.Management/commands/management/Process.cs @@ -4,6 +4,7 @@ using System; using System.Text; +using System.Text.RegularExpressions; using System.Collections; using System.Collections.Generic; using System.Collections.Specialized; @@ -1779,6 +1780,28 @@ public ProcessWindowStyle WindowStyle [Parameter] public SwitchParameter Wait { get; set; } + /// + /// If specified, wait for this number of seconds + /// + [Parameter] + [Alias("TimeoutSec")] + [ValidateNotNullOrEmpty] + [ValidateRange(1, 32767)] + public int Timeout + { + get + { + return _timeout; + } + set + { + _timeout = value * 1000; + _timeoutSpecified = true; + } + } + private int _timeout = System.Threading.Timeout.Infinite; + private bool _timeoutSpecified; + /// /// Default Environment /// @@ -2013,14 +2036,17 @@ protected override void BeginProcessing() } } - if (Wait.IsPresent) + if (Wait.IsPresent || _timeoutSpecified) { if (process != null) { if (!process.HasExited) { #if UNIX - process.WaitForExit(); + if (!process.WaitForExit(_timeout)) + { + StopProcessOnTimeout(process); + } #else _waithandle = new ManualResetEvent(false); @@ -2028,15 +2054,22 @@ protected override void BeginProcessing() ProcessCollection jobObject = new ProcessCollection(); if (jobObject.AssignProcessToJobObject(process)) { - // Wait for the job object to finish - jobObject.WaitOne(_waithandle); + // Wait for the job object to finish, or kill it if a timeout occurs + jobObject.WaitOne(_waithandle, _timeout); + if (!process.HasExited) + { + StopProcessOnTimeout(process); + } } else if (!process.HasExited) { // WinBlue: 27537 Start-Process -Wait doesn't work in a remote session on Windows 7 or lower. process.Exited += new EventHandler(myProcess_Exited); process.EnableRaisingEvents = true; - process.WaitForExit(); + if (!process.WaitForExit(_timeout)) + { + StopProcessOnTimeout(process); + } } #endif } @@ -2123,6 +2156,123 @@ private void LoadEnvironmentVariable(ProcessStartInfo startinfo, IDictionary Env } } + /// + /// Attempt to stop the process when the timeout has expired. + /// is used to stop the process + /// + /// + /// The process that should be stopped + /// + private void StopProcessOnTimeout(Process process) + { + string message = StringUtil.Format(ProcessResources.StartProcessTimeoutExceeded, process.ProcessName); + ErrorRecord er = new ErrorRecord(new TimeoutException(message), "StartProcessTimeoutExceeded", ErrorCategory.OperationTimeout, process); + + StopProcessCommand stop = new StopProcessCommand(); + stop.Id = GetProcessTreeIds(process); + foreach (Process p in stop.Invoke()) { } + + ThrowTerminatingError(er); + } + + /// + /// Gets IDs of descendant processes, started by a process + /// On Windows, this reads output from WMI commands + /// On UNIX, this reads output from `ps axo pid,ppid` + /// + /// + /// The parent process to use to resolve the process tree IDs + /// + /// + /// IDs of the parent process and all its descendants + /// + private int[] GetProcessTreeIds(Process parentProcess) + { + List stopProcessIds = new List {parentProcess.Id}; + string processRelationships = ""; +#if UNIX + try + { + Process ps = new Process(); + ps.StartInfo.FileName = "ps"; + ps.StartInfo.Arguments = "axo pid,ppid"; + ps.StartInfo.UseShellExecute = false; + ps.StartInfo.RedirectStandardOutput = true; + ps.Start(); + + processRelationships = ps.StandardOutput.ReadToEnd(); + + ps.WaitForExit(); + ps.Close(); + } + catch (Win32Exception) + { + WriteWarning( + StringUtil.Format(ProcessResources.CouldNotResolveProcessTree, parentProcess.ProcessName) + + " " + + ProcessResources.DescendantProcessesPossiblyRunning + ); + return stopProcessIds.ToArray(); + } +#else + string searchQuery = "Select ProcessID, ParentProcessID From Win32_Process"; + try + { + using (CimSession cimSession = CimSession.Create(null)) + { + IEnumerable processCollection = + cimSession.QueryInstances("root/cimv2", "WQL", searchQuery); + StringBuilder sb = new StringBuilder(); + foreach (CimInstance processInstance in processCollection) + { + sb.Append(processInstance.CimInstanceProperties["ProcessID"].Value.ToString()); + sb.Append(' '); + sb.Append(processInstance.CimInstanceProperties["ParentProcessID"].Value.ToString()); + sb.Append(Environment.NewLine); + } + processRelationships = sb.ToString(); + } + } + catch (CimException) + { + WriteWarning( + StringUtil.Format(ProcessResources.CouldNotResolveProcessTree, parentProcess.ProcessName) + + " " + + ProcessResources.DescendantProcessesPossiblyRunning + ); + return stopProcessIds.ToArray(); + } +#endif + + // processList - key: process ID, value: parent process ID + Dictionary processList = new Dictionary(); + int pid = 0; + int ppid = 0; + string relationshipPattern = @"\s?([0-9]+)\s+([0-9]+)"; + foreach (Match psLine in Regex.Matches(processRelationships, relationshipPattern)) + { + pid = int.Parse(psLine.Groups[1].Value); + ppid = int.Parse(psLine.Groups[2].Value); + processList.Add(pid, ppid); + } + + int position = 0; + do + { + foreach (KeyValuePair process in processList) + { + if (process.Value == stopProcessIds[position]) + { + stopProcessIds.Add(process.Key); + } + } + + position++; + } while (position <= (stopProcessIds.Count - 1)); + + return stopProcessIds.ToArray(); + } + private Process Start(ProcessStartInfo startInfo) { #if UNIX @@ -2605,12 +2755,15 @@ internal void CheckJobStatus(Object stateInfo) /// /// WaitHandle to use for waiting on the job object. /// - internal void WaitOne(ManualResetEvent waitHandleToUse) + /// + /// Wait for this number of milliseconds before a time-out occurs + /// + internal void WaitOne(ManualResetEvent waitHandleToUse, Int32 timeout = Timeout.Infinite) { TimerCallback jobObjectStatusCb = this.CheckJobStatus; using (Timer stateTimer = new Timer(jobObjectStatusCb, waitHandleToUse, 0, 1000)) { - waitHandleToUse.WaitOne(); + waitHandleToUse.WaitOne(timeout); } } } diff --git a/src/Microsoft.PowerShell.Commands.Management/resources/ProcessResources.resx b/src/Microsoft.PowerShell.Commands.Management/resources/ProcessResources.resx index e460ad2f016..4ae20abd339 100644 --- a/src/Microsoft.PowerShell.Commands.Management/resources/ProcessResources.resx +++ b/src/Microsoft.PowerShell.Commands.Management/resources/ProcessResources.resx @@ -129,6 +129,12 @@ Cannot stop process "{0} ({1})" because of the following error: {2} + + The process tree cannot be resolved for "{0}". + + + Descendant processes may still be running. + {0} ({1}) @@ -171,6 +177,9 @@ This command stopped operation because process "{0} ({1})" is not stopped in the specified time-out. + + This command stopped operation because process "{0}" did not complete within the specified time-out. + This command cannot be run due to the error: {0}. diff --git a/test/powershell/Modules/Microsoft.PowerShell.Management/Start-Process.Tests.ps1 b/test/powershell/Modules/Microsoft.PowerShell.Management/Start-Process.Tests.ps1 index ba5f3583923..e49fe1dd496 100644 --- a/test/powershell/Modules/Microsoft.PowerShell.Management/Start-Process.Tests.ps1 +++ b/test/powershell/Modules/Microsoft.PowerShell.Management/Start-Process.Tests.ps1 @@ -116,6 +116,52 @@ Describe "Start-Process" -Tags @("Feature") { } } +Describe "Start-Process -Timeout" -Tags "Feature","Slow" { + + BeforeAll { + if ($IsWindows) { + $pingParam = "-n 30 localhost" + } + elseif ($IsLinux -Or $IsMacOS) { + $pingParam = "-c 30 localhost" + } + + # Find where test/powershell is so we can find the testexe command relative to it + $powershellTestDir = $PSScriptRoot + while ($powershellTestDir -notmatch 'test[\\/]powershell$') { + $powershellTestDir = Split-Path $powershellTestDir + } + $testexe = Join-Path (Split-Path $powershellTestDir) tools/TestExe/bin/testexe + } + + It "Should work correctly if process completes before specified time-out" { + Start-Process ping -ArgumentList $pingParam -Timeout 40 -RedirectStandardOutput "$TESTDRIVE/output" | Should Be $null + } + + It "Should give an error when the specified time-out is exceeded" { + { Start-Process ping -ArgumentList $pingParam -Timeout 20 -RedirectStandardOutput "$TESTDRIVE/output" } | ShouldBeErrorId "StartProcessTimeoutExceeded,Microsoft.PowerShell.Commands.StartProcessCommand" + } + + It "Should use time-out value when both -Timeout and -Wait are passed" { + { Start-Process ping -ArgumentList $pingParam -Timeout 20 -Wait -RedirectStandardOutput "$TESTDRIVE/output" } | ShouldBeErrorId "StartProcessTimeoutExceeded,Microsoft.PowerShell.Commands.StartProcessCommand" + } + + # This is based on the test "Should kill native process tree" in + # test\powershell\Language\Scripting\NativeExecution\NativeCommandProcessor.Tests.ps1 + It "Should stop any descendant processes when the specified time-out is exceeded" { + Get-Process testexe -ErrorAction SilentlyContinue | Stop-Process + + { Start-Process $testexe -ArgumentList "-createchildprocess 6" -Timeout 10 -RedirectStandardOutput "$TESTDRIVE/output" } | ShouldBeErrorId "StartProcessTimeoutExceeded,Microsoft.PowerShell.Commands.StartProcessCommand" + + # Waiting for a second, as the $testexe processes may still be exiting + # and the Get-Process cmdlet will count them accidentally + Start-Sleep 1 + + $childprocesses = Get-Process testexe -ErrorAction SilentlyContinue + $childprocesses.count | Should Be 0 + } +} + Describe "Start-Process tests requiring admin" -Tags "Feature","RequireAdminOnWindows" { BeforeEach { From a0cc045dab15fb43ed1712933d8a734667c199cb Mon Sep 17 00:00:00 2001 From: Jonathan Hart Date: Fri, 17 Nov 2017 23:32:23 +0000 Subject: [PATCH 2/2] Changed `Timeout` parameter to `ExitTimeout` [feature] Based on PR feedback, also: * Skipped searching for `testexe`, as `Start-PSPester` puts it into the path * Set the upper limit for `ExitTimeout` to be a positive range, rather than 32767 * Used autogenerated property for `ExitTimeout` * Set a timeout and checked the exit code for the `ps axo` process * Used common coding convention for comments --- .../commands/management/Process.cs | 54 ++++++++----------- .../resources/ProcessResources.resx | 4 +- .../Start-Process.Tests.ps1 | 23 +++----- 3 files changed, 32 insertions(+), 49 deletions(-) diff --git a/src/Microsoft.PowerShell.Commands.Management/commands/management/Process.cs b/src/Microsoft.PowerShell.Commands.Management/commands/management/Process.cs index 9de7612efae..5c949445e3d 100644 --- a/src/Microsoft.PowerShell.Commands.Management/commands/management/Process.cs +++ b/src/Microsoft.PowerShell.Commands.Management/commands/management/Process.cs @@ -1781,26 +1781,12 @@ public ProcessWindowStyle WindowStyle public SwitchParameter Wait { get; set; } /// - /// If specified, wait for this number of seconds + /// If specified, wait for this number of milliseconds for the process to exit. /// [Parameter] - [Alias("TimeoutSec")] [ValidateNotNullOrEmpty] - [ValidateRange(1, 32767)] - public int Timeout - { - get - { - return _timeout; - } - set - { - _timeout = value * 1000; - _timeoutSpecified = true; - } - } - private int _timeout = System.Threading.Timeout.Infinite; - private bool _timeoutSpecified; + [ValidateRange(ValidateRangeKind.Positive)] + public int ExitTimeout { get; set; } = Timeout.Infinite; /// /// Default Environment @@ -2036,14 +2022,14 @@ protected override void BeginProcessing() } } - if (Wait.IsPresent || _timeoutSpecified) + if (Wait.IsPresent || ExitTimeout != Timeout.Infinite) { if (process != null) { if (!process.HasExited) { #if UNIX - if (!process.WaitForExit(_timeout)) + if (!process.WaitForExit(ExitTimeout)) { StopProcessOnTimeout(process); } @@ -2055,7 +2041,7 @@ protected override void BeginProcessing() if (jobObject.AssignProcessToJobObject(process)) { // Wait for the job object to finish, or kill it if a timeout occurs - jobObject.WaitOne(_waithandle, _timeout); + jobObject.WaitOne(_waithandle, ExitTimeout); if (!process.HasExited) { StopProcessOnTimeout(process); @@ -2066,7 +2052,7 @@ protected override void BeginProcessing() // WinBlue: 27537 Start-Process -Wait doesn't work in a remote session on Windows 7 or lower. process.Exited += new EventHandler(myProcess_Exited); process.EnableRaisingEvents = true; - if (!process.WaitForExit(_timeout)) + if (!process.WaitForExit(ExitTimeout)) { StopProcessOnTimeout(process); } @@ -2165,8 +2151,8 @@ private void LoadEnvironmentVariable(ProcessStartInfo startinfo, IDictionary Env /// private void StopProcessOnTimeout(Process process) { - string message = StringUtil.Format(ProcessResources.StartProcessTimeoutExceeded, process.ProcessName); - ErrorRecord er = new ErrorRecord(new TimeoutException(message), "StartProcessTimeoutExceeded", ErrorCategory.OperationTimeout, process); + string message = StringUtil.Format(ProcessResources.StartProcessExitTimeoutExceeded, process.ProcessName); + ErrorRecord er = new ErrorRecord(new TimeoutException(message), "StartProcessExitTimeoutExceeded", ErrorCategory.OperationTimeout, process); StopProcessCommand stop = new StopProcessCommand(); stop.Id = GetProcessTreeIds(process); @@ -2190,6 +2176,7 @@ private int[] GetProcessTreeIds(Process parentProcess) { List stopProcessIds = new List {parentProcess.Id}; string processRelationships = ""; + bool processesCollected = true; #if UNIX try { @@ -2202,17 +2189,15 @@ private int[] GetProcessTreeIds(Process parentProcess) processRelationships = ps.StandardOutput.ReadToEnd(); - ps.WaitForExit(); + if (!ps.WaitForExit(4000) || ps.ExitCode != 0) + { + processesCollected = false; + } ps.Close(); } catch (Win32Exception) { - WriteWarning( - StringUtil.Format(ProcessResources.CouldNotResolveProcessTree, parentProcess.ProcessName) - + " " - + ProcessResources.DescendantProcessesPossiblyRunning - ); - return stopProcessIds.ToArray(); + processesCollected = false; } #else string searchQuery = "Select ProcessID, ParentProcessID From Win32_Process"; @@ -2234,6 +2219,12 @@ private int[] GetProcessTreeIds(Process parentProcess) } } catch (CimException) + { + processesCollected = false; + } +#endif + + if (!processesCollected) { WriteWarning( StringUtil.Format(ProcessResources.CouldNotResolveProcessTree, parentProcess.ProcessName) @@ -2242,7 +2233,6 @@ private int[] GetProcessTreeIds(Process parentProcess) ); return stopProcessIds.ToArray(); } -#endif // processList - key: process ID, value: parent process ID Dictionary processList = new Dictionary(); @@ -2756,7 +2746,7 @@ internal void CheckJobStatus(Object stateInfo) /// WaitHandle to use for waiting on the job object. /// /// - /// Wait for this number of milliseconds before a time-out occurs + /// Wait for this number of milliseconds before a time-out occurs. /// internal void WaitOne(ManualResetEvent waitHandleToUse, Int32 timeout = Timeout.Infinite) { diff --git a/src/Microsoft.PowerShell.Commands.Management/resources/ProcessResources.resx b/src/Microsoft.PowerShell.Commands.Management/resources/ProcessResources.resx index 4ae20abd339..d3083863eb5 100644 --- a/src/Microsoft.PowerShell.Commands.Management/resources/ProcessResources.resx +++ b/src/Microsoft.PowerShell.Commands.Management/resources/ProcessResources.resx @@ -177,8 +177,8 @@ This command stopped operation because process "{0} ({1})" is not stopped in the specified time-out. - - This command stopped operation because process "{0}" did not complete within the specified time-out. + + This command stopped operation because process "{0}" did not exit within the specified time-out. This command cannot be run due to the error: {0}. diff --git a/test/powershell/Modules/Microsoft.PowerShell.Management/Start-Process.Tests.ps1 b/test/powershell/Modules/Microsoft.PowerShell.Management/Start-Process.Tests.ps1 index e49fe1dd496..bc2714a7df3 100644 --- a/test/powershell/Modules/Microsoft.PowerShell.Management/Start-Process.Tests.ps1 +++ b/test/powershell/Modules/Microsoft.PowerShell.Management/Start-Process.Tests.ps1 @@ -125,33 +125,26 @@ Describe "Start-Process -Timeout" -Tags "Feature","Slow" { elseif ($IsLinux -Or $IsMacOS) { $pingParam = "-c 30 localhost" } - - # Find where test/powershell is so we can find the testexe command relative to it - $powershellTestDir = $PSScriptRoot - while ($powershellTestDir -notmatch 'test[\\/]powershell$') { - $powershellTestDir = Split-Path $powershellTestDir - } - $testexe = Join-Path (Split-Path $powershellTestDir) tools/TestExe/bin/testexe } - It "Should work correctly if process completes before specified time-out" { - Start-Process ping -ArgumentList $pingParam -Timeout 40 -RedirectStandardOutput "$TESTDRIVE/output" | Should Be $null + It "Should work correctly if process completes before specified exit time-out" { + Start-Process ping -ArgumentList $pingParam -ExitTimeout 40000 -RedirectStandardOutput "$TESTDRIVE/output" | Should Be $null } - It "Should give an error when the specified time-out is exceeded" { - { Start-Process ping -ArgumentList $pingParam -Timeout 20 -RedirectStandardOutput "$TESTDRIVE/output" } | ShouldBeErrorId "StartProcessTimeoutExceeded,Microsoft.PowerShell.Commands.StartProcessCommand" + It "Should give an error when the specified exit time-out is exceeded" { + { Start-Process ping -ArgumentList $pingParam -ExitTimeout 20000 -RedirectStandardOutput "$TESTDRIVE/output" } | ShouldBeErrorId "StartProcessExitTimeoutExceeded,Microsoft.PowerShell.Commands.StartProcessCommand" } - It "Should use time-out value when both -Timeout and -Wait are passed" { - { Start-Process ping -ArgumentList $pingParam -Timeout 20 -Wait -RedirectStandardOutput "$TESTDRIVE/output" } | ShouldBeErrorId "StartProcessTimeoutExceeded,Microsoft.PowerShell.Commands.StartProcessCommand" + It "Should use exit time-out value when both -ExitTimeout and -Wait are passed" { + { Start-Process ping -ArgumentList $pingParam -ExitTimeout 20000 -Wait -RedirectStandardOutput "$TESTDRIVE/output" } | ShouldBeErrorId "StartProcessExitTimeoutExceeded,Microsoft.PowerShell.Commands.StartProcessCommand" } # This is based on the test "Should kill native process tree" in # test\powershell\Language\Scripting\NativeExecution\NativeCommandProcessor.Tests.ps1 - It "Should stop any descendant processes when the specified time-out is exceeded" { + It "Should stop any descendant processes when the specified exit time-out is exceeded" { Get-Process testexe -ErrorAction SilentlyContinue | Stop-Process - { Start-Process $testexe -ArgumentList "-createchildprocess 6" -Timeout 10 -RedirectStandardOutput "$TESTDRIVE/output" } | ShouldBeErrorId "StartProcessTimeoutExceeded,Microsoft.PowerShell.Commands.StartProcessCommand" + { Start-Process testexe -ArgumentList "-createchildprocess 6" -ExitTimeout 10000 -RedirectStandardOutput "$TESTDRIVE/output" } | ShouldBeErrorId "StartProcessExitTimeoutExceeded,Microsoft.PowerShell.Commands.StartProcessCommand" # Waiting for a second, as the $testexe processes may still be exiting # and the Get-Process cmdlet will count them accidentally