Skip to content

Commit

Permalink
Close tasks channel after WaitGroup.Wait
Browse files Browse the repository at this point in the history
We could potentially run into an issue with writing to WorkerPool.tasks
in case the channel is closed concurrently:

  goroutine 1:                         goroutine 2:
   Submit                               Close
    -> wp.wg.Add(1)
    -> scheduled by runtime
                                        -> close(wp.tasks)
                                        -> scheduled by runtime
    -> wp.tasks <- &task{...}
    -> panic: write to closed channel

The situation can be provoked in some cases using the following test:

```go
func TestSubmitCloseConcurrent(t *testing.T) {
       n := runtime.NumCPU()
       wp := New(n)
       for i := 0; i < n+2; i++ {
               go func() {
                       _ = wp.Submit("", func() error {
                               time.Sleep(time.Millisecond)
                               return nil
                       })
               }()
       }
       wp.Close()
}
```

Avoid the panic case by closing the tasks channel after waiting for the
WaitGroup.

The reproducing test is not added yet, as there seem to be other
concurrency issues present which the test triggers.

Signed-off-by: Tobias Klauser <tobias@isovalent.com>
  • Loading branch information
tklauser authored and rolinh committed Feb 2, 2021
1 parent 625c1b2 commit 5ea8879
Showing 1 changed file with 1 addition and 1 deletion.
2 changes: 1 addition & 1 deletion workerpool.go
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,7 @@ func (wp *WorkerPool) Close() error {
return nil
}
wp.closed = true
close(wp.tasks)
wp.wg.Wait()
close(wp.tasks)
return nil
}

0 comments on commit 5ea8879

Please sign in to comment.