Skip to content

Instantly share code, notes, and snippets.

Embed
What would you like to do?
Use https://github.com/Tyrrrz/CliWrap instead, the following code has some shortcomings
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;
}
}
@GaikwadPratik

This comment has been minimized.

Copy link

@GaikwadPratik GaikwadPratik commented Aug 6, 2019

This is definitely improvement over original Gist. Thank you for that.
As a precaution, you may want to add CancellationTokenSource in Line #71 for timeout operation.

@GaikwadPratik

This comment has been minimized.

Copy link

@GaikwadPratik GaikwadPratik commented Aug 10, 2019

@georg-jung,

A quick question, if we return from this line when process doesn't start, how would we know what is the issue in starting the process?

@peterdijkstra

This comment has been minimized.

Copy link

@peterdijkstra peterdijkstra commented Aug 17, 2019

Thanks for this!

I added a quick and dirty change that allows for setting environment variables.

https://github.com/sloppycombo/UIElements-Pug/blob/master/ProcessAsyncHelper.cs

@JTSimmons

This comment has been minimized.

Copy link

@JTSimmons JTSimmons commented Aug 23, 2019

How do I implement this? The output and error data is being read async, but am I still just getting the data all at once when the process completes?

@georg-jung

This comment has been minimized.

Copy link
Owner Author

@georg-jung georg-jung commented Aug 26, 2019

@GaikwadPratik

A quick question, if we return from this line when process doesn't start, how would we know what is the issue in starting the process?

It isn't quite easy to answer when Process.Start() will return false, but it seems to be a rather rare case. See https://stackoverflow.com/questions/33042010/in-what-cases-does-the-process-start-method-return-false. If there is some kind of typical issue with starting the process (i.e. file not found), an exception will be thrown. I currently don't see a better way for this code to handle the falses, as we don't know any reasoning about a hypothetical false.

@georg-jung

This comment has been minimized.

Copy link
Owner Author

@georg-jung georg-jung commented Aug 26, 2019

How do I implement this? The output and error data is being read async, but am I still just getting the data all at once when the process completes?

Yes, you get the data when the process completes. This solution is not about streaming stdout/stderr asynchronously but about running the process in an async/await kind of way. If you need to call some external software which just offers a commandline interface, this can be quite useful. In the case I needed this solution for I did not need any streaming or parts of the stdout output. Consuming stdout as an async stream might be possible but is beyond this gist's scope.

@sysworx

This comment has been minimized.

Copy link

@sysworx sysworx commented Sep 10, 2019

How can i use it, i dont understand when the Task is over?

@georg-jung

This comment has been minimized.

Copy link
Owner Author

@georg-jung georg-jung commented Sep 10, 2019

For a full blown example with some context see https://github.com/georg-jung/PdfAnnotator/blob/master/PdfAnnotator/Pdf/Poppler/Analyzer.cs#L32

Minimal working example:
var res = await ProcessAsyncHelper.RunProcessAsync("your.exe", "--arg=1", 5000).ConfigureAwait(false);

This will start your.exe with the given arguments and a timeout of 5 seconds. res is of type
public struct ProcessResult { int? ExitCode; string Output; string Error; }
afterwards.

The task will run as long as the process it starts is running (mind the timeout), which means the await ...-call will return when the process it created exited.

@sysworx

This comment has been minimized.

Copy link

@sysworx sysworx commented Sep 11, 2019

Ok and how About admin priv? if i tries this with a exe that Need admin Rights...i got a Rights error :(

@georg-jung

This comment has been minimized.

Copy link
Owner Author

@georg-jung georg-jung commented Sep 11, 2019

Please see https://stackoverflow.com/a/133500/1200847 for how to elevate a process using the Process class. This way you could adapt the given class above easily to fit your needs.

@Indigo744

This comment has been minimized.

Copy link

@Indigo744 Indigo744 commented Nov 14, 2019

I made a generic one which can accept any ProcessStartInfo objet and with or without timeout: https://gist.github.com/Indigo744/b5f3bd50df4b179651c876416bf70d0a

I also took some improvements from the various answer from https://stackoverflow.com/questions/470256/process-waitforexit-asynchronously

@hidegh

This comment has been minimized.

Copy link

@hidegh hidegh commented Feb 27, 2020

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

@davidathompson

This comment has been minimized.

Copy link

@davidathompson davidathompson commented Apr 9, 2020

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;
        }
    }
@georg-jung

This comment has been minimized.

Copy link
Owner Author

@georg-jung georg-jung commented Apr 9, 2020

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 CliWrap fixed

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
@Tyrrrz

This comment has been minimized.

Copy link

@Tyrrrz Tyrrrz commented May 13, 2020

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.

@georg-jung

This comment has been minimized.

Copy link
Owner Author

@georg-jung georg-jung commented May 13, 2020

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!

@Tyrrrz

This comment has been minimized.

Copy link

@Tyrrrz Tyrrrz commented May 13, 2020

❤️

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment