From 5ea8879d41d5543a0cca2f869f348a30a943ae36 Mon Sep 17 00:00:00 2001
From: Tobias Klauser <tklauser@distanz.ch>
Date: Tue, 2 Feb 2021 20:07:42 +0100
Subject: [PATCH] Close tasks channel after WaitGroup.Wait

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>
---
 workerpool.go | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/workerpool.go b/workerpool.go
index 6eb20be..221497d 100644
--- a/workerpool.go
+++ b/workerpool.go
@@ -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
 }