fix(agent/reaper): stop reaper goroutine in tests to prevent ECHILD race (#22844)

Each ForkReap call started a reap.ReapChildren goroutine that never
stopped (done=nil). Goroutines accumulated across subtests, racing to 
call Wait4(-1, WNOHANG) and stealing the child's wait status before 
ForkReap's Wait4(pid) could collect it.

Add a WithDone option to pass the done channel through to ReapChildren,
and use it in tests via a withDone(t) helper.
This commit is contained in:
Mathias Fredriksson
2026-03-09 17:34:44 +00:00
committed by GitHub
parent 1a2eea5e76
commit 6e9e39a4e0
3 changed files with 24 additions and 1 deletions
+11
View File
@@ -42,9 +42,20 @@ func WithLogger(logger slog.Logger) Option {
}
}
// WithDone sets a channel that, when closed, stops the reaper
// goroutine. Callers that invoke ForkReap more than once in the
// same process (e.g. tests) should use this to prevent goroutine
// accumulation.
func WithDone(ch chan struct{}) Option {
return func(o *options) {
o.Done = ch
}
}
type options struct {
ExecArgs []string
PIDs reap.PidCh
CatchSignals []os.Signal
Logger slog.Logger
Done chan struct{}
}
+12
View File
@@ -18,6 +18,15 @@ import (
"github.com/coder/coder/v2/testutil"
)
// withDone returns an option that stops the reaper goroutine when t
// completes, preventing goroutine accumulation across subtests.
func withDone(t *testing.T) reaper.Option {
t.Helper()
done := make(chan struct{})
t.Cleanup(func() { close(done) })
return reaper.WithDone(done)
}
// TestReap checks that's the reaper is successfully reaping
// exited processes and passing the PIDs through the shared
// channel.
@@ -36,6 +45,7 @@ func TestReap(t *testing.T) {
reaper.WithPIDCallback(pids),
// Provide some argument that immediately exits.
reaper.WithExecArgs("/bin/sh", "-c", "exit 0"),
withDone(t),
)
require.NoError(t, err)
require.Equal(t, 0, exitCode)
@@ -89,6 +99,7 @@ func TestForkReapExitCodes(t *testing.T) {
t.Run(tt.name, func(t *testing.T) {
exitCode, err := reaper.ForkReap(
reaper.WithExecArgs("/bin/sh", "-c", tt.command),
withDone(t),
)
require.NoError(t, err)
require.Equal(t, tt.expectedCode, exitCode, "exit code mismatch for %q", tt.command)
@@ -118,6 +129,7 @@ func TestReapInterrupt(t *testing.T) {
exitCode, err := reaper.ForkReap(
reaper.WithPIDCallback(pids),
reaper.WithCatchSignals(os.Interrupt),
withDone(t),
// Signal propagation does not extend to children of children, so
// we create a little bash script to ensure sleep is interrupted.
reaper.WithExecArgs("/bin/sh", "-c", fmt.Sprintf("pid=0; trap 'kill -USR2 %d; kill -TERM $pid' INT; sleep 10 &\npid=$!; kill -USR1 %d; wait", os.Getpid(), os.Getpid())),
+1 -1
View File
@@ -64,7 +64,7 @@ func ForkReap(opt ...Option) (int, error) {
o(opts)
}
go reap.ReapChildren(opts.PIDs, nil, nil, nil)
go reap.ReapChildren(opts.PIDs, nil, opts.Done, nil)
pwd, err := os.Getwd()
if err != nil {