Warn against Process.StandardOutput and Process.StandardError

[bretehlert]

It is best practice to read process IO asynchronously to avoid deadlocks using OutputDataReceived/BeginOutputReadLine and ErrorDataRecieved/BeginErrorReadLine.

Warn against any access to Process.StandardOutput and Process.StandardError, suggesting to use the asynchronous operations instead.

[brian-reichle]

Using Process.StandardOutput and Process.StandardError can be perfectly valid if reading them asynchronously, eg.

async Task ReadOutput(StreamReader reader)
{
    string line;
    while ((line = await reader.ReadLineAsync()) != null)
    {
        // Do stuff with line.
    }
}

Process process = ...
var task = Task.WhenAll(
    ReadOutput(process.StandardOutput),
    ReadOutput(process.StandardError));
process.WaitForExit();
await task;

[bretehlert]

This is not safe at all, you have called WaitForExit() before reading from StandardOutput, when the stdio buffer fills up it will wait for you to read from the buffer but you will not because you are waiting for the process to extit => deadlock.

[brian-reichle]

Oops, out of order (fixed). I usually create a WaitForExitAsync extension method and include it in the WhenAll.

[bretehlert]

This is why we need a warning, you can't even get the code right the first time when discussing how to get the code right....

[brian-reichle]

Though using the events has it's own "gotcha's". You need to call WaitForExit() (without arguments) to ensure that the callbacks have been flushed.

[yaakov-h]

There are a lot of ways to do it wrong, and also ways to do it right, both of which use these properties.

I'm thinking perhaps we should simply warn against calling ReadToEnd or ReadToEndAsync on the resulting objects.

Would that suffice for the majority case?