fix(daemon): kill launcher workers via launch-time process group

internal/background/process_posix.go

@@ -49,8 +49,9 @@ func terminateProcess(pid int) error {
 // own convention. A command made its own session leader via Setsid (as opposed
 // to Setpgid) is also its own process-group leader in practice, but takes the
 // slower rediscovery path here since Setsid isn't checked. Harmless today
-// because TerminateCommand has exactly one caller in this codebase; worth
-// covering explicitly if Setsid-configured commands start using this path too.
+// because current group-owning callers use ConfigureChildProcessGroup's Setpgid
+// convention; worth covering explicitly if Setsid-configured commands start
+// using this path too.
 func terminateOwnedProcess(cmd *exec.Cmd) (bool, error) {
 	if cmd.SysProcAttr != nil && cmd.SysProcAttr.Setpgid && cmd.SysProcAttr.Pgid == 0 {
 		return false, execution.TerminateProcessGroup(cmd.Process.Pid, terminationGracePeriod, terminationPollInterval)

internal/background/terminate.go

@@ -22,6 +22,22 @@ func TerminateProcess(pid int) error {
 	return terminateProcess(pid)
 }
 
+// TerminateOwnedProcess stops a started command without reaping it. On POSIX,
+// commands prepared by ConfigureChildProcessGroup are stopped through their
+// launch-time process-group identity, so Darwin ESRCH on an unreaped leader
+// cannot leave descendants running (see execution.TerminateProcessGroup).
+// Commands without that configuration use the platform's safe PID/process-tree
+// fallback; Windows always uses its rooted process-tree implementation. Unlike
+// TerminateCommand, this does not Wait: callers such as execWorker.Kill and
+// CommandContext Cancel still own the subsequent reap.
+func TerminateOwnedProcess(cmd *exec.Cmd) error {
+	if cmd == nil || cmd.Process == nil {
+		return errors.New("terminate owned process: process was never started")
+	}
+	_, err := terminateOwnedProcess(cmd)
+	return err
+}
+
 // TerminateCommand stops a started command and reaps its leader. The caller must
 // have exclusive ownership of cmd: it must not have previously called Wait or
 // Process.Release, and no goroutine may call either concurrently. On POSIX it

internal/background/terminate_posix_test.go

@@ -0,0 +1,181 @@
+//go:build !windows
+
+package background
+
+import (
+	"bufio"
+	"io"
+	"os"
+	"os/exec"
+	"strconv"
+	"strings"
+	"syscall"
+	"testing"
+	"time"
+)
+
+func TestTerminateOwnedProcessDoesNotReap(t *testing.T) {
+	cmd := exec.Command("sleep", "30")
+	ConfigureChildProcessGroup(cmd)
+	if err := cmd.Start(); err != nil {
+		t.Fatalf("start: %v", err)
+	}
+	t.Cleanup(func() {
+		if cmd.ProcessState == nil {
+			_ = cmd.Process.Kill()
+			_ = cmd.Wait()
+		}
+	})
+
+	if err := TerminateOwnedProcess(cmd); err != nil {
+		t.Fatalf("TerminateOwnedProcess: %v", err)
+	}
+	if cmd.ProcessState != nil {
+		t.Fatal("TerminateOwnedProcess must not Wait; the caller still owns the reap")
+	}
+	if err := cmd.Wait(); err == nil {
+		t.Fatal("expected the terminated command's Wait to report a signal")
+	}
+}
+
+func TestTerminateOwnedProcessKillsChildAfterLeaderExits(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 and is left
+	// unreaped. TerminateOwnedProcess must signal the launch-time process
+	// group rather than rediscovering it via Getpgid (Darwin ESRCH, #861).
+	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)
+	}
+	finalized := false
+	var childPID int
+	t.Cleanup(func() {
+		if finalized {
+			return
+		}
+		if childPID > 0 {
+			_ = syscall.Kill(childPID, syscall.SIGKILL)
+		}
+		if cmd.Process != nil {
+			_ = syscall.Kill(-cmd.Process.Pid, syscall.SIGKILL)
+			_ = cmd.Process.Kill()
+			if cmd.ProcessState == nil {
+				_ = cmd.Wait()
+			}
+		}
+	})
+
+	childPID = readPIDWithTimeout(t, stdout, 3*time.Second)
+
+	// echo $! races ahead of exit 0. Wait until the leader is an unreaped
+	// zombie without calling Wait, so this exercises Darwin Getpgid ESRCH.
+	waitUntilUnreapedZombie(t, cmd.Process.Pid)
+
+	if err := TerminateOwnedProcess(cmd); err != nil {
+		t.Fatalf("TerminateOwnedProcess: %v", err)
+	}
+	if cmd.ProcessState != nil {
+		t.Fatal("TerminateOwnedProcess must not reap the leader")
+	}
+	if err := cmd.Wait(); err != nil && terminatingSignal(err) == 0 {
+		// Leader may have exited 0 before the group signal, or have been
+		// signalled; either is a successful reap.
+		t.Fatalf("Wait after TerminateOwnedProcess: %v", err)
+	}
+
+	deadline := time.Now().Add(2 * time.Second)
+	for !processStopped(childPID) {
+		if time.Now().After(deadline) {
+			t.Fatalf("forked child %d survived TerminateOwnedProcess — group kill failed", childPID)
+		}
+		time.Sleep(20 * time.Millisecond)
+	}
+	finalized = true
+}
+
+func TestTerminateOwnedProcessNil(t *testing.T) {
+	defer func() {
+		if r := recover(); r != nil {
+			t.Fatalf("TerminateOwnedProcess(nil) panicked: %v", r)
+		}
+	}()
+	if err := TerminateOwnedProcess(nil); err == nil {
+		t.Fatal("TerminateOwnedProcess(nil) must return an error")
+	}
+}
+
+func TestTerminateOwnedProcessUnstarted(t *testing.T) {
+	defer func() {
+		if r := recover(); r != nil {
+			t.Fatalf("TerminateOwnedProcess(unstarted) panicked: %v", r)
+		}
+	}()
+	cmd := exec.Command("true")
+	if err := TerminateOwnedProcess(cmd); err == nil {
+		t.Fatal("TerminateOwnedProcess on an unstarted command must return an error")
+	}
+}
+
+// waitUntilUnreapedZombie polls until pid is a zombie without calling Wait.
+func waitUntilUnreapedZombie(t *testing.T, pid int) {
+	t.Helper()
+	deadline := time.Now().Add(2 * time.Second)
+	for !isUnreapedZombie(pid) {
+		if time.Now().After(deadline) {
+			t.Fatalf("pid %d did not become an unreaped zombie before terminate", pid)
+		}
+		time.Sleep(20 * time.Millisecond)
+	}
+}
+
+func isUnreapedZombie(pid int) bool {
+	if data, err := os.ReadFile("/proc/" + strconv.Itoa(pid) + "/stat"); err == nil {
+		s := string(data)
+		i := strings.LastIndexByte(s, ')')
+		if i < 0 || i+2 >= len(s) {
+			return false
+		}
+		return s[i+2] == 'Z'
+	}
+	state, err := exec.Command("ps", "-o", "stat=", "-p", strconv.Itoa(pid)).Output()
+	if err != nil {
+		return false
+	}
+	return strings.HasPrefix(strings.TrimSpace(string(state)), "Z")
+}
+
+func readPIDWithTimeout(t *testing.T, r io.Reader, timeout time.Duration) int {
+	t.Helper()
+	type res struct {
+		pid int
+		err error
+	}
+	ch := make(chan res, 1)
+	go func() {
+		line, err := bufio.NewReader(r).ReadString('\n')
+		if err != nil {
+			ch <- res{err: err}
+			return
+		}
+		pid, err := strconv.Atoi(strings.TrimSpace(line))
+		ch <- res{pid: pid, err: err}
+	}()
+	select {
+	case out := <-ch:
+		if out.err != nil {
+			t.Fatalf("read child PID: %v", out.err)
+		}
+		return out.pid
+	case <-time.After(timeout):
+		t.Fatalf("timed out waiting for child PID after %v", timeout)
+		return 0
+	}
+}

internal/daemon/launcher.go

@@ -9,6 +9,7 @@ import (
 	"os"
 	"os/exec"
 	"strings"
+	"sync"
 
 	"github.com/Gitlawb/zero/internal/background"
 	"github.com/Gitlawb/zero/internal/sandbox"
@@ -47,6 +48,8 @@ func scrubWorkerEnv(env []string) []string {
 // execWorker is a WorkerHandle backed by a `zero exec` child process speaking
 // stream-json on stdout.
 type execWorker struct {
+	mu     sync.Mutex
+	reaped bool
 	cmd    *exec.Cmd
 	stdout io.ReadCloser
 	lines  Lines
@@ -57,7 +60,19 @@ func (w *execWorker) Stdout() Lines { return w.lines }
 func (w *execWorker) Pid() int      { return w.pid }
 
 func (w *execWorker) Wait() (int, error) {
+	w.mu.Lock()
+	if w.reaped {
+		w.mu.Unlock()
+		return 0, nil
+	}
+	w.mu.Unlock()
+
 	err := w.cmd.Wait()
+
+	w.mu.Lock()
+	w.reaped = true
+	w.mu.Unlock()
+
 	if err == nil {
 		return 0, nil
 	}
@@ -69,12 +84,17 @@ func (w *execWorker) Wait() (int, error) {
 }
 
 func (w *execWorker) Kill() error {
-	if w.cmd.Process == nil {
+	w.mu.Lock()
+	defer w.mu.Unlock()
+	if w.reaped || w.cmd.Process == nil {
 		return nil
 	}
-	// background.TerminateProcess is the cross-platform terminate (kills the
-	// process group on POSIX, taskkill /T on Windows).
-	return background.TerminateProcess(w.cmd.Process.Pid)
+	// TerminateOwnedProcess uses the launch-time group identity from
+	// ConfigureChildProcessGroup rather than TerminateProcess's Getpgid
+	// rediscovery. On Darwin, Getpgid of an unreaped group leader can return
+	// ESRCH and leave descendants running (#861, #774). TerminateCommand is
+	// the wrong helper here: it Wait()s, and the pool still owns the reap.
+	return background.TerminateOwnedProcess(w.cmd)
 }
 
 // readerLines adapts a bufio.Reader to the Lines interface. Unlike a capped
@@ -154,13 +174,13 @@ func NewExecLauncher(cfg ExecLauncherConfig) (Launcher, error) {
 		background.ConfigureChildProcessGroup(cmd)
 		// CommandContext's default cancel sends os.Process.Kill to the LEADER only,
 		// orphaning the process group we just configured (a stuck worker's children
-		// would survive ctx cancellation). Terminate the whole group instead — the
-		// same cross-platform group terminate Kill() uses (D11).
+		// would survive ctx cancellation). Terminate the whole group instead via
+		// the launch-time identity — the same primitive Kill() uses (D11, #861).
 		cmd.Cancel = func() error {
 			if cmd.Process == nil {
 				return nil
 			}
-			return background.TerminateProcess(cmd.Process.Pid)
+			return background.TerminateOwnedProcess(cmd)
 		}
 
 		stdout, err := cmd.StdoutPipe()