-
Notifications
You must be signed in to change notification settings - Fork 326
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’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
context cancel propagation does not work if all tasks are completed, but you haven't called Wait(). #78
Comments
Hi @serbrech! The problem with your example is you're reconfiguring it in every call of the for loop. Calling any of the ctx, cancel := signal.NotifyContext(context.Background(), os.Interrupt, os.Kill)
defer cancel()
p := pool.New().WithMaxGoroutines(20).WithContext(ctx)
for ctx.Err() == nil {
p.Go(func(ctx context.Context) error { return nil })
fmt.Println("pushed task")
}
p.Wait() However, |
That makes sense, and it now does behaves as I expected, thanks! I do think that setting up callbacks for errors/panics would be very useful, so that they can be surfaced/handled right away, and separately from the task itself. is that something you've considered? |
What did you have in mind here? In the case that you want to handle errors early with a callback, couldn't you just add that to the closure you're giving to Something like: errCallback := func(err error) {
println("live error", err)
}
p := pool.New().WithErrors()
for i := 0; i < 100; i++ {
p.Go(func() (err error) {
defer func() { if err != nil { errCallback(err) } }()
return thingThatMightError()
})
}
println("completed error", p.Wait()) |
Thanks! |
Thanks for the recommendation! An |
I just created #80 to track the addition of an |
repro:
ctrl+C is not exiting the program
sidenote:
I wrote this code thinking that, the pool would release a worker when a task is done.
I misunderstood the usecase of conc. I thought I could use it as a simple concurrency limiter/backpressure, and continuously push task to it as fast as it can handle them.
the fact that I have to call Wait makes it more of a batch prossessing helper than a concurrency helper.
I looked at Stream, and was surprised that it also required to call Wait().
The text was updated successfully, but these errors were encountered: