5f064ea fix(daemon): retain latest resize timer ownership · vev

M internal/usecase/daemon/pane.go +1 -1


@@ -19,7 +19,7 @@ type pane struct {
19	19		id layout.PaneID
20	20		stableID string
21	21		pty ports.PTY
22		-	mu sync.Mutex // guards screen, scrollback, syncGen, rect, and title
	22	+	mu sync.Mutex // guards screen, scrollback, syncGen, rect, resizeApplying, resizePending, PTY side effects, and title
23	23		resizeMu sync.Mutex // serializes PTY resizes without holding mu
24	24		screen *vt.Screen
25	25		scrollback *scopy.Scrollback


@@ -19,7 +19,7 @@ type pane struct {
19		id layout.PaneID	19		id layout.PaneID
20		stableID string	20		stableID string
21		pty ports.PTY	21		pty ports.PTY
22	-	mu sync.Mutex // guards screen, scrollback, syncGen, rect, and title	22	+	mu sync.Mutex // guards screen, scrollback, syncGen, rect, resizeApplying, resizePending, PTY side effects, and title
23		resizeMu sync.Mutex // serializes PTY resizes without holding mu	23		resizeMu sync.Mutex // serializes PTY resizes without holding mu
24		screen *vt.Screen	24		screen *vt.Screen
25		scrollback *scopy.Scrollback	25		scrollback *scopy.Scrollback

M internal/usecase/daemon/render_coordinator.go +13 -4


@@ -825,11 +825,8 @@ func (c renderCoordinator) scheduleResize(size domain.Size, source attachedCli
825	825		}
826	826		c.mu.Lock()
827	827		valid := !c.torndown && c.resizeGen == gen && c.resize.epoch == epoch && c.resize.source == source
828		-	c.resizeTimer = nil
829		-	if c.resizeGen == gen {
830		-	c.resizeCancel = nil
831		-	}
832	828		c.mu.Unlock()
	829	+	c.clearResizeTimer(gen)
833	830		if valid {
834	831		run(epoch)
835	832		}

@@ -837,6 +834,18 @@ func (c renderCoordinator) scheduleResize(size domain.Size, source attachedCli
837	834		return epoch
838	835		}
839	836
	837	+	// clearResizeTimer releases timer ownership only for the matching resize generation.
	838	+	// A stale callback must not clear a newer request's timer or cancellation channel.
	839	+	func (c *renderCoordinator) clearResizeTimer(gen uint64) {
	840	+	c.mu.Lock()
	841	+	defer c.mu.Unlock()
	842	+	if c.resizeGen != gen {
	843	+	return
	844	+	}
	845	+	c.resizeTimer = nil
	846	+	c.resizeCancel = nil
	847	+	}
	848	+
840	849		// resizeCurrent verifies that an apply/commit attempt still owns the newest
841	850		// request. commit also advances the separately observable committed epoch.
842	851		func (c renderCoordinator) resizeCurrent(epoch uint64, source attachedClient, commit bool) bool {

 		}
 		c.mu.Lock()
 		valid := !c.torndown && c.resizeGen == gen && c.resize.epoch == epoch && c.resize.source == source
-		c.resizeTimer = nil
-		if c.resizeGen == gen {
-			c.resizeCancel = nil
-		}
 		c.mu.Unlock()
+		c.clearResizeTimer(gen)
 		if valid {
 			run(epoch)
 		}
 	return epoch
 }
+// clearResizeTimer releases timer ownership only for the matching resize generation.
+// A stale callback must not clear a newer request's timer or cancellation channel.
+func (c *renderCoordinator) clearResizeTimer(gen uint64) {
+	c.mu.Lock()
+	defer c.mu.Unlock()
+	if c.resizeGen != gen {
+		return
+	}
+	c.resizeTimer = nil
+	c.resizeCancel = nil
+}
 // resizeCurrent verifies that an apply/commit attempt still owns the newest
 // request. commit also advances the separately observable committed epoch.
 func (c *renderCoordinator) resizeCurrent(epoch uint64, source *attachedClient, commit bool) bool {

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


@@ -1332,6 +1332,29 @@ func TestCoordinatorDeadlineCannotPaintPublishedReplacementBeforeOwnershipInstal
1332	1332		requireNoCoordinatorOutputFrame(t, ownerSends)
1333	1333		}
1334	1334
	1335	+	func TestRenderCoordinatorClearResizeTimerKeepsNewerTimer(t *testing.T) {
	1336	+	rc := newRenderCoordinator(renderCoordinatorOptions{})
	1337	+	newer := &portsmocks.MockTimer{}
	1338	+
	1339	+	rc.mu.Lock()
	1340	+	rc.resizeGen = 2
	1341	+	rc.resizeTimer = newer
	1342	+	rc.resizeCancel = make(chan struct{})
	1343	+	rc.mu.Unlock()
	1344	+
	1345	+	rc.clearResizeTimer(1)
	1346	+	rc.mu.Lock()
	1347	+	require.Same(t, newer, rc.resizeTimer)
	1348	+	require.NotNil(t, rc.resizeCancel)
	1349	+	rc.mu.Unlock()
	1350	+
	1351	+	rc.clearResizeTimer(2)
	1352	+	rc.mu.Lock()
	1353	+	require.Nil(t, rc.resizeTimer)
	1354	+	require.Nil(t, rc.resizeCancel)
	1355	+	rc.mu.Unlock()
	1356	+	}
	1357	+
1335	1358		func TestRenderCoordinatorInertTimerFiresSynchronouslyWithoutWorker(t *testing.T) {
1336	1359		clock := portsmocks.NewMockClock(t)
1337	1360		timer := portsmocks.NewMockTimer(t)


@@ -1332,6 +1332,29 @@ func TestCoordinatorDeadlineCannotPaintPublishedReplacementBeforeOwnershipInstal
1332	requireNoCoordinatorOutputFrame(t, ownerSends)	1332		requireNoCoordinatorOutputFrame(t, ownerSends)
1333	}	1333		}
1334		1334
		1335	+	func TestRenderCoordinatorClearResizeTimerKeepsNewerTimer(t *testing.T) {
		1336	+	rc := newRenderCoordinator(renderCoordinatorOptions{})
		1337	+	newer := &portsmocks.MockTimer{}
		1338	+
		1339	+	rc.mu.Lock()
		1340	+	rc.resizeGen = 2
		1341	+	rc.resizeTimer = newer
		1342	+	rc.resizeCancel = make(chan struct{})
		1343	+	rc.mu.Unlock()
		1344	+
		1345	+	rc.clearResizeTimer(1)
		1346	+	rc.mu.Lock()
		1347	+	require.Same(t, newer, rc.resizeTimer)
		1348	+	require.NotNil(t, rc.resizeCancel)
		1349	+	rc.mu.Unlock()
		1350	+
		1351	+	rc.clearResizeTimer(2)
		1352	+	rc.mu.Lock()
		1353	+	require.Nil(t, rc.resizeTimer)
		1354	+	require.Nil(t, rc.resizeCancel)
		1355	+	rc.mu.Unlock()
		1356	+	}
		1357	+
