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

cmd/go: ssh ControlPersist freezes go get #13453

Closed
cdelorme opened this issue Dec 2, 2015 · 10 comments
Closed

cmd/go: ssh ControlPersist freezes go get #13453

cdelorme opened this issue Dec 2, 2015 · 10 comments
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Milestone

Comments

@cdelorme
Copy link

cdelorme commented Dec 2, 2015

  • What version of Go are you using (go version)?
    • go1.5 (built using gvm & go1.4.2)
  • What operating system and processor architecture are you using?
    • osx 10.9.5, intel core i7, amd64
  • What did you do?

I was trying to redirect go get to use the ssh address instead of the https address when cloning from a given address, as a proof-of-concept for use inside a company with a private repository host.

I added lines to my ~/.gitconfig to redirect git clone to use ssh for github.com:

[url "git@github.com:"]
    insteadOf = https://github.com/

Prior to this project I had been using a globally applied ssh ControlPath to persist and reuse ssh connections:

Host *
    ControlMaster auto
    ControlPath ~/.ssh/%r@%h:%p
    CompressionLevel 9
    ControlPersist 2h
    ServerAliveInterval 60

I verified that the ControlPersist setting is what leads to the unexpected behavior, but I don't understand why go get is effected.

  • What did you expect to see?
    • in a terminal I execute go get against any go repo
    • it downloads the source
    • my terminal is released when it finishes
  • What did you see instead?
    • I ran go get
    • the repository is cloned
    • my terminal remains busy
  • additional troubleshooting

I tried with many repositories, and verified that when I disable ControlPersist in my ~/.ssh/config my terminal is freed after the operation (as desired/expected), but then I no-longer have persisted ssh.

If I run git clone with ControlPersist enabled, everything works as expected, including my terminal being relinquished after it finishes cloning.

If I establish an ssh connection first (eg. ControlPath exists for a given host), then run go get with ControlPersist enabled, it works as expected. So once the master connection is established it works fine.

@cdelorme
Copy link
Author

cdelorme commented Dec 2, 2015

Verified the same behavior on linux (debian jessie), also core i7, amd64.

@ianlancetaylor ianlancetaylor changed the title ssh ControlPersist freezes go get cmd/go: ssh ControlPersist freezes go get Dec 2, 2015
@ianlancetaylor ianlancetaylor added this to the Unplanned milestone Dec 2, 2015
@ianlancetaylor
Copy link
Contributor

From #16104: we could consider setting GIT_SSH_COMMAND='ssh -o ControlMaster=no' before invoking git.

The explanation for the behavior is also in #16104: go get is waiting for standard error to be closed.

@davecheney
Copy link
Contributor

Nice find.

@josharian josharian modified the milestones: Go1.8, Unplanned Jun 28, 2016
@docwhat
Copy link

docwhat commented Jul 29, 2016

Why does go get wait for stderr to be closed? Can't it just pass in go get's stderr and not worry about it? That'd also let stderr pass out to the controlling process (e.g. the terminal).

@cdelorme
Copy link
Author

I use ControlMaster as a means of boosting ssh performance by reusing/multiplexing on a single established connection. It also deals with connection rate limiting.

I would much rather see a fix to how go get handles stderr than an override to a perfectly valid ssh configuration. The fact that git clone works fine alone clearly shows the problem is with how go get is dealing with its subprocess.

If the objective of go get is to suppress errors from git clone why not just use io.Discard as the stderr, or a bytes.Buffer to collect the output and decide whether to relay it to the tty after execution?

@quentinmit quentinmit added the NeedsFix The path to resolution is known, but the work has not been done. label Oct 6, 2016
@gopherbot
Copy link

CL https://golang.org/cl/31353 mentions this issue.

@nightlyone
Copy link
Contributor

I use ControlMaster as a means of boosting ssh performance by reusing/multiplexing on a single established connection. It also deals with connection rate limiting.

I use it that way too. The most common use-case here is @github itself.

The common workaround to fix this problem is to kill the master process controlling the socket. Unless I am seeing a completely different problem here.

Since the workaround is so simple and can even be automated for builders, I would rather like to have a go get which is not disabling my carefully chosen non-default ssh options.

@minux
Copy link
Member

minux commented Oct 19, 2016 via email

@cespare
Copy link
Contributor

cespare commented Oct 19, 2016

I asked about this behavior on the openssh mailing list a couple of years ago but never got a reply:

https://lists.mindrot.org/pipermail/openssh-unix-dev/2014-January/031975.html

I eventually stopped using controlmaster because of it. There are problems with a lot of software besides go get.

@pwaller
Copy link
Contributor

pwaller commented Oct 21, 2016

Nice find! I was wondering why things were hanging.

For what it's worth, it looks like a fix will be in OpenSSH 7.3. Good to see a fix go-side though, considering it will take a while before >=7.3 is the norm.

Link to patch, was applied April 2016.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Development

No branches or pull requests