fix(term): order resize watcher init/stop under mu — no post-restore start

ResizeEvents initialized resizeCh/resizeQuit/sigCh under its own
sync.Once without holding t.mu, while restore()'s stop path used a
separate Once — the two weren't ordered, so a signal-driven restore
racing ahead of the first ResizeEvents call could leave a leaked
watcher goroutine, a leaked signal handler, and a never-closed resize
channel (hanging any consumer selecting on it), plus a data race.

Fold watcher start/stop under t.mu (the watcher goroutine never takes
t.mu, so waiting on it under the lock can't deadlock) and track a
resizeDone flag: once stop has run, ResizeEvents never starts a
watcher and returns an already-closed channel. Locked in by a
restore-before-first-ResizeEvents regression test and a concurrent
ResizeEvents-vs-restore stress test under -race.

Also: go mod tidy so x/term (direct) and x/sys (test-only) are no
longer marked indirect.

commit: 58001c6cafedf56408c50243f0979483622fd7f1
author: brice · 3mo ago · 2026-07-02 11:13 UTC
committer: brice · 3mo ago · 2026-07-02 11:16 UTC
parent: 786a71f
signature: GPG VerifiedCD88F9E564E0102646FB6077A1A07C7C7C3E349D · arch-gpg Verified for the committer email

2 files · +165 -27

M internal/adapters/term/terminal.go +50 -27


@@ -47,12 +47,15 @@ type Terminal struct {
47	47		rawSkipped bool // true if fd wasn't a tty, so MakeRaw/Restore were skipped
48	48		restoreFn func() error // the single idempotent restore closure for the current session
49	49
50		-	resizeStart sync.Once
51		-	resizeStop sync.Once
52		-	resizeCh chan domain.Size
53		-	resizeQuit chan struct{}
54		-	sigCh chan os.Signal
55		-	resizeWG sync.WaitGroup
	50	+	// Resize watcher state, also guarded by mu so watcher start
	51	+	// (ResizeEvents) and watcher stop (restore) are strictly ordered:
	52	+	// a restore that runs before the first ResizeEvents call must
	53	+	// prevent any watcher (and its signal.Notify) from ever starting.
	54	+	resizeCh chan domain.Size
	55	+	resizeQuit chan struct{}
	56	+	sigCh chan os.Signal
	57	+	resizeDone bool // set by stopResizeLocked; no watcher may start afterwards
	58	+	resizeWG sync.WaitGroup
56	59		}
57	60
58	61		// New creates a Terminal backed by os.Stdin and os.Stdout.

@@ -140,7 +143,7 @@ func (t *Terminal) restore() error {
140	143		t.entered = false
141	144		t.restoreFn = nil
142	145
143		-	t.stopResize()
	146	+	t.stopResizeLocked()
144	147
145	148		switch {
146	149		case werr != nil:

@@ -177,33 +180,53 @@ func (t *Terminal) Size() (domain.Size, error) {
177	180		// ResizeEvents returns a channel of coalesced terminal sizes, one per
178	181		// SIGWINCH burst. The SIGWINCH watcher starts lazily on the first call;
179	182		// subsequent calls return the same channel. The channel is closed once
180		-	// restore (returned by EnterRaw) runs.
	183	+	// restore (returned by EnterRaw) runs. If restore has already run when
	184	+	// ResizeEvents is first called, no watcher is started and the returned
	185	+	// channel is already closed.
181	186		func (t *Terminal) ResizeEvents() <-chan domain.Size {
182		-	t.resizeStart.Do(func() {
183		-	t.resizeCh = make(chan domain.Size)
184		-	t.resizeQuit = make(chan struct{})
185		-	t.sigCh = make(chan os.Signal, 1)
186		-	signal.Notify(t.sigCh, syscall.SIGWINCH)
187		-
188		-	t.resizeWG.Add(1)
189		-	go func() {
190		-	defer t.resizeWG.Done()
191		-	defer signal.Stop(t.sigCh)
192		-	resizeLoop(t.sigCh, t.resizeCh, t.resizeQuit, t.Size)
193		-	}()
194		-	})
	187	+	t.mu.Lock()
	188	+	defer t.mu.Unlock()
	189	+
	190	+	if t.resizeCh != nil {
	191	+	return t.resizeCh
	192	+	}
	193	+	t.resizeCh = make(chan domain.Size)
	194	+
	195	+	if t.resizeDone {
	196	+	// restore ran before the first ResizeEvents call: never start a
	197	+	// watcher (or a signal handler) after stop; hand back a closed
	198	+	// channel so a consumer selecting on it doesn't hang.
	199	+	close(t.resizeCh)
	200	+	return t.resizeCh
	201	+	}
	202	+
	203	+	t.resizeQuit = make(chan struct{})
	204	+	t.sigCh = make(chan os.Signal, 1)
	205	+	signal.Notify(t.sigCh, syscall.SIGWINCH)
	206	+
	207	+	t.resizeWG.Add(1)
	208	+	go func() {
	209	+	defer t.resizeWG.Done()
	210	+	defer signal.Stop(t.sigCh)
	211	+	resizeLoop(t.sigCh, t.resizeCh, t.resizeQuit, t.Size)
	212	+	}()
	213	+
195	214		return t.resizeCh
196	215		}
197	216
198		-	// stopResize stops the SIGWINCH watcher goroutine (if one was started)
199		-	// and waits for it to exit, so its channel is guaranteed closed before
200		-	// stopResize returns.
201		-	func (t *Terminal) stopResize() {
202		-	t.resizeStop.Do(func() {
	217	+	// stopResizeLocked stops the SIGWINCH watcher goroutine (if one was
	218	+	// started) and waits for it to exit, so its channel is guaranteed
	219	+	// closed before stopResizeLocked returns. It marks the watcher as done
	220	+	// so no watcher can start afterwards. Must be called with t.mu held;
	221	+	// the watcher goroutine never takes t.mu, so waiting under the lock
	222	+	// cannot deadlock.
	223	+	func (t *Terminal) stopResizeLocked() {
	224	+	if !t.resizeDone {
	225	+	t.resizeDone = true
203	226		if t.resizeQuit != nil {
204	227		close(t.resizeQuit)
205	228		}
206		-	})
	229	+	}
207	230		t.resizeWG.Wait()
208	231		}
209	232


@@ -47,12 +47,15 @@ type Terminal struct {
47		rawSkipped bool // true if fd wasn't a tty, so MakeRaw/Restore were skipped	47		rawSkipped bool // true if fd wasn't a tty, so MakeRaw/Restore were skipped
48		restoreFn func() error // the single idempotent restore closure for the current session	48		restoreFn func() error // the single idempotent restore closure for the current session
49			49
50	-	resizeStart sync.Once	50	+	// Resize watcher state, also guarded by mu so watcher start
51	-	resizeStop sync.Once	51	+	// (ResizeEvents) and watcher stop (restore) are strictly ordered:
52	-	resizeCh chan domain.Size	52	+	// a restore that runs before the first ResizeEvents call must
53	-	resizeQuit chan struct{}	53	+	// prevent any watcher (and its signal.Notify) from ever starting.
54	-	sigCh chan os.Signal	54	+	resizeCh chan domain.Size
55	-	resizeWG sync.WaitGroup	55	+	resizeQuit chan struct{}
			56	+	sigCh chan os.Signal
			57	+	resizeDone bool // set by stopResizeLocked; no watcher may start afterwards
			58	+	resizeWG sync.WaitGroup
56		}	59		}
57			60
58		// New creates a Terminal backed by os.Stdin and os.Stdout.	61		// New creates a Terminal backed by os.Stdin and os.Stdout.

@@ -140,7 +143,7 @@ func (t *Terminal) restore() error {
140		t.entered = false	143		t.entered = false
141		t.restoreFn = nil	144		t.restoreFn = nil
142			145
143	-	t.stopResize()	146	+	t.stopResizeLocked()
144			147
145		switch {	148		switch {
146		case werr != nil:	149		case werr != nil:

@@ -177,33 +180,53 @@ func (t *Terminal) Size() (domain.Size, error) {
177		// ResizeEvents returns a channel of coalesced terminal sizes, one per	180		// ResizeEvents returns a channel of coalesced terminal sizes, one per
178		// SIGWINCH burst. The SIGWINCH watcher starts lazily on the first call;	181		// SIGWINCH burst. The SIGWINCH watcher starts lazily on the first call;
179		// subsequent calls return the same channel. The channel is closed once	182		// subsequent calls return the same channel. The channel is closed once
180	-	// restore (returned by EnterRaw) runs.	183	+	// restore (returned by EnterRaw) runs. If restore has already run when
			184	+	// ResizeEvents is first called, no watcher is started and the returned
			185	+	// channel is already closed.
181		func (t *Terminal) ResizeEvents() <-chan domain.Size {	186		func (t *Terminal) ResizeEvents() <-chan domain.Size {
182	-	t.resizeStart.Do(func() {	187	+	t.mu.Lock()
183	-	t.resizeCh = make(chan domain.Size)	188	+	defer t.mu.Unlock()
184	-	t.resizeQuit = make(chan struct{})	189	+
185	-	t.sigCh = make(chan os.Signal, 1)	190	+	if t.resizeCh != nil {
186	-	signal.Notify(t.sigCh, syscall.SIGWINCH)	191	+	return t.resizeCh
187	-		192	+	}
188	-	t.resizeWG.Add(1)	193	+	t.resizeCh = make(chan domain.Size)
189	-	go func() {	194	+
190	-	defer t.resizeWG.Done()	195	+	if t.resizeDone {
191	-	defer signal.Stop(t.sigCh)	196	+	// restore ran before the first ResizeEvents call: never start a
192	-	resizeLoop(t.sigCh, t.resizeCh, t.resizeQuit, t.Size)	197	+	// watcher (or a signal handler) after stop; hand back a closed
193	-	}()	198	+	// channel so a consumer selecting on it doesn't hang.
194	-	})	199	+	close(t.resizeCh)
			200	+	return t.resizeCh
			201	+	}
			202	+
			203	+	t.resizeQuit = make(chan struct{})
			204	+	t.sigCh = make(chan os.Signal, 1)
			205	+	signal.Notify(t.sigCh, syscall.SIGWINCH)
			206	+
			207	+	t.resizeWG.Add(1)
			208	+	go func() {
			209	+	defer t.resizeWG.Done()
			210	+	defer signal.Stop(t.sigCh)
			211	+	resizeLoop(t.sigCh, t.resizeCh, t.resizeQuit, t.Size)
			212	+	}()
			213	+
195		return t.resizeCh	214		return t.resizeCh
196		}	215		}
197			216
198	-	// stopResize stops the SIGWINCH watcher goroutine (if one was started)	217	+	// stopResizeLocked stops the SIGWINCH watcher goroutine (if one was
199	-	// and waits for it to exit, so its channel is guaranteed closed before	218	+	// started) and waits for it to exit, so its channel is guaranteed
200	-	// stopResize returns.	219	+	// closed before stopResizeLocked returns. It marks the watcher as done
201	-	func (t *Terminal) stopResize() {	220	+	// so no watcher can start afterwards. Must be called with t.mu held;
202	-	t.resizeStop.Do(func() {	221	+	// the watcher goroutine never takes t.mu, so waiting under the lock
			222	+	// cannot deadlock.
			223	+	func (t *Terminal) stopResizeLocked() {
			224	+	if !t.resizeDone {
			225	+	t.resizeDone = true
203		if t.resizeQuit != nil {	226		if t.resizeQuit != nil {
204		close(t.resizeQuit)	227		close(t.resizeQuit)
205		}	228		}
206	-	})	229	+	}
207		t.resizeWG.Wait()	230		t.resizeWG.Wait()
208		}	231		}
209			232

M internal/adapters/term/terminal_test.go +115 -0


@@ -6,6 +6,8 @@ import (
6	6		"os"
7	7		"testing"
8	8		"time"
	9	+
	10	+	"github.com/bnema/vev/internal/domain"
9	11		)
10	12
11	13		// pipePair returns an os.Pipe and a goroutine that continuously copies

@@ -178,6 +180,119 @@ func TestTerminal_ResizeEvents_ReturnsSameChannelAndClosesOnRestore(t *testing.T
178	180		<-done
179	181		}
180	182
	183	+	func TestTerminal_ResizeEvents_AfterRestore_NoWatcherAndClosedChannel(t *testing.T) {
	184	+	inR, inW, err := os.Pipe()
	185	+	if err != nil {
	186	+	t.Fatalf("os.Pipe(in): %v", err)
	187	+	}
	188	+	defer inW.Close()
	189	+	defer inR.Close()
	190	+
	191	+	outR, outW, _, done := pipeCapture(t)
	192	+	defer outR.Close()
	193	+
	194	+	tm := NewWithFiles(inR, outW)
	195	+
	196	+	restore, err := tm.EnterRaw()
	197	+	if err != nil {
	198	+	t.Fatalf("EnterRaw: %v", err)
	199	+	}
	200	+	// restore BEFORE the first ResizeEvents call (signal-driven shutdown
	201	+	// racing ahead of the resize pump's startup).
	202	+	if err := restore(); err != nil {
	203	+	t.Fatalf("restore: %v", err)
	204	+	}
	205	+
	206	+	ch := tm.ResizeEvents()
	207	+
	208	+	// The channel must already be closed so a consumer selecting on it
	209	+	// doesn't hang.
	210	+	select {
	211	+	case _, ok := <-ch:
	212	+	if ok {
	213	+	t.Fatalf("expected closed resize channel, got a value")
	214	+	}
	215	+	case <-time.After(2 * time.Second):
	216	+	t.Fatalf("resize channel not closed after restore-before-ResizeEvents")
	217	+	}
	218	+
	219	+	// No watcher may have started: no signal handler installed, no quit
	220	+	// channel allocated, no goroutine registered.
	221	+	tm.mu.Lock()
	222	+	sigCh, quit := tm.sigCh, tm.resizeQuit
	223	+	tm.mu.Unlock()
	224	+	if sigCh != nil {
	225	+	t.Fatalf("expected no signal channel (no signal.Notify) after restore-first ordering")
	226	+	}
	227	+	if quit != nil {
	228	+	t.Fatalf("expected no quit channel (no watcher goroutine) after restore-first ordering")
	229	+	}
	230	+	// Returns immediately iff no watcher goroutine was leaked.
	231	+	tm.resizeWG.Wait()
	232	+
	233	+	outW.Close()
	234	+	<-done
	235	+	}
	236	+
	237	+	func TestTerminal_ResizeEvents_ConcurrentWithRestore(t *testing.T) {
	238	+	// Race ResizeEvents against a restore triggered from another
	239	+	// goroutine (the signal-driven shutdown shape). Under -race this
	240	+	// locks in that watcher start/stop are ordered by the mutex; in all
	241	+	// interleavings the returned channel must eventually be closed and
	242	+	// no watcher goroutine may leak.
	243	+	for i := 0; i < 50; i++ {
	244	+	inR, inW, err := os.Pipe()
	245	+	if err != nil {
	246	+	t.Fatalf("os.Pipe(in): %v", err)
	247	+	}
	248	+	outR, outW, _, done := pipeCapture(t)
	249	+
	250	+	tm := NewWithFiles(inR, outW)
	251	+	restore, err := tm.EnterRaw()
	252	+	if err != nil {
	253	+	t.Fatalf("EnterRaw: %v", err)
	254	+	}
	255	+
	256	+	start := make(chan struct{})
	257	+	chch := make(chan (<-chan domain.Size), 1)
	258	+	go func() {
	259	+	<-start
	260	+	chch <- tm.ResizeEvents()
	261	+	}()
	262	+	go func() {
	263	+	<-start
	264	+	_ = restore()
	265	+	}()
	266	+	close(start)
	267	+
	268	+	ch := <-chch
	269	+	// restore may still be in flight; it is idempotent and returns
	270	+	// only after the watcher (if any) has fully stopped.
	271	+	if err := restore(); err != nil {
	272	+	t.Fatalf("iteration %d: restore: %v", i, err)
	273	+	}
	274	+
	275	+	select {
	276	+	case _, ok := <-ch:
	277	+	if ok {
	278	+	// A real size emission is impossible here (pipe, not a
	279	+	// tty, and no SIGWINCH sent) — anything but a close is a
	280	+	// bug.
	281	+	t.Fatalf("iteration %d: expected closed channel, got a value", i)
	282	+	}
	283	+	case <-time.After(2 * time.Second):
	284	+	t.Fatalf("iteration %d: resize channel never closed", i)
	285	+	}
	286	+	tm.resizeWG.Wait()
	287	+
	288	+	outW.Close()
	289	+	<-done
	290	+	inR.Close()
	291	+	inW.Close()
	292	+	outR.Close()
	293	+	}
	294	+	}
	295	+
181	296		func TestTerminal_Size_NonTTY_ReturnsError(t *testing.T) {
182	297		inR, inW, err := os.Pipe()
183	298		if err != nil {


@@ -6,6 +6,8 @@ import (
6	"os"	6		"os"
7	"testing"	7		"testing"
8	"time"	8		"time"
		9	+
		10	+	"github.com/bnema/vev/internal/domain"
9	)	11		)
10		12
11	// pipePair returns an os.Pipe and a goroutine that continuously copies	13		// pipePair returns an os.Pipe and a goroutine that continuously copies

@@ -178,6 +180,119 @@ func TestTerminal_ResizeEvents_ReturnsSameChannelAndClosesOnRestore(t *testing.T
178	<-done	180		<-done
179	}	181		}
180		182
		183	+	func TestTerminal_ResizeEvents_AfterRestore_NoWatcherAndClosedChannel(t *testing.T) {
		184	+	inR, inW, err := os.Pipe()
		185	+	if err != nil {
		186	+	t.Fatalf("os.Pipe(in): %v", err)
		187	+	}
		188	+	defer inW.Close()
		189	+	defer inR.Close()
		190	+
		191	+	outR, outW, _, done := pipeCapture(t)
		192	+	defer outR.Close()
		193	+
		194	+	tm := NewWithFiles(inR, outW)
		195	+
		196	+	restore, err := tm.EnterRaw()
		197	+	if err != nil {
		198	+	t.Fatalf("EnterRaw: %v", err)
		199	+	}
		200	+	// restore BEFORE the first ResizeEvents call (signal-driven shutdown
		201	+	// racing ahead of the resize pump's startup).
		202	+	if err := restore(); err != nil {
		203	+	t.Fatalf("restore: %v", err)
		204	+	}
		205	+
		206	+	ch := tm.ResizeEvents()
		207	+
		208	+	// The channel must already be closed so a consumer selecting on it
		209	+	// doesn't hang.
		210	+	select {
		211	+	case _, ok := <-ch:
		212	+	if ok {
		213	+	t.Fatalf("expected closed resize channel, got a value")
		214	+	}
		215	+	case <-time.After(2 * time.Second):
		216	+	t.Fatalf("resize channel not closed after restore-before-ResizeEvents")
		217	+	}
		218	+
		219	+	// No watcher may have started: no signal handler installed, no quit
		220	+	// channel allocated, no goroutine registered.
		221	+	tm.mu.Lock()
		222	+	sigCh, quit := tm.sigCh, tm.resizeQuit
		223	+	tm.mu.Unlock()
		224	+	if sigCh != nil {
		225	+	t.Fatalf("expected no signal channel (no signal.Notify) after restore-first ordering")
		226	+	}
		227	+	if quit != nil {
		228	+	t.Fatalf("expected no quit channel (no watcher goroutine) after restore-first ordering")
		229	+	}
		230	+	// Returns immediately iff no watcher goroutine was leaked.
		231	+	tm.resizeWG.Wait()
		232	+
		233	+	outW.Close()
		234	+	<-done
		235	+	}
		236	+
		237	+	func TestTerminal_ResizeEvents_ConcurrentWithRestore(t *testing.T) {
		238	+	// Race ResizeEvents against a restore triggered from another
		239	+	// goroutine (the signal-driven shutdown shape). Under -race this
		240	+	// locks in that watcher start/stop are ordered by the mutex; in all
		241	+	// interleavings the returned channel must eventually be closed and
		242	+	// no watcher goroutine may leak.
		243	+	for i := 0; i < 50; i++ {
		244	+	inR, inW, err := os.Pipe()
		245	+	if err != nil {
		246	+	t.Fatalf("os.Pipe(in): %v", err)
		247	+	}
		248	+	outR, outW, _, done := pipeCapture(t)
		249	+
		250	+	tm := NewWithFiles(inR, outW)
		251	+	restore, err := tm.EnterRaw()
		252	+	if err != nil {
		253	+	t.Fatalf("EnterRaw: %v", err)
		254	+	}
		255	+
		256	+	start := make(chan struct{})
		257	+	chch := make(chan (<-chan domain.Size), 1)
		258	+	go func() {
		259	+	<-start
		260	+	chch <- tm.ResizeEvents()
		261	+	}()
		262	+	go func() {
		263	+	<-start
		264	+	_ = restore()
		265	+	}()
		266	+	close(start)
		267	+
		268	+	ch := <-chch
		269	+	// restore may still be in flight; it is idempotent and returns
		270	+	// only after the watcher (if any) has fully stopped.
		271	+	if err := restore(); err != nil {
		272	+	t.Fatalf("iteration %d: restore: %v", i, err)
		273	+	}
		274	+
		275	+	select {
		276	+	case _, ok := <-ch:
		277	+	if ok {
		278	+	// A real size emission is impossible here (pipe, not a
		279	+	// tty, and no SIGWINCH sent) — anything but a close is a
		280	+	// bug.
		281	+	t.Fatalf("iteration %d: expected closed channel, got a value", i)
		282	+	}
		283	+	case <-time.After(2 * time.Second):
		284	+	t.Fatalf("iteration %d: resize channel never closed", i)
		285	+	}
		286	+	tm.resizeWG.Wait()
		287	+
		288	+	outW.Close()
		289	+	<-done
		290	+	inR.Close()
		291	+	inW.Close()
		292	+	outR.Close()
		293	+	}
		294	+	}
		295	+
181	func TestTerminal_Size_NonTTY_ReturnsError(t *testing.T) {	296		func TestTerminal_Size_NonTTY_ReturnsError(t *testing.T) {
182	inR, inW, err := os.Pipe()	297		inR, inW, err := os.Pipe()
183	if err != nil {	298		if err != nil {