7c216c5 fix(daemon): validate coordinator wake incarnation · vev

M internal/usecase/daemon/client.go +18 -14


@@ -15,14 +15,18 @@ import (
15	15		)
16	16
17	17		type attachedClient struct {
18		-	tr ports.Transport
19		-	output *outputStateStream
20		-	overlays *overlayRuntime
21		-	overlayOnce sync.Once
22		-	clientID [16]byte
23		-	resumeCapable bool
24		-	resumeToken uint64
25		-	parked bool
	18	+	tr ports.Transport
	19	+	output *outputStateStream
	20	+	overlays *overlayRuntime
	21	+	overlayOnce sync.Once
	22	+	clientID [16]byte
	23	+	resumeCapable bool
	24	+	resumeToken uint64
	25	+	parked bool
	26	+	// coordinatorEpoch is published atomically by renderCoordinator lifecycle
	27	+	// transitions. A wake verifies it again after acquiring sendMu, so a parked
	28	+	// attachment reused by resume cannot carry an old wake to its new transport.
	29	+	coordinatorEpoch atomic.Uint64
26	30		echoAck atomic.Uint64
27	31		bars barCache // only touched while sendMu is held
28	32		composed composedFrameCache // only touched while sendMu is held

@@ -390,12 +394,12 @@ func (d Daemon) attachCoordinator(sess session, old, current *attachedClient)
390	394		rc = newRenderCoordinator(renderCoordinatorOptions{
391	395		clock: d.clock,
392	396		wake: func(w renderWake) {
393		-	// w.attachment was validated and captured under the coordinator
394		-	// lock. Never reread sess.client here: attach publishes that field
395		-	// before it replaces the coordinator, so doing so can paint the
396		-	// new connection before Welcome.
397		-	if w.attachment != nil {
398		-	d.paint(sess, w.attachment, w.reset)
	397	+	// Never reread sess.client here: a wake is bound to both its
	398	+	// attachment and its coordinator incarnation. The second check in
	399	+	// paint occurs under sendMu, closing a park/resume race after this
	400	+	// unlocked coordinator validation.
	401	+	if w.attachment != nil && rc.wakeCurrent(w) {
	402	+	d.paintCoordinatorWake(sess, w)
399	403		}
400	404		},
401	405		ackReady: func() bool {


@@ -15,14 +15,18 @@ import (
15		)	15		)
16			16
17		type attachedClient struct {	17		type attachedClient struct {
18	-	tr ports.Transport	18	+	tr ports.Transport
19	-	output *outputStateStream	19	+	output *outputStateStream
20	-	overlays *overlayRuntime	20	+	overlays *overlayRuntime
21	-	overlayOnce sync.Once	21	+	overlayOnce sync.Once
22	-	clientID [16]byte	22	+	clientID [16]byte
23	-	resumeCapable bool	23	+	resumeCapable bool
24	-	resumeToken uint64	24	+	resumeToken uint64
25	-	parked bool	25	+	parked bool
			26	+	// coordinatorEpoch is published atomically by renderCoordinator lifecycle
			27	+	// transitions. A wake verifies it again after acquiring sendMu, so a parked
			28	+	// attachment reused by resume cannot carry an old wake to its new transport.
			29	+	coordinatorEpoch atomic.Uint64
26		echoAck atomic.Uint64	30		echoAck atomic.Uint64
27		bars barCache // only touched while sendMu is held	31		bars barCache // only touched while sendMu is held
28		composed composedFrameCache // only touched while sendMu is held	32		composed composedFrameCache // only touched while sendMu is held

@@ -390,12 +394,12 @@ func (d Daemon) attachCoordinator(sess session, old, current *attachedClient)
390		rc = newRenderCoordinator(renderCoordinatorOptions{	394		rc = newRenderCoordinator(renderCoordinatorOptions{
391		clock: d.clock,	395		clock: d.clock,
392		wake: func(w renderWake) {	396		wake: func(w renderWake) {
393	-	// w.attachment was validated and captured under the coordinator	397	+	// Never reread sess.client here: a wake is bound to both its
394	-	// lock. Never reread sess.client here: attach publishes that field	398	+	// attachment and its coordinator incarnation. The second check in
395	-	// before it replaces the coordinator, so doing so can paint the	399	+	// paint occurs under sendMu, closing a park/resume race after this
396	-	// new connection before Welcome.	400	+	// unlocked coordinator validation.
397	-	if w.attachment != nil {	401	+	if w.attachment != nil && rc.wakeCurrent(w) {
398	-	d.paint(sess, w.attachment, w.reset)	402	+	d.paintCoordinatorWake(sess, w)
399		}	403		}
400		},	404		},
401		ackReady: func() bool {	405		ackReady: func() bool {

M internal/usecase/daemon/render.go +9 -3


@@ -278,10 +278,16 @@ func (d Daemon) invalidateForResizeGeneration(sess session, ac *attachedClient
278	278		}
279	279
280	280		func (d Daemon) paint(sess session, ac *attachedClient, reset bool) {
281		-	d.paintForResizeGeneration(sess, ac, reset, 0)
	281	+	d.paintForResizeGeneration(sess, ac, reset, 0, 0)
282	282		}
283	283
284		-	func (d Daemon) paintForResizeGeneration(sess session, ac *attachedClient, reset bool, resizeGeneration uint64) {
	284	+	// paintCoordinatorWake composes a coordinator wake only for its captured
	285	+	// attachment incarnation. The epoch is checked after sendMu is acquired.
	286	+	func (d Daemon) paintCoordinatorWake(sess session, w renderWake) {
	287	+	d.paintForResizeGeneration(sess, w.attachment, w.reset, 0, w.attachmentEpoch)
	288	+	}
	289	+
	290	+	func (d Daemon) paintForResizeGeneration(sess session, ac *attachedClient, reset bool, resizeGeneration, attachmentEpoch uint64) {
285	291		tb := sess.activeTab()
286	292		if tb == nil {
287	293		return

@@ -294,7 +300,7 @@ func (d Daemon) paintForResizeGeneration(sess session, ac *attachedClient, res
294	300		sess.mu.Lock()
295	301		owned := sess.client == ac
296	302		sess.mu.Unlock()
297		-	if !owned \|\| ac.currentSession() != sess {
	303	+	if !owned \|\| ac.currentSession() != sess \|\| (attachmentEpoch != 0 && ac.coordinatorEpoch.Load() != attachmentEpoch) {
298	304		ac.sendMu.Unlock()
299	305		return
300	306		}

 }
 func (d *Daemon) paint(sess *session, ac *attachedClient, reset bool) {
-	d.paintForResizeGeneration(sess, ac, reset, 0)
+	d.paintForResizeGeneration(sess, ac, reset, 0, 0)
 }
-func (d *Daemon) paintForResizeGeneration(sess *session, ac *attachedClient, reset bool, resizeGeneration uint64) {
+// paintCoordinatorWake composes a coordinator wake only for its captured
+// attachment incarnation. The epoch is checked after sendMu is acquired.
+func (d *Daemon) paintCoordinatorWake(sess *session, w renderWake) {
+	d.paintForResizeGeneration(sess, w.attachment, w.reset, 0, w.attachmentEpoch)
+}
+func (d *Daemon) paintForResizeGeneration(sess *session, ac *attachedClient, reset bool, resizeGeneration, attachmentEpoch uint64) {
 	tb := sess.activeTab()
 	if tb == nil {
 		return
 	sess.mu.Lock()
 	owned := sess.client == ac
 	sess.mu.Unlock()
-	if !owned || ac.currentSession() != sess {
+	if !owned || ac.currentSession() != sess || (attachmentEpoch != 0 && ac.coordinatorEpoch.Load() != attachmentEpoch) {
 		ac.sendMu.Unlock()
 		return
 	}

M internal/usecase/daemon/render_coordinator.go +36 -1


@@ -60,6 +60,9 @@ type renderWake struct {
60	60		attachment *attachedClient
61	61		// watchdog marks a flush forced by the synchronized-output watchdog.
62	62		watchdog bool
	63	+	// attachmentEpoch is captured with attachment and must still be current
	64	+	// before this wake may compose or send.
	65	+	attachmentEpoch uint64
63	66		}
64	67
65	68		// resizeRequestMetadata is the coordinator-owned latest requested resize

@@ -145,6 +148,9 @@ type renderCoordinator struct {
145	148		// attachment is the currently bound client identity; callbacks from any
146	149		// other identity are stale and must not mutate coordinator state.
147	150		attachment *attachedClient
	151	+	// attachmentEpoch distinguishes consecutive bindings of the same client
	152	+	// object (notably park/resume, which replaces its transport in place).
	153	+	attachmentEpoch uint64
148	154		// detached rejects invalidations after detach/park until a new attachment.
149	155		detached bool
150	156		torndown bool

@@ -537,16 +543,38 @@ func (c renderCoordinator) teardownPreviewFor(viewer attachedClient) {
537	543		c.mu.Unlock()
538	544		}
539	545
	546	+	// advanceAttachmentEpochLocked invalidates every previously dispatched wake.
	547	+	// It mirrors the epoch onto the client so paint can recheck ownership after it
	548	+	// acquires sendMu without taking c.mu and creating a lock cycle.
	549	+	func (c renderCoordinator) advanceAttachmentEpochLocked(clients ...attachedClient) {
	550	+	c.attachmentEpoch++
	551	+	for _, ac := range clients {
	552	+	if ac != nil {
	553	+	ac.coordinatorEpoch.Store(c.attachmentEpoch)
	554	+	}
	555	+	}
	556	+	}
	557	+
540	558		// attach binds ac as the coordinator's current attachment identity.
541	559		func (c renderCoordinator) attach(ac attachedClient) {
542	560		c.mu.Lock()
543	561		if !c.torndown {
	562	+	c.advanceAttachmentEpochLocked(ac)
544	563		c.attachment = ac
545	564		c.detached = false
546	565		}
547	566		c.mu.Unlock()
548	567		}
549	568
	569	+	// wakeCurrent validates a dispatched wake without entering sendMu. The paint
	570	+	// path repeats the epoch check after taking sendMu, closing the remaining
	571	+	// lifecycle handoff window without holding c.mu across composition.
	572	+	func (c *renderCoordinator) wakeCurrent(w renderWake) bool {
	573	+	c.mu.Lock()
	574	+	defer c.mu.Unlock()
	575	+	return !c.torndown && !c.detached && c.attachment == w.attachment && c.attachmentEpoch == w.attachmentEpoch
	576	+	}
	577	+
550	578		// noteDetach invalidates pending wakes and stale callbacks for a detaching
551	579		// attachment.
552	580		func (c renderCoordinator) noteDetach(ac attachedClient) {

@@ -554,6 +582,7 @@ func (c renderCoordinator) noteDetach(ac attachedClient) {
554	582		var timer ports.Timer
555	583		var timers []ports.Timer
556	584		if c.attachment == ac {
	585	+	c.advanceAttachmentEpochLocked(ac)
557	586		c.attachment = nil
558	587		c.detached = true
559	588		c.pending = false

@@ -580,6 +609,7 @@ func (c renderCoordinator) noteReplace(old, replacement attachedClient) {
580	609		var timer ports.Timer
581	610		var timers []ports.Timer
582	611		if c.attachment == old \|\| c.attachment == nil {
	612	+	c.advanceAttachmentEpochLocked(old, replacement)
583	613		c.attachment = replacement
584	614		c.detached = false
585	615		c.pending = false

@@ -602,6 +632,7 @@ func (c renderCoordinator) notePark(ac attachedClient) { c.noteDetach(ac) }
602	632		func (c *renderCoordinator) noteSessionTeardown() {
603	633		c.mu.Lock()
604	634		c.torndown = true
	635	+	c.advanceAttachmentEpochLocked(c.attachment)
605	636		c.attachment = nil
606	637		c.previewWake = nil
607	638		c.previewWakes = nil

@@ -721,7 +752,11 @@ func (c *renderCoordinator) fire(gen uint64, watchdog, deadline bool) {
721	752		c.mu.Unlock()
722	753		return
723	754		}
724		-	w := renderWake{reset: c.pendingReset, urgent: c.pendingUrgent, coalesced: c.coalesced, watchdog: watchdog, attachment: c.attachment}
	755	+	attachmentEpoch := uint64(0)
	756	+	if c.attachment != nil {
	757	+	attachmentEpoch = c.attachmentEpoch
	758	+	}
	759	+	w := renderWake{reset: c.pendingReset, urgent: c.pendingUrgent, coalesced: c.coalesced, watchdog: watchdog, attachment: c.attachment, attachmentEpoch: attachmentEpoch}
725	760		if !ready {
726	761		if deadline {
727	762		c.ackDeferred = true

 	attachment *attachedClient
 	// watchdog marks a flush forced by the synchronized-output watchdog.
 	watchdog bool
+	// attachmentEpoch is captured with attachment and must still be current
+	// before this wake may compose or send.
+	attachmentEpoch uint64
 }
 // resizeRequestMetadata is the coordinator-owned latest requested resize
 	// attachment is the currently bound client identity; callbacks from any
 	// other identity are stale and must not mutate coordinator state.
 	attachment *attachedClient
+	// attachmentEpoch distinguishes consecutive bindings of the same client
+	// object (notably park/resume, which replaces its transport in place).
+	attachmentEpoch uint64
 	// detached rejects invalidations after detach/park until a new attachment.
 	detached bool
 	torndown bool
 	c.mu.Unlock()
 }
+// advanceAttachmentEpochLocked invalidates every previously dispatched wake.
+// It mirrors the epoch onto the client so paint can recheck ownership after it
+// acquires sendMu without taking c.mu and creating a lock cycle.
+func (c *renderCoordinator) advanceAttachmentEpochLocked(clients ...*attachedClient) {
+	c.attachmentEpoch++
+	for _, ac := range clients {
+		if ac != nil {
+			ac.coordinatorEpoch.Store(c.attachmentEpoch)
+		}
+	}
+}
 // attach binds ac as the coordinator's current attachment identity.
 func (c *renderCoordinator) attach(ac *attachedClient) {
 	c.mu.Lock()
 	if !c.torndown {
+		c.advanceAttachmentEpochLocked(ac)
 		c.attachment = ac
 		c.detached = false
 	}
 	c.mu.Unlock()
 }
+// wakeCurrent validates a dispatched wake without entering sendMu. The paint
+// path repeats the epoch check after taking sendMu, closing the remaining
+// lifecycle handoff window without holding c.mu across composition.
+func (c *renderCoordinator) wakeCurrent(w renderWake) bool {
+	c.mu.Lock()
+	defer c.mu.Unlock()
+	return !c.torndown && !c.detached && c.attachment == w.attachment && c.attachmentEpoch == w.attachmentEpoch
+}
 // noteDetach invalidates pending wakes and stale callbacks for a detaching
 // attachment.
 func (c *renderCoordinator) noteDetach(ac *attachedClient) {
 	var timer ports.Timer
 	var timers []ports.Timer
 	if c.attachment == ac {
+		c.advanceAttachmentEpochLocked(ac)
 		c.attachment = nil
 		c.detached = true
 		c.pending = false
 	var timer ports.Timer
 	var timers []ports.Timer
 	if c.attachment == old || c.attachment == nil {
+		c.advanceAttachmentEpochLocked(old, replacement)
 		c.attachment = replacement
 		c.detached = false
 		c.pending = false
 func (c *renderCoordinator) noteSessionTeardown() {
 	c.mu.Lock()
 	c.torndown = true
+	c.advanceAttachmentEpochLocked(c.attachment)
 	c.attachment = nil
 	c.previewWake = nil
 	c.previewWakes = nil
 		c.mu.Unlock()
 		return
 	}
-	w := renderWake{reset: c.pendingReset, urgent: c.pendingUrgent, coalesced: c.coalesced, watchdog: watchdog, attachment: c.attachment}
+	attachmentEpoch := uint64(0)
+	if c.attachment != nil {
+		attachmentEpoch = c.attachmentEpoch
+	}
+	w := renderWake{reset: c.pendingReset, urgent: c.pendingUrgent, coalesced: c.coalesced, watchdog: watchdog, attachment: c.attachment, attachmentEpoch: attachmentEpoch}
 	if !ready {
 		if deadline {
 			c.ackDeferred = true

M internal/usecase/daemon/render_coordinator_test.go +3 -0


@@ -388,6 +388,7 @@ resize:
388	388		wake := awaitWake(t, wakes)
389	389		require.Same(t, owner, wake.attachment)
390	390		wake.attachment = nil
	391	+	wake.attachmentEpoch = 0
391	392		require.Equal(t, renderWake{coalesced: 1}, wake)
392	393		for range 4096 {
393	394		select {

@@ -776,6 +777,7 @@ func TestRenderCoordinatorPreviewWakesDoNotWaitForTargetAck(t *testing.T) {
776	777		require.NotNil(t, preview.attachment)
777	778		}
778	779		preview.attachment = nil
	780	+	preview.attachmentEpoch = 0
779	781		require.Equal(t, renderWake{coalesced: 1}, preview,
780	782		"the viewer preview must receive target output without target ACK capacity")
781	783		requireNoWake(t, h.wakes)

@@ -788,6 +790,7 @@ func TestRenderCoordinatorPreviewWakesDoNotWaitForTargetAck(t *testing.T) {
788	790		require.NotNil(t, wake.attachment)
789	791		}
790	792		wake.attachment = nil
	793	+	wake.attachmentEpoch = 0
791	794		require.Equal(t, renderWake{coalesced: 1}, wake,
792	795		"the target primary frame remains pending for its own ACK")
793	796		requireNoWake(t, h.previews)


@@ -388,6 +388,7 @@ resize:
388	wake := awaitWake(t, wakes)	388		wake := awaitWake(t, wakes)
389	require.Same(t, owner, wake.attachment)	389		require.Same(t, owner, wake.attachment)
390	wake.attachment = nil	390		wake.attachment = nil
		391	+	wake.attachmentEpoch = 0
391	require.Equal(t, renderWake{coalesced: 1}, wake)	392		require.Equal(t, renderWake{coalesced: 1}, wake)
392	for range 4096 {	393		for range 4096 {
393	select {	394		select {

@@ -776,6 +777,7 @@ func TestRenderCoordinatorPreviewWakesDoNotWaitForTargetAck(t *testing.T) {
776	require.NotNil(t, preview.attachment)	777		require.NotNil(t, preview.attachment)
777	}	778		}
778	preview.attachment = nil	779		preview.attachment = nil
		780	+	preview.attachmentEpoch = 0
779	require.Equal(t, renderWake{coalesced: 1}, preview,	781		require.Equal(t, renderWake{coalesced: 1}, preview,
780	"the viewer preview must receive target output without target ACK capacity")	782		"the viewer preview must receive target output without target ACK capacity")
781	requireNoWake(t, h.wakes)	783		requireNoWake(t, h.wakes)

@@ -788,6 +790,7 @@ func TestRenderCoordinatorPreviewWakesDoNotWaitForTargetAck(t *testing.T) {
788	require.NotNil(t, wake.attachment)	790		require.NotNil(t, wake.attachment)
789	}	791		}
790	wake.attachment = nil	792		wake.attachment = nil
		793	+	wake.attachmentEpoch = 0
791	require.Equal(t, renderWake{coalesced: 1}, wake,	794		require.Equal(t, renderWake{coalesced: 1}, wake,
792	"the target primary frame remains pending for its own ACK")	795		"the target primary frame remains pending for its own ACK")
793	requireNoWake(t, h.previews)	796		requireNoWake(t, h.previews)

M internal/usecase/daemon/resume_wake_incarnation_test.go +4 -0


@@ -53,6 +53,10 @@ func TestParkedResumeRejectsDispatchedWakeFromPriorAttachment(t *testing.T) {
53	53		close(releaseWake)
54	54		<-wakeDone
55	55		require.Empty(t, newTransport.Sends(), "a stale pre-resume wake must not emit Output before Welcome")
	56	+	ac.sendMu.Lock()
	57	+	staleWakeComposed := ac.composed.valid
	58	+	ac.sendMu.Unlock()
	59	+	require.False(t, staleWakeComposed, "a stale wake must not mutate the resumed attachment shadow")
56	60
57	61		// The new attachment incarnation still accepts its normal required paint.
58	62		rc.opts.wake = originalWake


@@ -53,6 +53,10 @@ func TestParkedResumeRejectsDispatchedWakeFromPriorAttachment(t *testing.T) {
53	close(releaseWake)	53		close(releaseWake)
54	<-wakeDone	54		<-wakeDone
55	require.Empty(t, newTransport.Sends(), "a stale pre-resume wake must not emit Output before Welcome")	55		require.Empty(t, newTransport.Sends(), "a stale pre-resume wake must not emit Output before Welcome")
		56	+	ac.sendMu.Lock()
		57	+	staleWakeComposed := ac.composed.valid
		58	+	ac.sendMu.Unlock()
		59	+	require.False(t, staleWakeComposed, "a stale wake must not mutate the resumed attachment shadow")
56		60
57	// The new attachment incarnation still accepts its normal required paint.	61		// The new attachment incarnation still accepts its normal required paint.
58	rc.opts.wake = originalWake	62		rc.opts.wake = originalWake