5c0dde8 fix(daemon): prepare coordinator ownership before publication · vev

M internal/usecase/daemon/client.go +9 -4


@@ -350,21 +350,26 @@ func (d Daemon) attachClient(sess session, tr ports.Transport, sz domain.Size,
350	350		ac.initOverlays()
351	351		ac.setSession(sess)
352	352		ac.keys = keys.NewRouter(d.clock, daemonKeyHandler{d: d, ac: ac}, &d.bindings)
	353	+	// Daemon attachment callers hold d.mu, serialising this preparation with
	354	+	// other publications. Bind coordinator ownership before sess.client becomes
	355	+	// visible: an old deadline can then never target this new output chain.
353	356		sess.mu.Lock()
354	357		old := sess.client
355		-	sess.client = ac
356	358		name := sess.name
357	359		sess.mu.Unlock()
358	360		d.attachCoordinator(sess, old, ac)
	361	+	sess.mu.Lock()
	362	+	sess.client = ac
	363	+	sess.mu.Unlock()
359	364		d.touchMRU(sess)
360	365		d.log.Info("client attached", "session", name, "resume", opts.resumeCapable)
361	366		d.applyHostTheme(sess, ac, d.effectiveTheme(themeui.Theme{}), true)
362	367		return ac, old
363	368		}
364	369
365		-	// handoffCoordinator completes a cross-session ownership transfer before the
366		-	// destination's first paint. It invalidates source callbacks, rebases the
367		-	// attachment's output dependency chain, then installs the destination owner.
	370	+	// handoffCoordinator prepares a cross-session ownership transfer before the
	371	+	// destination publishes sess.client. It never takes daemon/session locks while
	372	+	// taking sendMu, avoiding a d.mu/sendMu cycle with transport callbacks.
