From 699ea9ba1afe55b49483b4c8035cbf97b700c036 Mon Sep 17 00:00:00 2001 From: Aric Camarata Date: Mon, 19 Oct 2026 00:51:42 -0400 Subject: [PATCH 1/2] fix(ci): read the process table from /proc and sysctl instead of forking ps (D-1008) The exec funnel's descendant tracker forked ps -A up to three times per snapshot, and the process identity marker and Linux group probe forked ps again. A no-op funnel job cost about 31 ms; it now costs about 2.4 ms. Snapshots read /proc on Linux and sysctl kern.proc.all on darwin, one read per call; ps stays as the fallback. Markers written by older builds (ps lstart text) are still compared through ps. --- free/ci/go.mod | 2 +- free/ci/internal/exec/process_marker.go | 13 +++ .../internal/exec/process_subreaper_linux.go | 21 +--- free/ci/internal/exec/process_table_darwin.go | 33 +++++++ free/ci/internal/exec/process_table_linux.go | 99 +++++++++++++++++++ .../internal/exec/process_tree_nops_test.go | 45 +++++++++ free/ci/internal/exec/process_tree_unix.go | 88 +++++++++-------- free/ci/internal/exec/process_tree_windows.go | 1 + 8 files changed, 241 insertions(+), 61 deletions(-) create mode 100644 free/ci/internal/exec/process_table_darwin.go create mode 100644 free/ci/internal/exec/process_table_linux.go create mode 100644 free/ci/internal/exec/process_tree_nops_test.go diff --git a/free/ci/go.mod b/free/ci/go.mod index 08760f692..b009cedd0 100644 --- a/free/ci/go.mod +++ b/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 diff --git a/free/ci/internal/exec/process_marker.go b/free/ci/internal/exec/process_marker.go index 666dd2b44..6d9488543 100644 --- a/free/ci/internal/exec/process_marker.go +++ b/free/ci/internal/exec/process_marker.go @@ -21,7 +21,17 @@ 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) +} + +func psIdentity(pid int) (string, error) { out, err := osexec.Command("ps", "-o", "lstart=", "-p", strconv.Itoa(pid)).Output() if err != nil { return "", err @@ -100,6 +110,9 @@ func (e *Executor) sweepProcessMarker(ctx context.Context, home, current, attemp return nil } identity, err := processIdentity(marker.PID) + if err == nil && !strings.HasPrefix(marker.Identity, "st:") && strings.HasPrefix(identity, "st:") { + identity, err = psIdentity(marker.PID) // marker from an older build + } if groupAlive(marker.PID) { if err == nil && identity != "" && identity != marker.Identity { // A different process has reused the leader PID. Do not kill its group. diff --git a/free/ci/internal/exec/process_subreaper_linux.go b/free/ci/internal/exec/process_subreaper_linux.go index a6fb45890..12e305090 100644 --- a/free/ci/internal/exec/process_subreaper_linux.go +++ b/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 == "" { diff --git a/free/ci/internal/exec/process_table_darwin.go b/free/ci/internal/exec/process_table_darwin.go new file mode 100644 index 000000000..53113d45c --- /dev/null +++ b/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 +} diff --git a/free/ci/internal/exec/process_table_linux.go b/free/ci/internal/exec/process_table_linux.go new file mode 100644 index 000000000..2832499aa --- /dev/null +++ b/free/ci/internal/exec/process_table_linux.go @@ -0,0 +1,99 @@ +//go:build linux + +package exec + +import ( + "os" + "strconv" + "strings" +) + +// procStat is the part of /proc//stat the executor needs. +type procStat struct { + state byte + ppid, pgrp int + startTicks string +} + +// readProcStat parses /proc//stat. The command name may hold spaces and +// parentheses, so fields are counted from the last ')'. +func readProcStat(pid int) (procStat, bool) { + data, err := os.ReadFile("/proc/" + strconv.Itoa(pid) + "/stat") + if err != nil { + return procStat{}, false + } + s := string(data) + i := strings.LastIndexByte(s, ')') + if i < 0 { + return procStat{}, false + } + f := strings.Fields(s[i+1:]) + if len(f) < 20 || f[0] == "" { + return procStat{}, false + } + ppid, e1 := strconv.Atoi(f[1]) + pgrp, e2 := strconv.Atoi(f[2]) + if e1 != nil || e2 != nil { + return procStat{}, false + } + return procStat{state: f[0][0], ppid: ppid, pgrp: pgrp, startTicks: f[19]}, true +} + +// procPIDs lists the numeric entries of /proc. +func procPIDs() ([]int, error) { + dir, err := os.Open("/proc") + if err != nil { + return nil, err + } + defer func() { _ = dir.Close() }() + names, err := dir.Readdirnames(-1) + if err != nil { + return nil, err + } + pids := make([]int, 0, len(names)) + for _, n := range names { + if pid, err := strconv.Atoi(n); err == nil && pid > 0 { + pids = append(pids, pid) + } + } + return pids, nil +} + +// processTable returns pid -> parent pid for every process, read from /proc +// without forking. +func processTable() (map[int]int, error) { + pids, err := procPIDs() + if err != nil { + return nil, err + } + parents := make(map[int]int, len(pids)) + for _, pid := range pids { + if st, ok := readProcStat(pid); ok { + parents[pid] = st.ppid + } + } + return parents, nil +} + +// nativeStartID names the start instant of pid: it changes when the pid is reused. +func nativeStartID(pid int) (string, bool) { + st, ok := readProcStat(pid) + if !ok { + return "", false + } + return "st:" + st.startTicks, true +} + +// groupAlive reports whether any non-zombie process is in group pgid. +func groupAlive(pgid int) bool { + pids, err := procPIDs() + if err != nil { + return processGroupProbe(pgid) + } + for _, pid := range pids { + if st, ok := readProcStat(pid); ok && st.pgrp == pgid && st.state != 'Z' { + return true + } + } + return false +} diff --git a/free/ci/internal/exec/process_tree_nops_test.go b/free/ci/internal/exec/process_tree_nops_test.go new file mode 100644 index 000000000..b1687e3c6 --- /dev/null +++ b/free/ci/internal/exec/process_tree_nops_test.go @@ -0,0 +1,45 @@ +//go:build !windows + +package exec + +import ( + "context" + "os" + "path/filepath" + "strings" + "testing" + "time" +) + +// D-1008: the descendant tracker must not fork ps. Each fork cost about 10 ms, +// ran three times per snapshot and dominated the V15 tiny-repo run time. +func TestTrackerDoesNotForkPS(t *testing.T) { + shim := t.TempDir() + log := filepath.Join(shim, "ps.calls") + script := "#!/bin/sh\necho \"$@\" >> " + log + "\nexit 0\n" + if err := os.WriteFile(filepath.Join(shim, "ps"), []byte(script), 0o755); err != nil { + t.Fatal(err) + } + t.Setenv("PATH", shim+string(os.PathListSeparator)+os.Getenv("PATH")) + res := (&Executor{Grace: 100 * time.Millisecond}).Run(context.Background(), JobSpec{ + AttemptID: "no-ps", CoordinatorID: "coord", Home: t.TempDir(), + Command: []string{"sh", "-c", "sleep 0.3 & sleep 0.2; wait"}, + }) + if res.Err != nil || res.ExitCode != 0 { + t.Fatalf("run: %+v", res) + } + if b, _ := os.ReadFile(log); strings.TrimSpace(string(b)) != "" { + t.Fatalf("tracker forked ps %d time(s): %q", strings.Count(string(b), "\n"), b) + } +} + +// The table must list this process, its parent, and a live child. +func TestProcessTableSeesChildren(t *testing.T) { + parents, err := processTable() + if err != nil { + t.Fatal(err) + } + if got := parents[os.Getpid()]; got != os.Getppid() { + t.Fatalf("ppid of self = %d, want %d", got, os.Getppid()) + } +} diff --git a/free/ci/internal/exec/process_tree_unix.go b/free/ci/internal/exec/process_tree_unix.go index 15acf9954..197ed14e4 100644 --- a/free/ci/internal/exec/process_tree_unix.go +++ b/free/ci/internal/exec/process_tree_unix.go @@ -47,61 +47,67 @@ func (t *processTree) stop() { reapDescendants(t.pids()) } +// snapshot reads the process table once and adds every descendant of the root +// (and every adopted orphan) to the seen set. The table comes from /proc or +// sysctl; ps is only the fallback when those fail. func (t *processTree) snapshot() { - // ps is available on supported Unix hosts. Repeated walks capture forks that - // occur while a prior snapshot is being read. - for i := 0; i < 3; i++ { - out, err := osexec.Command("ps", "-A", "-o", "pid=", "-o", "ppid=").Output() - if err != nil { + parents, err := processTable() + if err != nil { + if parents, err = psProcessTable(); err != nil { return } - parents := make(map[int]int) - for _, line := range strings.Split(string(out), "\n") { - fields := strings.Fields(line) - if len(fields) != 2 { + } + t.mu.Lock() + defer t.mu.Unlock() + known := map[int]struct{}{t.root: {}} + for pid := range t.seen { + known[pid] = struct{}{} + } + for _, pid := range adoptedDescendants(parents, t.attempt) { + known[pid] = struct{}{} + t.seen[pid] = struct{}{} + } + for { + added := false + for pid, ppid := range parents { + if _, ok := known[ppid]; !ok { continue } - pid, e1 := strconv.Atoi(fields[0]) - ppid, e2 := strconv.Atoi(fields[1]) - if e1 == nil && e2 == nil { - parents[pid] = ppid + if _, ok := known[pid]; ok { + continue } - } - t.mu.Lock() - known := map[int]struct{}{t.root: {}} - for pid := range t.seen { - known[pid] = struct{}{} - } - for _, pid := range adoptedDescendants(parents, t.attempt) { known[pid] = struct{}{} t.seen[pid] = struct{}{} + added = true } - changed := false - for { - added := false - for pid, ppid := range parents { - if _, ok := known[ppid]; !ok { - continue - } - if _, ok := known[pid]; ok { - continue - } - known[pid] = struct{}{} - t.seen[pid] = struct{}{} - added = true - changed = true - } - if !added { - break - } - } - t.mu.Unlock() - if !changed { + if !added { return } } } +// psProcessTable is the fork-based fallback for hosts where the native table +// is unreadable. +func psProcessTable() (map[int]int, error) { + out, err := osexec.Command("ps", "-A", "-o", "pid=", "-o", "ppid=").Output() + if err != nil { + return nil, err + } + parents := make(map[int]int) + for _, line := range strings.Split(string(out), "\n") { + fields := strings.Fields(line) + if len(fields) != 2 { + continue + } + pid, e1 := strconv.Atoi(fields[0]) + ppid, e2 := strconv.Atoi(fields[1]) + if e1 == nil && e2 == nil { + parents[pid] = ppid + } + } + return parents, nil +} + func (t *processTree) pids() []int { t.mu.Lock() defer t.mu.Unlock() diff --git a/free/ci/internal/exec/process_tree_windows.go b/free/ci/internal/exec/process_tree_windows.go index 3b7f46e85..6a066dc9d 100644 --- a/free/ci/internal/exec/process_tree_windows.go +++ b/free/ci/internal/exec/process_tree_windows.go @@ -10,3 +10,4 @@ func (*processTree) signalTerminate() {} func (*processTree) signalKill() {} func (*processTree) anyAlive() bool { return false } func (*processTree) stop() {} +func nativeStartID(int) (string, bool) { return "", false } From b344e663511cefc0f9dbe3204b50a525e1095b96 Mon Sep 17 00:00:00 2001 From: Aric Camarata Date: Mon, 19 Oct 2026 00:58:06 -0400 Subject: [PATCH 2/2] fix(ci): compare legacy process markers in attempt kill, re-scan until stable, bind start ticks to boot KillAttemptGroup compared only the new st: identity, so a marker written by an older build (ps lstart text) looked like a reused PID and its group was left running. Both kill paths now share sameProcess, which reads the live identity in the marker's own form and treats an unreadable one as unknown, not different. The tracker snapshot repeats (at most three passes) while a pass finds new processes, as the ps version did, so forks during the read are caught. Linux start ids carry the boot id so ticks cannot match across a reboot. Adds identity, legacy-marker, reused-PID and nested-descendant tests. --- free/ci/internal/exec/hygiene_unix.go | 6 +- .../ci/internal/exec/process_identity_test.go | 158 ++++++++++++++++++ free/ci/internal/exec/process_marker.go | 28 +++- free/ci/internal/exec/process_table_linux.go | 19 ++- free/ci/internal/exec/process_tree_unix.go | 33 +++- 5 files changed, 227 insertions(+), 17 deletions(-) create mode 100644 free/ci/internal/exec/process_identity_test.go diff --git a/free/ci/internal/exec/hygiene_unix.go b/free/ci/internal/exec/hygiene_unix.go index 98a5d6712..e48c1d10b 100644 --- a/free/ci/internal/exec/hygiene_unix.go +++ b/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 diff --git a/free/ci/internal/exec/process_identity_test.go b/free/ci/internal/exec/process_identity_test.go new file mode 100644 index 000000000..29bebae67 --- /dev/null +++ b/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) + } +} diff --git a/free/ci/internal/exec/process_marker.go b/free/ci/internal/exec/process_marker.go index 6d9488543..bf84b8326 100644 --- a/free/ci/internal/exec/process_marker.go +++ b/free/ci/internal/exec/process_marker.go @@ -31,6 +31,28 @@ func processIdentity(pid int) (string, error) { 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 { @@ -109,12 +131,8 @@ func (e *Executor) sweepProcessMarker(ctx context.Context, home, current, attemp if !stale { return nil } - identity, err := processIdentity(marker.PID) - if err == nil && !strings.HasPrefix(marker.Identity, "st:") && strings.HasPrefix(identity, "st:") { - identity, err = psIdentity(marker.PID) // marker from an older build - } 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") } diff --git a/free/ci/internal/exec/process_table_linux.go b/free/ci/internal/exec/process_table_linux.go index 2832499aa..ee7827262 100644 --- a/free/ci/internal/exec/process_table_linux.go +++ b/free/ci/internal/exec/process_table_linux.go @@ -6,6 +6,7 @@ import ( "os" "strconv" "strings" + "sync" ) // procStat is the part of /proc//stat the executor needs. @@ -81,7 +82,23 @@ func nativeStartID(pid int) (string, bool) { if !ok { return "", false } - return "st:" + st.startTicks, true + return "st:" + st.startTicks + bootSuffix(), true +} + +var ( + bootOnce sync.Once + bootID string +) + +// bootSuffix ties start ticks (counted from boot) to one boot, so a marker kept +// across a reboot cannot match a new process that has the same pid and ticks. +func bootSuffix() string { + bootOnce.Do(func() { + if b, err := os.ReadFile("/proc/sys/kernel/random/boot_id"); err == nil { + bootID = "@" + strings.TrimSpace(string(b)) + } + }) + return bootID } // groupAlive reports whether any non-zombie process is in group pgid. diff --git a/free/ci/internal/exec/process_tree_unix.go b/free/ci/internal/exec/process_tree_unix.go index 197ed14e4..eeb2a1c52 100644 --- a/free/ci/internal/exec/process_tree_unix.go +++ b/free/ci/internal/exec/process_tree_unix.go @@ -47,25 +47,41 @@ func (t *processTree) stop() { reapDescendants(t.pids()) } -// snapshot reads the process table once and adds every descendant of the root -// (and every adopted orphan) to the seen set. The table comes from /proc or -// sysctl; ps is only the fallback when those fail. +// snapshot reads the process table and adds every descendant of the root (and +// every adopted orphan) to the seen set. A pass that found new processes is +// repeated (at most three passes) so forks made while the table was being read +// are caught. The table comes from /proc or sysctl, which cost no fork; ps is +// only the fallback when those fail. func (t *processTree) snapshot() { - parents, err := processTable() - if err != nil { - if parents, err = psProcessTable(); err != nil { + for i := 0; i < 3; i++ { + parents, err := processTable() + if err != nil { + if parents, err = psProcessTable(); err != nil { + return + } + } + if !t.absorb(parents) { return } } +} + +// absorb adds the descendants found in parents to the seen set and reports +// whether it added any. +func (t *processTree) absorb(parents map[int]int) bool { t.mu.Lock() defer t.mu.Unlock() known := map[int]struct{}{t.root: {}} for pid := range t.seen { known[pid] = struct{}{} } + changed := false for _, pid := range adoptedDescendants(parents, t.attempt) { known[pid] = struct{}{} - t.seen[pid] = struct{}{} + if _, ok := t.seen[pid]; !ok { + t.seen[pid] = struct{}{} + changed = true + } } for { added := false @@ -79,9 +95,10 @@ func (t *processTree) snapshot() { known[pid] = struct{}{} t.seen[pid] = struct{}{} added = true + changed = true } if !added { - return + return changed } } }