Merge pull request #115 from bnema/fix/perf-harness-teardown-trace

fix: survive client teardown hangup without truncating perf traces

commit: 092c79957e77325b3068ad9a826b3c4d1b3cde1a
author: bnema · 3mo ago · 2026-07-19 06:01 UTC
committer: GitHub · 3mo ago · 2026-07-19 06:01 UTC
merge of: 156ea64 + dae912d · diff against the first parent
signature: GPG UnverifiedB5690EEEBB952194 The key is unknown

4 files · +384 -61

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

@@ -0,0 +1,171 @@
	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	+	// Close the client on every exit path so a failed assertion cannot leak the
	103	+	// process. Close is idempotent (sync.Once): the explicit call below, once
	104	+	// reached, is the one whose result is asserted; this defer is a backstop.
	105	+	defer func() {
	106	+	if err := client.Close(); err != nil {
	107	+	t.Errorf("close client role: %v", err)
	108	+	}
	109	+	}()
	110	+
	111	+	// Wait until the client is attached with a receive pump blocked mid-frame:
	112	+	// its trace then holds one more adapter_receive_start than end. That in-flight
	113	+	// span is exactly what an ungraceful death truncates.
	114	+	if err := waitForInflightReceive(clientMapping.TracePath, 10*time.Second); err != nil {
	115	+	t.Fatalf("client never reached a blocked receive: %v", err)
	116	+	}
	117	+
	118	+	// Sever the controlling terminal with no graceful exit. This is the harness
	119	+	// hangup escalation stage, delivering SIGHUP to the client session leader.
	120	+	if err := client.pty.Close(); err != nil {
	121	+	t.Fatalf("close client PTY master: %v", err)
	122	+	}
	123	+	// Close runs the bounded escalation ladder; the hangup above already severed
	124	+	// the PTY, so this should reap gracefully without reaching SIGTERM/SIGKILL.
	125	+	if err := client.Close(); err != nil {
	126	+	t.Fatalf("client did not exit cleanly after PTY hangup: %v", err)
	127	+	}
	128	+
	129	+	// The severed client's trace must pair. On the unfixed client this fails with
	130	+	// "missing process-local span pair" for the truncated adapter_receive span.
	131	+	if _, err := mergeProcessTraces([]processMapping{clientMapping}); err != nil {
	132	+	t.Fatalf("client trace did not pair after PTY hangup: %v", err)
	133	+	}
	134	+	}
	135	+
	136	+	// waitForInflightReceive polls a process trace until it holds an unpaired
	137	+	// adapter_receive_start (more starts than ends). It tolerates a partially
	138	+	// written trailing line while the traced process appends concurrently.
	139	+	func waitForInflightReceive(path string, timeout time.Duration) error {
	140	+	deadline := time.Now().Add(timeout)
	141	+	for {
	142	+	starts, ends := 0, 0
	143	+	data, err := os.ReadFile(path)
	144	+	if err != nil {
	145	+	return err
	146	+	}
	147	+	for _, line := range strings.Split(string(data), "\n") {
	148	+	line = strings.TrimSpace(line)
	149	+	if line == "" {
	150	+	continue
	151	+	}
	152	+	var r traceRecord
	153	+	if json.Unmarshal([]byte(line), &r) != nil {
	154	+	continue // a concurrently appended, not-yet-complete final line
	155	+	}
	156	+	switch r.Kind {
	157	+	case "adapter_receive_start":
	158	+	starts++
	159	+	case "adapter_receive_end":
	160	+	ends++
	161	+	}
	162	+	}
	163	+	if starts > ends {
	164	+	return nil
	165	+	}
	166	+	if time.Now().After(deadline) {
	167	+	return fmt.Errorf("adapter_receive starts=%d ends=%d, want an in-flight start", starts, ends)
	168	+	}
	169	+	time.Sleep(10 * time.Millisecond)
	170	+	}
	171	+	}

