b9a79f3 test(daemon): synchronize handoff contention · vev

M internal/usecase/daemon/client.go +6 -3


@@ -51,7 +51,7 @@ type attachedClient struct {
51	51		theme themeui.Theme
52	52		clientTheme themeui.Theme
53	53		lastCursor cursorOut
54		-	renderStages renderStageHooks // invoked at real pipeline boundaries while sendMu is held
	54	+	renderStages renderStageHooks // optional render and handoff observability hooks
55	55		// previousSession is guarded independently. It is retained through temporary
56	56		// setSession(nil) hand-offs and cleared only on terminal teardown.
57	57		previousSession Guarded[*session]

@@ -437,13 +437,16 @@ func (d Daemon) attachClientDeferred(sess session, tr ports.Transport, sz doma
437	437		}
438	438
439	439		// handoffCoordinator prepares a cross-session ownership transfer before the
440		-	// destination publishes sess.client. It never takes daemon/session locks while
441		-	// taking sendMu, avoiding a d.mu/sendMu cycle with transport callbacks.
	440	+	// destination publishes sess.client. Callers may hold d.mu, but this function
	441	+	// acquires no daemon or session locks while it holds sendMu.
442	442		func (d Daemon) handoffCoordinator(from, target session, old, current *attachedClient) []renderLifecycleCleanup {
443	443		cleanups := make([]renderLifecycleCleanup, 0, 2)
444	444		if rc := from.renderCoordinator(); rc != nil {
445	445		cleanups = append(cleanups, rc.beginDetach(current))
446	446		}
	447	+	if current.renderStages.handoffRebase != nil {
	448	+	current.renderStages.handoffRebase()
	449	+	}
447	450		current.sendMu.Lock()
448	451		current.output.rebase()
449	452		// Capture snapshots are attachment-owned. Discard them while the attachment

 	theme         themeui.Theme
 	clientTheme   themeui.Theme
 	lastCursor    cursorOut
-	renderStages  renderStageHooks // invoked at real pipeline boundaries while sendMu is held
+	renderStages  renderStageHooks // optional render and handoff observability hooks
 	// previousSession is guarded independently. It is retained through temporary
 	// setSession(nil) hand-offs and cleared only on terminal teardown.
 	previousSession Guarded[*session]
 }
 // handoffCoordinator prepares a cross-session ownership transfer before the
-// destination publishes sess.client. It never takes daemon/session locks while
+// destination publishes sess.client. Callers may hold d.mu, but this function
-// taking sendMu, avoiding a d.mu/sendMu cycle with transport callbacks.
+// acquires no daemon or session locks while it holds sendMu.
 func (d *Daemon) handoffCoordinator(from, target *session, old, current *attachedClient) []renderLifecycleCleanup {
 	cleanups := make([]renderLifecycleCleanup, 0, 2)
 	if rc := from.renderCoordinator(); rc != nil {
 		cleanups = append(cleanups, rc.beginDetach(current))
 	}
+	if current.renderStages.handoffRebase != nil {
+		current.renderStages.handoffRebase()
+	}
 	current.sendMu.Lock()
 	current.output.rebase()
 	// Capture snapshots are attachment-owned. Discard them while the attachment

M internal/usecase/daemon/picker_test.go +15 -15


@@ -421,6 +421,10 @@ func TestPickerSessionSwitchWaitsForInFlightPaintSend(t *testing.T) {
421	421		tr.EXPECT().Close().Return(nil).Maybe()
422	422		ac := &attachedClient{tr: tr, output: newOutputStateStream(), size: domain.Size{Cols: 80, Rows: 24}}
423	423		ac.initOverlays()
	424	+	handoffAtSendMu := make(chan struct{})
	425	+	ac.renderStages = renderStageHooks{
	426	+	handoffRebase: func() { close(handoffAtSendMu) },
	427	+	}
424	428		sctx1, cancel1 := context.WithCancel(d.serveCtx)
425	429		sctx2, cancel2 := context.WithCancel(d.serveCtx)
426	430		defer cancel1()

@@ -439,29 +443,25 @@ func TestPickerSessionSwitchWaitsForInFlightPaintSend(t *testing.T) {
439	443		}()
440	444		<-enteredSend
441	445
442		-	switchStarted := make(chan struct{})
443	446		switchDone := make(chan struct{})
444	447		go func() {
445		-	close(switchStarted)
446		-	d.stealClientForTarget(sess1, ac, sess2, picker.Target{Session: sess2.id})
	448	+	cleanups := d.handoffCoordinator(sess1, sess2, nil, ac)
	449	+	finishRenderLifecycleCleanups(cleanups)
447	450		close(switchDone)
448	451		}()
449		-	<-switchStarted
	452	+	awaitTestCompletion(t, handoffAtSendMu, "session switch did not reach the handoff sendMu boundary")
450	453
451		-	// enteredSend proves paint owns ac.sendMu. The real handoff also takes that
452		-	// lock, so releasing the transport send is the deterministic handoff gate.
453		-	close(releaseSend)
454		-	select {
455		-	case <-paintDone:
456		-	case <-time.After(time.Second):
457		-	t.Fatal("paint did not finish after Send was released")
458		-	}
	454	+	// The handoff is immediately about to acquire sendMu, which paint still owns
	455	+	// through its blocked transport send.
459	456		select {
460	457		case <-switchDone:
461		-	case <-time.After(time.Second):
462		-	t.Fatal("session switch did not complete after paint finished")
	458	+	t.Fatal("session switch completed while paint owned sendMu")
	459	+	default:
463	460		}
464		-	require.Same(t, sess2, ac.currentSession())
	461	+
	462	+	close(releaseSend)
	463	+	awaitTestCompletion(t, paintDone, "paint did not finish after Send was released")
	464	+	awaitTestCompletion(t, switchDone, "session handoff did not complete after paint finished")
465	465		}
466	466
467	467		func TestPickerCrossSessionSwitchCopiesTerminalEnvForFutureTabs(t *testing.T) {


@@ -421,6 +421,10 @@ func TestPickerSessionSwitchWaitsForInFlightPaintSend(t *testing.T) {
421		tr.EXPECT().Close().Return(nil).Maybe()	421		tr.EXPECT().Close().Return(nil).Maybe()
422		ac := &attachedClient{tr: tr, output: newOutputStateStream(), size: domain.Size{Cols: 80, Rows: 24}}	422		ac := &attachedClient{tr: tr, output: newOutputStateStream(), size: domain.Size{Cols: 80, Rows: 24}}
423		ac.initOverlays()	423		ac.initOverlays()
			424	+	handoffAtSendMu := make(chan struct{})
			425	+	ac.renderStages = renderStageHooks{
			426	+	handoffRebase: func() { close(handoffAtSendMu) },
			427	+	}
424		sctx1, cancel1 := context.WithCancel(d.serveCtx)	428		sctx1, cancel1 := context.WithCancel(d.serveCtx)
425		sctx2, cancel2 := context.WithCancel(d.serveCtx)	429		sctx2, cancel2 := context.WithCancel(d.serveCtx)
426		defer cancel1()	430		defer cancel1()

@@ -439,29 +443,25 @@ func TestPickerSessionSwitchWaitsForInFlightPaintSend(t *testing.T) {
439		}()	443		}()
440		<-enteredSend	444		<-enteredSend
441			445
442	-	switchStarted := make(chan struct{})
443		switchDone := make(chan struct{})	446		switchDone := make(chan struct{})
444		go func() {	447		go func() {
445	-	close(switchStarted)	448	+	cleanups := d.handoffCoordinator(sess1, sess2, nil, ac)
446	-	d.stealClientForTarget(sess1, ac, sess2, picker.Target{Session: sess2.id})	449	+	finishRenderLifecycleCleanups(cleanups)
447		close(switchDone)	450		close(switchDone)
448		}()	451		}()
449	-	<-switchStarted	452	+	awaitTestCompletion(t, handoffAtSendMu, "session switch did not reach the handoff sendMu boundary")
450			453
451	-	// enteredSend proves paint owns ac.sendMu. The real handoff also takes that	454	+	// The handoff is immediately about to acquire sendMu, which paint still owns
452	-	// lock, so releasing the transport send is the deterministic handoff gate.	455	+	// through its blocked transport send.
453	-	close(releaseSend)
454	-	select {
455	-	case <-paintDone:
456	-	case <-time.After(time.Second):
457	-	t.Fatal("paint did not finish after Send was released")
458	-	}
459		select {	456		select {
460		case <-switchDone:	457		case <-switchDone:
461	-	case <-time.After(time.Second):	458	+	t.Fatal("session switch completed while paint owned sendMu")
462	-	t.Fatal("session switch did not complete after paint finished")	459	+	default:
463		}	460		}
464	-	require.Same(t, sess2, ac.currentSession())	461	+
			462	+	close(releaseSend)
			463	+	awaitTestCompletion(t, paintDone, "paint did not finish after Send was released")
			464	+	awaitTestCompletion(t, switchDone, "session handoff did not complete after paint finished")
465		}	465		}
466			466
467		func TestPickerCrossSessionSwitchCopiesTerminalEnvForFutureTabs(t *testing.T) {	467		func TestPickerCrossSessionSwitchCopiesTerminalEnvForFutureTabs(t *testing.T) {

M internal/usecase/daemon/render_pipeline.go +10 -8


@@ -16,15 +16,17 @@ import (
16	16
17	17		// composeCacheInput is an attachment-local value snapshot. composeFrame never
18	18		// retains or accesses the attachment that owns it.
19		-	// renderStageHooks are optional observability hooks. They are invoked only at
20		-	// completed production boundaries: capture and compose after their result is
21		-	// available, and emit after prepare plus transport success (including a
22		-	// successful no-byte emission). Failed attempts count for capture/compose but
23		-	// never for emit.
	19	+	// renderStageHooks are optional observability hooks. capture and compose run
	20	+	// after their result is available, and emit after prepare plus transport success
	21	+	// (including a successful no-byte emission); those hooks run while sendMu is
	22	+	// held. Failed attempts count for capture/compose but never for emit.
	23	+	// handoffRebase marks the boundary immediately before a handoff acquires sendMu
	24	+	// to rebase attachment-owned output state, so it intentionally runs unlocked.
24	25		type renderStageHooks struct {
25		-	capture func()
26		-	compose func()
27		-	emit func()
	26	+	capture func()
	27	+	compose func()
	28	+	emit func()
	29	+	handoffRebase func()
28	30		}
29	31
30	32		type composeCacheInput struct {


@@ -16,15 +16,17 @@ import (
16			16
17		// composeCacheInput is an attachment-local value snapshot. composeFrame never	17		// composeCacheInput is an attachment-local value snapshot. composeFrame never
18		// retains or accesses the attachment that owns it.	18		// retains or accesses the attachment that owns it.
19	-	// renderStageHooks are optional observability hooks. They are invoked only at	19	+	// renderStageHooks are optional observability hooks. capture and compose run
20	-	// completed production boundaries: capture and compose after their result is	20	+	// after their result is available, and emit after prepare plus transport success
21	-	// available, and emit after prepare plus transport success (including a	21	+	// (including a successful no-byte emission); those hooks run while sendMu is
22	-	// successful no-byte emission). Failed attempts count for capture/compose but	22	+	// held. Failed attempts count for capture/compose but never for emit.
23	-	// never for emit.	23	+	// handoffRebase marks the boundary immediately before a handoff acquires sendMu
			24	+	// to rebase attachment-owned output state, so it intentionally runs unlocked.
24		type renderStageHooks struct {	25		type renderStageHooks struct {
25	-	capture func()	26	+	capture func()
26	-	compose func()	27	+	compose func()
27	-	emit func()	28	+	emit func()
			29	+	handoffRebase func()
28		}	30		}
29			31
30		type composeCacheInput struct {	32		type composeCacheInput struct {