fix(ci): read the process table without forking ps in the exec tracker (D-1008)

free/ci/go.mod

@@ -20,7 +20,7 @@ require (
 	golang.org/x/mod v0.41.0 // indirect
 	golang.org/x/net v0.60.0 // indirect
 	golang.org/x/sync v0.23.0 // indirect
-	golang.org/x/sys v0.48.0 // indirect
+	golang.org/x/sys v0.48.0
 	golang.org/x/tools v0.50.0 // indirect
 	modernc.org/libc v1.77.1 // indirect
 	modernc.org/mathutil v1.7.1 // indirect

free/ci/internal/exec/hygiene_unix.go

@@ -47,16 +47,16 @@ func KillAttemptGroup(home, attempt string) error {
 		return nil
 	}
 	if groupAlive(marker.PID) {
-		identity, idErr := processIdentity(marker.PID)
+		same, known := sameProcess(marker.PID, marker.Identity)
 		leaderGone := errors.Is(syscall.Kill(marker.PID, 0), syscall.ESRCH)
 		switch {
 		case leaderGone:
 			// A live group whose leader has exited is the attempt's own: the kernel does not reuse a
 			// PID that still names a process group.
-		case idErr != nil || identity == "":
+		case !known:
 			// the leader exists but its start time is unreadable: it cannot be told from a reused PID
 			return coded("E610", "attempt process identity unavailable")
-		case identity != marker.Identity:
+		case !same:
 			// the leader PID now belongs to another process; its group is not ours to kill
 			removeProcessMarker(home, attempt)
 			return nil

free/ci/internal/exec/process_identity_test.go

@@ -0,0 +1,158 @@
+//go:build !windows
+
+package exec
+
+import (
+	"context"
+	"os"
+	osexec "os/exec"
+	"strings"
+	"testing"
+	"time"
+)
+
+// startGroup runs a sleeping process group and returns its leader.
+func startGroup(t *testing.T) *osexec.Cmd {
+	t.Helper()
+	cmd := osexec.Command("sh", "-c", "sleep 30")
+	if err := prepareProcess(cmd); err != nil {
+		t.Fatal(err)
+	}
+	if err := cmd.Start(); err != nil {
+		t.Fatal(err)
+	}
+	t.Cleanup(func() {
+		_ = killGroup(cmd.Process.Pid)
+		_ = cmd.Wait()
+	})
+	return cmd
+}
+
+func waitGroupGone(pid int) bool {
+	deadline := time.Now().Add(2 * time.Second)
+	for groupAlive(pid) && time.Now().Before(deadline) {
+		time.Sleep(20 * time.Millisecond)
+	}
+	return !groupAlive(pid)
+}
+
+func TestNativeStartIDIsStableAndPerProcess(t *testing.T) {
+	self, ok := nativeStartID(os.Getpid())
+	if !ok || !strings.HasPrefix(self, "st:") {
+		t.Fatalf("self id = %q ok=%v", self, ok)
+	}
+	if again, _ := nativeStartID(os.Getpid()); again != self {
+		t.Fatalf("id changed between reads: %q vs %q", self, again)
+	}
+	cmd := startGroup(t)
+	other, ok := nativeStartID(cmd.Process.Pid)
+	if !ok || other == self {
+		t.Fatalf("child id %q must differ from self %q", other, self)
+	}
+	if _, ok := nativeStartID(1 << 30); ok {
+		t.Fatal("a pid that cannot exist reported a start id")
+	}
+}
+
+// A marker written by an older build holds ps lstart text; it must still match
+// the live leader, so a legacy marker's group is killed and not leaked.
+func TestKillAttemptGroupAcceptsLegacyMarker(t *testing.T) {
+	home := t.TempDir()
+	cmd := startGroup(t)
+	pid := cmd.Process.Pid
+	legacy, err := psIdentity(pid)
+	if err != nil || legacy == "" {
+		t.Fatalf("ps identity: %q %v", legacy, err)
+	}
+	writeMarkerWith(t, home, "legacy", "old", pid, legacy)
+	if err := KillAttemptGroup(home, "legacy"); err != nil {
+		t.Fatal(err)
+	}
+	_ = cmd.Wait()
+	if !waitGroupGone(pid) {
+		t.Fatal("group with a legacy marker survived KillAttemptGroup")
+	}
+}
+
+// A marker whose identity differs from the live leader means the PID was
+// reused: the group belongs to someone else and must not be killed.
+func TestKillAttemptGroupSparesReusedPID(t *testing.T) {
+	for name, identity := range map[string]string{"native": "st:1", "legacy": "Thu Jan  1 00:00:00 1970"} {
+		t.Run(name, func(t *testing.T) {
+			home := t.TempDir()
+			cmd := startGroup(t)
+			pid := cmd.Process.Pid
+			writeMarkerWith(t, home, "reused", "old", pid, identity)
+			if err := KillAttemptGroup(home, "reused"); err != nil {
+				t.Fatal(err)
+			}
+			if !groupAlive(pid) {
+				t.Fatal("KillAttemptGroup killed a group whose leader PID was reused")
+			}
+			if _, err := os.Stat(markerPath(home, "reused")); !os.IsNotExist(err) {
+				t.Fatalf("stale marker retained: %v", err)
+			}
+		})
+	}
+}
+
+func TestSweepLegacyMarkerKillsAndReusedPIDRefuses(t *testing.T) {
+	home := t.TempDir()
+	legacyCmd := startGroup(t)
+	reusedCmd := startGroup(t)
+	legacy, err := psIdentity(legacyCmd.Process.Pid)
+	if err != nil {
+		t.Fatal(err)
+	}
+	writeMarkerWith(t, home, "legacy", "old", legacyCmd.Process.Pid, legacy)
+	writeMarkerWith(t, home, "reused", "old", reusedCmd.Process.Pid, "st:1")
+	e := &Executor{Stale: staleChecker{"old": true}}
+	if err := e.sweepProcessMarker(context.Background(), home, "current", "legacy"); err != nil {
+		t.Fatal(err)
+	}
+	_ = legacyCmd.Wait()
+	if !waitGroupGone(legacyCmd.Process.Pid) {
+		t.Fatal("legacy-marker group survived sweep")
+	}
+	if err := e.sweepProcessMarker(context.Background(), home, "current", "reused"); err == nil {
+		t.Fatal("sweep of a reused PID must refuse")
+	}
+	if !groupAlive(reusedCmd.Process.Pid) {
+		t.Fatal("sweep killed a group whose leader PID was reused")
+	}
+}
+
+// The tracker must follow nested descendants, not only direct children.
+func TestTrackerFollowsNestedDescendants(t *testing.T) {
+	cmd := osexec.Command("sh", "-c", "sh -c 'sleep 30 & sleep 30 & wait' & wait")
+	if err := prepareProcess(cmd); err != nil {
+		t.Fatal(err)
+	}
+	if err := cmd.Start(); err != nil {
+		t.Fatal(err)
+	}
+	tr := trackDescendants(cmd.Process.Pid, "")
+	deadline := time.Now().Add(2 * time.Second)
+	for len(tr.pids()) < 3 && time.Now().Before(deadline) {
+		time.Sleep(20 * time.Millisecond)
+	}
+	got := len(tr.pids())
+	tr.signalKill()
+	_ = killGroup(cmd.Process.Pid)
+	_ = cmd.Wait()
+	tr.stop()
+	if got < 3 {
+		t.Fatalf("tracker saw %d descendants, want at least 3", got)
+	}
+}
+
+func writeMarkerWith(t *testing.T, home, attempt, coordinator string, pid int, identity string) {
+	t.Helper()
+	if err := os.MkdirAll(home+"/processes", 0o700); err != nil {
+		t.Fatal(err)
+	}
+	data := `{"pid":` + itoa(pid) + `,"coordinator":"` + coordinator + `","identity":"` + identity + `"}`
+	if err := os.WriteFile(markerPath(home, attempt), []byte(data), 0o600); err != nil {
+		t.Fatal(err)
+	}
+}

free/ci/internal/exec/process_marker.go

@@ -21,7 +21,39 @@ func markerPath(home, attempt string) string {
 	return filepath.Join(home, "processes", attempt+".json")
 }
 
+// processIdentity names a process by its start instant. The native form
+// ("st:" prefix) needs no fork; markers written by older builds hold the ps
+// lstart text, so psIdentity stays for comparing against those.
 func processIdentity(pid int) (string, error) {
+	if id, ok := nativeStartID(pid); ok {
+		return id, nil
+	}
+	return psIdentity(pid)
+}
+
+// sameProcess compares the live start instant of pid with a marker's recorded
+// identity, in the form the marker was written in: "st:" markers use the native
+// reader, older markers hold ps lstart text. known is false when the live
+// identity cannot be read in that form, which is not the same as "different".
+func sameProcess(pid int, recorded string) (same, known bool) {
+	var now string
+	if strings.HasPrefix(recorded, "st:") {
+		id, ok := nativeStartID(pid)
+		if !ok {
+			return false, false
+		}
+		now = id
+	} else {
+		id, err := psIdentity(pid)
+		if err != nil || id == "" {
+			return false, false
+		}
+		now = id
+	}
+	return now == recorded, true
+}
+
+func psIdentity(pid int) (string, error) {
 	out, err := osexec.Command("ps", "-o", "lstart=", "-p", strconv.Itoa(pid)).Output()
 	if err != nil {
 		return "", err
@@ -99,9 +131,8 @@ func (e *Executor) sweepProcessMarker(ctx context.Context, home, current, attemp
 	if !stale {
 		return nil
 	}
-	identity, err := processIdentity(marker.PID)
 	if groupAlive(marker.PID) {
-		if err == nil && identity != "" && identity != marker.Identity {
+		if same, known := sameProcess(marker.PID, marker.Identity); known && !same {
 			// A different process has reused the leader PID. Do not kill its group.
 			return coded("E610", "stale group identity changed")
 		}

free/ci/internal/exec/process_subreaper_linux.go

@@ -5,10 +5,8 @@ package exec
 import (
 	"bytes"
 	"os"
-	osexec "os/exec"
 	"path/filepath"
 	"strconv"
-	"strings"
 	"sync"
 	"syscall"
 	"time"
@@ -49,23 +47,8 @@ func processExists(pid int) bool {
 	return syscall.Kill(pid, 0) == nil || os.IsPermission(syscall.Kill(pid, 0))
 }
 
-func groupAlive(pgid int) bool {
-	out, err := osexec.Command("ps", "-A", "-o", "pgid=", "-o", "stat=").Output()
-	if err != nil {
-		return syscall.Kill(-pgid, 0) == nil
-	}
-	for _, line := range strings.Split(string(out), "\n") {
-		fields := strings.Fields(line)
-		if len(fields) != 2 {
-			continue
-		}
-		group, parseErr := strconv.Atoi(fields[0])
-		if parseErr == nil && group == pgid && fields[1][0] != 'Z' {
-			return true
-		}
-	}
-	return false
-}
+// processGroupProbe is the signal-based fallback when /proc cannot be read.
+func processGroupProbe(pgid int) bool { return syscall.Kill(-pgid, 0) == nil }
 
 func adoptedDescendants(parents map[int]int, attempt string) []int {
 	if attempt == "" {

free/ci/internal/exec/process_table_darwin.go

@@ -0,0 +1,33 @@
+//go:build darwin
+
+package exec
+
+import (
+	"strconv"
+
+	"golang.org/x/sys/unix"
+)
+
+// processTable returns pid -> parent pid for every process, read with one
+// sysctl (no fork, no cgo).
+func processTable() (map[int]int, error) {
+	procs, err := unix.SysctlKinfoProcSlice("kern.proc.all")
+	if err != nil {
+		return nil, err
+	}
+	parents := make(map[int]int, len(procs))
+	for i := range procs {
+		parents[int(procs[i].Proc.P_pid)] = int(procs[i].Eproc.Ppid)
+	}
+	return parents, nil
+}
+
+// nativeStartID names the start instant of pid: it changes when the pid is reused.
+func nativeStartID(pid int) (string, bool) {
+	p, err := unix.SysctlKinfoProc("kern.proc.pid", pid)
+	if err != nil || int(p.Proc.P_pid) != pid {
+		return "", false
+	}
+	tv := p.Proc.P_starttime
+	return "st:" + strconv.FormatInt(int64(tv.Sec), 10) + "." + strconv.FormatInt(int64(tv.Usec), 10), true
+}