Skip to content

os/exec: clarify requirements for stdin/stdout #6008

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

Closed
dvyukov opened this issue Aug 1, 2013 · 4 comments
Closed

os/exec: clarify requirements for stdin/stdout #6008

dvyukov opened this issue Aug 1, 2013 · 4 comments
Labels
Documentation Issues describing a change to documentation. FrozenDueToAge
Milestone

Comments

@dvyukov
Copy link
Member

dvyukov commented Aug 1, 2013

Here is code from golang-nuts:
        cmd := exec.Command("cat")
        in, _ := cmd.StdinPipe()
        out, _ := cmd.StdoutPipe()
        cmd.Start()
        go func() { defer os.Stdout.Close(); io.Copy(os.Stdout, out) }()
        go func() { defer in.Close(); io.Copy(in, os.Stdin) }()
        cmd.Wait()
https://groups.google.com/forum/#!topic/golang-nuts/gjkbC6M4cAU

This has 2 data races:
1. Both cmd and user close 'in'.
2. Cmd closes 'out', while user reads from it.

I've seen similar races in several projects.

os/exec documentation must clearly explain:
1. When it is allowed to close stdin/stdout (only after Wait?)
2. How to stream stdout (it's easy to race with stdout.Close in Wait)
3. How to close stdin to notify the process about end of input data (e.g. cat)
4. How/when to use thread-safe Reader/Writer

It all is basically impossible to get right first time.
@robpike
Copy link
Contributor

robpike commented Aug 4, 2013

Comment 1:

Labels changed: added priority-later, go1.2, removed priority-triage, go1.2maybe.

Status changed to Accepted.

@bradfitz
Copy link
Contributor

Comment 2:

See the somewhat unfortunate issue #4290 for history.
Don't use goroutines.

@rsc
Copy link
Contributor

rsc commented Sep 10, 2013

Comment 3:

Labels changed: added documentation.

@rsc
Copy link
Contributor

rsc commented Sep 13, 2013

Comment 4:

This issue was closed by revision 22e8f82.

Status changed to Fixed.

@dvyukov dvyukov added fixed Documentation Issues describing a change to documentation. labels Sep 13, 2013
@rsc rsc added this to the Go1.2 milestone Apr 14, 2015
@rsc rsc removed the go1.2 label Apr 14, 2015
@golang golang locked and limited conversation to collaborators Jun 24, 2016
This issue was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Documentation Issues describing a change to documentation. FrozenDueToAge
Projects
None yet
Development

No branches or pull requests

5 participants