(Hi, errgroup author here. Comments inline below.) On Thursday, October 25, 2018 at 7:40:43 AM UTC-4, Andrew Chambers wrote: > > @Sebastien > > The biggest difference is my package makes guarantees that when a bundle > is garbage collected the context and thus child goroutines > are cancelled. This lets you be more creative having shared ownership of > groups of goroutines, e.g. a collection of goroutines producing a lazy > infinite series on a channel, which would be safely closed once all > references to the bundle are gone. >
I don't think I understand your use-case here. If you're producing an infinite series on a channel, what prevents the bundle from being collected while some goroutine is still reading on that channel? It seems like you always need some higher-level structure to own both the bundle and the communication mechanism, and at that point you can toss a `Close` method on that structure and not need to worry about finalizers. On the other hand, relying on finalizers to clean up goroutines is itself quite risky. Per the documentation <http://go/godoc/runtime/#SetFinalizer>: - “If a cyclic structure includes a block with a finalizer, that cycle is not guaranteed to be garbage collected and the finalizer is not guaranteed to run, because there is no ordering that respects the dependencies.” - “The finalizer is scheduled to run at some arbitrary time after the program can no longer reach the object to which obj points. There is no guarantee that finalizers will run before a program exits […].” So if the thing you're storing the bundle in participates in a cycle, the goroutines might never be cancelled; if the goroutines in your bundle are actively performing work, you might waste an arbitrary amount of CPU before they finally stop. Moreover, if you don't have some explicit point of cancellation, you also don't have an explicit point at which you can block for the goroutines to return. So how will you test for goroutine leaks? With explicit cancellation it's easy: you cancel the bundle and wait for everything to return, and if one of the goroutines leaks or deadlocks then your test will hang — and you can send it SIGQUIT and look at the goroutine stacks to figure out what went wrong. In contrast, with the finalizer approach you can't distinguish between a goroutine leak and the collector just taking its time. (See also the first section of my 2018 GopherCon talk, *Rethinking Classical Concurrency Patterns* (slides) <https://drive.google.com/file/d/1nPdvhB0PutEJzdCq5ms6UI58dp50fcAN/view> (video) <https://www.youtube.com/watch?v=5zXAHh5tJqQ>.) errgroup seem to have a 'heavier' api, focused on aggregating errors, > If you aren't collecting errors, and *are* using synchronous functions to check for goroutine leaks, then the bundling doesn't provide much benefit over using context.WithCancel and a sync.WaitGroup separately: it drops the paired Add/Done boilerplate from the WaitGroup, and that's about it. `errgroup` has a heavier API because a thinner API doesn't provide enough benefit to justify another layer of abstraction. (See also chapter 4 of John Ousterhout's *A Philosophy of Software Design*.) > It also seems strange to me that the 'group' function in errgroup does not > accept a context which is idiomatic go. Being biased, I prefer my own > API, but actually, use whichever you prefer :) > In `errgroup`, the `ctx context.Context` parameter is omitted to reduce boilerplate: it is always the same for every function, so the caller only needs to obtain it once, and there is no need to repeat the same boilerplate at every call. Furthermore, it is sometimes useful to be able to make additional modifications (such as an additional cancellation layer or a watchdog timeout) for a subset of the goroutines in the group. So the explicit parameter would impose a cost (boilerplate) and remove a benefit (injecting finer-grained cancellation) while providing relatively little benefit of its own (a slightly lower likelihood of using the wrong context, which is already being reduced somewhat in https://golang.org/cl/134395). Plus, omitting the context parameter makes the zero `errgroup.Group` somewhat useful for aggregating errors without cancellation. (A context parameter passed in by a Group that was constructed without one would be misleading at best: it would suggest propagation of values when it only really provides cancellation. See also #28342 <https://golang.org/issue/28342>.) If the language didn't require so much boilerplate for each function parameter (see #21498 <https://golang.org/issue/21498>), the cost/benefit balance might swing back in favor of the explicit parameter. -- You received this message because you are subscribed to the Google Groups "golang-nuts" group. To unsubscribe from this group and stop receiving emails from it, send an email to golang-nuts+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout.