@@ -0,0 +1,171 @@
	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	+	// Close the client on every exit path so a failed assertion cannot leak the
	103	+	// process. Close is idempotent (sync.Once): the explicit call below, once
	104	+	// reached, is the one whose result is asserted; this defer is a backstop.
	105	+	defer func() {
	106	+	if err := client.Close(); err != nil {
	107	+	t.Errorf("close client role: %v", err)
	108	+	}
	109	+	}()
	110	+
	111	+	// Wait until the client is attached with a receive pump blocked mid-frame:
	112	+	// its trace then holds one more adapter_receive_start than end. That in-flight
	113	+	// span is exactly what an ungraceful death truncates.
	114	+	if err := waitForInflightReceive(clientMapping.TracePath, 10*time.Second); err != nil {
	115	+	t.Fatalf("client never reached a blocked receive: %v", err)
	116	+	}
	117	+
	118	+	// Sever the controlling terminal with no graceful exit. This is the harness
	119	+	// hangup escalation stage, delivering SIGHUP to the client session leader.
	120	+	if err := client.pty.Close(); err != nil {
	121	+	t.Fatalf("close client PTY master: %v", err)
	122	+	}
	123	+	// Close runs the bounded escalation ladder; the hangup above already severed
	124	+	// the PTY, so this should reap gracefully without reaching SIGTERM/SIGKILL.
	125	+	if err := client.Close(); err != nil {
	126	+	t.Fatalf("client did not exit cleanly after PTY hangup: %v", err)
	127	+	}
	128	+
	129	+	// The severed client's trace must pair. On the unfixed client this fails with
	130	+	// "missing process-local span pair" for the truncated adapter_receive span.
	131	+	if _, err := mergeProcessTraces([]processMapping{clientMapping}); err != nil {
	132	+	t.Fatalf("client trace did not pair after PTY hangup: %v", err)
	133	+	}
	134	+	}
	135	+
	136	+	// waitForInflightReceive polls a process trace until it holds an unpaired
	137	+	// adapter_receive_start (more starts than ends). It tolerates a partially
	138	+	// written trailing line while the traced process appends concurrently.
	139	+	func waitForInflightReceive(path string, timeout time.Duration) error {
	140	+	deadline := time.Now().Add(timeout)
	141	+	for {
	142	+	starts, ends := 0, 0
	143	+	data, err := os.ReadFile(path)
	144	+	if err != nil {
	145	+	return err
	146	+	}
	147	+	for _, line := range strings.Split(string(data), "\n") {
	148	+	line = strings.TrimSpace(line)
	149	+	if line == "" {
	150	+	continue
	151	+	}
	152	+	var r traceRecord
	153	+	if json.Unmarshal([]byte(line), &r) != nil {
	154	+	continue // a concurrently appended, not-yet-complete final line
	155	+	}
	156	+	switch r.Kind {
	157	+	case "adapter_receive_start":
	158	+	starts++
	159	+	case "adapter_receive_end":
	160	+	ends++
	161	+	}
	162	+	}
	163	+	if starts > ends {
	164	+	return nil
	165	+	}
	166	+	if time.Now().After(deadline) {
	167	+	return fmt.Errorf("adapter_receive starts=%d ends=%d, want an in-flight start", starts, ends)
	168	+	}
	169	+	time.Sleep(10 * time.Millisecond)
	170	+	}
	171	+	}

M cmd/vev-perf-harness/launcher.go +128 -61


@@ -131,6 +131,7 @@ type cliProcess struct {
131	131		cleanupRuntime func() error
132	132		terminalReady bool
133	133		readyPath string
	134	+	closeResult error
134	135		}
135	136
136	137		func (l *cliLauncher) Launch(m processMapping, role roleCommand) (launchedProcess, error) {

@@ -625,89 +626,150 @@ func (e *ptyLocalEcho) applicationOutput(chunk []byte) []byte {
625	626		return nil
626	627		}
627	628
	629	+	// Close tears down a launched role along a bounded escalation ladder. Each rung
	630	+	// performs one action, then waits a single closeDeadline for the process to be
	631	+	// reaped before the next rung escalates. The graceful rungs — the public daemon
	632	+	// stop, the client shell exit, the controlling-terminal hangup, and SIGTERM —
	633	+	// let a role close its blocked transport operations and serialize their end
	634	+	// marks; only a SIGKILL severs a process before it can, so only that rung fails
	635	+	// teardown. Every path closes the PTY exactly once and finalizes output and
	636	+	// runtime state.
628	637		func (p *cliProcess) Close() error {
629		-	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	+	p.closeResult = errors.New("public CLI process did not exit after hangup and SIGTERM; killed")
	719	+	} else {
	720	+	p.closeResult = 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		-	if err := p.output.Close(); result == nil {
701		-	result = err
	734	+	if err := p.output.Close(); p.closeResult == nil {
	735	+	p.closeResult = err
702	736		}
703	737		}
704	738		if p.cleanupRuntime != nil {
705		-	if err := p.cleanupRuntime(); result == nil {
706		-	result = err
	739	+	if err := p.cleanupRuntime(); p.closeResult == nil {
	740	+	p.closeResult = err
707	741		}
708	742		}
709	743		})
710		-	return result
	744	+	return p.closeResult
	745	+	}
	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
