Version
main
Describe the bug.
Describe the bug
WorkerPool.SubmitWithResult checks wp.stopped and then sends on wp.jobs. Stop() sets wp.stopped and then closes wp.jobs. Between the check and the send, Stop() can close the channel from another goroutine — this is a genuine data race (confirmed with go test -race) and a real, reproducible panic :
- panic: send on closed channel
This is a realistic pattern in general (any graceful shutdown racing against in-flight submissions), though I haven't found any current
caller of this package elsewhere in the repo — it looks like a general-purpose utility that isn't wired into a real service yet. So this is likely a latent bug in code intended for future use, rather than something actively causing incidents today. Still worth fixing before it's adopted somewhere, since the failure mode (an unrecovered panic) would be a real problem once it is.
Affected component
rest-api/flow/pkg/workerpool/workerpool.go — Submit, SubmitWithResult, Stop.
Minimum reproducible example
func TestWorkerPool_StressSubmitDuringStop(t *testing.T) {
for iter := 0; iter < 200; iter++ {
pool := New(&Config{MaxWorkers: 4, QueueSize: 10})
_ = pool.Start()
var wg sync.WaitGroup
for g := 0; g < 20; g++ {
wg.Add(1)
go func(id int) {
defer wg.Done()
for j := 0; j < 50; j++ {
_ = pool.Submit(newMockTask(fmt.Sprintf("t-%d-%d", id, j), 0, false))
}
}(g)
}
_ = pool.Stop() // concurrently, while Submit is still in flight
wg.Wait()
}
}
Run with: go test -race -run TestWorkerPool_StressSubmitDuringStop ./pkg/workerpool/...
Relevant log output
workerpool_test.go:478: Submit panicked: send on closed channel
workerpool_test.go:478: Submit panicked: send on closed channel
Suggested fix (verified)
Add a sync.RWMutex: Submit/SubmitWithResult take a read lock around the stopped-check-and-send (allowing concurrent submissions to proceed together); Stop takes a write lock around closing the jobs channel, so it can never run between another goroutine's check and send. Verified this resolves both the panic and the race, with the above stress test passing cleanly (200 iterations, under -race) after the fix, and the full existing test suite for this package still passing.
Minimum reproducible example
Relevant log output
Other/Misc.
No response
Code of Conduct
Version
main
Describe the bug.
Describe the bug
WorkerPool.SubmitWithResultcheckswp.stoppedand then sends onwp.jobs.Stop()setswp.stoppedand then closeswp.jobs. Between the check and the send,Stop()can close the channel from another goroutine — this is a genuine data race (confirmed withgo test -race) and a real, reproducible panic :This is a realistic pattern in general (any graceful shutdown racing against in-flight submissions), though I haven't found any current
caller of this package elsewhere in the repo — it looks like a general-purpose utility that isn't wired into a real service yet. So this is likely a latent bug in code intended for future use, rather than something actively causing incidents today. Still worth fixing before it's adopted somewhere, since the failure mode (an unrecovered panic) would be a real problem once it is.
Affected component
rest-api/flow/pkg/workerpool/workerpool.go—Submit,SubmitWithResult,Stop.Minimum reproducible example
Run with:
go test -race -run TestWorkerPool_StressSubmitDuringStop ./pkg/workerpool/...Relevant log output
workerpool_test.go:478: Submit panicked: send on closed channel
workerpool_test.go:478: Submit panicked: send on closed channel
Suggested fix (verified)
Add a
sync.RWMutex:Submit/SubmitWithResulttake a read lock around the stopped-check-and-send (allowing concurrent submissions to proceed together);Stoptakes a write lock around closing the jobs channel, so it can never run between another goroutine's check and send. Verified this resolves both the panic and the race, with the above stress test passing cleanly (200 iterations, under-race) after the fix, and the full existing test suite for this package still passing.Minimum reproducible example
Relevant log output
Other/Misc.
No response
Code of Conduct