Skip to content

Start-Process: add parameter -ExitTimeout #5471

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
stuajnht wants to merge 2 commits into from
Closed

Start-Process: add parameter -ExitTimeout #5471

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
bb5db45
Start-Process: add parameter 'Timeout'
stuajnht Nov 16, 2017
a0cc045
Changed `Timeout` parameter to `ExitTimeout`
stuajnht Nov 17, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
157 changes: 150 additions & 7 deletions src/Microsoft.PowerShell.Commands.Management/commands/management/Process.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

using System;
using System.Text;
using System.Text.RegularExpressions;
using System.Collections;
using System.Collections.Generic;
using System.Collections.Specialized;
Expand Down Expand Up @@ -1779,6 +1780,14 @@ public ProcessWindowStyle WindowStyle
[Parameter]
public SwitchParameter Wait { get; set; }

/// <summary>
/// If specified, wait for this number of milliseconds for the process to exit.
/// </summary>
[Parameter]
[ValidateNotNullOrEmpty]
[ValidateRange(ValidateRangeKind.Positive)]
public int ExitTimeout { get; set; } = Timeout.Infinite;
Copy link
Member

@TravisEz13 TravisEz13 Feb 8, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

needs to be updated per committee review


/// <summary>
/// Default Environment
/// </summary>
Expand Down Expand Up @@ -2013,30 +2022,40 @@ protected override void BeginProcessing()
}
}

if (Wait.IsPresent)
if (Wait.IsPresent || ExitTimeout != Timeout.Infinite)
{
if (process != null)
{
if (!process.HasExited)
{
#if UNIX
Copy link
Collaborator

@iSazonov iSazonov Nov 16, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should remove two code paths and use public bool WaitForExit(int milliseconds); for all plaforms.

Copy link
Author

@stuajnht stuajnht Nov 17, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm sorry, I don't quite understand what you are asking here. Are you saying to remove the ProcessCollection / jobObject and just have a WaitForExit for all platforms? e.g.

if (!process.WaitForExit(ExitTimeout))
{
    StopProcessOnTimeout(process);
}

From what I can see, the Windows specific code keeps track of any child processes that are created, which doesn't happen for UNIX. I am unsure of the reasoning behind this, as the earliest commit on GitHub for these lines does not contain any helpful information.

process.WaitForExit();
if (!process.WaitForExit(ExitTimeout))
{
StopProcessOnTimeout(process);
}
#else
_waithandle = new ManualResetEvent(false);

// Create and start the job object
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, ExitTimeout);
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(ExitTimeout))
{
StopProcessOnTimeout(process);
}
}
#endif
}
Expand Down Expand Up @@ -2123,6 +2142,127 @@ private void LoadEnvironmentVariable(ProcessStartInfo startinfo, IDictionary Env
}
}

/// <summary>
/// Attempt to stop the process when the timeout has expired.
/// <see cref="StopProcessCommand" /> is used to stop the process
/// </summary>
/// <param name="process">
/// The process that should be stopped
/// </param>
private void StopProcessOnTimeout(Process 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);
foreach (Process p in stop.Invoke<Process>()) { }

ThrowTerminatingError(er);
}

/// <summary>
/// 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`
/// </summary>
/// <param name="parentProcess">
/// The parent process to use to resolve the process tree IDs
/// </param>
/// <returns>
/// IDs of the parent process and all its descendants
/// </returns>
private int[] GetProcessTreeIds(Process parentProcess)
{
List<int> stopProcessIds = new List<int> {parentProcess.Id};
string processRelationships = "";
bool processesCollected = true;
#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();

if (!ps.WaitForExit(4000) || ps.ExitCode != 0)
{
processesCollected = false;
}
ps.Close();
}
catch (Win32Exception)
{
processesCollected = false;
}
#else
string searchQuery = "Select ProcessID, ParentProcessID From Win32_Process";
try
{
using (CimSession cimSession = CimSession.Create(null))
{
IEnumerable<CimInstance> 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)
{
processesCollected = false;
}
#endif

if (!processesCollected)
{
WriteWarning(
StringUtil.Format(ProcessResources.CouldNotResolveProcessTree, parentProcess.ProcessName)
+ " "
+ ProcessResources.DescendantProcessesPossiblyRunning
);
return stopProcessIds.ToArray();
}

// processList - key: process ID, value: parent process ID
Dictionary<int, int> processList = new Dictionary<int, int>();
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<int, int> 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
Expand Down Expand Up @@ -2605,12 +2745,15 @@ internal void CheckJobStatus(Object stateInfo)
/// <param name="waitHandleToUse">
/// WaitHandle to use for waiting on the job object.
/// </param>
internal void WaitOne(ManualResetEvent waitHandleToUse)
/// <param name="timeout">
/// Wait for this number of milliseconds before a time-out occurs.
/// </param>
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);
}
}
}
Expand Down
9 changes: 9 additions & 0 deletions src/Microsoft.PowerShell.Commands.Management/resources/ProcessResources.resx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,12 @@
<data name="CouldNotStopProcess" xml:space="preserve">
<value>Cannot stop process "{0} ({1})" because of the following error: {2}</value>
</data>
<data name="CouldNotResolveProcessTree" xml:space="preserve">
<value>The process tree cannot be resolved for "{0}".</value>
</data>
<data name="DescendantProcessesPossiblyRunning" xml:space="preserve">
<value>Descendant processes may still be running.</value>
</data>
<data name="ProcessNameForConfirmation" xml:space="preserve">
<value>{0} ({1})</value>
</data>
Expand Down Expand Up @@ -171,6 +177,9 @@
<data name="ProcessNotTerminated" xml:space="preserve">
<value>This command stopped operation because process "{0} ({1})" is not stopped in the specified time-out.</value>
</data>
<data name="StartProcessExitTimeoutExceeded" xml:space="preserve">
<value>This command stopped operation because process "{0}" did not exit within the specified time-out.</value>
</data>
<data name="InvalidStartProcess" xml:space="preserve">
<value>This command cannot be run due to the error: {0}.</value>
</data>
Expand Down
39 changes: 39 additions & 0 deletions test/powershell/Modules/Microsoft.PowerShell.Management/Start-Process.Tests.ps1
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,45 @@ 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"
}
}

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 exit time-out is exceeded" {
{ Start-Process ping -ArgumentList $pingParam -ExitTimeout 20000 -RedirectStandardOutput "$TESTDRIVE/output" } | ShouldBeErrorId "StartProcessExitTimeoutExceeded,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 exit time-out is exceeded" {
Get-Process testexe -ErrorAction SilentlyContinue | Stop-Process

{ 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
Start-Sleep 1

$childprocesses = Get-Process testexe -ErrorAction SilentlyContinue
$childprocesses.count | Should Be 0
}
}

Describe "Start-Process tests requiring admin" -Tags "Feature","RequireAdminOnWindows" {

BeforeEach {
Expand Down