-
-
Save georg-jung/3a8703946075d56423e418ea76212745 to your computer and use it in GitHub Desktop.
using System; | |
using System.Diagnostics; | |
using System.Text; | |
using System.Threading.Tasks; | |
// based on https://gist.github.com/AlexMAS/276eed492bc989e13dcce7c78b9e179d | |
public static class ProcessAsyncHelper | |
{ | |
public static async Task<ProcessResult> RunProcessAsync(string command, string arguments, int timeout) | |
{ | |
var result = new ProcessResult(); | |
using (var process = new Process()) | |
{ | |
process.StartInfo.FileName = command; | |
process.StartInfo.Arguments = arguments; | |
process.StartInfo.UseShellExecute = false; | |
//process.StartInfo.RedirectStandardInput = true; | |
process.StartInfo.RedirectStandardOutput = true; | |
process.StartInfo.RedirectStandardError = true; | |
process.StartInfo.CreateNoWindow = true; | |
var outputBuilder = new StringBuilder(); | |
var outputCloseEvent = new TaskCompletionSource<bool>(); | |
process.OutputDataReceived += (s, e) => | |
{ | |
if (e.Data == null) | |
{ | |
outputCloseEvent.SetResult(true); | |
} | |
else | |
{ | |
outputBuilder.Append(e.Data); | |
} | |
}; | |
var errorBuilder = new StringBuilder(); | |
var errorCloseEvent = new TaskCompletionSource<bool>(); | |
process.ErrorDataReceived += (s, e) => | |
{ | |
if (e.Data == null) | |
{ | |
errorCloseEvent.SetResult(true); | |
} | |
else | |
{ | |
errorBuilder.Append(e.Data); | |
} | |
}; | |
var isStarted = process.Start(); | |
if (!isStarted) | |
{ | |
result.ExitCode = process.ExitCode; | |
return result; | |
} | |
// Reads the output stream first and then waits because deadlocks are possible | |
process.BeginOutputReadLine(); | |
process.BeginErrorReadLine(); | |
// Creates task to wait for process exit using timeout | |
var waitForExit = WaitForExitAsync(process, timeout); | |
// Create task to wait for process exit and closing all output streams | |
var processTask = Task.WhenAll(waitForExit, outputCloseEvent.Task, errorCloseEvent.Task); | |
// Waits process completion and then checks it was not completed by timeout | |
if (await Task.WhenAny(Task.Delay(timeout), processTask) == processTask && waitForExit.Result) | |
{ | |
result.ExitCode = process.ExitCode; | |
result.Output = outputBuilder.ToString(); | |
result.Error = errorBuilder.ToString(); | |
} | |
else | |
{ | |
try | |
{ | |
// Kill hung process | |
process.Kill(); | |
} | |
catch | |
{ | |
// ignored | |
} | |
} | |
} | |
return result; | |
} | |
private static Task<bool> WaitForExitAsync(Process process, int timeout) | |
{ | |
return Task.Run(() => process.WaitForExit(timeout)); | |
} | |
public struct ProcessResult | |
{ | |
public int? ExitCode; | |
public string Output; | |
public string Error; | |
} | |
} |
One goals of async is to not tie up a thread while waiting. This WaitForExitAsync shouldn't tie up a thread while waiting for process to complete.
public static async Task WaitForExitAsync(this Process process, CancellationToken cancellationToken = default)
{
var tcs = new TaskCompletionSource<bool>(TaskCreationOptions.RunContinuationsAsynchronously);
void Process_Exited(object sender, EventArgs e)
{
tcs.TrySetResult(true);
}
process.EnableRaisingEvents = true;
process.Exited += Process_Exited;
try
{
if (process.HasExited)
{
return;
}
using (cancellationToken.Register(() => tcs.TrySetCanceled()))
{
await tcs.Task.ConfigureAwait(false);
}
}
finally
{
process.Exited -= Process_Exited;
}
}
I think you are right about that. The above code has different shortcomings. After reviewing (I read at least parts of the source code) some libraries solving this or similar problems, I think CliWrap is a quite complete and mature solution.
More lightweight (in the sense of you can read the source code completely in some minutes, similar to this gist; not in the sense of execution time, which I didn't test) would be ProcessStartAsync, but it has some shortcomings:
- not targeting ns2.0 (but 1.3)
- having dependencies that should be private
- not sure if it's actively maintained
- silently overwriting ProcessStartInfo properties (
startInfo.UseShellExecute = false; startInfo.CreateNoWindow = true;
). Throwing InvalidOperationExceptions might be better. - no handling of
ps.Start();
returning false. Not sure if it's needed though.- same applies to CliWrap
- duplicate WaitForExit seems wrong
- seems like a good idea to follow the guidance regarding completions of TCSs
same applies to CliWrapfixed
I liked both more than other options I reviewed:
https://github.com/Cysharp/ProcessX
- does many things that are imho out of scope for such a library
https://github.com/itn3000/AsyncProcessExecutor/
- more complicated than ProcessStartAsync (maybe interesting if one plans to pipe binary data/wants to stream output of one process to another), imho written worse than CliWrap
https://github.com/jamesmanning/RunProcessAsTask
- API with Lists for StdOut and StdErr just seems wrong
https://github.com/samoatesgames/AsyncProcess.Sharp
- rather good candidate, though: waits in a thread, targets .Net Framework
Thanks @georg-jung!
Just wanted to add a few comments since I've found your post:
no handling of ps.Start();
To the best of my knowledge, false
can only be returned with shell execute, although I may be wrong. I have never had a false
return though so in order to handle it I would need a test that exercises this behavior. Basically I'm waiting for the issue to happen first before solving it.
does many things that are imho out of scope for such a library
Interestingly enough, CliWrap also has all the features of ProcessX
, which is the event stream execution model. So you don't have to compromise.
more complicated than ProcessStartAsync (maybe interesting if one plans to pipe binary data/wants to stream output of one process to another)
Same here, CliWrap has a powerful support for piping in which the configuration and execution are also separate.
While they do share features, I liked yours better in terms of separation of concerns and think it's less opinionated, that's what I meant. Like deciding "developers tend to write things to the console instead of an ILogger" (by offering an API for one but not the other) is a thing I considered "out of scope" ;-); but after all that's just my opinion too, I guess. So - keep up the great work!
❤️
Once I was experimenting with something similar, focusing on streams in/out/error... Unfortunately the original link provided in the sample is not avail, but there were some consideration (at least in the past) why the event based approach was not used. What I miss is actually the ExitCode handling in case of regular completition (this includes everything except the custom timeout, so errors as well). This is what I could grab from the mails I've exchanged at that time: https://gist.github.com/hidegh/07d5588702e2b56a3fc2a3d73848d9f3