fix(perf-harness): escalate client teardown without truncating traces

The bounded 2s teardown closed the PTY master while the client could
still be mid-teardown on a loaded runner. With hangup now a graceful
detach, restructure Close into an explicit escalation ladder (shell
exit, hangup, SIGTERM, SIGKILL), drain terminal output for the whole
teardown so a full chunks channel can never wedge copyTerminal, and
fail teardown with a named error when only SIGKILL reaps the role
instead of surfacing the truncation later as a merge mystery.

Open the harness PTY with O_CLOEXEC: the leaked master previously held
the terminal open inside the client, so closing the harness master never
delivered a hangup at all and escalation went straight to signals.

Fixes the intermittent Ubuntu-only failure of
TestHarnessResolvesCanonicalRelativePathsBeforeRoleCWDChanges (missing
process-local span pair for the final in-flight adapter receive).

commit: 681ca80179a868f6f2c145986a4029a3bd09ebdb
author: brice · 3mo ago · 2026-07-18 16:28 UTC
committer: brice · 3mo ago · 2026-07-18 16:36 UTC
parent: 05e5006
signature: GPG VerifiedCD88F9E564E0102646FB6077A1A07C7C7C3E349D · arch-gpg Verified for the committer email

3 files · +332 -55

A cmd/vev-perf-harness/hangup_test.go +168 -0

@@ -0,0 +1,168 @@
	1	+	//go:build linux
	2	+
	3	+	package main
	4	+
	5	+	import (
	6	+	"encoding/json"
	7	+	"fmt"
	8	+	"os"
	9	+	"os/exec"
	10	+	"path/filepath"
	11	+	"strings"
	12	+	"testing"
	13	+	"time"
	14	+	)
	15	+
	16	+	// TestClientTracePairsAfterPTYHangup drives the real teardown escalation: a
	17	+	// live client is severed by closing its controlling PTY master (the harness
	18	+	// hangup stage) with no graceful "exit\n". A client that dies from the default
	19	+	// SIGHUP disposition truncates its receive pump's in-flight span, so its trace
	20	+	// merge fails with an unpaired adapter_receive_start. A client that survives the
	21	+	// hangup closes its transport and serializes the matching receive-end first, so
	22	+	// the merge pairs.
	23	+	func TestClientTracePairsAfterPTYHangup(t *testing.T) {
	24	+	if testing.Short() {
	25	+	t.Skip("public CLI hangup lifecycle")
	26	+	}
	27	+	root, err := filepath.Abs(filepath.Join("..", ".."))
	28	+	if err != nil {
	29	+	t.Fatal(err)
	30	+	}
	31	+	// Keep the run directory (and thus the Unix socket path) short: MkdirTemp in
	32	+	// the system temp root stays well under the platform socket-path limit.
	33	+	runDir, err := os.MkdirTemp("", "vev-hangup-")
	34	+	if err != nil {
	35	+	t.Fatal(err)
	36	+	}
	37	+	removeTestTree(t, runDir)
	38	+	bin := filepath.Join(runDir, "vev")
	39	+	build := exec.Command("go", "build", "-o", bin, "./")
	40	+	build.Dir = root
	41	+	if output, err := build.CombinedOutput(); err != nil {
	42	+	t.Fatalf("build public CLI: %v\n%s", err, output)
	43	+	}
	44	+
	45	+	s := scenario{ID: "hangup-local", Transport: "local", Workload: "idle", Roles: []string{"daemon", "client"}}
	46	+	selected, err := manifestTransport(manifest{}, s.Transport)
	47	+	if err != nil {
	48	+	t.Fatal(err)
	49	+	}
	50	+
	51	+	maps := make([]processMapping, 0, len(s.Roles))
	52	+	for i, role := range s.Roles {
	53	+	pid := fmt.Sprintf("%s-r%03d-%s-%02d", safeName(s.ID), 1, safeName(role), i+1)
	54	+	path := filepath.Join(runDir, pid+".jsonl")
	55	+	f, err := os.OpenFile(path, os.O_WRONLY\|os.O_CREATE\|os.O_EXCL, 0o600)
	56	+	if err != nil {
	57	+	t.Fatal(err)
	58	+	}
	59	+	if err := f.Close(); err != nil {
	60	+	t.Fatal(err)
	61	+	}
	62	+	maps = append(maps, processMapping{ProcessID: pid, ClockDomain: pid, TracePath: path, Role: role, Scenario: s.ID, Run: 1, Identity: "harness-assigned:" + pid})
	63	+	}
	64	+
	65	+	launcher := &cliLauncher{bin: bin}
	66	+	var daemonProc launchedProcess
	67	+	var client *cliProcess
	68	+	var clientMapping processMapping
	69	+	for _, pm := range launchOrder(maps) {
	70	+	p, err := launcher.Launch(pm, routeRoleArgs(s, pm, selected))
	71	+	if err != nil {
	72	+	t.Fatalf("launch %s: %v", pm.Role, err)
	73	+	}
	74	+	switch pm.Role {
	75	+	case "daemon":
	76	+	daemonProc = p
	77	+	ready, ok := p.(interface{ WaitReady() error })
	78	+	if !ok {
	79	+	t.Fatal("daemon role does not expose WaitReady")
	80	+	}
	81	+	if err := ready.WaitReady(); err != nil {
	82	+	t.Fatalf("daemon readiness: %v", err)
	83	+	}
	84	+	case "client":
	85	+	cp, ok := p.(*cliProcess)
	86	+	if !ok {
	87	+	t.Fatalf("client role is %T, want *cliProcess", p)
	88	+	}
	89	+	client, clientMapping = cp, pm
	90	+	}
	91	+	}
	92	+	if client == nil \|\| daemonProc == nil {
	93	+	t.Fatal("did not launch both roles")
	94	+	}
	95	+	// Close the daemon last, on every exit path, so a failed assertion cannot
	96	+	// leak a foreground daemon or its runtime directory.
	97	+	defer func() {
	98	+	if err := daemonProc.Close(); err != nil {
	99	+	t.Errorf("close daemon role: %v", err)
	100	+	}
	101	+	}()
	102	+
	103	+	// Wait until the client is attached with a receive pump blocked mid-frame:
	104	+	// its trace then holds one more adapter_receive_start than end. That in-flight
	105	+	// span is exactly what an ungraceful death truncates.
	106	+	if err := waitForInflightReceive(clientMapping.TracePath, 10*time.Second); err != nil {
	107	+	t.Fatalf("client never reached a blocked receive: %v", err)
	108	+	}
	109	+
	110	+	// Sever the controlling terminal with no graceful exit. This is the harness
	111	+	// hangup escalation stage, delivering SIGHUP to the client session leader.
	112	+	if err := client.pty.Close(); err != nil {
	113	+	t.Fatalf("close client PTY master: %v", err)
	114	+	}
	115	+	select {
	116	+	case <-client.waitErr:
	117	+	case <-time.After(10 * time.Second):
	118	+	t.Fatal("client did not exit after PTY hangup")
	119	+	}
	120	+	if client.output != nil {
	121	+	if err := client.output.Close(); err != nil {
	122	+	t.Errorf("close client terminal output: %v", err)
	123	+	}
	124	+	}
	125	+
	126	+	// The severed client's trace must pair. On the unfixed client this fails with
	127	+	// "missing process-local span pair" for the truncated adapter_receive span.
	128	+	if _, err := mergeProcessTraces([]processMapping{clientMapping}); err != nil {
	129	+	t.Fatalf("client trace did not pair after PTY hangup: %v", err)
	130	+	}
	131	+	}
	132	+
	133	+	// waitForInflightReceive polls a process trace until it holds an unpaired
	134	+	// adapter_receive_start (more starts than ends). It tolerates a partially
	135	+	// written trailing line while the traced process appends concurrently.
	136	+	func waitForInflightReceive(path string, timeout time.Duration) error {
	137	+	deadline := time.Now().Add(timeout)
	138	+	for {
	139	+	starts, ends := 0, 0
	140	+	data, err := os.ReadFile(path)
	141	+	if err != nil {
	142	+	return err
	143	+	}
	144	+	for _, line := range strings.Split(string(data), "\n") {
	145	+	line = strings.TrimSpace(line)
	146	+	if line == "" {
	147	+	continue
	148	+	}
	149	+	var r traceRecord
	150	+	if json.Unmarshal([]byte(line), &r) != nil {
	151	+	continue // a concurrently appended, not-yet-complete final line
	152	+	}
	153	+	switch r.Kind {
	154	+	case "adapter_receive_start":
	155	+	starts++
	156	+	case "adapter_receive_end":
	157	+	ends++
	158	+	}
	159	+	}
	160	+	if starts > ends {
	161	+	return nil
	162	+	}
	163	+	if time.Now().After(deadline) {
	164	+	return fmt.Errorf("adapter_receive starts=%d ends=%d, want an in-flight start", starts, ends)
	165	+	}
	166	+	time.Sleep(10 * time.Millisecond)
	167	+	}
	168	+	}

