Process execution in a multi task enveríroment

Question

I am trying to execute some commands in my program, for that I am using System.Diagnostics.Process. I managed to set it working, when I execute the commands 1 by 1 the returns are correct. I then tried to speed up the process by creating tasks for each process execution, and here is where I am having problems.

Here is the class to execute the commands:

class ProcessExec
{
    public string Start(string command)
    {
        string res = "";

        Process process = new Process();
        process.EnableRaisingEvents = true;
        process.StartInfo.FileName = "powershell.exe";
        process.StartInfo.Arguments = command;
        process.StartInfo.UseShellExecute = false;
        process.StartInfo.RedirectStandardOutput = true;
        process.StartInfo.CreateNoWindow = true;

        process.OutputDataReceived += (object sender, DataReceivedEventArgs e) =>
        {
            res = res + e.Data;
        };

        process.Start();
        process.BeginOutputReadLine();
        process.WaitForExit(10000);

        return res;
    }
}

And this is my Main:

class Program
{
    static void Main(string[] args)
    {
        Console.WriteLine("Start");

        List<Task> tasks = new List<Task>();

        ProcessExec exec = new ProcessExec();

        Stopwatch sw = new Stopwatch();
        sw.Start();

        string res1 = "";
        tasks.Add(Task.Run(() => { res1 = exec.Start("date"); }));
        string res2 = "";
        tasks.Add(Task.Run(() => { res2 = exec.Start("hostname"); }));
        string res3 = "";
        tasks.Add(Task.Run(() => { res3 = exec.Start("date"); }));
        string res4 = "";
        tasks.Add(Task.Run(() => { res4 = exec.Start("date"); }));
        string res5 = "";
        tasks.Add(Task.Run(() => { res5 = exec.Start("date"); }));
        string res6 = "";
        tasks.Add(Task.Run(() => { res6 = exec.Start("ipconfig"); }));
        string res7 = "";
        tasks.Add(Task.Run(() => { res7 = exec.Start("date"); }));
        string res8 = "";
        tasks.Add(Task.Run(() => { res8 = exec.Start("date"); }));

        Task.WaitAll(tasks.ToArray());

        sw.Stop();

        Console.WriteLine(sw.Elapsed.TotalSeconds);

        Console.WriteLine("1 - " + res1);
        Console.WriteLine("2 - " + res2);
        Console.WriteLine("3 - " + res3);
        Console.WriteLine("4 - " + res4);
        Console.WriteLine("5 - " + res5);
        Console.WriteLine("6 - " + res6);
        Console.WriteLine("7 - " + res7);
        Console.WriteLine("8 - " + res8);

        Console.WriteLine("End");
        Console.ReadKey();
    }
}

This is my output:

Start
7,4867498
1 - 22 de julho de 2017 10:25:46    
2 -    
3 - 22 de julho de 2017 10:25:48
4 - 22 de julho de 2017 10:25:48    
5 -    
6 -    
7 - 22 de julho de 2017 10:25:48
8 - 22 de julho de 2017 10:25:48
End

Now, I think that my problem is related with the OutputDataReceived event being in a diferent thread, but I am not fully sure. Anyone knows what is the problem and how can I solve it?

Answer 1

It would be better if you could explain not just what output you got, but what output you expected. That said, it does seem like what you're asking about is why some (but not all) of your commands have empty output. And yes, that would be because those commands apparently take longer than the 10 seconds you've allotted, so your method returns before the output has been read.

Fact is, if you want your method to not return until the process has completed, there's no reason to use WaitForExit() at all. I know that sounds counter-intuitive, but the reason you have to use WaitForExit() here is that you've chosen to consume the process output asynchronously. But there's no reason to do that, because you want the handling of the process to be synchronous.

So, just do that. You can call Process.StandardOutput.ReadToEnd(), which will block until the process exits, and then return all of the output. Also, there does not appear to be any reason for your class to be non-static, since it doesn't have any state other than the local state in the method.

So something like this would be better:

static class ProcessExec
{
    public static string Start(string command)
    {
        Process process = new Process();
        process.StartInfo.FileName = "powershell.exe";
        process.StartInfo.Arguments = command;
        process.StartInfo.UseShellExecute = false;
        process.StartInfo.RedirectStandardOutput = true;
        process.StartInfo.CreateNoWindow = true;

        process.Start();

        return process.StandardOutput.ReadToEnd();
    }
}

The other thing I would change about your implementation is the use of local variables. It would be better, given your use of Task, to represent the result of the command in the Task object itself, rather than capturing a local variable for every command you execute.

For example:

static void Main(string[] args)
{
    Console.WriteLine("Start");

    List<Task<string>> tasks = new List<Task<string>>();

    Stopwatch sw = new Stopwatch();
    sw.Start();

    tasks.Add(Task.Run(() => ProcessExec.Start("date")));
    tasks.Add(Task.Run(() => ProcessExec.Start("hostname")));
    tasks.Add(Task.Run(() => ProcessExec.Start("date")));
    tasks.Add(Task.Run(() => ProcessExec.Start("date")));
    tasks.Add(Task.Run(() => ProcessExec.Start("date")));
    tasks.Add(Task.Run(() => ProcessExec.Start("ipconfig")));
    tasks.Add(Task.Run(() => ProcessExec.Start("date")));
    tasks.Add(Task.Run(() => ProcessExec.Start("date")));

    Task.WaitAll(tasks);

    sw.Stop();

    Console.WriteLine(sw.Elapsed.TotalSeconds);

    Console.WriteLine(string.Join(Environment.NewLine,
        tasks.Select((t, i) => $"{i + 1} - {t.Result}")));

    Console.WriteLine("End");
    Console.ReadKey();
}

Then you don't need the variables, and it's easy to retrieve the command results using a loop variable instead of having to name each one individually.