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

x/crypto/ssh: data race if session.Stdout and Stderr are the same writer #5582

Closed
davecheney opened this issue May 29, 2013 · 7 comments
Closed
Milestone

Comments

@davecheney
Copy link
Contributor

@davecheney davecheney commented May 29, 2013

https://golang.org/cl/9711043/ identified that the ssh package does not handle
the case where the Stdout and Stderr point to the same writer, as in the case of the
CombinedOutput() helper function.

This was worker around by introducing a blocking wrapper, however the proper solution is
to handle this common case the way os/exec handles it.
@davecheney
Copy link
Contributor Author

@davecheney davecheney commented Jun 1, 2013

Comment 1:

I had a quick look this afternoon and although they share the same API, os/exec an ssh
share a different underlying implementation. When os/exec detects that stdout and stderr
are the same writer, it passes the pipe representing stdout to the underlying exec as
stderr. 
ssh is a little different, we have two handlers for stdout and strerr, this goes all the
way up to the channel level, and they are established quite early. The easiest thing
will probably be to keep kr's singleWriter but make it internal.
@hanwen
Copy link
Contributor

@hanwen hanwen commented Oct 26, 2013

Comment 2:

should this bugreport be closed then?
@rsc
Copy link
Contributor

@rsc rsc commented Nov 27, 2013

Comment 3:

Labels changed: added go1.3maybe.

@rsc
Copy link
Contributor

@rsc rsc commented Dec 4, 2013

Comment 4:

Labels changed: added release-none, removed go1.3maybe.

@rsc
Copy link
Contributor

@rsc rsc commented Dec 4, 2013

Comment 5:

Labels changed: added repo-crypto.

@mikioh mikioh changed the title go.crypto/ssh: data race if session.Stdout and Stderr are the same writer ssh: data race if session.Stdout and Stderr are the same writer Jan 7, 2015
@rsc rsc added this to the Unplanned milestone Apr 10, 2015
@rsc rsc removed priority-soon labels Apr 10, 2015
@rsc rsc changed the title ssh: data race if session.Stdout and Stderr are the same writer x/crypto/ssh: data race if session.Stdout and Stderr are the same writer Apr 14, 2015
@rsc rsc modified the milestones: Unreleased, Unplanned Apr 14, 2015
@rsc rsc removed the repo-crypto label Apr 14, 2015
sarahhodne added a commit to travis-ci/worker that referenced this issue Sep 22, 2015
Apparently there's a data race when stdout and stderr is the same
writer: golang/go#5582. We're gonna attempt to work around it by letting
the library generate pipes for both stdout and stderr and then using
io.Copy to copy all of the data back to the log writer.
@odeke-em
Copy link
Member

@odeke-em odeke-em commented Oct 10, 2016

@hanwen @davecheney @rsc any thoughts on this bug?

@hanwen
Copy link
Contributor

@hanwen hanwen commented Oct 10, 2016

I'm pretty this is obsolete, but if you can repro a race, I can look into fixing it.

@hanwen hanwen closed this Oct 17, 2016
@golang golang locked and limited conversation to collaborators Oct 17, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
5 participants
You can’t perform that action at this time.