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

clientconn: buffer waitC on Dial goroutine #717

Merged
merged 1 commit into from
Jun 8, 2016

Conversation

heyitsanthony
Copy link
Contributor

Leaks a goroutine on dial timeout if unbuffered.

/cc @xiang90

Leaks a goroutine on dial timeout if unbuffered.
@xiang90
Copy link
Contributor

xiang90 commented Jun 7, 2016

@heyitsanthony This change looks good to me.

@iamqizhao By reading the code, I found another issue. So if we have multiple addresses, the routine will try to connect them one by one until succeeds. However the main routine will exit at the first error. This seems to be broken.

@iamqizhao
Copy link
Contributor

LGTM

@xiang90 This spawned goroutine will also exit at the first error. Do I miss anything?

@xiang90
Copy link
Contributor

xiang90 commented Jun 7, 2016

@iamqizhao Should it exit at the first error? Or it should continue to try other address?

@iamqizhao
Copy link
Contributor

@xiang90 For simplicity, I let it exit at the first error now. I agree it is debatable what is the best thing to do here. Probably it should be exit if all the connections.

@xiang90
Copy link
Contributor

xiang90 commented Jun 7, 2016

@iamqizhao ACK. Works for me.

@iamqizhao iamqizhao merged commit e9855a1 into grpc:master Jun 8, 2016
@heyitsanthony heyitsanthony deleted the fix-dial-leak branch July 1, 2016 17:12
@lock lock bot locked as resolved and limited conversation to collaborators Jan 19, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants