Starting a process asynchronous

Question

For executing a Process I've created an separate class which binds the possibilities together, for example reading output, start as Admin, catch Exceptions and also start all this asynchronous.

Are there any suggestions to improve it further?

public class ExternalProcess
{
    public string FileName { get; set; }
    public string Arguments { get; set; } = "";
    public string WorkingDirectory { get; set; } = "";
    public int Timeout_milliseconds { get; set; } = -1;
    public bool ReadOutput { get; set; }
    public bool ShowWindow { get; set; }
    public bool StartAsAdministrator { get; set; }
    public string Output { get; private set; } = "";
    public string OutputError { get; private set; } = "";
    public int ExitCode { get; private set; }
    public bool WasKilled { get; private set; }
    public bool WasSuccessful { get; private set; }

    public event EventHandler OutputChanged;

    public event EventHandler OutputErrorChanged;

    public bool Start(CancellationToken cancellationToken = default(CancellationToken))
    {
        var task = StartAsync(cancellationToken);
        task.Wait();
        return task.Result;
    }

    public async Task<bool> StartAsync(CancellationToken cancellationToken = default(CancellationToken))
    {
        if (String.IsNullOrWhiteSpace(FileName))
        {
            throw new ArgumentNullException(nameof(FileName));
        }
        //if (!File.Exists(FileName))   // removed because also commands could be executed (for example: ping)
        if (!ReadOutput)
        {
            Output = OutputError = $"Enable {nameof(ReadOutput)} to get Output  ";
        }
        if (StartAsAdministrator)
        {
            ReadOutput = false;  // Verb="runas" only possible with ShellExecute=true.
            Output = OutputError = "Output couldn't be read when started as Administrator  ";
        }
        var useShellExecute = !ReadOutput;  // true when started as admin, false for reading output
        if (!StartAsAdministrator && !ShowWindow)
            useShellExecute = false;    // false for hiding the window
        using (var p = new Process
        {
            EnableRaisingEvents = true,
            StartInfo = new ProcessStartInfo()
            {
                FileName = FileName,
                Arguments = Arguments,
                UseShellExecute = useShellExecute,
                RedirectStandardOutput = ReadOutput,
                RedirectStandardError = ReadOutput,
                RedirectStandardInput = ReadOutput,
                CreateNoWindow = !ShowWindow,
            }
        })
        {
            if (StartAsAdministrator)
            {
                p.StartInfo.Verb = "runas";
            }
            if (!String.IsNullOrWhiteSpace(WorkingDirectory))
            {
                p.StartInfo.WorkingDirectory = WorkingDirectory;
            }
            if (ReadOutput)
            {
                p.OutputDataReceived += (sender, args) =>
                {
                    if (!String.IsNullOrEmpty(args?.Data))
                    {
                        Output += args.Data + Environment.NewLine;
                        OutputChanged?.Invoke(this, EventArgs.Empty);
                    }
                };
                p.ErrorDataReceived += (sender, args) =>
                {
                    if (!String.IsNullOrEmpty(args?.Data))
                    {
                        OutputError += args.Data + Environment.NewLine;
                        OutputErrorChanged?.Invoke(this, EventArgs.Empty);
                    }
                };
            }

            try
            {
                p.Start();
            }
            catch (System.ComponentModel.Win32Exception ex)
            {
                if (ex.NativeErrorCode == 1223)
                {
                    OutputError += "AdminRights request Canceled by User!! " + ex;
                    ExitCode = -1;
                    WasSuccessful = false;
                    return WasSuccessful;
                }
                else
                {
                    OutputError += "Win32Exception thrown: " + ex;
                    ExitCode = -1;
                    WasSuccessful = false;
                    throw;
                }
            }
            catch (Exception ex)
            {
                OutputError += "Exception thrown: " + ex;
                ExitCode = -1;
                WasSuccessful = false;
                throw;
            }

            if (ReadOutput)
            {
                // var writer = p.StandardInput; writer.WriteLine("");
                p.BeginOutputReadLine();
                p.BeginErrorReadLine();
            }

            CancellationTokenSource timeoutCancellationToken = null;
            if (Timeout_milliseconds > 0)
            {
                timeoutCancellationToken = new CancellationTokenSource(Timeout_milliseconds);
            }
            while (!p.HasExited && !cancellationToken.IsCancellationRequested && !(timeoutCancellationToken?.IsCancellationRequested ?? false))
            {
                await Task.Delay(10).ConfigureAwait(false);  // configureAwait is that the task doesn't continue after the first await is done (prevent deadlock)
            }
            if (!p.HasExited)
            {
                WasKilled = true;
                OutputError += "  Process was cancelled!";
                try
                {
                    p.CloseMainWindow();
                    int waitForKill = 30;
                    do
                    {
                        Thread.Sleep(10);
                        waitForKill--;
                    } while (!p.HasExited && waitForKill > 0);
                    if (!p.HasExited)
                    {
                        p.Kill();
                    }
                }
                catch { }
            }
            ExitCode = p.ExitCode;
            p.Close();
            if (ExitCode == -1073741510)
            {
                OutputError += $"Process exited by user, exitcode: {ExitCode}!";
            }
            WasSuccessful = !WasKilled && ExitCode == 0;
            return WasSuccessful;
        }
    }
}

Answer 1

Ideally, you wouldn't use Task.Delay, and instead find a way to bind to Process event(s), and thereby create a brand new TaskCompletionSource. Also, you generally shouldn't return true/false, and should use exception throwing instead.

• see https://stackoverflow.com/a/15316668/887092 for TaskCompletionSource
• see https://stackoverflow.com/questions/4504170/why-is-my-processs-exited-method-not-being-called#4504208 for example of Process.Exited
• see https://softwareengineering.stackexchange.com/a/159097/78712 - there are still good cases where true/false should be returned (eg. while(reader.Read()))

Example of the start of a re-write, which highlights the 3 chages:

static Task MostBasicProcess()
{
    var t = new TaskCompletionSource<bool>(); //Using bool, because TaskCompletionSource needs at least one generic param

    var p = new Process();
    //TODO: more setup
    p.EnableRaisingEvents = true; //VERY important
    p.Exited += (object sender, EventArgs e) =>
    {
        ////TODO: Exceptions will go first, followed by `return;`
        //t.SetException();

        //TODO: Finally, if there are no problems, return successfully
        t.SetResult(true);
    };
    p.Start();
    //TODO: wrap .Start() in try-block and call t.SetException on error

    return t.Task; //We actually don't want the caller using the bool param, it's implicitly casted to plain Task.
}

Note: That exception processing should appear first to avoid too-much {} nesting. Also, you should really put all the code in a try-block.

Usage:

try
{
    await MostBasicProcess();
}
catch (Exception ex)
{
}

Of course, you need to adapt this to your broader needs.