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: goroutine leak when ssh.Channel connection is closed early #16287

Closed
belak opened this issue Jul 7, 2016 · 2 comments
Closed

x/crypto/ssh: goroutine leak when ssh.Channel connection is closed early #16287

belak opened this issue Jul 7, 2016 · 2 comments
Milestone

Comments

@belak
Copy link

@belak belak commented Jul 7, 2016

  1. What version of Go are you using (go version)?
    go version go1.6.2 linux/amd64
  2. What operating system and processor architecture are you using (go env)?
GOARCH="amd64"
GOBIN=""
GOEXE=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOOS="linux"
GOPATH=""
GORACE=""
GOROOT="/home/kelwert/go1.6.2"
GOTOOLDIR="/home/kelwert/go1.6.2/pkg/tool/linux_amd64"
GO15VENDOREXPERIMENT="1"
CC="gcc"
GOGCCFLAGS="-fPIC -m64 -pthread -fmessage-length=0"
CXX="g++"
CGO_ENABLED="1"
  1. What did you do?

We've been working at rolling out a go-based ssh server to replace some old infra and we noticed that after handling a few million connections, the memory usage seems to go up from about 6MB to 30MB over the course of a day. This obviously isn't a very large leak, but it's definitely noticeable over a long period of time.

After investigating, I think we've narrowed it down to the fact that the only function which closes the channel's request channel (*channel.close()) is never called if a connection dies early. It is only closed if the channel gets a msgChannelClose in *channel.handlePacket.

This leaves any server implementation using ssh.DiscardRequests open to a goroutine leak because the channel that function is ranging over never gets closed and therefore the goroutine never exits even when the underlying connection (and in extension, ssh-channel) is closed.

I don't have a simple example yet, as writing ssh server implementation is not simple and distilling a bug down to a reproducible case isn't easy, but I'll update this when I have one.

  1. What did you expect to see?
    No memory leak.
  2. What did you see instead?
    A very small memory leak.

Additional notes:

*channel.Close never actually calls *channel.close. It simply sends a msgChannelClose.

@quentinmit quentinmit added this to the Unreleased milestone Jul 7, 2016
@ianlancetaylor
Copy link
Contributor

@ianlancetaylor ianlancetaylor commented Jul 7, 2016

@belak
Copy link
Author

@belak belak commented Jul 7, 2016

After trying to figure this out for a while, I'm fairly certain this was misdiagnosed.

In crypto/ssh/mux.go on line 194, all channels are closed if they're still open.

Sorry about that.

@belak belak closed this Jul 7, 2016
@golang golang locked and limited conversation to collaborators Jul 7, 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
4 participants
You can’t perform that action at this time.