Skip to content

bug: data race and panic (send on closed channel) between WorkerPool.Submit and Stop #3540

Description

@RajMandaliya

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.goSubmit, 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

  • I agree to follow NVIDIA Infra Controller's Code of Conduct
  • I have searched the open bugs and have found no duplicates for this bug report

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugA defect in existing software (deprecated - use issue type, but it's needed for reporting now)

    Type

    Fields

    No fields configured for Bug.

    Projects

    Status
    Triage

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions