conc
conc copied to clipboard
WithTimeout
Is it possible to add the wg.GoWithTimeout method, each coroutine needs to control the timeout period, or wg.WaitWithTimeout() The overall time-consuming control.
@stoneworld please see PR, https://github.com/sourcegraph/conc/pull/18.
Hi @stoneworld! Great question. The short answer is: probably not.
One guarantee of conc is if you spawn a goroutine with wg.Go(), it will be cleaned up when by the time wg.Wait() returns. This is the "scoped" part of "scoped concurrency". This is what allows us to propagate panics consistently.
Goroutines can't be killed, the only way to unblock the caller for a method like wg.WaitWithTimeout() is to leak the goroutines that are still running. This violates the scoping guarantees above. It also means that, if one of your goroutines panics, the panic is just ignored.
The recommended way of handling cancellation is to pass a context in to your tasks either capturing with a closure or using pool.New().WithContext(ctx). This will let you cancel your tasks early while still guaranteeing that there are no goroutine leaks.
func main() {
ctx, cancel := context.WithTimeout(context.Background(), time.Second)
defer cancel()
p := pool.New().WithContext(ctx)
p.Go(func(ctx context.Context) error {
<-ctx.Done() // exit once context is canceled
return ctx.Err()
})
fmt.Printf("%s\n", p.Wait())
}
This is not to say that leaking goroutines is always incorrect -- it's just not correct in conc. If you want to do this, it's possible to build this on top of conc, but it leaves the burden on the author to verify that it's okay to leak goroutines.
Example:
func main() {
ctx, cancel := context.WithTimeout(context.Background(), time.Second)
defer cancel()
var wg conc.WaitGroup
wg.Go(func() {
panic("test")
})
waitContext(ctx, &wg)
}
func waitContext(ctx context.Context, wg *conc.WaitGroup) {
done := make(chan any)
go func() {
defer func() { done <- recover() }()
wg.Wait()
}()
select {
case <-ctx.Done():
case panicVal := <-done:
if panicVal != nil {
panic(panicVal)
}
}
}
Closing this since I don't expect there to be a way around the issues brought up above. I'm happy to continue discussion if there is a way to make the above pattern more ergonomic without sacrificing the mentioned guarantees.
Closing this since I don't expect there to be a way around the issues brought up above. I'm happy to continue discussion if there is a way to make the above pattern more ergonomic without sacrificing the mentioned guarantees.
Thanks