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

os/exec: fix StdinPipe and maybe StdoutPipe #6270

Closed
robpike opened this issue Aug 28, 2013 · 4 comments
Closed

os/exec: fix StdinPipe and maybe StdoutPipe #6270

robpike opened this issue Aug 28, 2013 · 4 comments
Milestone

Comments

@robpike
Copy link
Contributor

robpike commented Aug 28, 2013

See also issue #4290.

StdinPipe (for instance) returns a WriteCloser but then closes it in Wait. That's bad
design: a package that returns a Closer shouldn't then close it for you silently.

But the real problem is that this property makes it impossible to use StdinPipe
correctly to deliver data to the process, since any program attempting to write to the
pipe to deliver input to the process must close the file descriptor itself to signal
EOF. There is a deadly embrace: The process won't exit until EOF; the EOF won't be
delivered until the process exits.

I believe StdinPipe should be deprecated, and a new method (call it XXX) that returns a
WriteCloser, but that Wait does not close that file descriptor. It would be the
well-documented responsibility of the client to close XXX explicitly.

I haven't thought through StdoutPipe but perhaps it has a related issue.
@robpike
Copy link
Contributor Author

robpike commented Aug 28, 2013

Comment 1:

New possibility: Have StdinPipe return a custom WriteCloser that closes only once (under
lock of course). That way either the client can close or or Wait can, race- and
problem-free. This would require no API change.

Labels changed: added go1.2, removed go1.2maybe.

@adg
Copy link
Contributor

adg commented Aug 28, 2013

Comment 3:

Owner changed to @adg.

Status changed to Started.

@bradfitz
Copy link
Contributor

Comment 4:

Re comment #1: I proposed that 10 months ago:
https://golang.org/cl/6789043/#msg7
But there was concern that we couldn't change the underlying type away from being an
*os.File: https://golang.org/cl/6789043/#msg8 etc

@adg
Copy link
Contributor

adg commented Aug 29, 2013

Comment 5:

This issue was closed by revision 10e2ffd.

Status changed to Fixed.

@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 25, 2016
@rsc rsc unassigned adg Jun 22, 2022
@golang golang unlocked this conversation Jun 21, 2023
@golang golang locked as resolved and limited conversation to collaborators Jun 21, 2023
This issue was closed.
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

5 participants