From f5c6f58e679bb6c7d41beaf92ffac86fd11da38b Mon Sep 17 00:00:00 2001 From: Said Saifi Date: Thu, 31 Mar 2022 18:27:04 +0300 Subject: [PATCH] fix: Fix deadlock raised in #11 --- errgroup.go | 57 +++++++++++++++++++++++++++++++++++++++++------------ 1 file changed, 44 insertions(+), 13 deletions(-) diff --git a/errgroup.go b/errgroup.go index e40a4d0..8c33454 100644 --- a/errgroup.go +++ b/errgroup.go @@ -40,8 +40,10 @@ type Group struct { wg sync.WaitGroup - errOnce sync.Once - err error + err error + + // errMu protects err. + errMu sync.RWMutex // numG is the maximum number of goroutines that can be started. numG int @@ -154,13 +156,28 @@ func (g *Group) Go(f func() error) { return } - g.qCh <- f + for { + g.errMu.RLock() + if g.err != nil { + g.errMu.RUnlock() + g.qMu.Unlock() - // Check if we can or should start a new goroutine? - g.maybeStartG() + return + } - g.qMu.Unlock() + select { + case g.qCh <- f: + g.errMu.RUnlock() + g.maybeStartG() + g.qMu.Unlock() + return + default: + break + } + + g.errMu.RUnlock() + } } // maybeStartG might start a new worker goroutine, if @@ -204,16 +221,30 @@ func (g *Group) startG() { return } - if err := f(); err != nil { - g.errOnce.Do(func() { - g.err = err - if g.cancel != nil { - g.cancel() - } - }) + err := f() + if err == nil { + // happy path + continue + } + // an error exists + // checking if it's the first group error + g.errMu.Lock() + if g.err != nil { + // this is not the first group error + // no need to set it + g.errMu.Unlock() return } + + g.err = err + g.errMu.Unlock() + + if g.cancel != nil { + g.cancel() + } + + return } }() }