711	773		}
712	774
713	775		func (p *cliProcess) closeDeadline() <-chan time.Time {

@@ -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		}


@@ -131,6 +131,7 @@ type cliProcess struct {
131		cleanupRuntime func() error	131		cleanupRuntime func() error
132		terminalReady bool	132		terminalReady bool
133		readyPath string	133		readyPath string
			134	+	closeResult error
134		}	135		}
135			136
136		func (l *cliLauncher) Launch(m processMapping, role roleCommand) (launchedProcess, error) {	137		func (l *cliLauncher) Launch(m processMapping, role roleCommand) (launchedProcess, error) {

@@ -625,89 +626,150 @@ func (e *ptyLocalEcho) applicationOutput(chunk []byte) []byte {
625		return nil	626		return nil
626		}	627		}
627			628
			629	+	// Close tears down a launched role along a bounded escalation ladder. Each rung
			630	+	// performs one action, then waits a single closeDeadline for the process to be
			631	+	// reaped before the next rung escalates. The graceful rungs — the public daemon
			632	+	// stop, the client shell exit, the controlling-terminal hangup, and SIGTERM —
			633	+	// let a role close its blocked transport operations and serialize their end
			634	+	// marks; only a SIGKILL severs a process before it can, so only that rung fails
			635	+	// teardown. Every path closes the PTY exactly once and finalizes output and
			636	+	// runtime state.
628		func (p *cliProcess) Close() error {	637		func (p *cliProcess) Close() error {
629	-	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	+	p.closeResult = errors.New("public CLI process did not exit after hangup and SIGTERM; killed")
			719	+	} else {
			720	+	p.closeResult = 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(); p.closeResult == nil {
701	-	result = err	735	+	p.closeResult = err
702		}	736		}
703		}	737		}
704		if p.cleanupRuntime != nil {	738		if p.cleanupRuntime != nil {
705	-	if err := p.cleanupRuntime(); result == nil {	739	+	if err := p.cleanupRuntime(); p.closeResult == nil {
706	-	result = err	740	+	p.closeResult = err
707		}	741		}
708		}	742		}
709		})	743		})
710	-	return result	744	+	return p.closeResult
			745	+	}
			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
