Closed chenyahui closed 2 years ago
Maybe we can invoke p.contextCancel()
before close(p.tasks)
? So that the purger goroutine has been closed when close(p.tasks)
.
func (p *WorkerPool) StopAndWait() {
p.stopOnce.Do(func() {
// Mark pool as stopped
p.stopped = true
// Terminate all workers & purger goroutine
p.contextCancel()
// Stop accepting new tasks
close(p.tasks)
// Wait for all workers to exit
p.waitGroup.Wait()
})
}
Calling p.contextCancel()
before close(p.tasks)
could prevent the panic under some circumstances, but there's still a (small) chance that the purger goroutine's select
statement decides to remove an idle worker after the tasks
channel is closed in the goroutine that called Stop
on the pool.
I opened a PR with a possible solution to this issue by avoiding closing the tasks channel altogether, which seems to be the recommended approach. Mind reviewing it and verifying if that prevents the panic? https://github.com/alitto/pond/pull/27
Hey @chenyahui!
Thanks for opening this PR! I think the issue you are trying to fix is definitely present, although I believe we can fix it in a slightly different way. Instead of closing the
tasks
channel inside the purger goroutine I would change it to do a non-blocking send to this channel, something like this (reference line):This way the lifecycle of the
tasks
channel is fully controlled by the struct that creates it (WorkerPool
). What do you think?