@@ -0,0 +1,168 @@
	1	+	//go:build linux
	2	+
	3	+	package main
	4	+
	5	+	import (
	6	+	"encoding/json"
	7	+	"fmt"
	8	+	"os"
	9	+	"os/exec"
	10	+	"path/filepath"
	11	+	"strings"
	12	+	"testing"
	13	+	"time"
	14	+	)
	15	+
	16	+	// TestClientTracePairsAfterPTYHangup drives the real teardown escalation: a
	17	+	// live client is severed by closing its controlling PTY master (the harness
	18	+	// hangup stage) with no graceful "exit\n". A client that dies from the default
	19	+	// SIGHUP disposition truncates its receive pump's in-flight span, so its trace
	20	+	// merge fails with an unpaired adapter_receive_start. A client that survives the
	21	+	// hangup closes its transport and serializes the matching receive-end first, so
	22	+	// the merge pairs.
	23	+	func TestClientTracePairsAfterPTYHangup(t *testing.T) {
	24	+	if testing.Short() {
	25	+	t.Skip("public CLI hangup lifecycle")
	26	+	}
	27	+	root, err := filepath.Abs(filepath.Join("..", ".."))
	28	+	if err != nil {
	29	+	t.Fatal(err)
	30	+	}
	31	+	// Keep the run directory (and thus the Unix socket path) short: MkdirTemp in
	32	+	// the system temp root stays well under the platform socket-path limit.
	33	+	runDir, err := os.MkdirTemp("", "vev-hangup-")
	34	+	if err != nil {
	35	+	t.Fatal(err)
	36	+	}
	37	+	removeTestTree(t, runDir)
	38	+	bin := filepath.Join(runDir, "vev")
	39	+	build := exec.Command("go", "build", "-o", bin, "./")
	40	+	build.Dir = root
	41	+	if output, err := build.CombinedOutput(); err != nil {
	42	+	t.Fatalf("build public CLI: %v\n%s", err, output)
	43	+	}
	44	+
	45	+	s := scenario{ID: "hangup-local", Transport: "local", Workload: "idle", Roles: []string{"daemon", "client"}}
	46	+	selected, err := manifestTransport(manifest{}, s.Transport)
	47	+	if err != nil {
	48	+	t.Fatal(err)
	49	+	}
	50	+
	51	+	maps := make([]processMapping, 0, len(s.Roles))
	52	+	for i, role := range s.Roles {
	53	+	pid := fmt.Sprintf("%s-r%03d-%s-%02d", safeName(s.ID), 1, safeName(role), i+1)
	54	+	path := filepath.Join(runDir, pid+".jsonl")
	55	+	f, err := os.OpenFile(path, os.O_WRONLY\|os.O_CREATE\|os.O_EXCL, 0o600)
	56	+	if err != nil {
	57	+	t.Fatal(err)
	58	+	}
	59	+	if err := f.Close(); err != nil {
	60	+	t.Fatal(err)
	61	+	}
	62	+	maps = append(maps, processMapping{ProcessID: pid, ClockDomain: pid, TracePath: path, Role: role, Scenario: s.ID, Run: 1, Identity: "harness-assigned:" + pid})
	63	+	}
	64	+
	65	+	launcher := &cliLauncher{bin: bin}
	66	+	var daemonProc launchedProcess
	67	+	var client *cliProcess
	68	+	var clientMapping processMapping
	69	+	for _, pm := range launchOrder(maps) {
	70	+	p, err := launcher.Launch(pm, routeRoleArgs(s, pm, selected))
	71	+	if err != nil {
	72	+	t.Fatalf("launch %s: %v", pm.Role, err)
	73	+	}
	74	+	switch pm.Role {
	75	+	case "daemon":
	76	+	daemonProc = p
	77	+	ready, ok := p.(interface{ WaitReady() error })
	78	+	if !ok {
	79	+	t.Fatal("daemon role does not expose WaitReady")
	80	+	}
	81	+	if err := ready.WaitReady(); err != nil {
	82	+	t.Fatalf("daemon readiness: %v", err)
	83	+	}
	84	+	case "client":
	85	+	cp, ok := p.(*cliProcess)
	86	+	if !ok {
	87	+	t.Fatalf("client role is %T, want *cliProcess", p)
	88	+	}
	89	+	client, clientMapping = cp, pm
	90	+	}
	91	+	}
	92	+	if client == nil \|\| daemonProc == nil {
	93	+	t.Fatal("did not launch both roles")
	94	+	}
	95	+	// Close the daemon last, on every exit path, so a failed assertion cannot
	96	+	// leak a foreground daemon or its runtime directory.
	97	+	defer func() {
	98	+	if err := daemonProc.Close(); err != nil {
	99	+	t.Errorf("close daemon role: %v", err)
	100	+	}
	101	+	}()
	102	+
	103	+	// Wait until the client is attached with a receive pump blocked mid-frame:
	104	+	// its trace then holds one more adapter_receive_start than end. That in-flight
	105	+	// span is exactly what an ungraceful death truncates.
	106	+	if err := waitForInflightReceive(clientMapping.TracePath, 10*time.Second); err != nil {
	107	+	t.Fatalf("client never reached a blocked receive: %v", err)
	108	+	}
	109	+
	110	+	// Sever the controlling terminal with no graceful exit. This is the harness
	111	+	// hangup escalation stage, delivering SIGHUP to the client session leader.
	112	+	if err := client.pty.Close(); err != nil {
	113	+	t.Fatalf("close client PTY master: %v", err)
	114	+	}
	115	+	select {
	116	+	case <-client.waitErr:
	117	+	case <-time.After(10 * time.Second):
	118	+	t.Fatal("client did not exit after PTY hangup")
	119	+	}
	120	+	if client.output != nil {
	121	+	if err := client.output.Close(); err != nil {
	122	+	t.Errorf("close client terminal output: %v", err)
	123	+	}
	124	+	}
	125	+
	126	+	// The severed client's trace must pair. On the unfixed client this fails with
	127	+	// "missing process-local span pair" for the truncated adapter_receive span.
	128	+	if _, err := mergeProcessTraces([]processMapping{clientMapping}); err != nil {
	129	+	t.Fatalf("client trace did not pair after PTY hangup: %v", err)
	130	+	}
	131	+	}
	132	+
	133	+	// waitForInflightReceive polls a process trace until it holds an unpaired
	134	+	// adapter_receive_start (more starts than ends). It tolerates a partially
	135	+	// written trailing line while the traced process appends concurrently.
	136	+	func waitForInflightReceive(path string, timeout time.Duration) error {
	137	+	deadline := time.Now().Add(timeout)
	138	+	for {
	139	+	starts, ends := 0, 0
	140	+	data, err := os.ReadFile(path)
	141	+	if err != nil {
	142	+	return err
	143	+	}
	144	+	for _, line := range strings.Split(string(data), "\n") {
	145	+	line = strings.TrimSpace(line)
	146	+	if line == "" {
	147	+	continue
	148	+	}
	149	+	var r traceRecord
	150	+	if json.Unmarshal([]byte(line), &r) != nil {
	151	+	continue // a concurrently appended, not-yet-complete final line
	152	+	}
	153	+	switch r.Kind {
	154	+	case "adapter_receive_start":
	155	+	starts++
	156	+	case "adapter_receive_end":
	157	+	ends++
	158	+	}
	159	+	}
	160	+	if starts > ends {
	161	+	return nil
	162	+	}
	163	+	if time.Now().After(deadline) {
	164	+	return fmt.Errorf("adapter_receive starts=%d ends=%d, want an in-flight start", starts, ends)
	165	+	}
	166	+	time.Sleep(10 * time.Millisecond)
	167	+	}
	168	+	}

