Plug a goroutine leak.#661
Conversation
|
Question: are we confident that readers are going away because the context is cancelled? Are there any other conditions where the reader may stop reading, but the context continues to be valid? |
| eventChan <- event | ||
| select { | ||
| case eventChan <- event: | ||
| case <-ctx.Done(): |
There was a problem hiding this comment.
While this may fix the issue if the caller context is correctly canceled, I believe it should have its own context that gets canceled on Stop(). I believe we should create a derived context with WithCancel, then make Stop() cancel it, in addition to the embedded resp.Stop()
There was a problem hiding this comment.
Yeah... I agree. Relying on the embedded resp's Stop() to be called and handle everything seems fragile. watcher should implement its own Stop that ensures the context is cancelled to stop the goroutine.
|
@aruiz14 Would you be able to take over from me? I only see how it's used from |
Signed-off-by: Brad Davidson <brad.davidson@rancher.com>
brandond
force-pushed
the
652-plug-goroutine-leak
branch
from
d529bc7 to
4166a88
Compare
|
@ericpromislow @aruiz14 how about that? |
4 participants
Applied @vvlisn's suggested fix from 652