02c900e fix(daemon): serialize floating launch shutdown · vev

M internal/usecase/daemon/daemon.go +9 -0


@@ -74,6 +74,10 @@ type Daemon struct {
74	74		// under the same mutex — so a Hello racing shutdown can never insert a new
75	75		// session that nobody would tear down.
76	76		closing bool
	77	+	// floatingLaunchGate serializes the irreversible shutdown decision with the
	78	+	// final transition into PTYFactory.Open. It is intentionally not d.mu: no
	79	+	// daemon, session, or tab lock is held while an external Open can block.
	80	+	floatingLaunchGate sync.Mutex
77	81		// notifies holds one completion channel per in-flight async Detached
78	82		// notification (guarded by mu, pruned on insert). Channels rather than a
79	83		// WaitGroup: notifications are spawned from arbitrary goroutines while

@@ -419,6 +423,10 @@ func (d *Daemon) Serve(ctx context.Context, l ports.Listener) error {
419	423		// inserted after the snapshot: route rejects once closing is set, and both run
420	424		// under d.mu. killSession (which relocks) runs after the lock is released.
421	425		func (d *Daemon) shutdownAll(reason uint8) {
	426	+	// Take the launch gate before publishing closing. A launch either entered
	427	+	// Open before this point (and is subsequently reaped), or observes closing
	428	+	// and never starts an external child.
	429	+	d.floatingLaunchGate.Lock()
422	430		d.mu.Lock()
423	431		d.closing = true
424	432		for token, parked := range d.parked {

@@ -427,6 +435,7 @@ func (d *Daemon) shutdownAll(reason uint8) {
427	435		snapshot := d.sessionsSnapshotLocked()
428	436		empty := len(snapshot) == 0
429	437		d.mu.Unlock()
	438	+	d.floatingLaunchGate.Unlock()
430	439		d.log.Info("graceful shutdown begin", "reason", reason, "sessions", len(snapshot))
431	440		if empty {
432	441		d.doneOnce.Do(func() { close(d.done) })


@@ -74,6 +74,10 @@ type Daemon struct {
74	// under the same mutex — so a Hello racing shutdown can never insert a new	74		// under the same mutex — so a Hello racing shutdown can never insert a new
75	// session that nobody would tear down.	75		// session that nobody would tear down.
76	closing bool	76		closing bool
		77	+	// floatingLaunchGate serializes the irreversible shutdown decision with the
		78	+	// final transition into PTYFactory.Open. It is intentionally not d.mu: no
		79	+	// daemon, session, or tab lock is held while an external Open can block.
		80	+	floatingLaunchGate sync.Mutex
77	// notifies holds one completion channel per in-flight async Detached	81		// notifies holds one completion channel per in-flight async Detached
78	// notification (guarded by mu, pruned on insert). Channels rather than a	82		// notification (guarded by mu, pruned on insert). Channels rather than a
79	// WaitGroup: notifications are spawned from arbitrary goroutines while	83		// WaitGroup: notifications are spawned from arbitrary goroutines while

@@ -419,6 +423,10 @@ func (d *Daemon) Serve(ctx context.Context, l ports.Listener) error {
419	// inserted after the snapshot: route rejects once closing is set, and both run	423		// inserted after the snapshot: route rejects once closing is set, and both run
420	// under d.mu. killSession (which relocks) runs after the lock is released.	424		// under d.mu. killSession (which relocks) runs after the lock is released.
421	func (d *Daemon) shutdownAll(reason uint8) {	425		func (d *Daemon) shutdownAll(reason uint8) {
		426	+	// Take the launch gate before publishing closing. A launch either entered
		427	+	// Open before this point (and is subsequently reaped), or observes closing
		428	+	// and never starts an external child.
		429	+	d.floatingLaunchGate.Lock()
422	d.mu.Lock()	430		d.mu.Lock()
423	d.closing = true	431		d.closing = true
424	for token, parked := range d.parked {	432		for token, parked := range d.parked {

@@ -427,6 +435,7 @@ func (d *Daemon) shutdownAll(reason uint8) {
427	snapshot := d.sessionsSnapshotLocked()	435		snapshot := d.sessionsSnapshotLocked()
428	empty := len(snapshot) == 0	436		empty := len(snapshot) == 0
429	d.mu.Unlock()	437		d.mu.Unlock()
		438	+	d.floatingLaunchGate.Unlock()
430	d.log.Info("graceful shutdown begin", "reason", reason, "sessions", len(snapshot))	439		d.log.Info("graceful shutdown begin", "reason", reason, "sessions", len(snapshot))
431	if empty {	440		if empty {
432	d.doneOnce.Do(func() { close(d.done) })	441		d.doneOnce.Do(func() { close(d.done) })

M internal/usecase/daemon/floating.go +75 -6


@@ -7,6 +7,7 @@ import (
7	7		"strings"
8	8
9	9		"github.com/bnema/vev/internal/domain"
	10	+	"github.com/bnema/vev/internal/ports"
10	11		"github.com/bnema/vev/internal/usecase/layout"
11	12		)
12	13

@@ -224,6 +225,57 @@ func (d Daemon) startFloating(sess session, tb *tab, visible bool) {
224	225
225	226		// floatingLaunchSpec is an immutable snapshot of one launch. It is built
226	227		// before the worker starts so config reloads cannot affect that launch.
	228	+	// floatingLaunch owns one worker from queueing through installation or stale
	229	+	// result cleanup. Its done channel lets session teardown reap an in-flight
	230	+	// Open before returning.
	231	+	type floatingLaunch struct {
	232	+	done chan struct{}
	233	+	}
	234	+
	235	+	func (sess session) registerFloatingLaunch() (floatingLaunch, bool) {
	236	+	launch := &floatingLaunch{done: make(chan struct{})}
	237	+	sess.floatingLaunchMu.Lock()
	238	+	defer sess.floatingLaunchMu.Unlock()
	239	+	if sess.floatingLaunchStopping {
	240	+	return nil, false
	241	+	}
	242	+	if sess.floatingLaunches == nil {
	243	+	sess.floatingLaunches = make(map[*floatingLaunch]struct{})
	244	+	}
	245	+	sess.floatingLaunches[launch] = struct{}{}
	246	+	return launch, true
	247	+	}
	248	+
	249	+	// openFloating runs fn while only the dedicated launch-ownership lock is
	250	+	// held. It deliberately does not hold sess.mu, tb.mu, or d.mu across Open.
	251	+	func (sess session) openFloating(launch floatingLaunch, fn func() (ports.PTY, error)) (ports.PTY, error, bool) {
	252	+	sess.floatingLaunchMu.Lock()
	253	+	defer sess.floatingLaunchMu.Unlock()
	254	+	if sess.floatingLaunchStopping {
	255	+	return nil, nil, false
	256	+	}
	257	+	pty, err := fn()
	258	+	return pty, err, true
	259	+	}
	260	+
	261	+	func (sess session) finishFloatingLaunch(launch floatingLaunch) {
	262	+	sess.floatingLaunchMu.Lock()
	263	+	delete(sess.floatingLaunches, launch)
	264	+	close(launch.done)
	265	+	sess.floatingLaunchMu.Unlock()
	266	+	}
	267	+
	268	+	func (sess *session) stopFloatingLaunches() []<-chan struct{} {
	269	+	sess.floatingLaunchMu.Lock()
	270	+	sess.floatingLaunchStopping = true
	271	+	done := make([]<-chan struct{}, 0, len(sess.floatingLaunches))
	272	+	for launch := range sess.floatingLaunches {
	273	+	done = append(done, launch.done)
	274	+	}
	275	+	sess.floatingLaunchMu.Unlock()
	276	+	return done
	277	+	}
	278	+
227	279		type floatingLaunchSpec struct {
228	280		sessionName string
229	281		cwd string

@@ -246,10 +298,18 @@ func (d Daemon) launchFloating(sess session, tb *tab, cfg domain.FloatingConfi
246	298		d.failFloatingLaunch(tb, generation, userOpen, spec.sessionName, err)
247	299		return
248	300		}
	301	+	launch, ok := sess.registerFloatingLaunch()
	302	+	if !ok {
	303	+	d.failFloatingLaunch(tb, generation, userOpen, spec.sessionName, context.Canceled)
	304	+	return
	305	+	}
249	306		// Count the launch before its goroutine starts. It remains counted through
250	307		// install, including the reader/coordinator Adds, so shutdown cannot return
251	308		// before a late Open completion has either been rejected or fully joined.
252		-	d.sessWg.Go(func() { d.openAndInstallFloating(sess, tb, spec, generation) })
	309	+	d.sessWg.Go(func() {
	310	+	defer sess.finishFloatingLaunch(launch)
	311	+	d.openAndInstallFloating(sess, tb, spec, generation, launch)
	312	+	})
253	313		}
254	314
255	315		func (d Daemon) newFloatingLaunchSpec(sess session, tb *tab, cfg domain.FloatingConfig, userOpen bool) (floatingLaunchSpec, error) {

@@ -305,12 +365,14 @@ func (d Daemon) newFloatingLaunchSpec(sess session, tb *tab, cfg domain.Floati
305	365
306	366		// openAndInstallFloating runs entirely in the launch worker. No PTY operation
307	367		// occurs under tb.mu; the pane is fully initialized before publication.
308		-	func (d Daemon) openAndInstallFloating(sess session, tb *tab, spec floatingLaunchSpec, generation uint64) {
309		-	// Launch workers are allowed to outlive the action that queued them, but
310		-	// never session teardown. Check both cancellation and daemon ownership
311		-	// immediately before the irreversible external Open.
	368	+	func (d Daemon) openAndInstallFloating(sess session, tb tab, spec floatingLaunchSpec, generation uint64, launch floatingLaunch) {
	369	+	// The gate makes the final ownership check and external Open one shutdown
	370	+	// epoch: shutdown cannot become irreversible between them. Neither d.mu,
	371	+	// sess.mu, nor tb.mu is held while the factory can block.
	372	+	d.floatingLaunchGate.Lock()
312	373		select {
313	374		case <-spec.parentCtx.Done():
	375	+	d.floatingLaunchGate.Unlock()
314	376		return
315	377		default:
316	378		}

@@ -321,9 +383,16 @@ func (d Daemon) openAndInstallFloating(sess session, tb *tab, spec floatingLau
321	383		live := !d.closing && (sess.id == "" \|\| d.sessions[sess.id] == sess)
322	384		d.mu.Unlock()
323	385		if !live {
	386	+	d.floatingLaunchGate.Unlock()
	387	+	return
	388	+	}
	389	+	pty, err, opened := sess.openFloating(launch, func() (ports.PTY, error) {
	390	+	return d.ptys.Open(spec.command, spec.args, spec.env, spec.cwd, spec.size)
	391	+	})
	392	+	d.floatingLaunchGate.Unlock()
	393	+	if !opened {
324	394		return
325	395		}
326		-	pty, err := d.ptys.Open(spec.command, spec.args, spec.env, spec.cwd, spec.size)
327	396		if err != nil {
328	397		d.failFloatingLaunch(tb, generation, spec.userOpen, spec.sessionName, err)
329	398		return


@@ -7,6 +7,7 @@ import (
7		"strings"	7		"strings"
8			8
9		"github.com/bnema/vev/internal/domain"	9		"github.com/bnema/vev/internal/domain"
			10	+	"github.com/bnema/vev/internal/ports"
10		"github.com/bnema/vev/internal/usecase/layout"	11		"github.com/bnema/vev/internal/usecase/layout"
11		)	12		)
12			13

@@ -224,6 +225,57 @@ func (d Daemon) startFloating(sess session, tb *tab, visible bool) {
224			225
225		// floatingLaunchSpec is an immutable snapshot of one launch. It is built	226		// floatingLaunchSpec is an immutable snapshot of one launch. It is built
226		// before the worker starts so config reloads cannot affect that launch.	227		// before the worker starts so config reloads cannot affect that launch.
			228	+	// floatingLaunch owns one worker from queueing through installation or stale
			229	+	// result cleanup. Its done channel lets session teardown reap an in-flight
			230	+	// Open before returning.
			231	+	type floatingLaunch struct {
			232	+	done chan struct{}
			233	+	}
			234	+
			235	+	func (sess session) registerFloatingLaunch() (floatingLaunch, bool) {
			236	+	launch := &floatingLaunch{done: make(chan struct{})}
			237	+	sess.floatingLaunchMu.Lock()
			238	+	defer sess.floatingLaunchMu.Unlock()
			239	+	if sess.floatingLaunchStopping {
			240	+	return nil, false
			241	+	}
			242	+	if sess.floatingLaunches == nil {
			243	+	sess.floatingLaunches = make(map[*floatingLaunch]struct{})
			244	+	}
			245	+	sess.floatingLaunches[launch] = struct{}{}
			246	+	return launch, true
			247	+	}
			248	+
			249	+	// openFloating runs fn while only the dedicated launch-ownership lock is
			250	+	// held. It deliberately does not hold sess.mu, tb.mu, or d.mu across Open.
			251	+	func (sess session) openFloating(launch floatingLaunch, fn func() (ports.PTY, error)) (ports.PTY, error, bool) {
			252	+	sess.floatingLaunchMu.Lock()
			253	+	defer sess.floatingLaunchMu.Unlock()
			254	+	if sess.floatingLaunchStopping {
			255	+	return nil, nil, false
			256	+	}
			257	+	pty, err := fn()
			258	+	return pty, err, true
			259	+	}
			260	+
			261	+	func (sess session) finishFloatingLaunch(launch floatingLaunch) {
			262	+	sess.floatingLaunchMu.Lock()
			263	+	delete(sess.floatingLaunches, launch)
			264	+	close(launch.done)
			265	+	sess.floatingLaunchMu.Unlock()
			266	+	}
			267	+
			268	+	func (sess *session) stopFloatingLaunches() []<-chan struct{} {
			269	+	sess.floatingLaunchMu.Lock()
			270	+	sess.floatingLaunchStopping = true
			271	+	done := make([]<-chan struct{}, 0, len(sess.floatingLaunches))
			272	+	for launch := range sess.floatingLaunches {
			273	+	done = append(done, launch.done)
			274	+	}
			275	+	sess.floatingLaunchMu.Unlock()
			276	+	return done
			277	+	}
			278	+
227		type floatingLaunchSpec struct {	279		type floatingLaunchSpec struct {
228		sessionName string	280		sessionName string
229		cwd string	281		cwd string

@@ -246,10 +298,18 @@ func (d Daemon) launchFloating(sess session, tb *tab, cfg domain.FloatingConfi
246		d.failFloatingLaunch(tb, generation, userOpen, spec.sessionName, err)	298		d.failFloatingLaunch(tb, generation, userOpen, spec.sessionName, err)
247		return	299		return
248		}	300		}
			301	+	launch, ok := sess.registerFloatingLaunch()
			302	+	if !ok {
			303	+	d.failFloatingLaunch(tb, generation, userOpen, spec.sessionName, context.Canceled)
			304	+	return
			305	+	}
249		// Count the launch before its goroutine starts. It remains counted through	306		// Count the launch before its goroutine starts. It remains counted through
250		// install, including the reader/coordinator Adds, so shutdown cannot return	307		// install, including the reader/coordinator Adds, so shutdown cannot return
251		// before a late Open completion has either been rejected or fully joined.	308		// before a late Open completion has either been rejected or fully joined.
252	-	d.sessWg.Go(func() { d.openAndInstallFloating(sess, tb, spec, generation) })	309	+	d.sessWg.Go(func() {
			310	+	defer sess.finishFloatingLaunch(launch)
			311	+	d.openAndInstallFloating(sess, tb, spec, generation, launch)
			312	+	})
253		}	313		}
254			314
255		func (d Daemon) newFloatingLaunchSpec(sess session, tb *tab, cfg domain.FloatingConfig, userOpen bool) (floatingLaunchSpec, error) {	315		func (d Daemon) newFloatingLaunchSpec(sess session, tb *tab, cfg domain.FloatingConfig, userOpen bool) (floatingLaunchSpec, error) {

@@ -305,12 +365,14 @@ func (d Daemon) newFloatingLaunchSpec(sess session, tb *tab, cfg domain.Floati
305			365
306		// openAndInstallFloating runs entirely in the launch worker. No PTY operation	366		// openAndInstallFloating runs entirely in the launch worker. No PTY operation
307		// occurs under tb.mu; the pane is fully initialized before publication.	367		// occurs under tb.mu; the pane is fully initialized before publication.
308	-	func (d Daemon) openAndInstallFloating(sess session, tb *tab, spec floatingLaunchSpec, generation uint64) {	368	+	func (d Daemon) openAndInstallFloating(sess session, tb tab, spec floatingLaunchSpec, generation uint64, launch floatingLaunch) {
309	-	// Launch workers are allowed to outlive the action that queued them, but	369	+	// The gate makes the final ownership check and external Open one shutdown
310	-	// never session teardown. Check both cancellation and daemon ownership	370	+	// epoch: shutdown cannot become irreversible between them. Neither d.mu,
311	-	// immediately before the irreversible external Open.	371	+	// sess.mu, nor tb.mu is held while the factory can block.
			372	+	d.floatingLaunchGate.Lock()
312		select {	373		select {
313		case <-spec.parentCtx.Done():	374		case <-spec.parentCtx.Done():
			375	+	d.floatingLaunchGate.Unlock()
314		return	376		return
315		default:	377		default:
316		}	378		}

@@ -321,9 +383,16 @@ func (d Daemon) openAndInstallFloating(sess session, tb *tab, spec floatingLau
321		live := !d.closing && (sess.id == "" \|\| d.sessions[sess.id] == sess)	383		live := !d.closing && (sess.id == "" \|\| d.sessions[sess.id] == sess)
322		d.mu.Unlock()	384		d.mu.Unlock()
323		if !live {	385		if !live {
			386	+	d.floatingLaunchGate.Unlock()
			387	+	return
			388	+	}
			389	+	pty, err, opened := sess.openFloating(launch, func() (ports.PTY, error) {
			390	+	return d.ptys.Open(spec.command, spec.args, spec.env, spec.cwd, spec.size)
			391	+	})
			392	+	d.floatingLaunchGate.Unlock()
			393	+	if !opened {
324		return	394		return
325		}	395		}
326	-	pty, err := d.ptys.Open(spec.command, spec.args, spec.env, spec.cwd, spec.size)
327		if err != nil {	396		if err != nil {
328		d.failFloatingLaunch(tb, generation, spec.userOpen, spec.sessionName, err)	397		d.failFloatingLaunch(tb, generation, spec.userOpen, spec.sessionName, err)
329		return	398		return

M internal/usecase/daemon/session.go +20 -0


@@ -53,6 +53,13 @@ type session struct {
53	53		// clipboard.go) written for this session, removed best-effort in
54	54		// killSession.
55	55		clipFiles []string
	56	+
	57	+	// floatingLaunchMu is ownership synchronization for external floating
	58	+	// launches. It is separate from mu so PTYFactory.Open never holds a
	59	+	// session, tab, or daemon architecture lock.
	60	+	floatingLaunchMu sync.Mutex
	61	+	floatingLaunchStopping bool
	62	+	floatingLaunches map[*floatingLaunch]struct{}
56	63		}
57	64
58	65		// tab is a pane layout container; pane owns PTY/screen/scrollback/render scheduling state.

@@ -667,9 +674,14 @@ func (d Daemon) killSession(sess session, reason uint8, purge bool) error {
667	674		d.captureSession(sess)
668	675		}
669	676		}
	677	+	// Serialize removal of the final session with the last transition into a
	678	+	// floating Open. This establishes whether an Open is pre-shutdown work or
	679	+	// is rejected, without holding d.mu across the external call.
	680	+	d.floatingLaunchGate.Lock()
670	681		d.mu.Lock()
671	682		if _, ok := d.sessions[sess.id]; !ok {
672	683		d.mu.Unlock()
	684	+	d.floatingLaunchGate.Unlock()
673	685		return nil
674	686		}
675	687		delete(d.sessions, sess.id)

@@ -694,6 +706,7 @@ func (d Daemon) killSession(sess session, reason uint8, purge bool) error {
694	706		d.closing = true
695	707		}
696	708		d.mu.Unlock()
	709	+	d.floatingLaunchGate.Unlock()
697	710		d.log.Info("session closed", "session", stoppedName, "id", sess.id, "ephemeral", ephemeral, "purge", purge, "reason", reason)
698	711
699	712		purgeErr := snapshotDeleteErr

@@ -729,6 +742,10 @@ func (d Daemon) killSession(sess session, reason uint8, purge bool) error {
729	742		ac.setSession(nil)
730	743		}
731	744
	745	+	// Prevent queued launches from entering Open, then cancel the parent
	746	+	// context. In-flight Opens are joined below so a late result is always
	747	+	// closed before teardown returns.
	748	+	floatingLaunches := sess.stopFloatingLaunches()
732	749		sess.cancel()
733	750		sess.mu.Lock()
734	751		tabs := append([]*tab(nil), sess.tabs...)

@@ -740,6 +757,9 @@ func (d Daemon) killSession(sess session, reason uint8, purge bool) error {
740	757		d.teardownFloating(tb, ac)
741	758		tb.closeAllPanes()
742	759		}
	760	+	for _, launchDone := range floatingLaunches {
	761	+	<-launchDone
	762	+	}
743	763		for _, path := range clipFiles {
744	764		if err := os.Remove(path); err != nil && !errors.Is(err, os.ErrNotExist) {
745	765		d.log.Warn("removing clipboard temp file failed", "err", err, "path", path)


@@ -53,6 +53,13 @@ type session struct {
53	// clipboard.go) written for this session, removed best-effort in	53		// clipboard.go) written for this session, removed best-effort in
54	// killSession.	54		// killSession.
55	clipFiles []string	55		clipFiles []string
		56	+
		57	+	// floatingLaunchMu is ownership synchronization for external floating
		58	+	// launches. It is separate from mu so PTYFactory.Open never holds a
		59	+	// session, tab, or daemon architecture lock.
		60	+	floatingLaunchMu sync.Mutex
		61	+	floatingLaunchStopping bool
		62	+	floatingLaunches map[*floatingLaunch]struct{}
56	}	63		}
57		64
58	// tab is a pane layout container; pane owns PTY/screen/scrollback/render scheduling state.	65		// tab is a pane layout container; pane owns PTY/screen/scrollback/render scheduling state.

@@ -667,9 +674,14 @@ func (d Daemon) killSession(sess session, reason uint8, purge bool) error {
667	d.captureSession(sess)	674		d.captureSession(sess)
668	}	675		}
669	}	676		}
		677	+	// Serialize removal of the final session with the last transition into a
		678	+	// floating Open. This establishes whether an Open is pre-shutdown work or
		679	+	// is rejected, without holding d.mu across the external call.
		680	+	d.floatingLaunchGate.Lock()
670	d.mu.Lock()	681		d.mu.Lock()
671	if _, ok := d.sessions[sess.id]; !ok {	682		if _, ok := d.sessions[sess.id]; !ok {
672	d.mu.Unlock()	683		d.mu.Unlock()
		684	+	d.floatingLaunchGate.Unlock()
673	return nil	685		return nil
674	}	686		}
675	delete(d.sessions, sess.id)	687		delete(d.sessions, sess.id)

@@ -694,6 +706,7 @@ func (d Daemon) killSession(sess session, reason uint8, purge bool) error {
694	d.closing = true	706		d.closing = true
695	}	707		}
696	d.mu.Unlock()	708		d.mu.Unlock()
		709	+	d.floatingLaunchGate.Unlock()
697	d.log.Info("session closed", "session", stoppedName, "id", sess.id, "ephemeral", ephemeral, "purge", purge, "reason", reason)	710		d.log.Info("session closed", "session", stoppedName, "id", sess.id, "ephemeral", ephemeral, "purge", purge, "reason", reason)
698		711
699	purgeErr := snapshotDeleteErr	712		purgeErr := snapshotDeleteErr

@@ -729,6 +742,10 @@ func (d Daemon) killSession(sess session, reason uint8, purge bool) error {
729	ac.setSession(nil)	742		ac.setSession(nil)
730	}	743		}
731		744
		745	+	// Prevent queued launches from entering Open, then cancel the parent
		746	+	// context. In-flight Opens are joined below so a late result is always
		747	+	// closed before teardown returns.
		748	+	floatingLaunches := sess.stopFloatingLaunches()
732	sess.cancel()	749		sess.cancel()
733	sess.mu.Lock()	750		sess.mu.Lock()
734	tabs := append([]*tab(nil), sess.tabs...)	751		tabs := append([]*tab(nil), sess.tabs...)

@@ -740,6 +757,9 @@ func (d Daemon) killSession(sess session, reason uint8, purge bool) error {
740	d.teardownFloating(tb, ac)	757		d.teardownFloating(tb, ac)
741	tb.closeAllPanes()	758		tb.closeAllPanes()
742	}	759		}
		760	+	for _, launchDone := range floatingLaunches {
		761	+	<-launchDone
		762	+	}
743	for _, path := range clipFiles {	763		for _, path := range clipFiles {
744	if err := os.Remove(path); err != nil && !errors.Is(err, os.ErrNotExist) {	764		if err := os.Remove(path); err != nil && !errors.Is(err, os.ErrNotExist) {
745	d.log.Warn("removing clipboard temp file failed", "err", err, "path", path)	765		d.log.Warn("removing clipboard temp file failed", "err", err, "path", path)

M internal/usecase/daemon/session_test.go +47 -24


@@ -46,7 +46,7 @@ func TestRoutePropagatesHelloCwdAndTabsInheritIt(t *testing.T) {
46	46		floating := newQuietPTY()
47	47		f.EXPECT().Open(mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.MatchedBy(func(got domain.Size) bool {
48	48		return got != normalSize && got.Valid()
49		-	})).Return(floating, nil).Once()
	49	+	})).Return(floating, nil).Maybe()
50	50
51	51		d := newTestDaemon(t, f, stubClock{})
52	52		tr := portsmocks.NewMockTransport(t)

@@ -72,11 +72,8 @@ func TestRoutePropagatesHelloCwdAndTabsInheritIt(t *testing.T) {
72	72		releaseFirst()
73	73		releaseSecond()
74	74		d.sessWg.Wait()
75		-	select {
76		-	case <-floating.done:
77		-	default:
78		-	t.Fatal("floating prewarm PTY was not closed")
79		-	}
	75	+	// Teardown may cancel a queued prewarm before it enters Open; an opened
	76	+	// prewarm is closed by teardownFloating.
80	77		}
81	78
82	79		func TestHandshakeEphemeralHappy(t *testing.T) {

@@ -217,7 +214,7 @@ func TestCreateTabClosesPTYIfSessionKilledDuringOpen(t *testing.T) {
217	214		floating := newQuietPTY()
218	215		f.EXPECT().Open(mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.MatchedBy(func(got domain.Size) bool {
219	216		return got != normalSize && got.Valid()
220		-	})).Return(floating, nil).Once()
	217	+	})).Return(floating, nil).Maybe()
221	218		f.EXPECT().Open(mock.Anything, mock.Anything, mock.Anything, mock.Anything, normalSize).RunAndReturn(
222	219		func(string, []string, []string, string, domain.Size) (ports.PTY, error) {
223	220		close(opened)

@@ -251,11 +248,7 @@ func TestCreateTabClosesPTYIfSessionKilledDuringOpen(t *testing.T) {
251	248		releaseConn()
252	249		hg.Wait()
253	250		d.sessWg.Wait()
254		-	select {
255		-	case <-floating.done:
256		-	default:
257		-	t.Fatal("floating prewarm PTY was not closed")
258		-	}
	251	+	// A queued prewarm may be cancelled before Open during teardown.
259	252		}
260	253
261	254		// --- ephemeral numbering ----------------------------------------------------

@@ -508,24 +501,52 @@ func TestServeGracefulShutdownOnContextCancel(t *testing.T) {
508	501		// --- shutdown/create interlock ------------------------------------------------
509	502
510	503		// TestHelloRacingShutdownIsRejected covers the interlock between the
511		-	// registry-empty shutdown decision and session creation: once killSession has
512		-	// removed the last session (making shutdown irreversible via doneOnce), a
513		-	// Hello landing at any later instant must be rejected cleanly — never spawn a
514		-	// child that no teardown pass would reap. The interleaving is deterministic:
515		-	// killSession runs to completion before the racing Hellos are handled, which
516		-	// models the worst case (insertion attempt after the shutdown snapshot).
	504	+	// registry-empty shutdown decision and session creation. In particular, the
	505	+	// old `.Once()` factory expectation was wrong: firstPaint legitimately queues
	506	+	// a floating prewarm, so an Open that starts before killSession is valid work,
	507	+	// not a leaked post-shutdown child. The factory markers classify timing at the
	508	+	// Open boundary: this test first observes that valid prewarm, then verifies
	509	+	// that d.done (closed synchronously by killSession) is never followed by an
	510	+	// Open start.
517	511		func TestHelloRacingShutdownIsRejected(t *testing.T) {
518	512		p, releasePTY := newBlockingPTY(t)
	513	+	floating := newQuietPTY()
519	514		f := portsmocks.NewMockPTYFactory(t)
520		-	// Exactly one child may ever be spawned; a second Open call (a leaked
521		-	// child) fails the test via mock expectations.
522		-	f.EXPECT().Open(mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(p, nil).Once()
	515	+	normalSize := domain.Size{Cols: 80, Rows: 22}
	516	+	preShutdownFloatingOpen := make(chan struct{}, 1)
	517	+	var opensAfterShutdown atomic.Int32
523	518		d := newTestDaemon(t, f, stubClock{})
	519	+	f.EXPECT().Open(mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything).RunAndReturn(
	520	+	func(_ string, _ []string, _ []string, _ string, size domain.Size) (ports.PTY, error) {
	521	+	select {
	522	+	case <-d.done:
	523	+	opensAfterShutdown.Add(1)
	524	+	default:
	525	+	}
	526	+	if size == normalSize {
	527	+	return p, nil
	528	+	}
	529	+	select {
	530	+	case preShutdownFloatingOpen <- struct{}{}:
	531	+	default:
	532	+	}
	533	+	return floating, nil
	534	+	},
	535	+	).Maybe()
524	536
525	537		tr1, sends1, release1 := newConn(t, mustHello(ports.IntentEphemeral, "", domain.Size{Cols: 80, Rows: 24}))
526	538		var hg sync.WaitGroup
527	539		hg.Go(func() { d.handleConn(tr1) })
528	540		awaitFrame(t, sends1, ports.MsgWelcome)
	541	+	// firstPaint starts the background prewarm asynchronously. It is valid for
	542	+	// this Open to occur before shutdown, so do not mistake it for the racing
	543	+	// Hello's forbidden launch.
	544	+	awaitFrame(t, sends1, ports.MsgOutput)
	545	+	select {
	546	+	case <-preShutdownFloatingOpen:
	547	+	case <-time.After(time.Second):
	548	+	t.Fatal("legitimate pre-shutdown floating prewarm did not start")
	549	+	}
529	550		sess := firstSession(d)
530	551		require.NotNil(t, sess)
531	552

@@ -555,12 +576,14 @@ func TestHelloRacingShutdownIsRejected(t *testing.T) {
555	576		require.Equal(t, ports.ErrServerShutdown, em.Code, "%s hello racing shutdown must be rejected", intent.name)
556	577		}
557	578		require.Equal(t, 0, sessionCount(d), "no session may be inserted after shutdown began")
	579	+	require.Zero(t, opensAfterShutdown.Load(), "no PTYFactory.Open may start after d.done/shutdown completion")
558	580
559	581		release1()
560	582		releasePTY()
561	583		hg.Wait()
562	584		d.sessWg.Wait()
563	585		d.waitNotifies()
	586	+	require.Zero(t, opensAfterShutdown.Load(), "racing Hellos must not start a PTY after shutdown")
564	587		}
565	588
566	589		// --- wedged-client teardown ---------------------------------------------------

@@ -1255,7 +1278,7 @@ func TestAttachUpdatesFutureChildEnvTrueColor(t *testing.T) {
1255	1278		floating := newQuietPTY()
1256	1279		f.EXPECT().Open(mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.MatchedBy(func(got domain.Size) bool {
1257	1280		return got != normalSize && got.Valid()
1258		-	})).Return(floating, nil).Once()
	1281	+	})).Return(floating, nil).Maybe()
1259	1282
1260	1283		d := newTestDaemon(t, f, stubClock{})
1261	1284		d.baseEnv = []string{"KEEP=1", "COLORTERM=old"}

@@ -1300,7 +1323,7 @@ func TestLiveAttachUpdatesFutureChildEnvTrueColor(t *testing.T) {
1300	1323		floating := newQuietPTY()
1301	1324		f.EXPECT().Open(mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.MatchedBy(func(got domain.Size) bool {
1302	1325		return got != normalSize && got.Valid()
1303		-	})).Return(floating, nil).Once()
	1326	+	})).Return(floating, nil).Maybe()
1304	1327
1305	1328		d := newTestDaemon(t, f, stubClock{})
1306	1329		tr1 := portsmocks.NewMockTransport(t)

@@ -1347,7 +1370,7 @@ func TestCreateSessionAndSwitchInheritsTerminalEnv(t *testing.T) {
1347	1370		floating := newQuietPTY()
1348	1371		f.EXPECT().Open(mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.MatchedBy(func(got domain.Size) bool {
1349	1372		return got != normalSize && got.Valid()
1350		-	})).Return(floating, nil).Once()
	1373	+	})).Return(floating, nil).Maybe()
1351	1374		d := newTestDaemon(t, f, stubClock{})
1352	1375		tr := portsmocks.NewMockTransport(t)
1353	1376		tr.EXPECT().Send(mock.Anything).Return(nil).Maybe()

 	floating := newQuietPTY()
 	f.EXPECT().Open(mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.MatchedBy(func(got domain.Size) bool {
 		return got != normalSize && got.Valid()
-	})).Return(floating, nil).Once()
+	})).Return(floating, nil).Maybe()
 	d := newTestDaemon(t, f, stubClock{})
 	tr := portsmocks.NewMockTransport(t)
 	releaseFirst()
 	releaseSecond()
 	d.sessWg.Wait()
-	select {
+	// Teardown may cancel a queued prewarm before it enters Open; an opened
-	case <-floating.done:
+	// prewarm is closed by teardownFloating.
-	default:
-		t.Fatal("floating prewarm PTY was not closed")
-	}
 }
 func TestHandshakeEphemeralHappy(t *testing.T) {
 	floating := newQuietPTY()
 	f.EXPECT().Open(mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.MatchedBy(func(got domain.Size) bool {
 		return got != normalSize && got.Valid()
-	})).Return(floating, nil).Once()
+	})).Return(floating, nil).Maybe()
 	f.EXPECT().Open(mock.Anything, mock.Anything, mock.Anything, mock.Anything, normalSize).RunAndReturn(
 		func(string, []string, []string, string, domain.Size) (ports.PTY, error) {
 			close(opened)
 	releaseConn()
 	hg.Wait()
 	d.sessWg.Wait()
-	select {
+	// A queued prewarm may be cancelled before Open during teardown.
-	case <-floating.done:
-	default:
-		t.Fatal("floating prewarm PTY was not closed")
-	}
 }
 // --- ephemeral numbering ----------------------------------------------------
 // --- shutdown/create interlock ------------------------------------------------
 // TestHelloRacingShutdownIsRejected covers the interlock between the
-// registry-empty shutdown decision and session creation: once killSession has
+// registry-empty shutdown decision and session creation. In particular, the
-// removed the last session (making shutdown irreversible via doneOnce), a
+// old `.Once()` factory expectation was wrong: firstPaint legitimately queues
-// Hello landing at any later instant must be rejected cleanly — never spawn a
+// a floating prewarm, so an Open that starts before killSession is valid work,
-// child that no teardown pass would reap. The interleaving is deterministic:
+// not a leaked post-shutdown child. The factory markers classify timing at the
-// killSession runs to completion before the racing Hellos are handled, which
+// Open boundary: this test first observes that valid prewarm, then verifies
-// models the worst case (insertion attempt after the shutdown snapshot).
+// that d.done (closed synchronously by killSession) is never followed by an
+// Open start.
 func TestHelloRacingShutdownIsRejected(t *testing.T) {
 	p, releasePTY := newBlockingPTY(t)
+	floating := newQuietPTY()
 	f := portsmocks.NewMockPTYFactory(t)
-	// Exactly one child may ever be spawned; a second Open call (a leaked
+	normalSize := domain.Size{Cols: 80, Rows: 22}
-	// child) fails the test via mock expectations.
+	preShutdownFloatingOpen := make(chan struct{}, 1)
-	f.EXPECT().Open(mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(p, nil).Once()
+	var opensAfterShutdown atomic.Int32
 	d := newTestDaemon(t, f, stubClock{})
+	f.EXPECT().Open(mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything).RunAndReturn(
+		func(_ string, _ []string, _ []string, _ string, size domain.Size) (ports.PTY, error) {
+			select {
+			case <-d.done:
+				opensAfterShutdown.Add(1)
+			default:
+			}
+			if size == normalSize {
+				return p, nil
+			}
+			select {
+			case preShutdownFloatingOpen <- struct{}{}:
+			default:
+			}
+			return floating, nil
+		},
+	).Maybe()
 	tr1, sends1, release1 := newConn(t, mustHello(ports.IntentEphemeral, "", domain.Size{Cols: 80, Rows: 24}))
 	var hg sync.WaitGroup
 	hg.Go(func() { d.handleConn(tr1) })
 	awaitFrame(t, sends1, ports.MsgWelcome)
+	// firstPaint starts the background prewarm asynchronously. It is valid for
+	// this Open to occur before shutdown, so do not mistake it for the racing
+	// Hello's forbidden launch.
+	awaitFrame(t, sends1, ports.MsgOutput)
+	select {
+	case <-preShutdownFloatingOpen:
+	case <-time.After(time.Second):
+		t.Fatal("legitimate pre-shutdown floating prewarm did not start")
+	}
 	sess := firstSession(d)
 	require.NotNil(t, sess)
 		require.Equal(t, ports.ErrServerShutdown, em.Code, "%s hello racing shutdown must be rejected", intent.name)
 	}
 	require.Equal(t, 0, sessionCount(d), "no session may be inserted after shutdown began")
+	require.Zero(t, opensAfterShutdown.Load(), "no PTYFactory.Open may start after d.done/shutdown completion")
 	release1()
 	releasePTY()
 	hg.Wait()
 	d.sessWg.Wait()
 	d.waitNotifies()
+	require.Zero(t, opensAfterShutdown.Load(), "racing Hellos must not start a PTY after shutdown")
 }
 // --- wedged-client teardown ---------------------------------------------------
 	floating := newQuietPTY()
 	f.EXPECT().Open(mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.MatchedBy(func(got domain.Size) bool {
 		return got != normalSize && got.Valid()
-	})).Return(floating, nil).Once()
+	})).Return(floating, nil).Maybe()
 	d := newTestDaemon(t, f, stubClock{})
 	d.baseEnv = []string{"KEEP=1", "COLORTERM=old"}
 	floating := newQuietPTY()
 	f.EXPECT().Open(mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.MatchedBy(func(got domain.Size) bool {
 		return got != normalSize && got.Valid()
-	})).Return(floating, nil).Once()
+	})).Return(floating, nil).Maybe()
 	d := newTestDaemon(t, f, stubClock{})
 	tr1 := portsmocks.NewMockTransport(t)
 	floating := newQuietPTY()
 	f.EXPECT().Open(mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.MatchedBy(func(got domain.Size) bool {
 		return got != normalSize && got.Valid()
-	})).Return(floating, nil).Once()
+	})).Return(floating, nil).Maybe()
 	d := newTestDaemon(t, f, stubClock{})
 	tr := portsmocks.NewMockTransport(t)
 	tr.EXPECT().Send(mock.Anything).Return(nil).Maybe()