M cmd/vev-perf-harness/launcher.go +122 -55


@@ -625,77 +625,111 @@ func (e *ptyLocalEcho) applicationOutput(chunk []byte) []byte {
625	625		return nil
626	626		}
627	627
	628	+	// Close tears down a launched role along a bounded escalation ladder. Each rung
	629	+	// performs one action, then waits a single closeDeadline for the process to be
	630	+	// reaped before the next rung escalates. The graceful rungs — the public daemon
	631	+	// stop, the client shell exit, the controlling-terminal hangup, and SIGTERM —
	632	+	// let a role close its blocked transport operations and serialize their end
	633	+	// marks; only a SIGKILL severs a process before it can, so only that rung fails
	634	+	// teardown. Every path closes the PTY exactly once and finalizes output and
	635	+	// runtime state.
628	636		func (p *cliProcess) Close() error {
629	637		var result error
630	638		p.closed.Do(func() {
631		-	// No measurement waits for terminal chunks during teardown. Continue
632		-	// consuming them so a full channel cannot block the client before it
633		-	// receives the shell exit request and closes its transport receive span.
634		-	if p.chunks != nil && p.done != nil {
635		-	go func() {
636		-	for {
637		-	select {
638		-	case _, ok := <-p.chunks:
639		-	if !ok {
640		-	return
641		-	}
642		-	case <-p.done:
643		-	return
644		-	}
645		-	}
646		-	}()
647		-	}
648		-	// A public terminal client owns a shell session. Ask that shell to exit
649		-	// before closing the PTY so its transport can finish its receive span;
650		-	// this avoids turning ordinary harness teardown into an aborted trace.
	639	+	// 1. Drain terminal output for the whole teardown. Without an active
	640	+	// consumer a full chunks channel stalls copyTerminal, which then stops
	641	+	// reading the PTY master and blocks the client mid-restore before it can
	642	+	// close its observed receive operation. The drain stops with p.done.
	643	+	drainDone := p.drainTerminalOutput()
	644	+
651	645		exited := false
652		-	if p.shutdown != nil && p.waitErr != nil {
	646	+	// waitForExit blocks up to one bounded deadline for the reap.
	647	+	waitForExit := func() {
	648	+	if p.waitErr == nil \|\| exited {
	649	+	return
	650	+	}
653	651		select {
654	652		case <-p.waitErr:
655	653		exited = true
656		-	default:
657		-	// The client may have just ended the last session and raced this
658		-	// best-effort public shutdown. The normal bounded reaping path below
659		-	// remains authoritative in that case.
660		-	_ = p.shutdown()
	654	+	case <-p.closeDeadline():
661	655		}
662	656		}
663		-	if p.pty != nil && p.waitErr != nil {
664		-	_, _ = p.pty.Write([]byte("exit\n"))
	657	+	// reaped reports whether the process is already gone; a role with no
	658	+	// process to reap needs no escalation at all.
	659	+	reaped := func() bool {
	660	+	if p.waitErr == nil \|\| exited {
	661	+	return true
	662	+	}
665	663		select {
666	664		case <-p.waitErr:
667	665		exited = true
668		-	case <-p.closeDeadline():
	666	+	return true
	667	+	default:
	668	+	return false
669	669		}
670	670		}
671		-	// Keep draining terminal output while the client exits gracefully. If the
672		-	// drain stops first, restoration output can fill the PTY and block the
673		-	// client before it closes its observed transport receive operation.
674		-	if p.done != nil {
675		-	close(p.done)
	671	+	ptyClosed := false
	672	+	closePTY := func() {
	673	+	if p.pty != nil && !ptyClosed {
	674	+	_ = p.pty.Close()
	675	+	ptyClosed = true
	676	+	}
676	677		}
677		-	if p.pty != nil {
678		-	_ = p.pty.Close()
	678	+
	679	+	// 2. Daemon role: ask the public daemon to stop before any signal so its
	680	+	// blocked carriage can close and serialize failed end marks.
	681	+	if p.shutdown != nil && !reaped() {
	682	+	_ = p.shutdown()
	683	+	waitForExit()
679	684		}
680		-	if p.waitErr != nil && !exited {
681		-	select {
682		-	case <-p.waitErr:
683		-	case <-p.closeDeadline():
684		-	// Teardown is deliberately bounded: a stuck public CLI or child
685		-	// must not hold a local harness run indefinitely.
686		-	p.forceProcessGroupCleanup()
687		-	select {
688		-	case <-p.waitErr:
689		-	case <-p.closeDeadline():
690		-	p.forceProcessGroupKill()
691		-	select {
692		-	case <-p.waitErr:
693		-	case <-p.closeDeadline():
694		-	result = errors.New("timed out reaping public CLI process")
695		-	}
696		-	}
	685	+	// 3. Client role: a public terminal client owns a shell session. Ask that
	686	+	// shell to exit so the client detaches and its transport finishes its
	687	+	// receive span cleanly.
	688	+	if p.pty != nil && !reaped() {
	689	+	_, _ = p.pty.Write([]byte("exit\n"))
	690	+	waitForExit()
	691	+	}
	692	+	// 4. Hang up the controlling terminal. The attach path installs a signal
	693	+	// handler, so this SIGHUP is a graceful detach — not an abrupt kill — and
	694	+	// the client still flushes its in-flight receive end mark.
	695	+	if p.pty != nil && !reaped() {
	696	+	closePTY()
	697	+	waitForExit()
	698	+	}
	699	+	// 5. SIGTERM the role's own process group. The daemon and, through its
	700	+	// attach signal handler, the client both treat this as a graceful stop.
	701	+	if !reaped() {
	702	+	p.forceProcessGroupCleanup()
	703	+	waitForExit()
	704	+	}
	705	+	// 6. SIGKILL the role's process group. This severs the process; an
	706	+	// in-flight observed operation cannot be closed, so its trace may truncate.
	707	+	killed := false
	708	+	if !reaped() {
	709	+	killed = true
	710	+	p.forceProcessGroupKill()
	711	+	waitForExit()
	712	+	}
	713	+	// 7. A SIGKILL escalation cannot guarantee a complete trace. Fail teardown
	714	+	// with a clear cause rather than letting the truncation surface later as a
	715	+	// downstream merge mystery. The graceful rungs never error on their own.
	716	+	if killed {
	717	+	if exited {
	718	+	result = errors.New("public CLI process did not exit after hangup and SIGTERM; killed")
	719	+	} else {
	720	+	result = errors.New("timed out reaping public CLI process")
697	721		}
698	722		}
	723	+
	724	+	// Close the PTY exactly once on every path: an early graceful exit skips
	725	+	// the hangup rung, so close it here before the drain stops.
	726	+	closePTY()
	727	+	if p.done != nil {
	728	+	close(p.done)
	729	+	}
	730	+	if drainDone != nil {
	731	+	<-drainDone
	732	+	}
699	733		if p.output != nil {
700	734		if err := p.output.Close(); result == nil {
701	735		result = err

@@ -710,6 +744,34 @@ func (p *cliProcess) Close() error {
710	744		return result
711	745		}
712	746
	747	+	// drainTerminalOutput consumes copyTerminal's chunks for the duration of a
	748	+	// teardown so a full channel can never wedge copyTerminal (and thus the client's
	749	+	// PTY). It returns nil when there is no terminal to drain, and otherwise a
	750	+	// channel closed once the consumer has stopped on p.done.
	751	+	func (p *cliProcess) drainTerminalOutput() <-chan struct{} {
	752	+	if p.chunks == nil \|\| p.done == nil {
	753	+	return nil
	754	+	}
	755	+	done := make(chan struct{})
	756	+	chunks := p.chunks
	757	+	go func() {
	758	+	defer close(done)
	759	+	for {
	760	+	select {
	761	+	case _, ok := <-chunks:
	762	+	if !ok {
	763	+	// copyTerminal closed the channel; a closed channel is always
	764	+	// ready, so stop selecting it and wait only for the stop signal.
	765	+	chunks = nil
	766	+	}
	767	+	case <-p.done:
	768	+	return
	769	+	}
	770	+	}
	771	+	}()
	772	+	return done
	773	+	}
	774	+
713	775		func (p *cliProcess) closeDeadline() <-chan time.Time {
714	776		if p.waitTimeout != nil {
715	777		return p.waitTimeout()

@@ -748,7 +810,12 @@ func (p *cliProcess) forceProcessGroupKill() {
748	810		// openPTY is the small Linux PTY boundary needed to drive the public terminal
749	811		// client. No daemon or adapter implementation is imported by this command.
750	812		func openPTY() (os.File, os.File, error) {
751		-	masterFD, err := syscall.Open("/dev/ptmx", syscall.O_RDWR\|syscall.O_NOCTTY, 0)
	813	+	// O_CLOEXEC keeps these harness-owned descriptors out of the launched client:
	814	+	// exec dup2s the slave onto the child's stdio, but an inherited master (or a
	815	+	// second slave) would otherwise leak in and hold the terminal open, so closing
	816	+	// the harness master could never hang up the client. cmd.Start still remaps the
	817	+	// slave onto fds 0-2, which are exempt from close-on-exec.
	818	+	masterFD, err := syscall.Open("/dev/ptmx", syscall.O_RDWR\|syscall.O_NOCTTY\|syscall.O_CLOEXEC, 0)
752	819		if err != nil {
753	820		return nil, nil, err
754	821		}

@@ -760,7 +827,7 @@ func openPTY() (os.File, os.File, error) {
760	827		if err := rawterm.UnlockPt(masterFD); err != nil {
761	828		return closeMaster(err)
762	829		}
763		-	slaveFD, err := syscall.Open(fmt.Sprintf("/dev/pts/%d", number), syscall.O_RDWR\|syscall.O_NOCTTY, 0)
	830	+	slaveFD, err := syscall.Open(fmt.Sprintf("/dev/pts/%d", number), syscall.O_RDWR\|syscall.O_NOCTTY\|syscall.O_CLOEXEC, 0)
764	831		if err != nil {
765	832		return closeMaster(err)
766	833		}


@@ -625,77 +625,111 @@ func (e *ptyLocalEcho) applicationOutput(chunk []byte) []byte {
625		return nil	625		return nil
626		}	626		}
627			627
			628	+	// Close tears down a launched role along a bounded escalation ladder. Each rung
			629	+	// performs one action, then waits a single closeDeadline for the process to be
			630	+	// reaped before the next rung escalates. The graceful rungs — the public daemon
			631	+	// stop, the client shell exit, the controlling-terminal hangup, and SIGTERM —
			632	+	// let a role close its blocked transport operations and serialize their end
			633	+	// marks; only a SIGKILL severs a process before it can, so only that rung fails
			634	+	// teardown. Every path closes the PTY exactly once and finalizes output and
			635	+	// runtime state.
628		func (p *cliProcess) Close() error {	636		func (p *cliProcess) Close() error {
629		var result error	637		var result error
630		p.closed.Do(func() {	638		p.closed.Do(func() {
631	-	// No measurement waits for terminal chunks during teardown. Continue	639	+	// 1. Drain terminal output for the whole teardown. Without an active
632	-	// consuming them so a full channel cannot block the client before it	640	+	// consumer a full chunks channel stalls copyTerminal, which then stops
633	-	// receives the shell exit request and closes its transport receive span.	641	+	// reading the PTY master and blocks the client mid-restore before it can
634	-	if p.chunks != nil && p.done != nil {	642	+	// close its observed receive operation. The drain stops with p.done.
635	-	go func() {	643	+	drainDone := p.drainTerminalOutput()
636	-	for {	644	+
637	-	select {
638	-	case _, ok := <-p.chunks:
639	-	if !ok {
640	-	return
641	-	}
642	-	case <-p.done:
643	-	return
644	-	}
645	-	}
646	-	}()
647	-	}
648	-	// A public terminal client owns a shell session. Ask that shell to exit
649	-	// before closing the PTY so its transport can finish its receive span;
650	-	// this avoids turning ordinary harness teardown into an aborted trace.
651		exited := false	645		exited := false
652	-	if p.shutdown != nil && p.waitErr != nil {	646	+	// waitForExit blocks up to one bounded deadline for the reap.
			647	+	waitForExit := func() {
			648	+	if p.waitErr == nil \|\| exited {
			649	+	return
			650	+	}
653		select {	651		select {
654		case <-p.waitErr:	652		case <-p.waitErr:
655		exited = true	653		exited = true
656	-	default:	654	+	case <-p.closeDeadline():
657	-	// The client may have just ended the last session and raced this
658	-	// best-effort public shutdown. The normal bounded reaping path below
659	-	// remains authoritative in that case.
660	-	_ = p.shutdown()
661		}	655		}
662		}	656		}
663	-	if p.pty != nil && p.waitErr != nil {	657	+	// reaped reports whether the process is already gone; a role with no
664	-	_, _ = p.pty.Write([]byte("exit\n"))	658	+	// process to reap needs no escalation at all.
			659	+	reaped := func() bool {
			660	+	if p.waitErr == nil \|\| exited {
			661	+	return true
			662	+	}
665		select {	663		select {
666		case <-p.waitErr:	664		case <-p.waitErr:
667		exited = true	665		exited = true
668	-	case <-p.closeDeadline():	666	+	return true
			667	+	default:
			668	+	return false
669		}	669		}
670		}	670		}
671	-	// Keep draining terminal output while the client exits gracefully. If the	671	+	ptyClosed := false
672	-	// drain stops first, restoration output can fill the PTY and block the	672	+	closePTY := func() {
673	-	// client before it closes its observed transport receive operation.	673	+	if p.pty != nil && !ptyClosed {
674	-	if p.done != nil {	674	+	_ = p.pty.Close()
675	-	close(p.done)	675	+	ptyClosed = true
			676	+	}
676		}	677		}
677	-	if p.pty != nil {	678	+
678	-	_ = p.pty.Close()	679	+	// 2. Daemon role: ask the public daemon to stop before any signal so its
			680	+	// blocked carriage can close and serialize failed end marks.
			681	+	if p.shutdown != nil && !reaped() {
			682	+	_ = p.shutdown()
			683	+	waitForExit()
679		}	684		}
680	-	if p.waitErr != nil && !exited {	685	+	// 3. Client role: a public terminal client owns a shell session. Ask that
681	-	select {	686	+	// shell to exit so the client detaches and its transport finishes its
682	-	case <-p.waitErr:	687	+	// receive span cleanly.
683	-	case <-p.closeDeadline():	688	+	if p.pty != nil && !reaped() {
684	-	// Teardown is deliberately bounded: a stuck public CLI or child	689	+	_, _ = p.pty.Write([]byte("exit\n"))
685	-	// must not hold a local harness run indefinitely.	690	+	waitForExit()
686	-	p.forceProcessGroupCleanup()	691	+	}
687	-	select {	692	+	// 4. Hang up the controlling terminal. The attach path installs a signal
688	-	case <-p.waitErr:	693	+	// handler, so this SIGHUP is a graceful detach — not an abrupt kill — and
689	-	case <-p.closeDeadline():	694	+	// the client still flushes its in-flight receive end mark.
690	-	p.forceProcessGroupKill()	695	+	if p.pty != nil && !reaped() {
691	-	select {	696	+	closePTY()
692	-	case <-p.waitErr:	697	+	waitForExit()
693	-	case <-p.closeDeadline():	698	+	}
694	-	result = errors.New("timed out reaping public CLI process")	699	+	// 5. SIGTERM the role's own process group. The daemon and, through its
695	-	}	700	+	// attach signal handler, the client both treat this as a graceful stop.
696	-	}	701	+	if !reaped() {
			702	+	p.forceProcessGroupCleanup()
			703	+	waitForExit()
			704	+	}
			705	+	// 6. SIGKILL the role's process group. This severs the process; an
			706	+	// in-flight observed operation cannot be closed, so its trace may truncate.
			707	+	killed := false
			708	+	if !reaped() {
			709	+	killed = true
			710	+	p.forceProcessGroupKill()
			711	+	waitForExit()
			712	+	}
			713	+	// 7. A SIGKILL escalation cannot guarantee a complete trace. Fail teardown
			714	+	// with a clear cause rather than letting the truncation surface later as a
			715	+	// downstream merge mystery. The graceful rungs never error on their own.
			716	+	if killed {
			717	+	if exited {
			718	+	result = errors.New("public CLI process did not exit after hangup and SIGTERM; killed")
			719	+	} else {
			720	+	result = errors.New("timed out reaping public CLI process")
697		}	721		}
698		}	722		}
			723	+
			724	+	// Close the PTY exactly once on every path: an early graceful exit skips
			725	+	// the hangup rung, so close it here before the drain stops.
			726	+	closePTY()
			727	+	if p.done != nil {
			728	+	close(p.done)
			729	+	}
			730	+	if drainDone != nil {
			731	+	<-drainDone
			732	+	}
699		if p.output != nil {	733		if p.output != nil {
700		if err := p.output.Close(); result == nil {	734		if err := p.output.Close(); result == nil {
701		result = err	735		result = err

@@ -710,6 +744,34 @@ func (p *cliProcess) Close() error {
710		return result	744		return result
711		}	745		}
712			746
			747	+	// drainTerminalOutput consumes copyTerminal's chunks for the duration of a
			748	+	// teardown so a full channel can never wedge copyTerminal (and thus the client's
			749	+	// PTY). It returns nil when there is no terminal to drain, and otherwise a
			750	+	// channel closed once the consumer has stopped on p.done.
			751	+	func (p *cliProcess) drainTerminalOutput() <-chan struct{} {
			752	+	if p.chunks == nil \|\| p.done == nil {
			753	+	return nil
			754	+	}
			755	+	done := make(chan struct{})
			756	+	chunks := p.chunks
			757	+	go func() {
			758	+	defer close(done)
			759	+	for {
			760	+	select {
			761	+	case _, ok := <-chunks:
			762	+	if !ok {
			763	+	// copyTerminal closed the channel; a closed channel is always
			764	+	// ready, so stop selecting it and wait only for the stop signal.
			765	+	chunks = nil
			766	+	}
			767	+	case <-p.done:
			768	+	return
			769	+	}
			770	+	}
			771	+	}()
			772	+	return done
			773	+	}
			774	+
713		func (p *cliProcess) closeDeadline() <-chan time.Time {	775		func (p *cliProcess) closeDeadline() <-chan time.Time {
714		if p.waitTimeout != nil {	776		if p.waitTimeout != nil {
715		return p.waitTimeout()	777		return p.waitTimeout()

@@ -748,7 +810,12 @@ func (p *cliProcess) forceProcessGroupKill() {
748		// openPTY is the small Linux PTY boundary needed to drive the public terminal	810		// openPTY is the small Linux PTY boundary needed to drive the public terminal
749		// client. No daemon or adapter implementation is imported by this command.	811		// client. No daemon or adapter implementation is imported by this command.
750		func openPTY() (os.File, os.File, error) {	812		func openPTY() (os.File, os.File, error) {
751	-	masterFD, err := syscall.Open("/dev/ptmx", syscall.O_RDWR\|syscall.O_NOCTTY, 0)	813	+	// O_CLOEXEC keeps these harness-owned descriptors out of the launched client:
			814	+	// exec dup2s the slave onto the child's stdio, but an inherited master (or a
			815	+	// second slave) would otherwise leak in and hold the terminal open, so closing
			816	+	// the harness master could never hang up the client. cmd.Start still remaps the
			817	+	// slave onto fds 0-2, which are exempt from close-on-exec.
			818	+	masterFD, err := syscall.Open("/dev/ptmx", syscall.O_RDWR\|syscall.O_NOCTTY\|syscall.O_CLOEXEC, 0)
752		if err != nil {	819		if err != nil {
753		return nil, nil, err	820		return nil, nil, err
754		}	821		}

@@ -760,7 +827,7 @@ func openPTY() (os.File, os.File, error) {
760		if err := rawterm.UnlockPt(masterFD); err != nil {	827		if err := rawterm.UnlockPt(masterFD); err != nil {
761		return closeMaster(err)	828		return closeMaster(err)
762		}	829		}
763	-	slaveFD, err := syscall.Open(fmt.Sprintf("/dev/pts/%d", number), syscall.O_RDWR\|syscall.O_NOCTTY, 0)	830	+	slaveFD, err := syscall.Open(fmt.Sprintf("/dev/pts/%d", number), syscall.O_RDWR\|syscall.O_NOCTTY\|syscall.O_CLOEXEC, 0)
764		if err != nil {	831		if err != nil {
765		return closeMaster(err)	832		return closeMaster(err)
766		}	833		}

M cmd/vev-perf-harness/launcher_test.go +42 -0


@@ -134,6 +134,48 @@ func TestCLIProcessCloseIsBoundedWhenPublicRoleDoesNotExit(t *testing.T) {
134	134		}
135	135		}
136	136
	137	+	func TestCLIProcessCloseSIGTERMStopIsGraceful(t *testing.T) {
	138	+	// A role reaped during the SIGTERM wait stopped gracefully: no SIGKILL, and
	139	+	// Close must not report an error for a stage that can still flush a trace.
	140	+	waitErr := make(chan error, 1)
	141	+	never := make(chan time.Time)
	142	+	var term, kill int
	143	+	p := &cliProcess{
	144	+	waitErr: waitErr,
	145	+	waitTimeout: func() <-chan time.Time { return never },
	146	+	forceCleanup: func() { term++; waitErr <- nil },
	147	+	forceKill: func() { kill++ },
	148	+	}
	149	+	if err := p.Close(); err != nil {
	150	+	t.Fatalf("graceful SIGTERM stop returned error: %v", err)
	151	+	}
	152	+	if term != 1 \|\| kill != 0 {
	153	+	t.Fatalf("TERM/KILL calls = %d/%d, want 1/0", term, kill)
	154	+	}
	155	+	}
	156	+
	157	+	func TestCLIProcessCloseErrorsAfterKillEscalation(t *testing.T) {
	158	+	// A role that only dies to SIGKILL may have truncated its trace, so Close must
	159	+	// surface a named escalation error rather than a silent success.
	160	+	waitErr := make(chan error, 1)
	161	+	deadline := make(chan time.Time, 1)
	162	+	deadline <- time.Now() // one ready deadline releases the SIGTERM wait
	163	+	var term, kill int
	164	+	p := &cliProcess{
	165	+	waitErr: waitErr,
	166	+	waitTimeout: func() <-chan time.Time { return deadline },
	167	+	forceCleanup: func() { term++ },
	168	+	forceKill: func() { kill++; waitErr <- nil },
	169	+	}
	170	+	err := p.Close()
	171	+	if err == nil \|\| !strings.Contains(err.Error(), "killed") {
	172	+	t.Fatalf("kill escalation error = %v, want a named killed escalation", err)
	173	+	}
	174	+	if term != 1 \|\| kill != 1 {
	175	+	t.Fatalf("TERM/KILL calls = %d/%d, want 1/1", term, kill)
	176	+	}
	177	+	}
	178	+
137	179		func TestCLIProcessWaitReadyRequiresDaemonSocket(t *testing.T) {
138	180		path := filepath.Join(t.TempDir(), "daemon.sock")
139	181		var lc net.ListenConfig


@@ -134,6 +134,48 @@ func TestCLIProcessCloseIsBoundedWhenPublicRoleDoesNotExit(t *testing.T) {
134	}	134		}
135	}	135		}
136		136
		137	+	func TestCLIProcessCloseSIGTERMStopIsGraceful(t *testing.T) {
		138	+	// A role reaped during the SIGTERM wait stopped gracefully: no SIGKILL, and
		139	+	// Close must not report an error for a stage that can still flush a trace.
		140	+	waitErr := make(chan error, 1)
		141	+	never := make(chan time.Time)
		142	+	var term, kill int
		143	+	p := &cliProcess{
		144	+	waitErr: waitErr,
		145	+	waitTimeout: func() <-chan time.Time { return never },
		146	+	forceCleanup: func() { term++; waitErr <- nil },
		147	+	forceKill: func() { kill++ },
		148	+	}
		149	+	if err := p.Close(); err != nil {
		150	+	t.Fatalf("graceful SIGTERM stop returned error: %v", err)
		151	+	}
		152	+	if term != 1 \|\| kill != 0 {
		153	+	t.Fatalf("TERM/KILL calls = %d/%d, want 1/0", term, kill)
		154	+	}
		155	+	}
		156	+
		157	+	func TestCLIProcessCloseErrorsAfterKillEscalation(t *testing.T) {
		158	+	// A role that only dies to SIGKILL may have truncated its trace, so Close must
		159	+	// surface a named escalation error rather than a silent success.
		160	+	waitErr := make(chan error, 1)
		161	+	deadline := make(chan time.Time, 1)
		162	+	deadline <- time.Now() // one ready deadline releases the SIGTERM wait
		163	+	var term, kill int
		164	+	p := &cliProcess{
		165	+	waitErr: waitErr,
		166	+	waitTimeout: func() <-chan time.Time { return deadline },
		167	+	forceCleanup: func() { term++ },
		168	+	forceKill: func() { kill++; waitErr <- nil },
		169	+	}
		170	+	err := p.Close()
		171	+	if err == nil \|\| !strings.Contains(err.Error(), "killed") {
		172	+	t.Fatalf("kill escalation error = %v, want a named killed escalation", err)
		173	+	}
		174	+	if term != 1 \|\| kill != 1 {
		175	+	t.Fatalf("TERM/KILL calls = %d/%d, want 1/1", term, kill)
		176	+	}
		177	+	}
		178	+
137	func TestCLIProcessWaitReadyRequiresDaemonSocket(t *testing.T) {	179		func TestCLIProcessWaitReadyRequiresDaemonSocket(t *testing.T) {
138	path := filepath.Join(t.TempDir(), "daemon.sock")	180		path := filepath.Join(t.TempDir(), "daemon.sock")
139	var lc net.ListenConfig	181		var lc net.ListenConfig