-
Notifications
You must be signed in to change notification settings - Fork 195
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
cmux leaks goroutines #55
Comments
We can reproduce this fairly easily also, I'm looking into this but if you have something in mind and need some testing, I can give access to a vm which can reproduce the leak. |
Thanks for the report. Do you know which cmux commit (git sha1) is vendored in your project? AFAICT, you're using cmux on a unix socket and the only way to get around this is to add a read timeout to cmux. I'll figure something out for that. |
Please try setting a reasonable read timeout: |
Thanks! I'll try that and report back! |
BTW we're using your latest release, which timeout do you think is reasonable though? |
It really depends on the characteristics of that unix socket. The one above is blocked on read for 215 minutes. I'd say something about 1-2 minutes should be enough to give up on the unix socket. |
thanks I'll try with a 1 minute timeout |
unfortunately even setting a read timeout isn't helping:
|
All of these go routines are stuck in IO wait? can you send me the stack trace of a couple of them? I'm not sure how to reproduce. |
I can send you a stack trace but they look exactly the same as the one I pasted in the first comment |
do you still see "215 minutes" of poll time? if yes, there might be a bug in how we set read timeout in cmux. |
I'll provide more goroutine stacks tomorrow but I'm pretty sure the goroutine counts wasn't lowering after the read timeout (in my case I set 1 minute). So maybe read timeout isn't fully respected. |
Oh wait. I just took a deeper look and it seems like that the go-routine isn't leaked by cmux and in fact cmux has already multiplexed the connection to the actual listener. As you can see in the stack trace, the "src/net/textproto/reader.go" is trying to read the proto payload from the connection. The reason that you see cmux in the stack trace is that cmux wraps the connection to pass the sniffed payload first to Read calls. In this case, the sniffed payload was read and the proto library is still trying to read from the connection. This is exactly why setting the read timeout on cmux isn't effective. The read connection timeout is only for when cmux is reading from the connection. IMO, this is not a cmux issue but rather an issue on your listener. You probably want to set read timeout when your listener accepts the connection. Here is the annotated stacktrace:
|
So the listener is a normal golang unix listener, should I set the read timeout there then? |
Yes, exactly. On the connection returned from listener.Accept(), you need to set https://golang.org/pkg/net/#UnixConn.SetReadDeadline |
wait, how do we set that? Our low is basically:
|
actually, we can set |
alright, that seems to be draining goroutines, I set it to 5 * time.Second and it seems to work :) Thanks! |
Great! :-) |
Hey there, we're using cmux to serve HTTP and gRPC together but our application is receiving a very large amount of requests (both gRPC and HTTP). We see tons of goroutines leaking around like:
Do you guys know what's happening?
The text was updated successfully, but these errors were encountered: