Skip to content

Fix leak of handleTableEvents #1787

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.

Sign up for GitHub

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

Jump to bottom
Merged
mavenugo merged 1 commit into from
Jun 6, 2017
Merged

Fix leak of handleTableEvents #1787

mavenugo merged 1 commit into from
Jun 6, 2017

Conversation

@fcrisciani
Copy link

@fcrisciani fcrisciani commented May 31, 2017 •
edited
Loading

The channel ch.C is never closed, so the condition ok in the if will never be false.
Added the listen of the ch.Done() to guarantee
that the goroutine is exiting once the event channel
is closed

Signed-off-by: Flavio Crisciani flavio.crisciani@docker.com

@fcrisciani
Copy link
Author

fcrisciani commented May 31, 2017 •
edited
Loading

Stacktrace related to 17.03 (line not matching on master code):

goroutine 1140550 [chan receive, 8614 minutes]:
github.com/docker/docker/vendor/github.com/docker/libnetwork.(*controller).handleTableEvents(0xc4208d6f00, 0xc424222660, 0xc423b0ca60)
	/root/rpmbuild/BUILD/docker-ee/.gopath/src/github.com/docker/docker/vendor/github.com/docker/libnetwork/agent.go:595 +0x5c
created by github.com/docker/docker/vendor/github.com/docker/libnetwork.(*network).addDriverWatches
	/root/rpmbuild/BUILD/docker-ee/.gopath/src/github.com/docker/docker/vendor/github.com/docker/libnetwork/agent.go:555 +0x317

@fcrisciani fcrisciani mentioned this pull request May 31, 2017
Merged
Fix leak of handleTableEvents
1045cfe 
The channel ch.C is never closed.
Added the listen of the ch.Done() to guarantee
that the goroutine is exiting once the event channel
is closed

Signed-off-by: Flavio Crisciani <flavio.crisciani@docker.com>
@aaronlehmann
Copy link

aaronlehmann commented Jun 6, 2017

LGTM (not a maintainer)

Hoping we can get this into 17.06

@aaronlehmann aaronlehmann mentioned this pull request Jun 6, 2017
Closed
23 tasks
@fcrisciani
Copy link
Author

fcrisciani commented Jun 6, 2017

@mavenugo can we let this one go in?

@mavenugo
Copy link
Contributor

mavenugo commented Jun 6, 2017

LGTM

@mavenugo mavenugo merged commit 0bce3e1 into moby:master Jun 6, 2017
@andrewhsu andrewhsu mentioned this pull request Jun 6, 2017
Closed
40 tasks
@vovimayhem vovimayhem mentioned this pull request Jun 7, 2017
Closed
@mavenugo mavenugo mentioned this pull request Jun 7, 2017
Merged
@fcrisciani fcrisciani deleted the goroutine_leak branch June 8, 2017 22:40
@andrewhsu andrewhsu mentioned this pull request Jun 9, 2017
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@fcrisciani @aaronlehmann @mavenugo