diff --git a/internal/background/process_posix.go b/internal/background/process_posix.go index 62a30dea..61f92e8b 100644 --- a/internal/background/process_posix.go +++ b/internal/background/process_posix.go @@ -39,3 +39,14 @@ func ConfigureChildProcessGroup(cmd *exec.Cmd) { func terminateProcess(pid int) error { return execution.TerminateProcessTree(pid, terminationGracePeriod, terminationPollInterval) } + +// terminateOwnedProcess terminates cmd's process for a caller that launched it +// through this package (ConfigureChildProcessGroup was called before Start), so +// cmd.Process.Pid is known — from launch time — to be its own process-group +// leader. It signals the group directly via execution.TerminateProcessGroup +// rather than terminateProcess's TerminateProcessTree, whose Getpgid +// rediscovery is fragile once the leader may have already exited (see +// TerminateProcessGroup's doc comment for why). +func terminateOwnedProcess(cmd *exec.Cmd) error { + return execution.TerminateProcessGroup(cmd.Process.Pid, terminationGracePeriod, terminationPollInterval) +} diff --git a/internal/background/process_posix_test.go b/internal/background/process_posix_test.go index fedd9205..83d6a704 100644 --- a/internal/background/process_posix_test.go +++ b/internal/background/process_posix_test.go @@ -147,14 +147,10 @@ func TestTerminateProcessKillsForkedChildren(t *testing.T) { t.Fatalf("terminateProcess: %v", err) } - // The forked child must be gone too (poll until reaped by init). + // The forked child must no longer be running. An orphaned zombie is already + // dead but may remain visible briefly until the platform's init reaps it. deadline := time.Now().Add(2 * time.Second) - for { - // Only ESRCH proves the child is gone; any other error (e.g. EPERM) would - // wrongly pass the test, so treat it as still-present and keep polling. - if errors.Is(syscall.Kill(childPID, syscall.Signal(0)), syscall.ESRCH) { - break // child no longer exists - } + for !processStopped(childPID) { if time.Now().After(deadline) { _ = syscall.Kill(childPID, syscall.SIGKILL) t.Fatalf("forked child %d survived terminateProcess — group kill failed", childPID) @@ -162,3 +158,54 @@ func TestTerminateProcessKillsForkedChildren(t *testing.T) { time.Sleep(20 * time.Millisecond) } } + +func TestTerminateCommandKillsChildAfterLeaderExits(t *testing.T) { + grace, poll := terminationGracePeriod, terminationPollInterval + terminationGracePeriod, terminationPollInterval = 2*time.Second, 20*time.Millisecond + t.Cleanup(func() { terminationGracePeriod, terminationPollInterval = grace, poll }) + + // The leader exits immediately after launching the child. TerminateCommand + // must capture and signal the process group before Wait reaps the leader and + // makes its group identity unavailable. + cmd := exec.Command("sh", "-c", "sleep 300 & echo $!; exit 0") + ConfigureChildProcessGroup(cmd) + stdout, err := cmd.StdoutPipe() + if err != nil { + t.Fatalf("stdout pipe: %v", err) + } + if err := cmd.Start(); err != nil { + t.Fatalf("start: %v", err) + } + line, err := bufio.NewReader(stdout).ReadString('\n') + if err != nil { + t.Fatalf("read forked child pid: %v", err) + } + childPID, err := strconv.Atoi(strings.TrimSpace(line)) + if err != nil { + t.Fatalf("parse forked child pid %q: %v", line, err) + } + + if err := TerminateCommand(cmd); err != nil { + t.Fatalf("TerminateCommand: %v", err) + } + + deadline := time.Now().Add(2 * time.Second) + for !processStopped(childPID) { + if time.Now().After(deadline) { + _ = syscall.Kill(childPID, syscall.SIGKILL) + t.Fatalf("forked child %d survived TerminateCommand", childPID) + } + time.Sleep(20 * time.Millisecond) + } +} + +func processStopped(pid int) bool { + if errors.Is(syscall.Kill(pid, syscall.Signal(0)), syscall.ESRCH) { + return true + } + state, err := exec.Command("ps", "-o", "stat=", "-p", strconv.Itoa(pid)).Output() + if err != nil { + return errors.Is(syscall.Kill(pid, syscall.Signal(0)), syscall.ESRCH) + } + return strings.HasPrefix(strings.TrimSpace(string(state)), "Z") +} diff --git a/internal/background/process_windows.go b/internal/background/process_windows.go index c248bc85..e9b14ef7 100644 --- a/internal/background/process_windows.go +++ b/internal/background/process_windows.go @@ -16,3 +16,11 @@ func ConfigureChildProcessGroup(cmd *exec.Cmd) { execution.ConfigureProcessGroup func terminateProcess(pid int) error { return execution.TerminateProcessTree(pid, 0, 0) } + +// terminateOwnedProcess terminates cmd's process. Windows has no process-group +// rediscovery concern — KillProcessTree always operates on the whole process +// tree via taskkill /T regardless of how the PID was obtained — so this is the +// same as terminateProcess. +func terminateOwnedProcess(cmd *exec.Cmd) error { + return terminateProcess(cmd.Process.Pid) +} diff --git a/internal/background/process_windows_test.go b/internal/background/process_windows_test.go new file mode 100644 index 00000000..1d5dc7ae --- /dev/null +++ b/internal/background/process_windows_test.go @@ -0,0 +1,30 @@ +//go:build windows + +package background + +import ( + "os/exec" + "testing" + "time" +) + +func TestTerminateCommandReapsExitedLeader(t *testing.T) { + cmd := exec.Command("cmd.exe", "/c", "exit", "0") + if err := cmd.Start(); err != nil { + t.Fatalf("start: %v", err) + } + // Let the child exit without calling Wait so cleanup exercises the Windows + // taskkill/TerminateProcess race while it still owns the process handle. + time.Sleep(500 * time.Millisecond) + + // jatmn's #774 finding: taskkill /T racing an already-exited PID (and the + // Process.Kill fallback) both fail here, but the reap below still succeeds — + // that's authoritative proof the leader is gone, so TerminateCommand must + // report success rather than surfacing the earlier kill-attempt error. + if err := TerminateCommand(cmd); err != nil { + t.Fatalf("TerminateCommand: %v, want nil (leader was already gone and got reaped)", err) + } + if cmd.ProcessState == nil { + t.Fatal("exited process was not reaped") + } +} diff --git a/internal/background/terminate.go b/internal/background/terminate.go index bc9ce7df..76dae318 100644 --- a/internal/background/terminate.go +++ b/internal/background/terminate.go @@ -1,5 +1,14 @@ package background +import ( + "errors" + "fmt" + "os/exec" + "time" +) + +const commandReapTimeout = 3 * time.Second + // TerminateProcess stops a background process by PID — on Windows its process // tree; on POSIX its whole process group when the PID leads its own group (the // invariant ConfigureChildProcessGroup establishes for processes started through @@ -12,3 +21,60 @@ package background func TerminateProcess(pid int) error { return terminateProcess(pid) } + +// TerminateCommand stops a started command's process tree/group AND reaps the +// leader, which TerminateProcess alone cannot do: a caller that still owns the +// exec.Cmd must Wait for it, or the exited leader lingers as a zombie for the +// lifetime of the parent. `zero daemon start` needs exactly this when readiness +// times out — it launched the child, so it must both stop the tree and collect +// the leader. +// +// The order matters: the tree is signalled first, because Wait releases the +// leader's PID and a later group lookup could then resolve to nothing (or, worse, +// to a recycled PID). Termination itself goes through terminateProcess, so this +// package keeps ONE cross-platform kill path (execution.TerminateProcessTree) +// rather than a second platform-specific killer beside it. +func TerminateCommand(cmd *exec.Cmd) error { + if cmd == nil || cmd.Process == nil { + return errors.New("terminate command: process was never started") + } + terminateErr := terminateOwnedProcess(cmd) + reapErr := waitForTerminatedCommandWithin(cmd, commandReapTimeout) + if reapErr != nil { + if terminateErr != nil { + return fmt.Errorf("%v (reap failed: %w)", terminateErr, reapErr) + } + return reapErr + } + // A successful reap is authoritative: cmd.Wait returned, so the leader is + // confirmed gone and collected. A terminateErr alongside that (e.g. a kill + // attempt racing a process that had already exited on its own, such as + // taskkill /T failing on Windows because the PID is already gone) describes + // how the attempt went, not whether the goal — the process is gone — was met. + return nil +} + +// classifyWaitError treats a non-zero exit status as success: a process being +// terminated is expected to report one (or a signal), so the only interesting +// failure is Wait itself not working. +func classifyWaitError(waitErr error) error { + var exitErr *exec.ExitError + if waitErr != nil && !errors.As(waitErr, &exitErr) { + return fmt.Errorf("reap process: %w", waitErr) + } + return nil +} + +// waitForTerminatedCommandWithin bounds the reap so a child that somehow survives +// termination cannot hang the caller — the daemon-start cleanup path runs while a +// user waits at the CLI. +func waitForTerminatedCommandWithin(cmd *exec.Cmd, timeout time.Duration) error { + waitDone := make(chan error, 1) + go func() { waitDone <- cmd.Wait() }() + select { + case waitErr := <-waitDone: + return classifyWaitError(waitErr) + case <-time.After(timeout): + return fmt.Errorf("process %d did not reap after termination", cmd.Process.Pid) + } +} diff --git a/internal/cli/daemon.go b/internal/cli/daemon.go index 5074312e..2dfebd0b 100644 --- a/internal/cli/daemon.go +++ b/internal/cli/daemon.go @@ -1,6 +1,7 @@ package cli import ( + "errors" "fmt" "io" "os" @@ -160,20 +161,56 @@ func runDaemonStartDetached(paths daemon.Paths, stdout io.Writer, stderr io.Writ cmd.Stdout = logFile cmd.Stderr = logFile background.ConfigureChildProcessGroup(cmd) // own process group: outlives this shell - if err := cmd.Start(); err != nil { + if err := startAndAwaitDaemonProcess(cmd, func() bool { return daemonReachable(paths) }, 5*time.Second, 25*time.Millisecond); err != nil { + if errors.Is(err, errDaemonStartTimeout) { + return writeAppError(stderr, err.Error()+"; see "+logPath, exitCrash) + } return writeAppError(stderr, err.Error(), exitCrash) } - _ = cmd.Process.Release() + fmt.Fprintf(stdout, "zero daemon started (socket %s)\n", paths.Socket) + return exitSuccess +} + +var errDaemonStartTimeout = errors.New("daemon did not come up within timeout") - deadline := time.Now().Add(5 * time.Second) - for time.Now().Before(deadline) { - if daemonReachable(paths) { - fmt.Fprintf(stdout, "zero daemon started (socket %s)\n", paths.Socket) - return exitSuccess +func startAndAwaitDaemonProcess(cmd *exec.Cmd, reachable func() bool, timeout time.Duration, pollInterval time.Duration) error { + if err := cmd.Start(); err != nil { + return err + } + if waitForDaemonReadiness(reachable, timeout, pollInterval) { + if err := cmd.Process.Release(); err != nil { + cleanupErr := terminateAndReapDaemonProcess(cmd) + if cleanupErr != nil { + return fmt.Errorf("release daemon process: %w (cleanup failed: %v)", err, cleanupErr) + } + return fmt.Errorf("release daemon process: %w", err) } - time.Sleep(25 * time.Millisecond) + return nil + } + if err := terminateAndReapDaemonProcess(cmd); err != nil { + return fmt.Errorf("%w; cleanup failed: %v", errDaemonStartTimeout, err) } - return writeAppError(stderr, "daemon did not come up within timeout; see "+logPath, exitCrash) + return errDaemonStartTimeout +} + +func waitForDaemonReadiness(reachable func() bool, timeout time.Duration, pollInterval time.Duration) bool { + deadline := time.Now().Add(timeout) + for { + if reachable() { + return true + } + remaining := time.Until(deadline) + if remaining <= 0 { + break + } + time.Sleep(min(pollInterval, remaining)) + } + // Avoid killing a daemon that became reachable at the timeout boundary. + return reachable() +} + +func terminateAndReapDaemonProcess(cmd *exec.Cmd) error { + return background.TerminateCommand(cmd) } func runDaemonStop(args []string, stdout io.Writer, stderr io.Writer) int { diff --git a/internal/cli/daemon_test.go b/internal/cli/daemon_test.go index 5418537c..3e316b8c 100644 --- a/internal/cli/daemon_test.go +++ b/internal/cli/daemon_test.go @@ -2,8 +2,15 @@ package cli import ( "bytes" + "errors" + "os" + "os/exec" + "path/filepath" "strings" "testing" + "time" + + "github.com/Gitlawb/zero/internal/background" ) // isolateDaemonPaths points DefaultPaths at a temp dir so the test never touches @@ -121,3 +128,84 @@ func TestDaemonSubcommandsRejectExtraArgs(t *testing.T) { } } } + +func TestWaitForDaemonReadinessChecksTimeoutBoundary(t *testing.T) { + checks := 0 + ready := waitForDaemonReadiness(func() bool { + checks++ + return checks == 2 + }, 0, time.Millisecond) + if !ready { + t.Fatal("daemon that became reachable at the timeout boundary was not detected") + } + if checks != 2 { + t.Fatalf("reachability checks = %d, want initial and final checks", checks) + } +} + +func TestTerminateAndReapDaemonProcess(t *testing.T) { + cmd := daemonDetachedChildCommand("hang") + if err := cmd.Start(); err != nil { + t.Fatalf("start helper process: %v", err) + } + + if err := terminateAndReapDaemonProcess(cmd); err != nil { + t.Fatalf("terminate and reap helper process: %v", err) + } + if cmd.ProcessState == nil { + t.Fatalf("helper process was not reaped: state=%v", cmd.ProcessState) + } + if cmd.ProcessState.Success() { + t.Fatalf("helper process unexpectedly exited successfully: %v", cmd.ProcessState) + } +} + +func TestStartAndAwaitDaemonProcessTimeoutTerminatesAndReaps(t *testing.T) { + cmd := daemonDetachedChildCommand("hang") + if err := startAndAwaitDaemonProcess(cmd, func() bool { return false }, 0, time.Millisecond); !errors.Is(err, errDaemonStartTimeout) { + t.Fatalf("start and await error = %v, want timeout", err) + } + if cmd.ProcessState == nil { + t.Fatal("timed-out helper process was not reaped") + } +} + +func TestStartAndAwaitDaemonProcessReleasesReadyChild(t *testing.T) { + marker := filepath.Join(t.TempDir(), "child-finished") + cmd := daemonDetachedChildCommand("mark", "ZERO_TEST_DAEMON_CHILD_MARKER="+marker) + if err := startAndAwaitDaemonProcess(cmd, func() bool { return true }, time.Second, time.Millisecond); err != nil { + t.Fatalf("start and await ready helper: %v", err) + } + + deadline := time.Now().Add(5 * time.Second) + for time.Now().Before(deadline) { + if _, err := os.Stat(marker); err == nil { + return + } + time.Sleep(10 * time.Millisecond) + } + t.Fatal("released helper process did not remain alive to write its marker") +} + +func daemonDetachedChildCommand(mode string, extraEnv ...string) *exec.Cmd { + cmd := exec.Command(os.Args[0], "-test.run=TestDaemonDetachedChildProcess") + cmd.Env = append(os.Environ(), "ZERO_TEST_DAEMON_DETACHED_CHILD="+mode) + cmd.Env = append(cmd.Env, extraEnv...) + background.ConfigureChildProcessGroup(cmd) + return cmd +} + +func TestDaemonDetachedChildProcess(t *testing.T) { + switch os.Getenv("ZERO_TEST_DAEMON_DETACHED_CHILD") { + case "": + return + case "mark": + if err := os.WriteFile(os.Getenv("ZERO_TEST_DAEMON_CHILD_MARKER"), []byte("ready"), 0o600); err != nil { + os.Exit(2) + } + os.Exit(0) + } + for { + time.Sleep(time.Hour) + } +} diff --git a/internal/execution/process_unix.go b/internal/execution/process_unix.go index 27302afd..8464dd02 100644 --- a/internal/execution/process_unix.go +++ b/internal/execution/process_unix.go @@ -6,6 +6,8 @@ import ( "errors" "fmt" "os/exec" + "strconv" + "strings" "syscall" "time" ) @@ -38,12 +40,42 @@ func KillProcessTree(pid int) error { // TerminateProcessTree requests graceful termination, then force-kills the // process tree after grace. Callers retain their distinct persistence models; // this function owns only the OS lifecycle primitive. +// +// The signal target is REDISCOVERED here via processSignalTarget(pid), which +// asks the OS for pid's current group. A caller that already knows pid was +// configured as its own group leader at launch (e.g. via ConfigureProcessGroup) +// should use TerminateProcessGroup instead — see its doc comment for why this +// rediscovery is fragile once the leader may have already exited. func TerminateProcessTree(pid int, grace, poll time.Duration) error { target, err := processSignalTarget(pid) if err != nil { return err } - alive := func() bool { return syscall.Kill(target, syscall.Signal(0)) == nil } + return terminateTarget(pid, target, grace, poll) +} + +// TerminateProcessGroup is like TerminateProcessTree, but for a caller that +// already knows pid is (or was, at launch) its own process-group leader — e.g. +// a command started after ConfigureProcessGroup(cmd), which sets Setpgid so +// cmd.Process.Pid always leads its own group at the moment it is signalled. +// +// TerminateProcessTree instead rediscovers the group via syscall.Getpgid(pid) +// at call time. On Darwin, that lookup can return ESRCH once an unreaped group +// leader has already exited, even though live descendants remain in the group +// it configured — TerminateProcessTree would then silently fall back to +// signalling only the (already dead) leader PID and leave those descendants +// running. Skipping rediscovery in favor of the launch-time invariant avoids +// that: the negative PID is used directly, regardless of whether the leader is +// still alive to be looked up. +func TerminateProcessGroup(pid int, grace, poll time.Duration) error { + return terminateTarget(pid, -pid, grace, poll) +} + +// terminateTarget signals target (an individual PID or, negative, a process +// group) with SIGTERM then, after grace, SIGKILL. reportPid identifies the +// original process in error messages regardless of which form target takes. +func terminateTarget(reportPid, target int, grace, poll time.Duration) error { + alive := func() bool { return signalTargetRunning(target) } if err := syscall.Kill(target, syscall.SIGTERM); err != nil { if errors.Is(err, syscall.ESRCH) { return nil @@ -77,11 +109,91 @@ func TerminateProcessTree(pid int, grace, poll time.Duration) error { time.Sleep(poll) } if alive() { - return fmt.Errorf("process %d did not exit after SIGKILL", pid) + return fmt.Errorf("process %d did not exit after SIGKILL", reportPid) } return nil } +// signalTargetRunning reports whether the signal target still has a process that +// is actually RUNNING, as opposed to one waiting to be reaped. +// +// kill(2) with signal 0 succeeds for a zombie: the PID still exists until its +// parent collects it. That matters because the target here is usually a process +// GROUP, and a terminated leader's child is briefly a zombie before init/launchd +// reaps it. Treating that window as "still alive" made termination sit through +// both grace periods, SIGKILL an already-dead group, and then report +// "did not exit after SIGKILL" for a tree that had in fact stopped — turning a +// successful cleanup into a spurious failure (and a flaky one, since it depends on +// reap timing). +// +// Zombie detection goes through ps, which is available on every Unix we target and +// avoids a /proc dependency Darwin does not have. It is only consulted when the +// cheap kill check says something is still there, so the common path costs nothing +// extra. If ps cannot be run or its output cannot be parsed, the conservative +// pre-existing answer (the kill result) stands. +func signalTargetRunning(target int) bool { + if syscall.Kill(target, syscall.Signal(0)) != nil { + return false + } + // A negative target is a process GROUP: check whether any member is + // non-zombie. A positive target is an individual PID that is NOT its own + // group leader (processSignalTarget's fallback) — its actual PGID differs + // from its own PID, so matching rows by PGID here would look for a group + // that doesn't exist and always report "unknown" (conservatively alive). + // Check its own PID's state instead. + var running, ok bool + if target < 0 { + running, ok = processGroupHasRunningMember(-target) + } else { + running, ok = processIsRunning(target) + } + if !ok { + return true + } + return running +} + +// processGroupHasRunningMember reports whether any member of pgid is in a state +// other than zombie. ok is false when the group's states could not be determined. +func processGroupHasRunningMember(pgid int) (running bool, ok bool) { + return processTableStateMatches(func(_, rowPgid int) bool { return rowPgid == pgid }) +} + +// processIsRunning reports whether pid itself is in a state other than zombie. +// ok is false when pid's state could not be determined. +func processIsRunning(pid int) (running bool, ok bool) { + return processTableStateMatches(func(rowPid, _ int) bool { return rowPid == pid }) +} + +// processTableStateMatches scans the process table once, via ps, and reports +// whether any row satisfying match is non-zombie, and whether at least one +// matching row was found at all (ok). No rows found means either the process +// (or group) genuinely doesn't exist (a race with exit) or ps's output was +// unparseable/restricted; either way, the caller should not claim knowledge. +func processTableStateMatches(match func(pid, pgid int) bool) (running bool, ok bool) { + output, err := exec.Command("ps", "-A", "-o", "pid=,pgid=,stat=").Output() + if err != nil { + return false, false + } + found := false + for line := range strings.SplitSeq(string(output), "\n") { + fields := strings.Fields(line) + if len(fields) < 3 { + continue + } + rowPid, errPid := strconv.Atoi(fields[0]) + rowPgid, errPgid := strconv.Atoi(fields[1]) + if errPid != nil || errPgid != nil || !match(rowPid, rowPgid) { + continue + } + found = true + if !strings.HasPrefix(fields[2], "Z") { + return true, true + } + } + return false, found +} + func processSignalTarget(pid int) (int, error) { if pid <= 1 { return 0, fmt.Errorf("refusing to signal invalid pid %d", pid) diff --git a/internal/execution/process_unix_test.go b/internal/execution/process_unix_test.go new file mode 100644 index 00000000..36ba9918 --- /dev/null +++ b/internal/execution/process_unix_test.go @@ -0,0 +1,133 @@ +//go:build !windows + +package execution + +import ( + "os/exec" + "strconv" + "syscall" + "testing" + "time" +) + +// TestTerminateProcessTreeTreatsZombieGroupAsExited is the regression test for +// counting unreaped processes as alive: kill(2) with signal 0 succeeds for a +// zombie, so a group whose only remaining member is waiting to be reaped used to +// sit through both grace periods, take a pointless SIGKILL, and then be reported +// as "did not exit after SIGKILL" — a spurious cleanup failure whose timing +// depended on when the reaper ran. +func TestTerminateProcessTreeTreatsZombieGroupAsExited(t *testing.T) { + cmd := exec.Command("sh", "-c", "exit 0") + ConfigureProcessGroup(cmd) + if err := cmd.Start(); err != nil { + t.Fatalf("start: %v", err) + } + pid := cmd.Process.Pid + // Deliberately do NOT Wait: the child stays a zombie in its own group, which + // is exactly the state a terminated tree passes through. + t.Cleanup(func() { _ = cmd.Wait() }) + + deadline := time.Now().Add(5 * time.Second) + for { + if zombie, ok := processIsZombie(pid); ok && zombie { + break + } + if time.Now().After(deadline) { + t.Skip("child did not reach the zombie state; ps output is unavailable in this environment") + } + time.Sleep(10 * time.Millisecond) + } + if syscall.Kill(-pid, syscall.Signal(0)) != nil { + t.Skip("the zombie group is no longer signalable; cannot exercise the case") + } + + if signalTargetRunning(-pid) { + t.Fatal("a group holding only a zombie must not count as running") + } + if err := TerminateProcessTree(pid, 50*time.Millisecond, 10*time.Millisecond); err != nil { + t.Fatalf("TerminateProcessTree on an already-exited tree: %v", err) + } +} + +// TestTerminateProcessTreeStopsRunningGroup is the positive control: a group with +// a live member must still be seen as running and then actually stopped, so the +// zombie tolerance above cannot degrade into ignoring real processes. +func TestTerminateProcessTreeStopsRunningGroup(t *testing.T) { + cmd := exec.Command("sh", "-c", "sleep 30") + ConfigureProcessGroup(cmd) + if err := cmd.Start(); err != nil { + t.Fatalf("start: %v", err) + } + pid := cmd.Process.Pid + defer func() { _ = cmd.Wait() }() + + if !signalTargetRunning(-pid) { + t.Fatal("a group with a live member must count as running") + } + if err := TerminateProcessTree(pid, time.Second, 10*time.Millisecond); err != nil { + t.Fatalf("TerminateProcessTree: %v", err) + } + if running, ok := processGroupHasRunningMember(pid); ok && running { + t.Fatal("the group still has a running member after termination") + } +} + +// TestSignalTargetRunningTreatsZombieIndividualPIDAsExited is the regression +// test for jatmn's second #774 finding: signalTargetRunning's zombie check +// matched the individual-PID fallback target (a process that is NOT its own +// group leader — processSignalTarget's positive-PID case) against process-table +// rows by PGID. That target's actual PGID differs from its own PID by +// definition (that's exactly why processSignalTarget fell back to the +// individual PID instead of a negative group target), so no row ever matched, +// "unknown" resulted every time, and signalTargetRunning conservatively (and +// incorrectly) treated a zombie individual-PID target as still running. +func TestSignalTargetRunningTreatsZombieIndividualPIDAsExited(t *testing.T) { + cmd := exec.Command("sh", "-c", "exit 0") + // Deliberately do NOT call ConfigureProcessGroup: the child inherits this + // test process's group, so it is not its own leader and processSignalTarget + // falls back to the individual (positive) PID. + if err := cmd.Start(); err != nil { + t.Fatalf("start: %v", err) + } + pid := cmd.Process.Pid + t.Cleanup(func() { _ = cmd.Wait() }) + + deadline := time.Now().Add(5 * time.Second) + for { + if zombie, ok := processIsZombie(pid); ok && zombie { + break + } + if time.Now().After(deadline) { + t.Skip("child did not reach the zombie state; ps output is unavailable in this environment") + } + time.Sleep(10 * time.Millisecond) + } + + target, err := processSignalTarget(pid) + if err != nil { + t.Fatalf("processSignalTarget: %v", err) + } + if target != pid { + t.Skip("child unexpectedly became its own group leader; cannot exercise the individual-PID fallback") + } + if signalTargetRunning(target) { + t.Fatal("a zombie individual-PID target must not count as running") + } +} + +// processIsZombie reports a process's zombie state via ps. ok is false when the +// state could not be read. +func processIsZombie(pid int) (zombie bool, ok bool) { + output, err := exec.Command("ps", "-o", "stat=", "-p", strconv.Itoa(pid)).Output() + if err != nil { + return false, false + } + state := string(output) + for len(state) > 0 && (state[0] == ' ' || state[0] == '\n' || state[0] == '\t') { + state = state[1:] + } + if state == "" { + return false, false + } + return state[0] == 'Z', true +}