Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Using Process.BeginOutputReadLine can lead to lost stdout lines #18789

Closed
joelverhagen opened this issue Sep 30, 2016 · 3 comments
Closed

Using Process.BeginOutputReadLine can lead to lost stdout lines #18789

joelverhagen opened this issue Sep 30, 2016 · 3 comments

Comments

@joelverhagen
Copy link
Member

When writing code that creates child processes and needs to capture stdout, I have found that lines of output go missing the following condition:

  • lots of parallelism...
  • ProcessStartInfo.RedirectStandardOutput = true
  • Process.OutputDataReceived += CollectLines (that is, using the async flow)
  • Process.BeginOutputReadLine()
  • Process.WaitForExit(int.MaxValue)

Note that passing -1 (infinite timespan) to WaitForExit does not cause lines go missing (based on my own testing).

I have created a little demo app to do this. It repro's on both netcoreapp1.0 and net45: processredirect.zip. Just use dotnet run to try it out.

That being said, you can capture stdout by using Process.StandardOutput (which is a StreamReader). However to consume this asynchronously and safely you need to use background threads/tasks. In other words:

stdout capture method using infinite timeout is flaky
StandardOutput StreamReader yes no
StandardOutput StreamReader no no
OutputDataReceived event yes no
OutputDataReceived event no yes

I think this a problem because it's pretty easy to step into this pitfall. Using the events to capture output is very convenient since you don't have to worry about making background threads or tasks. Also, the internet recommends this method:

I have only tried this on Windows, .NET Framework 4.5 and .NET Core (netcoreapp1.0). Also, all of this information is related to stdout, but I imagine stderr has the same problem. My dotnet info is:

Microsoft .NET Core Shared Framework Host

  Version  : 1.0.1
  Build    : cee57bf6c981237d80aa1631cfe83cb9ba329f12

> dotnet --version
1.0.0-preview2-003131
@joelverhagen
Copy link
Member Author

I've updated the test app to support non-Windows systems (by using echo):
processredirect2.zip.

The results are the same for Windows, Linux, and Mac OS X.

@stephentoub
Copy link
Member

stephentoub commented Oct 7, 2016

@joelverhagen, for better or worse, what you're seeing is documented design...

For Process.WaitForExit():
https://msdn.microsoft.com/en-us/library/fb4aw7b8(v=vs.110).aspx

This overload ensures that all processing has been completed, including the handling of asynchronous events for redirected standard output. You should use this overload after a call to the WaitForExit(Int32) overload when standard output has been redirected to asynchronous event handlers.

And for Process.WaitForExit(Int32):
https://msdn.microsoft.com/en-us/library/ty0d8k56(v=vs.110).aspx

When standard output has been redirected to asynchronous event handlers, it is possible that output processing will not have completed when this method returns. To ensure that asynchronous event handling has been completed, call the WaitForExit() overload that takes no parameter after receiving a true from this overload.

You can see here that the code explicitly does not wait for the redirected output to complete if it was passed a non-infinite timeout:

That said, I don't know why it was designed this way.

@Priya91
Copy link
Contributor

Priya91 commented Dec 7, 2016

@joelverhagen Using WaitForExit without timeout after the other overload should unblock you, as pointed out in the docs. Closing this issue, please reopen if further assistance needed.

@Priya91 Priya91 closed this as completed Dec 7, 2016
@msftgits msftgits transferred this issue from dotnet/corefx Jan 31, 2020
@msftgits msftgits added this to the 2.0.0 milestone Jan 31, 2020
@dotnet dotnet locked as resolved and limited conversation to collaborators Dec 28, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

4 participants