just had to drop an update, I further improved it, now when it stops it resets itself and you can use Add again, I removed the unnecessary positive/negative checks and condensed the Add and Done function into an Add that can take a negative (tried to think of a better word but Modify and Change... just didn't quite fit)
https://play.golang.org/p/cO3sV1w9-Re One could add back separate Add and Done functions, even conveniences that just inc 1 dec 1 for each but I don't see the point in that. It just simply allows you to make Wait block while the count is above zero, which is all you need to prevent goroutine leaks. On Sunday, 5 May 2019 17:18:52 UTC+2, Louki Sumirniy wrote: > > I figured out that the API of my design had a flaw separating starting the > goroutine and adding a new item, so as you can see in this code, I have > merged them and notice that there is basically no extraneous 'helper' > functions also: > > https://play.golang.org/p/hR1sTOAwkOm > > The flaw I made relates to API abuse - the contract of the library is > quite simply to concurrently keep track of the adjacent 'go' calls starting > goroutines, which doesn't apply until there is an increment. So you should > not be able to abuse this one the way you did the last. > > I'm pretty sure at somewhere between 50 and 100 worker routines the lack > of complex extra code makes this a better implementation. Probably the > original is fine for smaller worker pools. > > On Sunday, 5 May 2019 13:01:58 UTC+2, Jan Mercl wrote: >> >> On Sun, May 5, 2019 at 12:45 PM Louki Sumirniy >> <louki.sumi...@gmail.com> wrote: >> > >> > https://play.golang.org/p/5KwJQcTsUPg >> > >> > I fixed it. >> >> Not really. You've introduced a data race. >> >> jnml@4670:~/src/tmp> cat main.go >> package main >> >> type WaitGroup struct { >> workers int >> ops chan int >> } >> >> func New() *WaitGroup { >> wg := new(WaitGroup) >> return wg >> } >> >> func startWait(wg *WaitGroup) { >> wg.ops = make(chan int) >> >> go func() { >> done := false >> for !done { >> select { >> case op := <-wg.ops: >> wg.workers += op >> if wg.workers < 1 { >> done = true >> close(wg.ops) >> } >> } >> } >> }() >> } >> >> // Add adds a non-negative number >> func (wg *WaitGroup) Add(delta int) { >> if delta < 0 { >> return >> } >> if wg.ops == nil { >> startWait(wg) >> } >> wg.ops <- delta >> } >> >> // Done subtracts a non-negative value from the workers count >> func (wg *WaitGroup) Done(delta int) { >> if delta < 0 { >> return >> } >> wg.ops <- -delta >> } >> >> // Wait blocks until the waitgroup decrements to zero >> func (wg *WaitGroup) Wait() { >> for { >> if wg.workers < 1 { >> break >> } >> op, ok := <-wg.ops >> if !ok { >> break >> } else { >> wg.ops <- op >> } >> } >> } >> >> var wg = New() >> >> func main() { >> for i := 0; i < 2; i++ { >> wg.Add(1) >> go f() >> } >> wg.Wait() >> } >> >> func f() { >> defer wg.Done(1) >> >> for i := 0; i < 2; i++ { >> wg.Add(1) >> go g() >> } >> } >> >> func g() { wg.Done(1) } >> jnml@4670:~/src/tmp> go run -race main.go >> ================== >> WARNING: DATA RACE >> Read at 0x00c00008a000 by main goroutine: >> main.(*WaitGroup).Wait() >> /home/jnml/src/tmp/main.go:53 +0x6f >> main.main() >> /home/jnml/src/tmp/main.go:72 +0x104 >> >> Previous write at 0x00c00008a000 by goroutine 5: >> main.startWait.func1() >> /home/jnml/src/tmp/main.go:21 +0xbb >> >> Goroutine 5 (running) created at: >> main.startWait() >> /home/jnml/src/tmp/main.go:16 +0x9e >> main.main() >> /home/jnml/src/tmp/main.go:37 +0xdf >> ================== >> Found 1 data race(s) >> exit status 66 >> jnml@4670:~/src/tmp> >> > -- 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.
