From c1a674634e93c19d0389ad2a2f33b37b2d9878d2 Mon Sep 17 00:00:00 2001 From: mintaka Date: Fri, 11 Sep 2026 16:05:03 -0400 Subject: [PATCH 1/2] fix(compass-app): reap the gtk4 e2e WebKit children before exit (RIG-2526) WebKitGTK fork+execs its GPU/network helpers in C. They inherit the test binary's stdout, and upstream teardown never joins them: linuxApp.destroy tears down at the GTK level only. On a GPU-less runner a helper takes seconds to abort on EGL, so go test waits out its WaitDelay and a PASS is reported as "Test I/O incomplete". Teardown now joins them: SIGTERM, SIGKILL after a grace, bounded at 10s, and a survivor past the bound reds loudly instead of tripping the opaque WaitDelay. A real test failure keeps its own exit code. The helpers block until signaled, so a passive wait never ends. Sealing our stdio FD_CLOEXEC was measured as the wrong fix: the gate went green while orphaning a WebKitWebProcess to init on every run. Co-authored-by: Matt Wilkinson --- .../multiwindow_e2e_reap_linux_test.go | 92 +++++++++++++++++++ .../multiwindow_e2e_reap_other_test.go | 8 ++ go/cmd/compass-app/multiwindow_e2e_test.go | 72 ++++++++++++++- 3 files changed, 171 insertions(+), 1 deletion(-) create mode 100644 go/cmd/compass-app/multiwindow_e2e_reap_linux_test.go create mode 100644 go/cmd/compass-app/multiwindow_e2e_reap_other_test.go diff --git a/go/cmd/compass-app/multiwindow_e2e_reap_linux_test.go b/go/cmd/compass-app/multiwindow_e2e_reap_linux_test.go new file mode 100644 index 000000000..10a0d4470 --- /dev/null +++ b/go/cmd/compass-app/multiwindow_e2e_reap_linux_test.go @@ -0,0 +1,92 @@ +//go:build linux && gtk4 + +package main + +import ( + "os" + "strconv" + "strings" +) + +// liveChildren returns every process still descended from this one, discovered +// by walking /proc for entries whose ppid chains back to os.Getpid(). WebKit's +// GPU/network helpers are our direct children; scanning the whole subtree also +// catches any helper-of-a-helper before it reparents to init. +func liveChildren() []procInfo { + self := os.Getpid() + + entries, err := os.ReadDir("/proc") + if err != nil { + // Without procfs there is nothing to reap and no way to prove otherwise; + // treat as clean so the gate never wedges on an unreadable /proc. + return nil + } + + ppids := make(map[int]int, len(entries)) + comms := make(map[int]string, len(entries)) + for _, e := range entries { + pid, err := strconv.Atoi(e.Name()) + if err != nil { + continue // not a pid directory + } + ppid, comm, ok := readStat(pid) + if !ok { + continue // vanished between readdir and read; not our concern + } + ppids[pid] = ppid + comms[pid] = comm + } + + var out []procInfo + for pid := range ppids { + if pid != self && descendsFrom(pid, self, ppids) { + out = append(out, procInfo{pid: pid, comm: comms[pid]}) + } + } + return out +} + +// readStat parses ppid and comm out of /proc//stat. comm can hold spaces +// and parens, so key off the final ')': the two space-separated fields after it +// are state then ppid. +func readStat(pid int) (ppid int, comm string, ok bool) { + raw, err := os.ReadFile("/proc/" + strconv.Itoa(pid) + "/stat") + if err != nil { + return 0, "", false + } + s := string(raw) + open := strings.IndexByte(s, '(') + shut := strings.LastIndexByte(s, ')') + if open < 0 || shut < 0 || shut < open { + return 0, "", false + } + comm = s[open+1 : shut] + fields := strings.Fields(s[shut+1:]) + if len(fields) < 2 { + return 0, "", false + } + ppid, err = strconv.Atoi(fields[1]) + if err != nil { + return 0, "", false + } + return ppid, comm, true +} + +// descendsFrom reports whether pid chains up to root through the ppid map. The +// walk is bounded by the map size so pid reuse mid-scan cannot loop forever. +func descendsFrom(pid, root int, ppids map[int]int) bool { + for hops := 0; hops <= len(ppids); hops++ { + ppid, seen := ppids[pid] + if !seen { + return false + } + if ppid == root { + return true + } + if ppid <= 1 { + return false // reached init/kernel without hitting root + } + pid = ppid + } + return false +} diff --git a/go/cmd/compass-app/multiwindow_e2e_reap_other_test.go b/go/cmd/compass-app/multiwindow_e2e_reap_other_test.go new file mode 100644 index 000000000..c69d0a5f7 --- /dev/null +++ b/go/cmd/compass-app/multiwindow_e2e_reap_other_test.go @@ -0,0 +1,8 @@ +//go:build unix && gtk4 && !linux + +package main + +// liveChildren has no portable non-linux implementation; the leaked-child reap +// targets WebKitGTK under the linux e2e gate, and darwin does not run it. A +// no-op keeps the non-linux unix build compiling and the reap a clean pass. +func liveChildren() []procInfo { return nil } diff --git a/go/cmd/compass-app/multiwindow_e2e_test.go b/go/cmd/compass-app/multiwindow_e2e_test.go index 40b21ba5b..a03736251 100644 --- a/go/cmd/compass-app/multiwindow_e2e_test.go +++ b/go/cmd/compass-app/multiwindow_e2e_test.go @@ -24,9 +24,14 @@ package main import ( + "errors" + "fmt" "net/http" "os" + "strings" + "syscall" "testing" + "time" "github.com/RigelBuild/compass/go/internal/bridge" "github.com/wailsapp/wails/v3/pkg/application" @@ -69,7 +74,72 @@ func TestMain(m *testing.M) { os.Stderr.WriteString("compass-app multi-window e2e: app.Run: " + err.Error() + "\n") os.Exit(1) } - os.Exit(e2eExitCode) + // WebKitGTK fork+execs its GPU/network helpers in C as our direct children; + // upstream teardown never joins them. Left alive they hold go test's captured + // pipe until its WaitDelay fires (a PASS turns into "Test I/O incomplete"), or + // orphan to init on a reused runner. Reap them here, bounded and loud. + os.Exit(reapChildren(e2eExitCode)) +} + +// reapWait hard-bounds the join; past it a child is wedged, so the gate reds +// loudly instead of tripping go test's opaque WaitDelay. reapEscalate is the +// SIGTERM grace before SIGKILL — the helpers honor SIGTERM slowly. +const ( + reapWait = 10 * time.Second + reapEscalate = 2 * time.Second +) + +// procInfo names one surviving child for the timeout diagnostic. +type procInfo struct { + pid int + comm string +} + +// reapChildren joins the WebKit helpers WebKitGTK fork+exec'd as our children. +// They block until signaled (the app has quit), so a passive wait never ends: +// SIGTERM, then SIGKILL if they dawdle, until none remain or reapWait elapses. +// A survivor past the bound reds loudly and non-zero, never masking a failure. +func reapChildren(exitCode int) int { + start := time.Now() + signalChildren(liveChildren(), syscall.SIGTERM) + + ticker := time.NewTicker(100 * time.Millisecond) + defer ticker.Stop() + escalated := false + for { + remaining := liveChildren() + if len(remaining) == 0 { + return exitCode + } + if time.Since(start) >= reapWait { + var b strings.Builder + fmt.Fprintf(&b, "compass-app multi-window e2e: %d child process(es) survived the %s reap deadline:\n", len(remaining), reapWait) + for _, p := range remaining { + fmt.Fprintf(&b, " pid=%d comm=%q\n", p.pid, p.comm) + } + os.Stderr.WriteString(b.String()) + if exitCode == 0 { + return 1 + } + return exitCode + } + if !escalated && time.Since(start) >= reapEscalate { + signalChildren(remaining, syscall.SIGKILL) + escalated = true + } + <-ticker.C + } +} + +// signalChildren sends sig to each named process. ESRCH is expected and ignored +// — the child exited between discovery and the signal; any other error is a real +// fault worth surfacing, never swallowed. +func signalChildren(procs []procInfo, sig syscall.Signal) { + for _, p := range procs { + if err := syscall.Kill(p.pid, sig); err != nil && !errors.Is(err, syscall.ESRCH) { + fmt.Fprintf(os.Stderr, "compass-app multi-window e2e: signal %d pid=%d: %v\n", sig, p.pid, err) + } + } } // TestMultiWindowCloseCancelsOnlyClosingWindowE2E is the leak-gate proof through From 1e12217ce4b7d72858fa3810b25b28b1e6372ac7 Mon Sep 17 00:00:00 2001 From: mintaka Date: Fri, 11 Sep 2026 16:32:16 -0400 Subject: [PATCH 2/2] test(compass-app): unit-cover the reaper's signal-target logic Review: deciding which pid gets signalled is the highest-stakes logic in the change and had only integration cover, so a regressed chain walk would ship green. Splits the pure parse out of readStat and table-tests both. Proven to catch the real regressions: dropping descendsFrom's ppid==root precedence, and keying the parse off the first ')' instead of the last, each fail six subtests. Also re-signals each pass instead of latching, so a helper first seen after the grace still gets SIGKILL. Co-authored-by: Matt Wilkinson --- .../multiwindow_e2e_reap_linux_test.go | 15 +++-- .../multiwindow_e2e_reap_linux_unit_test.go | 66 +++++++++++++++++++ go/cmd/compass-app/multiwindow_e2e_test.go | 6 +- 3 files changed, 79 insertions(+), 8 deletions(-) create mode 100644 go/cmd/compass-app/multiwindow_e2e_reap_linux_unit_test.go diff --git a/go/cmd/compass-app/multiwindow_e2e_reap_linux_test.go b/go/cmd/compass-app/multiwindow_e2e_reap_linux_test.go index 10a0d4470..b21b63c6b 100644 --- a/go/cmd/compass-app/multiwindow_e2e_reap_linux_test.go +++ b/go/cmd/compass-app/multiwindow_e2e_reap_linux_test.go @@ -46,15 +46,20 @@ func liveChildren() []procInfo { return out } -// readStat parses ppid and comm out of /proc//stat. comm can hold spaces -// and parens, so key off the final ')': the two space-separated fields after it -// are state then ppid. +// readStat reads ppid and comm for one pid, tolerating a process that exits +// mid-scan. func readStat(pid int) (ppid int, comm string, ok bool) { raw, err := os.ReadFile("/proc/" + strconv.Itoa(pid) + "/stat") if err != nil { return 0, "", false } - s := string(raw) + return parseStat(string(raw)) +} + +// parseStat pulls ppid and comm out of a /proc//stat line. comm can hold +// spaces and parens, so key off the final ')': the two fields after it are +// state then ppid. +func parseStat(s string) (ppid int, comm string, ok bool) { open := strings.IndexByte(s, '(') shut := strings.LastIndexByte(s, ')') if open < 0 || shut < 0 || shut < open { @@ -65,7 +70,7 @@ func readStat(pid int) (ppid int, comm string, ok bool) { if len(fields) < 2 { return 0, "", false } - ppid, err = strconv.Atoi(fields[1]) + ppid, err := strconv.Atoi(fields[1]) if err != nil { return 0, "", false } diff --git a/go/cmd/compass-app/multiwindow_e2e_reap_linux_unit_test.go b/go/cmd/compass-app/multiwindow_e2e_reap_linux_unit_test.go new file mode 100644 index 000000000..291e621cc --- /dev/null +++ b/go/cmd/compass-app/multiwindow_e2e_reap_linux_unit_test.go @@ -0,0 +1,66 @@ +//go:build linux && gtk4 + +package main + +import "testing" + +// The chain walk decides which pids get signalled, so a regression here would +// aim SIGKILL at something we do not own. Every non-descendant case must answer +// false: under-signalling leaks a process, over-signalling kills a stranger. +func TestDescendsFrom(t *testing.T) { + const self = 100 + + tests := []struct { + name string + pid int + ppids map[int]int + want bool + }{ + {"direct child", 200, map[int]int{200: self, self: 1}, true}, + {"deep grandchild", 400, map[int]int{400: 300, 300: 200, 200: self, self: 1}, true}, + {"reparented to init", 200, map[int]int{200: 1, self: 1}, false}, + {"sibling under a subreaper", 200, map[int]int{200: 50, 50: 1, self: 1}, false}, + {"broken link", 400, map[int]int{400: 300, self: 1}, false}, + {"cycle terminates", 200, map[int]int{200: 300, 300: 200, self: 1}, false}, + {"self is not its own descendant", self, map[int]int{self: 1}, false}, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + if got := descendsFrom(tc.pid, self, tc.ppids); got != tc.want { + t.Fatalf("descendsFrom(%d, %d) = %v, want %v", tc.pid, self, got, tc.want) + } + }) + } +} + +// A comm holding spaces or parens would break a naive field split, and a +// misparsed ppid is a wrong signal target. +func TestParseStat(t *testing.T) { + tests := []struct { + name string + line string + wantPPID int + wantComm string + wantOK bool + }{ + {"plain", "42 (bash) S 7 42 42 0 -1 4194304", 7, "bash", true}, + {"comm with spaces", "42 (Web Content) S 7 42", 7, "Web Content", true}, + {"comm with parens", "42 (odd (name)) S 7 42", 7, "odd (name)", true}, + {"comm with a space and a paren", "42 (a b) c) R 9 42", 9, "a b) c", true}, + {"truncated after comm", "42 (bash)", 0, "", false}, + {"no parens", "42 bash S 7", 0, "", false}, + {"non-numeric ppid", "42 (bash) S x 42", 0, "", false}, + {"empty", "", 0, "", false}, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + ppid, comm, ok := parseStat(tc.line) + if ok != tc.wantOK || ppid != tc.wantPPID || comm != tc.wantComm { + t.Fatalf("parseStat(%q) = (%d, %q, %v), want (%d, %q, %v)", + tc.line, ppid, comm, ok, tc.wantPPID, tc.wantComm, tc.wantOK) + } + }) + } +} diff --git a/go/cmd/compass-app/multiwindow_e2e_test.go b/go/cmd/compass-app/multiwindow_e2e_test.go index a03736251..b37f2b711 100644 --- a/go/cmd/compass-app/multiwindow_e2e_test.go +++ b/go/cmd/compass-app/multiwindow_e2e_test.go @@ -105,7 +105,6 @@ func reapChildren(exitCode int) int { ticker := time.NewTicker(100 * time.Millisecond) defer ticker.Stop() - escalated := false for { remaining := liveChildren() if len(remaining) == 0 { @@ -123,9 +122,10 @@ func reapChildren(exitCode int) int { } return exitCode } - if !escalated && time.Since(start) >= reapEscalate { + // Re-signal each pass rather than latching: a helper first seen after + // the grace would otherwise get neither SIGTERM nor SIGKILL. + if time.Since(start) >= reapEscalate { signalChildren(remaining, syscall.SIGKILL) - escalated = true } <-ticker.C }