1335	func TestRenderCoordinatorInertTimerFiresSynchronouslyWithoutWorker(t *testing.T) {	1358		func TestRenderCoordinatorInertTimerFiresSynchronouslyWithoutWorker(t *testing.T) {
1336	clock := portsmocks.NewMockClock(t)	1359		clock := portsmocks.NewMockClock(t)
1337	timer := portsmocks.NewMockTimer(t)	1360		timer := portsmocks.NewMockTimer(t)

M internal/usecase/daemon/snapshot.go +1 -1


@@ -141,7 +141,7 @@ func (d *Daemon) restoreSession(ctx context.Context, snap snapcodec.Session) err
141	141		return fmt.Errorf("snapshot: generating pane identity: %w", err)
142	142		}
143	143		}
144		-	pty, err := d.ptys.Open(d.serveCtx, d.shell, d.shellArgs, d.childEnv(snap.Name, tabStableID, paneStableID, restoreTerm), paneSnap.Cwd, contentSize)
	144	+	pty, err := d.ptys.Open(ctx, d.shell, d.shellArgs, d.childEnv(snap.Name, tabStableID, paneStableID, restoreTerm), paneSnap.Cwd, contentSize)
145	145		if err != nil {
146	146		closeOpened()
147	147		return err


@@ -141,7 +141,7 @@ func (d *Daemon) restoreSession(ctx context.Context, snap snapcodec.Session) err
141		return fmt.Errorf("snapshot: generating pane identity: %w", err)	141		return fmt.Errorf("snapshot: generating pane identity: %w", err)
142		}	142		}
143		}	143		}
144	-	pty, err := d.ptys.Open(d.serveCtx, d.shell, d.shellArgs, d.childEnv(snap.Name, tabStableID, paneStableID, restoreTerm), paneSnap.Cwd, contentSize)	144	+	pty, err := d.ptys.Open(ctx, d.shell, d.shellArgs, d.childEnv(snap.Name, tabStableID, paneStableID, restoreTerm), paneSnap.Cwd, contentSize)
145		if err != nil {	145		if err != nil {
146		closeOpened()	146		closeOpened()
147		return err	147		return err

M internal/usecase/daemon/snapshot_restore_test.go +16 -2


@@ -251,6 +251,19 @@ func TestRestoreSnapshotsProcessCommandWriteFailureIsNonFatal(t *testing.T) {
251	251		require.Equal(t, []string{"pi --resume abc123\n"}, factory.opens[0].pty.writes)
252	252		}
253	253
	254	+	func TestRestoreSessionPassesRestoreContextToPTYOpen(t *testing.T) {
	255	+	factory := &restorePTYFactory{}
	256	+	d := newTestDaemon(t, factory, stubClock{})
	257	+	ctx, cancel := context.WithCancel(context.Background())
	258	+	defer cancel()
	259	+
	260	+	require.NoError(t, d.restoreSession(ctx, snapcodec.Session{Name: "restore-context", Tabs: []snapcodec.Tab{{
	261	+	Cols: 80, Rows: 24, Tree: layout.NewTree("pane-1"), Panes: []snapcodec.Pane{{ID: "pane-1", Cwd: "/tmp"}},
	262	+	}}}))
	263	+	require.Len(t, factory.opens, 1)
	264	+	require.Same(t, ctx, factory.opens[0].ctx)
	265	+	}
	266	+
254	267		func TestRestoreSnapshotsOpensCollapsedStackPanesWithValidPTYSize(t *testing.T) {
255	268		store := &restoreSnapshotStore{blobs: []ports.SnapshotBlob{{Name: "stacked", Data: mustSnapshotBytes(t, snapcodec.Session{
256	269		Name: "stacked",

@@ -438,18 +451,19 @@ type restorePTYFactory struct {
438	451		}
439	452
440	453		type restorePTYOpen struct {
	454	+	ctx context.Context
441	455		dir string
442	456		size domain.Size
443	457		env []string
444	458		pty *restorePTY
445	459		}
446	460
447		-	func (f *restorePTYFactory) Open(_ context.Context, _ string, _ []string, env []string, dir string, sz domain.Size) (ports.PTY, error) {
	461	+	func (f *restorePTYFactory) Open(ctx context.Context, _ string, _ []string, env []string, dir string, sz domain.Size) (ports.PTY, error) {
448	462		f.mu.Lock()
449	463		defer f.mu.Unlock()
450	464		pty := newRestorePTY()
451	465		pty.writeErr = f.writeErr
452		-	f.opens = append(f.opens, restorePTYOpen{dir: dir, size: sz, env: append([]string(nil), env...), pty: pty})
	466	+	f.opens = append(f.opens, restorePTYOpen{ctx: ctx, dir: dir, size: sz, env: append([]string(nil), env...), pty: pty})
453	467		return pty, nil
454	468		}
455	469

 	require.Equal(t, []string{"pi --resume abc123\n"}, factory.opens[0].pty.writes)
 }
+func TestRestoreSessionPassesRestoreContextToPTYOpen(t *testing.T) {
+	factory := &restorePTYFactory{}
+	d := newTestDaemon(t, factory, stubClock{})
+	ctx, cancel := context.WithCancel(context.Background())
+	defer cancel()
+	require.NoError(t, d.restoreSession(ctx, snapcodec.Session{Name: "restore-context", Tabs: []snapcodec.Tab{{
+		Cols: 80, Rows: 24, Tree: layout.NewTree("pane-1"), Panes: []snapcodec.Pane{{ID: "pane-1", Cwd: "/tmp"}},
+	}}}))
+	require.Len(t, factory.opens, 1)
+	require.Same(t, ctx, factory.opens[0].ctx)
+}
 func TestRestoreSnapshotsOpensCollapsedStackPanesWithValidPTYSize(t *testing.T) {
 	store := &restoreSnapshotStore{blobs: []ports.SnapshotBlob{{Name: "stacked", Data: mustSnapshotBytes(t, snapcodec.Session{
 		Name: "stacked",
 }
 type restorePTYOpen struct {
+	ctx  context.Context
 	dir  string
 	size domain.Size
 	env  []string
 	pty  *restorePTY
 }
-func (f *restorePTYFactory) Open(_ context.Context, _ string, _ []string, env []string, dir string, sz domain.Size) (ports.PTY, error) {
+func (f *restorePTYFactory) Open(ctx context.Context, _ string, _ []string, env []string, dir string, sz domain.Size) (ports.PTY, error) {
 	f.mu.Lock()
 	defer f.mu.Unlock()
 	pty := newRestorePTY()
 	pty.writeErr = f.writeErr
-	f.opens = append(f.opens, restorePTYOpen{dir: dir, size: sz, env: append([]string(nil), env...), pty: pty})
+	f.opens = append(f.opens, restorePTYOpen{ctx: ctx, dir: dir, size: sz, env: append([]string(nil), env...), pty: pty})
 	return pty, nil
 }