711		}	773		}
712			774
713		func (p *cliProcess) closeDeadline() <-chan time.Time {	775		func (p *cliProcess) closeDeadline() <-chan time.Time {

@@ -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 +76 -0


@@ -134,6 +134,82 @@ func TestCLIProcessCloseIsBoundedWhenPublicRoleDoesNotExit(t *testing.T) {
134	134		}
135	135		}
136	136
	137	+	func TestCLIProcessCloseEscalationOutcome(t *testing.T) {
	138	+	tests := []struct {
	139	+	name string
	140	+	reapsAt string // "term" or "kill": which rung's callback reaps the process
	141	+	waitTimeout func() func() <-chan time.Time
	142	+	wantErrSub string // "" means Close must return nil
	143	+	wantTerm int
	144	+	wantKill int
	145	+	}{
	146	+	{
	147	+	// A role reaped during the SIGTERM wait stopped gracefully: no SIGKILL,
	148	+	// and Close must not report an error for a stage that can still flush a
	149	+	// trace. The deadline channel never fires, so only the SIGTERM callback
	150	+	// reaping the process can end the wait.
	151	+	name: "SIGTERM stop is graceful",
	152	+	reapsAt: "term",
	153	+	waitTimeout: func() func() <-chan time.Time {
	154	+	never := make(chan time.Time)
	155	+	return func() <-chan time.Time { return never }
	156	+	},
	157	+	wantErrSub: "",
	158	+	wantTerm: 1,
	159	+	wantKill: 0,
	160	+	},
	161	+	{
	162	+	// A role that only dies to SIGKILL may have truncated its trace, so
	163	+	// Close must surface a named escalation error rather than a silent
	164	+	// success. One ready deadline releases the SIGTERM wait so the ladder
	165	+	// escalates to SIGKILL, whose callback then reaps the process.
	166	+	name: "errors after kill escalation",
	167	+	reapsAt: "kill",
	168	+	waitTimeout: func() func() <-chan time.Time {
	169	+	deadline := make(chan time.Time, 1)
	170	+	deadline <- time.Now()
	171	+	return func() <-chan time.Time { return deadline }
	172	+	},
	173	+	wantErrSub: "killed",
	174	+	wantTerm: 1,
	175	+	wantKill: 1,
	176	+	},
	177	+	}
	178	+	for _, tt := range tests {
	179	+	t.Run(tt.name, func(t *testing.T) {
	180	+	waitErr := make(chan error, 1)
	181	+	var term, kill int
	182	+	p := &cliProcess{
	183	+	waitErr: waitErr,
	184	+	waitTimeout: tt.waitTimeout(),
	185	+	forceCleanup: func() {
	186	+	term++
	187	+	if tt.reapsAt == "term" {
	188	+	waitErr <- nil
	189	+	}
	190	+	},
	191	+	forceKill: func() {
	192	+	kill++
	193	+	if tt.reapsAt == "kill" {
	194	+	waitErr <- nil
	195	+	}
	196	+	},
	197	+	}
	198	+	err := p.Close()
	199	+	if tt.wantErrSub == "" {
	200	+	if err != nil {
	201	+	t.Fatalf("Close() error = %v, want nil", err)
	202	+	}
	203	+	} else if err == nil \|\| !strings.Contains(err.Error(), tt.wantErrSub) {
	204	+	t.Fatalf("Close() error = %v, want an error containing %q", err, tt.wantErrSub)
	205	+	}
	206	+	if term != tt.wantTerm \|\| kill != tt.wantKill {
	207	+	t.Fatalf("TERM/KILL calls = %d/%d, want %d/%d", term, kill, tt.wantTerm, tt.wantKill)
	208	+	}
	209	+	})
	210	+	}
	211	+	}
	212	+
137	213		func TestCLIProcessWaitReadyRequiresDaemonSocket(t *testing.T) {
138	214		path := filepath.Join(t.TempDir(), "daemon.sock")
139	215		var lc net.ListenConfig


@@ -134,6 +134,82 @@ func TestCLIProcessCloseIsBoundedWhenPublicRoleDoesNotExit(t *testing.T) {
134	}	134		}
135	}	135		}
136		136
		137	+	func TestCLIProcessCloseEscalationOutcome(t *testing.T) {
		138	+	tests := []struct {
		139	+	name string
		140	+	reapsAt string // "term" or "kill": which rung's callback reaps the process
		141	+	waitTimeout func() func() <-chan time.Time
		142	+	wantErrSub string // "" means Close must return nil
		143	+	wantTerm int
		144	+	wantKill int
		145	+	}{
		146	+	{
		147	+	// A role reaped during the SIGTERM wait stopped gracefully: no SIGKILL,
		148	+	// and Close must not report an error for a stage that can still flush a
		149	+	// trace. The deadline channel never fires, so only the SIGTERM callback
		150	+	// reaping the process can end the wait.
		151	+	name: "SIGTERM stop is graceful",
		152	+	reapsAt: "term",
		153	+	waitTimeout: func() func() <-chan time.Time {
		154	+	never := make(chan time.Time)
		155	+	return func() <-chan time.Time { return never }
		156	+	},
		157	+	wantErrSub: "",
		158	+	wantTerm: 1,
		159	+	wantKill: 0,
		160	+	},
		161	+	{
		162	+	// A role that only dies to SIGKILL may have truncated its trace, so
		163	+	// Close must surface a named escalation error rather than a silent
		164	+	// success. One ready deadline releases the SIGTERM wait so the ladder
		165	+	// escalates to SIGKILL, whose callback then reaps the process.
		166	+	name: "errors after kill escalation",
		167	+	reapsAt: "kill",
		168	+	waitTimeout: func() func() <-chan time.Time {
		169	+	deadline := make(chan time.Time, 1)
		170	+	deadline <- time.Now()
		171	+	return func() <-chan time.Time { return deadline }
		172	+	},
		173	+	wantErrSub: "killed",
		174	+	wantTerm: 1,
		175	+	wantKill: 1,
		176	+	},
		177	+	}
		178	+	for _, tt := range tests {
		179	+	t.Run(tt.name, func(t *testing.T) {
		180	+	waitErr := make(chan error, 1)
		181	+	var term, kill int
		182	+	p := &cliProcess{
		183	+	waitErr: waitErr,
		184	+	waitTimeout: tt.waitTimeout(),
		185	+	forceCleanup: func() {
		186	+	term++
		187	+	if tt.reapsAt == "term" {
		188	+	waitErr <- nil
		189	+	}
		190	+	},
		191	+	forceKill: func() {
		192	+	kill++
		193	+	if tt.reapsAt == "kill" {
		194	+	waitErr <- nil
		195	+	}
		196	+	},
		197	+	}
		198	+	err := p.Close()
		199	+	if tt.wantErrSub == "" {
		200	+	if err != nil {
		201	+	t.Fatalf("Close() error = %v, want nil", err)
		202	+	}
		203	+	} else if err == nil \|\| !strings.Contains(err.Error(), tt.wantErrSub) {
		204	+	t.Fatalf("Close() error = %v, want an error containing %q", err, tt.wantErrSub)
		205	+	}
		206	+	if term != tt.wantTerm \|\| kill != tt.wantKill {
		207	+	t.Fatalf("TERM/KILL calls = %d/%d, want %d/%d", term, kill, tt.wantTerm, tt.wantKill)
		208	+	}
		209	+	})
		210	+	}
		211	+	}
		212	+
