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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Prevent a goroutine leak when healthcheck gets stopped #33781

Merged
merged 1 commit into from Jun 26, 2017

Conversation

Projects
None yet
6 participants
@mlaventure
Contributor

mlaventure commented Jun 22, 2017

Signed-off-by: Kenfe-Mickael Laventure mickael.laventure@gmail.com


ping @talex5 I couldn't understand why we don't want to wait for the probe to exit here, so I just put it in a go routine. However, the code is waiting if the context is cancelled a few lines below, can the same be done here?

- What I did

Prevent a go routine leak when a probe has been started and we're asked to stop healthchecks

- How I did it

Started a goroutine that will wait on the channel

- How to verify it

- Description for the changelog

Fix a case were healthcheck could leak a goroutine

- A picture of a cute animal (not mandatory but encouraged)

馃惗

@aaronlehmann

This comment has been minimized.

Show comment
Hide comment
@aaronlehmann

aaronlehmann Jun 22, 2017

Contributor

Started a goroutine that will wait on the channel

What about changing the channel to a buffered channel?

Contributor

aaronlehmann commented Jun 22, 2017

Started a goroutine that will wait on the channel

What about changing the channel to a buffered channel?

@mlaventure

This comment has been minimized.

Show comment
Hide comment
@mlaventure

mlaventure Jun 22, 2017

Contributor
Contributor

mlaventure commented Jun 22, 2017

@talex5

This comment has been minimized.

Show comment
Hide comment
@talex5

talex5 Jun 23, 2017

Contributor

I don't see any reason why it can't wait either (sorry for the poor comment in the code). Nothing is waiting for monitor() to finish as far as I can see. Looks like making it buffered would be fine too.

Contributor

talex5 commented Jun 23, 2017

I don't see any reason why it can't wait either (sorry for the poor comment in the code). Nothing is waiting for monitor() to finish as far as I can see. Looks like making it buffered would be fine too.

Prevent a goroutine leak when healthcheck gets stopped
Signed-off-by: Kenfe-Mickael Laventure <mickael.laventure@gmail.com>
@mlaventure

This comment has been minimized.

Show comment
Hide comment
@mlaventure

mlaventure Jun 23, 2017

Contributor

Alright, made it a blocking wait.

If we want to take it easy, we could make the channel buffered and not wait, but there's a risk that a few go routine would piles up before the context cancellation gets picked up upon.

Contributor

mlaventure commented Jun 23, 2017

Alright, made it a blocking wait.

If we want to take it easy, we could make the channel buffered and not wait, but there's a risk that a few go routine would piles up before the context cancellation gets picked up upon.

@cpuguy83

LGTM

@aaronlehmann

This comment has been minimized.

Show comment
Hide comment
@aaronlehmann

aaronlehmann Jun 26, 2017

Contributor

LGTM

Contributor

aaronlehmann commented Jun 26, 2017

LGTM

@aaronlehmann aaronlehmann merged commit da28210 into moby:master Jun 26, 2017

6 checks passed

dco-signed All commits are signed
experimental Jenkins build Docker-PRs-experimental 35239 has succeeded
Details
janky Jenkins build Docker-PRs 43846 has succeeded
Details
powerpc Jenkins build Docker-PRs-powerpc 4214 has succeeded
Details
windowsRS1 Jenkins build Docker-PRs-WoW-RS1 15183 has succeeded
Details
z Jenkins build Docker-PRs-s390x 3941 has succeeded
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment