fix(perf-harness): drain client output during shutdown

commit: dbc6bb740c6d75d139e6eb1c9ed2054a6c5397d3
author: brice · 3mo ago · 2026-07-18 13:55 UTC
parent: 68c86a5
signature: GPG VerifiedCD88F9E564E0102646FB6077A1A07C7C7C3E349D · arch-gpg Verified for the committer email

2 files · +55 -3

M cmd/vev-perf-harness/launcher.go +6 -3


@@ -628,9 +628,6 @@ func (e *ptyLocalEcho) applicationOutput(chunk []byte) []byte {
628	628		func (p *cliProcess) Close() error {
629	629		var result error
630	630		p.closed.Do(func() {
631		-	if p.done != nil {
632		-	close(p.done)
633		-	}
634	631		// A public terminal client owns a shell session. Ask that shell to exit
635	632		// before closing the PTY so its transport can finish its receive span;
636	633		// this avoids turning ordinary harness teardown into an aborted trace.

@@ -654,6 +651,12 @@ func (p *cliProcess) Close() error {
654	651		case <-p.closeDeadline():
655	652		}
656	653		}
	654	+	// Keep draining terminal output while the client exits gracefully. If the
	655	+	// drain stops first, restoration output can fill the PTY and block the
	656	+	// client before it closes its observed transport receive operation.
	657	+	if p.done != nil {
	658	+	close(p.done)
	659	+	}
657	660		if p.pty != nil {
658	661		_ = p.pty.Close()
659	662		}

 func (p *cliProcess) Close() error {
 	var result error
 	p.closed.Do(func() {
-		if p.done != nil {
-			close(p.done)
-		}
 		// A public terminal client owns a shell session. Ask that shell to exit
 		// before closing the PTY so its transport can finish its receive span;
 		// this avoids turning ordinary harness teardown into an aborted trace.
 			case <-p.closeDeadline():
 			}
 		}
+		// Keep draining terminal output while the client exits gracefully. If the
+		// drain stops first, restoration output can fill the PTY and block the
+		// client before it closes its observed transport receive operation.
+		if p.done != nil {
+			close(p.done)
+		}
 		if p.pty != nil {
 			_ = p.pty.Close()
 		}

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


@@ -11,12 +11,61 @@ import (
11	11		"os/exec"
12	12		"path/filepath"
13	13		"strings"
	14	+	"sync"
14	15		"testing"
15	16		"time"
16	17
17	18		"github.com/bnema/vev/pkg/safedir"
18	19		)
19	20
	21	+	func TestCLIProcessCloseKeepsTerminalDrainActiveUntilClientExit(t *testing.T) {
	22	+	pty, err := os.CreateTemp(t.TempDir(), "pty")
	23	+	if err != nil {
	24	+	t.Fatal(err)
	25	+	}
	26	+	waiting := make(chan struct{})
	27	+	deadline := make(chan time.Time)
	28	+	waitErr := make(chan error)
	29	+	done := make(chan struct{})
	30	+	closed := make(chan error, 1)
	31	+	var waitingOnce sync.Once
	32	+	p := &cliProcess{
	33	+	pty: pty,
	34	+	done: done,
	35	+	waitErr: waitErr,
	36	+	waitTimeout: func() <-chan time.Time {
	37	+	waitingOnce.Do(func() { close(waiting) })
	38	+	return deadline
	39	+	},
	40	+	}
	41	+	go func() { closed <- p.Close() }()
	42	+
	43	+	select {
	44	+	case <-waiting:
	45	+	case <-time.After(time.Second):
	46	+	t.Fatal("Close did not begin waiting for graceful client exit")
	47	+	}
	48	+	select {
	49	+	case <-done:
	50	+	t.Fatal("terminal drain stopped before the client exited")
	51	+	default:
	52	+	}
	53	+	waitErr <- nil
	54	+	select {
	55	+	case err := <-closed:
	56	+	if err != nil {
	57	+	t.Fatal(err)
	58	+	}
	59	+	case <-time.After(time.Second):
	60	+	t.Fatal("Close did not finish after the client exited")
	61	+	}
	62	+	select {
	63	+	case <-done:
	64	+	default:
	65	+	t.Fatal("terminal drain remained active after client exit")
	66	+	}
	67	+	}
	68	+
20	69		func TestCLIProcessCloseIsBoundedWhenPublicRoleDoesNotExit(t *testing.T) {
21	70		deadline := make(chan time.Time)
22	71		close(deadline)


@@ -11,12 +11,61 @@ import (
11	"os/exec"	11		"os/exec"
12	"path/filepath"	12		"path/filepath"
13	"strings"	13		"strings"
		14	+	"sync"
14	"testing"	15		"testing"
15	"time"	16		"time"
16		17
17	"github.com/bnema/vev/pkg/safedir"	18		"github.com/bnema/vev/pkg/safedir"
18	)	19		)
19		20
		21	+	func TestCLIProcessCloseKeepsTerminalDrainActiveUntilClientExit(t *testing.T) {
		22	+	pty, err := os.CreateTemp(t.TempDir(), "pty")
		23	+	if err != nil {
		24	+	t.Fatal(err)
		25	+	}
		26	+	waiting := make(chan struct{})
		27	+	deadline := make(chan time.Time)
		28	+	waitErr := make(chan error)
		29	+	done := make(chan struct{})
		30	+	closed := make(chan error, 1)
		31	+	var waitingOnce sync.Once
		32	+	p := &cliProcess{
		33	+	pty: pty,
		34	+	done: done,
		35	+	waitErr: waitErr,
		36	+	waitTimeout: func() <-chan time.Time {
		37	+	waitingOnce.Do(func() { close(waiting) })
		38	+	return deadline
		39	+	},
		40	+	}
		41	+	go func() { closed <- p.Close() }()
		42	+
		43	+	select {
		44	+	case <-waiting:
		45	+	case <-time.After(time.Second):
		46	+	t.Fatal("Close did not begin waiting for graceful client exit")
		47	+	}
		48	+	select {
		49	+	case <-done:
		50	+	t.Fatal("terminal drain stopped before the client exited")
		51	+	default:
		52	+	}
		53	+	waitErr <- nil
		54	+	select {
		55	+	case err := <-closed:
		56	+	if err != nil {
		57	+	t.Fatal(err)
		58	+	}
		59	+	case <-time.After(time.Second):
		60	+	t.Fatal("Close did not finish after the client exited")
		61	+	}
		62	+	select {
		63	+	case <-done:
		64	+	default:
		65	+	t.Fatal("terminal drain remained active after client exit")
		66	+	}
		67	+	}
		68	+
20	func TestCLIProcessCloseIsBoundedWhenPublicRoleDoesNotExit(t *testing.T) {	69		func TestCLIProcessCloseIsBoundedWhenPublicRoleDoesNotExit(t *testing.T) {
21	deadline := make(chan time.Time)	70		deadline := make(chan time.Time)
22	close(deadline)	71		close(deadline)