Skip to content

Commit 90fa389

Browse files
committed
workerpool: WaitGroup synchronization fix
Before this patch, Submit() would increment the WaitGroup without holding the WorkerPool lock. This could potentially lead to a panic when Submit() and Close() were executed concurrently: 1. The Submit() goroutine g0 acquire the lock, successfully check that the WorkerPool is neither closed nor draining, and release the lock. 2. The Close() goroutine g1 acquire the lock, update the WorkerPool state to closed, wait on the WaitGroup, and close the tasks channel (wrongly) assuming that all tasks have been submitted. 3. g0 increment the WaitGroup, and attempt to write to the (now closed) tasks channel leading to a panic. The same reasoning can be applied with Submit() and Drain() leading to a data race on the results slice. Signed-off-by: Alexandre Perrin <[email protected]>
1 parent 4cf96b5 commit 90fa389

File tree

1 file changed

+1
-2
lines changed

1 file changed

+1
-2
lines changed

workerpool.go

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -93,9 +93,8 @@ func (wp *WorkerPool) Submit(id string, f func(ctx context.Context) error) error
9393
wp.mu.Unlock()
9494
return ErrDraining
9595
}
96-
97-
wp.mu.Unlock()
9896
wp.wg.Add(1)
97+
wp.mu.Unlock()
9998
wp.tasks <- &task{
10099
id: id,
101100
run: f,

0 commit comments

Comments
 (0)