137	func TestCLIProcessWaitReadyRequiresDaemonSocket(t *testing.T) {	213		func TestCLIProcessWaitReadyRequiresDaemonSocket(t *testing.T) {
138	path := filepath.Join(t.TempDir(), "daemon.sock")	214		path := filepath.Join(t.TempDir(), "daemon.sock")
139	var lc net.ListenConfig	215		var lc net.ListenConfig

M internal/app/run.go +9 -0


@@ -480,6 +480,15 @@ func runDaemon() (retErr error) {
480	480		// attach loop. Logging goes to the shared file: the client must never write
481	481		// to the console while the terminal is raw.
482	482		func runAttach(ctx context.Context, intent uint8, name, remoteTarget string) (retErr error) {
	483	+	// Treat a controlling-terminal hangup or termination request as a graceful
	484	+	// detach rather than an abrupt process death. Cancelling the context unwinds
	485	+	// the client's pumps, closes its transport, and lets the deferred trace closer
	486	+	// flush the in-flight receive's end mark — so teardown never truncates a span.
	487	+	// Raw mode disables ISIG, so catching SIGINT here does not affect interactive
	488	+	// Ctrl+C, which the daemon delivers to the remote shell as a normal keystroke.
	489	+	ctx, stop := signal.NotifyContext(ctx, syscall.SIGHUP, syscall.SIGTERM, syscall.SIGINT)
	490	+	defer stop()
	491	+
483	492		log, logCloser, err := configureLogging(logging.Client, false)
484	493		if err != nil {
485	494		return err


@@ -480,6 +480,15 @@ func runDaemon() (retErr error) {
480	// attach loop. Logging goes to the shared file: the client must never write	480		// attach loop. Logging goes to the shared file: the client must never write
481	// to the console while the terminal is raw.	481		// to the console while the terminal is raw.
482	func runAttach(ctx context.Context, intent uint8, name, remoteTarget string) (retErr error) {	482		func runAttach(ctx context.Context, intent uint8, name, remoteTarget string) (retErr error) {
		483	+	// Treat a controlling-terminal hangup or termination request as a graceful
		484	+	// detach rather than an abrupt process death. Cancelling the context unwinds
		485	+	// the client's pumps, closes its transport, and lets the deferred trace closer
		486	+	// flush the in-flight receive's end mark — so teardown never truncates a span.
		487	+	// Raw mode disables ISIG, so catching SIGINT here does not affect interactive
		488	+	// Ctrl+C, which the daemon delivers to the remote shell as a normal keystroke.
		489	+	ctx, stop := signal.NotifyContext(ctx, syscall.SIGHUP, syscall.SIGTERM, syscall.SIGINT)
		490	+	defer stop()
		491	+
483	log, logCloser, err := configureLogging(logging.Client, false)	492		log, logCloser, err := configureLogging(logging.Client, false)
484	if err != nil {	493		if err != nil {
485	return err	494		return err