[13.4-stable] pillar/devicenetwork: fix goroutine leak #4425
Conversation
| resolvedIPsChan <- response | ||
| if response != nil && len(response) > 0 { | ||
| select { | ||
| case resolvedIPsChan <- response: |
There was a problem hiding this comment.
Sorry I didn't check your latest changes before they were merged, but this will panic if resolvedIPsChan is already closed.
https://go.dev/play/p/I96AZ_2-kcf
It was better when select was used between this channel and reading quit, while resolvedIPsChan was left for GC.
There was a problem hiding this comment.
Thx, I will change it.
There was a problem hiding this comment.
@milan-zededa go back to use quit or go back and let the GC close resolvedIPsChan?
There was a problem hiding this comment.
I think we need both but maybe you have simpler solution in mind.
There was a problem hiding this comment.
Actually @OhmSpectator and I discussed and it seems, no change is necessary, because resolvedIPsChan will only be closed after
defer func() {
close(quit)
<-wgChan
}()
which will only finish after
go func() {
wg.Wait()
close(wgChan)
}()
which will only finish after all goroutines of the waitgroup finished and that means, that no goroutine can write into resolvedIPsChan
There was a problem hiding this comment.
Ah, true, I missed this.
This function is not for the faint-hearthed :)
There was a problem hiding this comment.
No risk, no fun :-)
use fancy uber leak checker to check for leaks which have been fixed in the commit before Signed-off-by: Christoph Ostarek <christoph@zededa.com> (cherry picked from commit 145e98e)
introduced in 8573372
without this fix, the goroutine is blocked by trying to send into a channel that has no receiver
Signed-off-by: Christoph Ostarek christoph@zededa.com
(cherry picked from commit d93cf8b)