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

clientv3: use waitgroup to wait for substream goroutine teardown #7582

Merged
merged 1 commit into from
Mar 23, 2017

Conversation

heyitsanthony
Copy link
Contributor

When a grpc watch stream is torn down, it will join on its logical substream
goroutines by waiting for each to close a channel. This doesn't guarantee
the substream is fully exited, though, but only about to exit and can be
waiting to resume even after Watch.Close finishes. Instead, use a
waitgroup.Done at the very end of the substream defer.

Fixes #7573

reproduced reliably by allocating 10x more watch channels in TestWatchStressResumeClose

When a grpc watch stream is torn down, it will join on its logical substream
goroutines by waiting for each to close a channel. This doesn't guarantee
the substream is fully exited, though, but only about to exit and can be
waiting to resume even after Watch.Close finishes. Instead, use a
waitgroup.Done at the very end of the substream defer.

Fixes etcd-io#7573
@xiang90
Copy link
Contributor

xiang90 commented Mar 23, 2017

lgtm

1 similar comment
@fanminshi
Copy link
Member

lgtm

@heyitsanthony heyitsanthony merged commit 36ece32 into etcd-io:master Mar 23, 2017
@heyitsanthony
Copy link
Contributor Author

might be worth backporting, but kind of minor issue

@heyitsanthony heyitsanthony deleted the fix-watch-stream-leak branch March 23, 2017 19:25
@gyuho
Copy link
Contributor

gyuho commented Mar 23, 2017

@heyitsanthony Yeah was going to ask. Let's backport.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

TestWatchStressResumeClose: watch stream goroutine leak
4 participants