368	373		func (d Daemon) handoffCoordinator(from, target session, old, current *attachedClient) {
369	374		if rc := from.renderCoordinator(); rc != nil {
370	375		rc.noteDetach(current)

 	ac.initOverlays()
 	ac.setSession(sess)
 	ac.keys = keys.NewRouter(d.clock, daemonKeyHandler{d: d, ac: ac}, &d.bindings)
+	// Daemon attachment callers hold d.mu, serialising this preparation with
+	// other publications. Bind coordinator ownership before sess.client becomes
+	// visible: an old deadline can then never target this new output chain.
 	sess.mu.Lock()
 	old := sess.client
-	sess.client = ac
 	name := sess.name
 	sess.mu.Unlock()
 	d.attachCoordinator(sess, old, ac)
+	sess.mu.Lock()
+	sess.client = ac
+	sess.mu.Unlock()
 	d.touchMRU(sess)
 	d.log.Info("client attached", "session", name, "resume", opts.resumeCapable)
 	d.applyHostTheme(sess, ac, d.effectiveTheme(themeui.Theme{}), true)
 	return ac, old
 }
-// handoffCoordinator completes a cross-session ownership transfer before the
+// handoffCoordinator prepares a cross-session ownership transfer before the
-// destination's first paint. It invalidates source callbacks, rebases the
+// destination publishes sess.client. It never takes daemon/session locks while
-// attachment's output dependency chain, then installs the destination owner.
+// taking sendMu, avoiding a d.mu/sendMu cycle with transport callbacks.
 func (d *Daemon) handoffCoordinator(from, target *session, old, current *attachedClient) {
 	if rc := from.renderCoordinator(); rc != nil {
 		rc.noteDetach(current)

M internal/usecase/daemon/picker.go +10 -6


@@ -396,17 +396,21 @@ func (d Daemon) stealClientForTarget(from session, ac *attachedClient, targetS
396	396		targetSess.mu.Lock()
397	397		old := targetSess.client
398	398		targetSess.terminal = term
399		-	targetSess.client = ac
400	399		if target.TabIndex >= 0 && target.TabIndex < len(targetSess.tabs) {
401	400		targetSess.active = target.TabIndex
402	401		}
403	402		targetSess.mu.Unlock()
404	403
405		-	d.touchMRU(targetSess)
	404	+	// Do not expose targetSess.client until the target coordinator has claimed
	405	+	// this attachment and the output dependency chain has been rebased.
	406	+	d.handoffCoordinator(from, targetSess, old, ac)
406	407		ac.setSession(targetSess)
	408	+	targetSess.mu.Lock()
	409	+	targetSess.client = ac
	410	+	targetSess.mu.Unlock()
	411	+	d.touchMRU(targetSess)
407	412		ac.recordPreviousSession(from)
408	413		d.mu.Unlock()
409		-	d.handoffCoordinator(from, targetSess, old, ac)
410	414		if old != nil && old != ac {
411	415		// Displacement is a detach lifecycle transition too. Cancel the
412	416		// attachment-owned PR #71 timer only after releasing daemon/session

@@ -443,15 +447,15 @@ func (d Daemon) resumeStoppedAndSwitch(from session, ac *attachedClient, targe
443	447		from.client = nil
444	448		ac.setSession(nil)
445	449		from.mu.Unlock()
	450	+
	451	+	d.handoffCoordinator(from, targetSess, nil, ac)
	452	+	ac.setSession(targetSess)
446	453		targetSess.mu.Lock()
447	454		targetSess.client = ac
448	455		targetSess.mu.Unlock()
449		-
450	456		d.touchMRU(targetSess)
451		-	ac.setSession(targetSess)
452	457		ac.recordPreviousSession(from)
453	458		d.mu.Unlock()
454		-	d.handoffCoordinator(from, targetSess, nil, ac)
455	459		d.firstPaint(targetSess, ac, ac.size)
456	460		return true
457	461		}

 	targetSess.mu.Lock()
 	old := targetSess.client
 	targetSess.terminal = term
-	targetSess.client = ac
 	if target.TabIndex >= 0 && target.TabIndex < len(targetSess.tabs) {
 		targetSess.active = target.TabIndex
 	}
 	targetSess.mu.Unlock()
-	d.touchMRU(targetSess)
+	// Do not expose targetSess.client until the target coordinator has claimed
+	// this attachment and the output dependency chain has been rebased.
+	d.handoffCoordinator(from, targetSess, old, ac)
 	ac.setSession(targetSess)
+	targetSess.mu.Lock()
+	targetSess.client = ac
+	targetSess.mu.Unlock()
+	d.touchMRU(targetSess)
 	ac.recordPreviousSession(from)
 	d.mu.Unlock()
-	d.handoffCoordinator(from, targetSess, old, ac)
 	if old != nil && old != ac {
 		// Displacement is a detach lifecycle transition too. Cancel the
 		// attachment-owned PR #71 timer only after releasing daemon/session
 	from.client = nil
 	ac.setSession(nil)
 	from.mu.Unlock()
+	d.handoffCoordinator(from, targetSess, nil, ac)
+	ac.setSession(targetSess)
 	targetSess.mu.Lock()
 	targetSess.client = ac
 	targetSess.mu.Unlock()
 	d.touchMRU(targetSess)
-	ac.setSession(targetSess)
 	ac.recordPreviousSession(from)
 	d.mu.Unlock()
-	d.handoffCoordinator(from, targetSess, nil, ac)
 	d.firstPaint(targetSess, ac, ac.size)
 	return true
 }

M internal/usecase/daemon/render_coordinator.go +16 -6


@@ -230,12 +230,22 @@ func stopTimers(timers []ports.Timer) {
230	230		}
231	231		}
232	232
233		-	// invalidate publishes one producer state transition. Consecutive
234		-	// invalidations coalesce under a single armed deadline; the latest state and
235		-	// the stickiest reset win.
	233	+	// invalidate publishes a coordinator-owned producer transition. Callers that
	234	+	// carry an attachment identity must use invalidateForAttachment instead.
236	235		func (c *renderCoordinator) invalidate(inv renderInvalidation) {
	236	+	c.invalidateForAttachment(nil, inv)
	237	+	}
	238	+
	239	+	// invalidateForAttachment publishes only while source is still the bound
	240	+	// attachment. This check shares c.mu with replacement, making a stale
	241	+	// attachment's timer callback unable to enqueue work for its replacement.
	242	+	// A nil source represents an internal pane/session invalidation.
	243	+	func (c renderCoordinator) invalidateForAttachment(source attachedClient, inv renderInvalidation) {
237	244		c.mu.Lock()
238		-	if c.torndown \|\| c.detached {
	245	+	// An unbound coordinator is the manual/headless harness state; there is no
	246	+	// published production attachment to protect there. Once an attachment has
	247	+	// been bound (or detached), identity must match exactly.
	248	+	if c.torndown \|\| c.detached \|\| (source != nil && c.attachment != nil && c.attachment != source) {
239	249		c.mu.Unlock()
240	250		return
241	251		}

@@ -646,7 +656,7 @@ func (c *renderCoordinator) fireCurrent(watchdog bool) {
646	656		func (d Daemon) invalidateRender(sess session, ac *attachedClient, reset bool, producer string) {
647	657		if sess != nil {
648	658		if rc := sess.renderCoordinator(); rc != nil {
649		-	rc.invalidate(renderInvalidation{class: invalidateUrgent, reset: reset, producer: producer})
	659	+	rc.invalidateForAttachment(ac, renderInvalidation{class: invalidateUrgent, reset: reset, producer: producer})
650	660		return
651	661		}
652	662		}

@@ -661,7 +671,7 @@ func (d Daemon) invalidateRender(sess session, ac *attachedClient, reset bool,
661	671		func (d Daemon) invalidateRenderNow(sess session, ac *attachedClient, reset bool, producer string) {
662	672		if sess != nil {
663	673		if rc := sess.renderCoordinator(); rc != nil {
664		-	rc.invalidate(renderInvalidation{class: invalidateUrgent, reset: reset, producer: producer})
	674	+	rc.invalidateForAttachment(ac, renderInvalidation{class: invalidateUrgent, reset: reset, producer: producer})
665	675		rc.fireCurrent(false)
666	676		return
667	677		}

 	}
 }
-// invalidate publishes one producer state transition. Consecutive
+// invalidate publishes a coordinator-owned producer transition. Callers that
-// invalidations coalesce under a single armed deadline; the latest state and
+// carry an attachment identity must use invalidateForAttachment instead.
-// the stickiest reset win.
 func (c *renderCoordinator) invalidate(inv renderInvalidation) {
+	c.invalidateForAttachment(nil, inv)
+}
+// invalidateForAttachment publishes only while source is still the bound
+// attachment. This check shares c.mu with replacement, making a stale
+// attachment's timer callback unable to enqueue work for its replacement.
+// A nil source represents an internal pane/session invalidation.
+func (c *renderCoordinator) invalidateForAttachment(source *attachedClient, inv renderInvalidation) {
 	c.mu.Lock()
-	if c.torndown || c.detached {
+	// An unbound coordinator is the manual/headless harness state; there is no
+	// published production attachment to protect there. Once an attachment has
+	// been bound (or detached), identity must match exactly.
+	if c.torndown || c.detached || (source != nil && c.attachment != nil && c.attachment != source) {
 		c.mu.Unlock()
 		return
 	}
 func (d *Daemon) invalidateRender(sess *session, ac *attachedClient, reset bool, producer string) {
 	if sess != nil {
 		if rc := sess.renderCoordinator(); rc != nil {
-			rc.invalidate(renderInvalidation{class: invalidateUrgent, reset: reset, producer: producer})
+			rc.invalidateForAttachment(ac, renderInvalidation{class: invalidateUrgent, reset: reset, producer: producer})
 			return
 		}
 	}
 func (d *Daemon) invalidateRenderNow(sess *session, ac *attachedClient, reset bool, producer string) {
 	if sess != nil {
 		if rc := sess.renderCoordinator(); rc != nil {
-			rc.invalidate(renderInvalidation{class: invalidateUrgent, reset: reset, producer: producer})
+			rc.invalidateForAttachment(ac, renderInvalidation{class: invalidateUrgent, reset: reset, producer: producer})
 			rc.fireCurrent(false)
 			return
 		}

M internal/usecase/daemon/session.go +4 -5


@@ -237,19 +237,18 @@ func (d Daemon) createSessionAndSwitch(from session, ac *attachedClient, name
237	237		ac.setSession(nil)
238	238		from.mu.Unlock()
239	239
	240	+	// Prepare source invalidation, output rebase, and destination coordinator
	241	+	// identity before publishing the destination attachment.
	242	+	d.handoffCoordinator(from, newSess, nil, ac)
	243	+	ac.setSession(newSess)
240	244		newSess.mu.Lock()
241	245		newSess.client = ac
242	246		newSess.mu.Unlock()
243	247
244	248		d.touchMRU(newSess)
245		-	ac.setSession(newSess)
246	249		ac.recordPreviousSession(from)
247	250		d.log.Info("client attached", "session", newSess.name, "resume", ac.resumeCapable)
248	251		d.mu.Unlock()
249		-
250		-	// Match every other cross-session path: source callbacks are stale and
251		-	// destination ownership is installed before its first full frame.
252		-	d.handoffCoordinator(from, newSess, nil, ac)
253	252		d.firstPaint(newSess, ac, sz)
254	253		return nil
255	254		}

 	ac.setSession(nil)
 	from.mu.Unlock()
+	// Prepare source invalidation, output rebase, and destination coordinator
+	// identity before publishing the destination attachment.
+	d.handoffCoordinator(from, newSess, nil, ac)
+	ac.setSession(newSess)
 	newSess.mu.Lock()
 	newSess.client = ac
 	newSess.mu.Unlock()
 	d.touchMRU(newSess)
-	ac.setSession(newSess)
 	ac.recordPreviousSession(from)
 	d.log.Info("client attached", "session", newSess.name, "resume", ac.resumeCapable)
 	d.mu.Unlock()
-	// Match every other cross-session path: source callbacks are stale and
-	// destination ownership is installed before its first full frame.
-	d.handoffCoordinator(from, newSess, nil, ac)
 	d.firstPaint(newSess, ac, sz)
 	return nil
 }