Skip to content

Commit 5eff187

Browse files
test(daemon): stop what a worker left running after it was reaped
A daemon worker is launched into its own process group, but Kill and the Cancel hook pass its PID to background.TerminateProcess, which looks the group up again with Getpgid when it signals. Once the worker has exited and been waited, that lookup fails, only the dead PID is signalled, and a child the worker left running survives. This pins the case with a shell worker that backgrounds a sleep and exits: after the wait, Kill and the Cancel hook must still stop the sleep. Committed ahead of the fix so CI shows it failing on the current behaviour on Linux and macOS.
1 parent 99721c7 commit 5eff187

1 file changed

Lines changed: 90 additions & 0 deletions

File tree

Lines changed: 90 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,90 @@
1+
//go:build !windows
2+
3+
package daemon
4+
5+
import (
6+
"context"
7+
"errors"
8+
"os/exec"
9+
"strconv"
10+
"strings"
11+
"syscall"
12+
"testing"
13+
"time"
14+
)
15+
16+
// A WORKER'S CHILDREN ARE STOPPED THROUGH THE GROUP IT WAS LAUNCHED INTO.
17+
//
18+
// Kill and the Cancel hook used to hand the worker's PID to
19+
// background.TerminateProcess, which looks the group up again with Getpgid when it
20+
// signals. Once the worker has exited and been waited, that lookup fails, only the
21+
// dead PID is signalled, and whatever the worker left running survives; on Darwin
22+
// the lookup already fails while the exited worker is still a zombie. The worker
23+
// here is a shell that backgrounds a sleep, prints its PID and exits, so after the
24+
// wait the sleep is the only process left in the worker's group. Reported in #861.
25+
func TestExecWorkerStopsWhatItLeftRunningAfterItExits(t *testing.T) {
26+
for _, stop := range []struct {
27+
name string
28+
run func(*execWorker) error
29+
}{
30+
{"Kill", func(worker *execWorker) error { return worker.Kill() }},
31+
{"Cancel hook", func(worker *execWorker) error { return worker.cmd.Cancel() }},
32+
} {
33+
t.Run(stop.name, func(t *testing.T) {
34+
launcher, err := NewExecLauncher(ExecLauncherConfig{
35+
Executable: "/bin/sh",
36+
BaseArgs: []string{"-c", "sleep 300 >/dev/null 2>&1 & echo $!"},
37+
})
38+
if err != nil {
39+
t.Fatalf("NewExecLauncher: %v", err)
40+
}
41+
handle, err := launcher(context.Background(), WorkerSpec{})
42+
if err != nil {
43+
t.Fatalf("launch: %v", err)
44+
}
45+
worker, ok := handle.(*execWorker)
46+
if !ok {
47+
t.Fatalf("SETUP INVALID: the launcher returned %T, not the exec worker under test", handle)
48+
}
49+
line, ok, err := worker.Stdout().Next()
50+
if err != nil || !ok {
51+
t.Fatalf("read the child's pid: ok=%v err=%v", ok, err)
52+
}
53+
childPID, err := strconv.Atoi(strings.TrimSpace(line))
54+
if err != nil {
55+
t.Fatalf("parse the child's pid %q: %v", line, err)
56+
}
57+
t.Cleanup(func() { _ = syscall.Kill(childPID, syscall.SIGKILL) })
58+
59+
if _, err := worker.Wait(); err != nil {
60+
t.Fatalf("wait for the worker: %v", err)
61+
}
62+
if childStopped(childPID) {
63+
t.Fatalf("SETUP INVALID: the worker's child %d is not running, so there is nothing left to stop", childPID)
64+
}
65+
66+
// The pool ignores what Kill returns; the outcome is what matters.
67+
_ = stop.run(worker)
68+
69+
deadline := time.Now().Add(5 * time.Second)
70+
for !childStopped(childPID) {
71+
if time.Now().After(deadline) {
72+
t.Fatalf("the worker's child %d survived %s after the worker was reaped", childPID, stop.name)
73+
}
74+
time.Sleep(20 * time.Millisecond)
75+
}
76+
})
77+
}
78+
}
79+
80+
// childStopped reports whether pid is gone or only waiting to be reaped.
81+
func childStopped(pid int) bool {
82+
if errors.Is(syscall.Kill(pid, syscall.Signal(0)), syscall.ESRCH) {
83+
return true
84+
}
85+
state, err := exec.Command("ps", "-o", "stat=", "-p", strconv.Itoa(pid)).Output()
86+
if err != nil {
87+
return errors.Is(syscall.Kill(pid, syscall.Signal(0)), syscall.ESRCH)
88+
}
89+
return strings.HasPrefix(strings.TrimSpace(string(state)), "Z")
90+
}

0 commit comments

Comments
 (0)