Process sometimes hangs while waiting for Exit
Let's start with a recap of the accepted answer in a related post.
The problem is that if you redirect StandardOutput and/or StandardError the internal buffer can become full. Whatever order you use, there can be a problem:
- If you wait for the process to exit before reading StandardOutput the process can block trying to write to it, so the process never ends.
- If you read from StandardOutput using ReadToEnd then your process can block if the process never closes StandardOutput (for example if it never terminates, or if it is blocked writing to StandardError).
Even the accepted answer, however, struggles with the order of execution in certain cases.
EDIT: See answers below for how avoid an ObjectDisposedException if the timeout occurs.
It's in these kind of situations, where you want to orchestrate several events, that Rx really shines.
Note the .NET implementation of Rx is available as the System.Reactive NuGet package.
Let's dive in to see how Rx facilitates working with events.
// Subscribe to OutputData
Observable.FromEventPattern<DataReceivedEventArgs>(process, nameof(Process.OutputDataReceived))
.Subscribe(
eventPattern => output.AppendLine(eventPattern.EventArgs.Data),
exception => error.AppendLine(exception.Message)
).DisposeWith(disposables);
FromEventPattern allows us to map distinct occurrences of an event to a unified stream (aka observable). This allows us to handle the events in a pipeline (with LINQ-like semantics). The Subscribe overload used here is provided with an Action<EventPattern<...>> and an Action<Exception>. Whenever the observed event is raised, its sender and args will be wrapped by EventPattern and pushed through the Action<EventPattern<...>>. When an exception is raised in the pipeline, Action<Exception> is used.
One of the drawbacks of the Event pattern, clearly illustrated in this use case (and by all the workarounds in the referenced post), is that it's not apparent when / where to unsubscribe the event handlers.
With Rx we get back an IDisposable when we make a subscription. When we dispose of it, we effectively end the subscription. With the addition of the DisposeWith extension method (borrowed from RxUI), we can add multiple IDisposables to a CompositeDisposable (named disposables in the code samples). When we're all done, we can end all subscriptions with one call to disposables.Dispose().
To be sure, there's nothing we can do with Rx, that we wouldn't be able to do with vanilla .NET. The resulting code is just a lot easier to reason about, once you've adapted to the functional way of thinking.
public static void ExecuteScriptRx(string path, int processTimeOutMilliseconds, out string logs, out bool success, params string[] args)
{
StringBuilder output = new StringBuilder();
StringBuilder error = new StringBuilder();
using (var process = new Process())
using (var disposables = new CompositeDisposable())
{
process.StartInfo = new ProcessStartInfo
{
WindowStyle = ProcessWindowStyle.Hidden,
FileName = "powershell.exe",
RedirectStandardOutput = true,
RedirectStandardError = true,
UseShellExecute = false,
Arguments = $"-ExecutionPolicy Bypass -File \"{path}\"",
WorkingDirectory = Path.GetDirectoryName(path)
};
if (args.Length > 0)
{
var arguments = string.Join(" ", args.Select(x => $"\"{x}\""));
process.StartInfo.Arguments += $" {arguments}";
}
output.AppendLine($"args:'{process.StartInfo.Arguments}'");
// Raise the Process.Exited event when the process terminates.
process.EnableRaisingEvents = true;
// Subscribe to OutputData
Observable.FromEventPattern<DataReceivedEventArgs>(process, nameof(Process.OutputDataReceived))
.Subscribe(
eventPattern => output.AppendLine(eventPattern.EventArgs.Data),
exception => error.AppendLine(exception.Message)
).DisposeWith(disposables);
// Subscribe to ErrorData
Observable.FromEventPattern<DataReceivedEventArgs>(process, nameof(Process.ErrorDataReceived))
.Subscribe(
eventPattern => error.AppendLine(eventPattern.EventArgs.Data),
exception => error.AppendLine(exception.Message)
).DisposeWith(disposables);
var processExited =
// Observable will tick when the process has gracefully exited.
Observable.FromEventPattern<EventArgs>(process, nameof(Process.Exited))
// First two lines to tick true when the process has gracefully exited and false when it has timed out.
.Select(_ => true)
.Timeout(TimeSpan.FromMilliseconds(processTimeOutMilliseconds), Observable.Return(false))
// Force termination when the process timed out
.Do(exitedSuccessfully => { if (!exitedSuccessfully) { try { process.Kill(); } catch {} } } );
// Subscribe to the Process.Exited event.
processExited
.Subscribe()
.DisposeWith(disposables);
// Start process(ing)
process.Start();
process.BeginOutputReadLine();
process.BeginErrorReadLine();
// Wait for the process to terminate (gracefully or forced)
processExited.Take(1).Wait();
logs = output + Environment.NewLine + error;
success = process.ExitCode == 0;
}
}
We already discussed the first part, where we map our events to observables, so we can jump straight to the meaty part. Here we assign our observable to the processExited variable, because we want to use it more than once.
First, when we activate it, by calling Subscribe. And later on when we want to 'await' its first value.
var processExited =
// Observable will tick when the process has gracefully exited.
Observable.FromEventPattern<EventArgs>(process, nameof(Process.Exited))
// First two lines to tick true when the process has gracefully exited and false when it has timed out.
.Select(_ => true)
.Timeout(TimeSpan.FromMilliseconds(processTimeOutMilliseconds), Observable.Return(false))
// Force termination when the process timed out
.Do(exitedSuccessfully => { if (!exitedSuccessfully) { try { process.Kill(); } catch {} } } );
// Subscribe to the Process.Exited event.
processExited
.Subscribe()
.DisposeWith(disposables);
// Start process(ing)
...
// Wait for the process to terminate (gracefully or forced)
processExited.Take(1).Wait();
One of the problems with OP is that it assumes process.WaitForExit(processTimeOutMiliseconds) will terminate the process when it times out. From MSDN:
Instructs the Process component to wait the specified number of milliseconds for the associated process to exit.
Instead, when it times out, it merely returns control to the current thread (i.e. it stops blocking). You need to manually force termination when the process times out. To know when time out has occurred, we can map the Process.Exited event to a processExited observable for processing. This way we can prepare the input for the Do operator.
The code is pretty self-explanatory. If exitedSuccessfully the process will have terminated gracefully. If not exitedSuccessfully, termination will need to be forced. Note that process.Kill() is executed asynchronously, ref remarks. However, calling process.WaitForExit() right after will open up the possibility for deadlocks again. So even in the case of forced termination, it's better to let all disposables be cleaned up when the using scope ends, as the output can be considered interrupted / corrupted anyway.
The try catch construct is reserved for the exceptional case (no pun intended) where you've aligned processTimeOutMilliseconds with the actual time needed by the process to complete. In other words, a race condition occurs between the Process.Exited event and the timer. The possibility of this happening is again magnified by the asynchronous nature of process.Kill(). I've encountered it once during testing.
For completeness, the DisposeWith extension method.
/// <summary>
/// Extension methods associated with the IDisposable interface.
/// </summary>
public static class DisposableExtensions
{
/// <summary>
/// Ensures the provided disposable is disposed with the specified <see cref="CompositeDisposable"/>.
/// </summary>
public static T DisposeWith<T>(this T item, CompositeDisposable compositeDisposable)
where T : IDisposable
{
if (compositeDisposable == null)
{
throw new ArgumentNullException(nameof(compositeDisposable));
}
compositeDisposable.Add(item);
return item;
}
}
For the benefit of readers I'm going to divide this to 2 Sections
Section A: Problem & how to handle similar scenarios
Section B: Problem recreation & Solution
Section A: Problem
When this problem happens - process appears in task manager, then after 2-3sec disappears (its fine), then it waits for timeout and then exception is thrown System.InvalidOperationException: Process must exit before requested information can be determined.
& See Scenario 4 below
In your code:
Process.WaitForExit(ProcessTimeOutMiliseconds);With this you're waiting forProcessto Timeout or Exit, which ever takes place first.OutputWaitHandle.WaitOne(ProcessTimeOutMiliseconds)anderrorWaitHandle.WaitOne(ProcessTimeOutMiliseconds);With this you're waiting forOutputData&ErrorDatastream read operation to signal its completeProcess.ExitCode == 0Gets status of process when it exited
Different settings & their caveats:
- Scenario 1 (Happy Path): Process completes before the timeout, and thus your stdoutput and stderror also finishes before it and all is well.
- Scenario 2: Process, OutputWaitHandle & ErrorWaitHandle timesout however stdoutput & stderror is still being read and completes after time out WaitHandlers. This leads to another exception
ObjectDisposedException() - Scenario 3: Process times-out first (19 sec) but stdout and stderror is in action, you wait for WaitHandler's to times out (19 sec), causing a added delay of + 19sec.
- Scenario 4: Process times out and code attempts to prematurely query
Process.ExitCoderesulting in the errorSystem.InvalidOperationException: Process must exit before requested information can be determined.
I have tested this scenario over a dozen times and works fine, following settings have been used while testing
- Size of Output stream ranging from 5KB to 198KB by initiating build of about 2-15 projects
- Premature timeouts & process exits within the timeout window
Updated Code
.
.
.
process.BeginOutputReadLine();
process.BeginErrorReadLine();
//First waiting for ReadOperations to Timeout and then check Process to Timeout
if (!outputWaitHandle.WaitOne(ProcessTimeOutMiliseconds) && !errorWaitHandle.WaitOne(ProcessTimeOutMiliseconds)
&& !process.WaitForExit(ProcessTimeOutMiliseconds) )
{
//To cancel the Read operation if the process is stil reading after the timeout this will prevent ObjectDisposeException
process.CancelOutputRead();
process.CancelErrorRead();
Console.ForegroundColor = ConsoleColor.Red;
Console.WriteLine("Timed Out");
Logs = output + Environment.NewLine + error;
//To release allocated resource for the Process
process.Close();
return (false, logs);
}
Console.ForegroundColor = ConsoleColor.Green;
Console.WriteLine("Completed On Time");
Logs = output + Environment.NewLine + error;
ExitCode = process.ExitCode.ToString();
// Close frees the memory allocated to the exited process
process.Close();
//ExitCode now accessible
return process.ExitCode == 0 ? (true, logs) : (false, logs);
}
}
finally{}
EDIT:
After hours of playing around with MSBuild I was finally able to reproduce the issue at my system
Section B: Problem Recreation & Solution
MSBuild has
-m[:number]switch which is used to specify the maximum number of concurrent processes to use when building.When this is enabled, MSBuild spawns a number of nodes that lives on even after the Build is complete. Now,
Process.WaitForExit(milliseconds)would wait never exit and eventually timeout
I was able to solve this in couple of ways
Spawn MSBuild process indirectly through CMD
$path1 = """C:\Program Files (x86)\Microsoft Visual Studio\2017\Community\MSBuild\15.0\Bin\MSBuild.exe"" ""C:\Users\John\source\repos\Test\Test.sln"" -maxcpucount:3" $cmdOutput = cmd.exe /c $path1 '2>&1' $cmdOutputContinue to use MSBuild but be sure to set the nodeReuse to False
$filepath = "C:\Program Files (x86)\Microsoft Visual Studio\2017\Community\MSBuild\15.0\Bin\MSBuild.exe" $arg1 = "C:\Users\John\source\repos\Test\Test.sln" $arg2 = "-m:3" $arg3 = "-nr:False" Start-Process -FilePath $filepath -ArgumentList $arg1,$arg2,$arg3 -Wait -NoNewWindowEven If parallel build is not enabled, you could still prevent your process from hanging at
WaitForExitby launching the Build via CMD & therefore you do not create a direct dependency on Build process$path1 = """C:\....\15.0\Bin\MSBuild.exe"" ""C:\Users\John\source\Test.sln""" $cmdOutput = cmd.exe /c $path1 '2>&1' $cmdOutput
The 2nd approach is preferred since you do not want too many MSBuild nodes to be lying around.
The problem is that if you redirect StandardOutput and/or StandardError the internal buffer can become full.
To solve the issues aforementioned you can run the process in separate threads. I do not use WaitForExit, I utilize the process exited event which will return the ExitCode of the process asynchronously ensuring it has completed.
public async Task<int> RunProcessAsync(params string[] args)
{
try
{
var tcs = new TaskCompletionSource<int>();
var process = new Process
{
StartInfo = {
FileName = 'file path',
RedirectStandardOutput = true,
RedirectStandardError = true,
Arguments = "shell command",
UseShellExecute = false,
CreateNoWindow = true
},
EnableRaisingEvents = true
};
process.Exited += (sender, args) =>
{
tcs.SetResult(process.ExitCode);
process.Dispose();
};
process.Start();
// Use asynchronous read operations on at least one of the streams.
// Reading both streams synchronously would generate another deadlock.
process.BeginOutputReadLine();
string tmpErrorOut = await process.StandardError.ReadToEndAsync();
//process.WaitForExit();
return await tcs.Task;
}
catch (Exception ee) {
Console.WriteLine(ee.Message);
}
return -1;
}
The above code is battle tested calling FFMPEG.exe with command line arguments. I was converting mp4 files to mp3 files and doing over 1000 videos at a time without failing. Unfortunately I do not have direct power shell experience but hope this helps.