detached ac88d25

merge(stack): integrate render coordinator fixes

commit
ac88d250394452ba38d1afa1724e03e42a8ca1cd
author
brice · · 2026-07-12 05:34 UTC
merge of
f68fdb7 + 6bc1da2 · diff against the first parent
signature
GPG VerifiedCD88F9E564E0102646FB6077A1A07C7C7C3E349D · arch-gpg Verified for the committer email
10 files · +52 -43
Split
UnifiedSplit
M internal/usecase/daemon/client.go +4 -0
@@ -1,4 +1,8 @@
11 // Package daemon holds vev's server-side session multiplexer use case.
2+//
3+// Lock ordering: never acquire sendMu or session/daemon locks while holding a
4+// pane lock. Coordinator callbacks and timer methods run without coordinator.mu;
5+// see the lock-specific comments on attachedClient and renderCoordinator.
26 package daemon
37
48 import (
@@ -1,4 +1,8 @@
1 // Package daemon holds vev's server-side session multiplexer use case.1 // Package daemon holds vev's server-side session multiplexer use case.
2+//
3+// Lock ordering: never acquire sendMu or session/daemon locks while holding a
4+// pane lock. Coordinator callbacks and timer methods run without coordinator.mu;
5+// see the lock-specific comments on attachedClient and renderCoordinator.
2 package daemon6 package daemon
3 7
4 import (8 import (
M internal/usecase/daemon/copymode_test.go +13 -15
@@ -5,7 +5,6 @@ import (
55 "errors"
66 "fmt"
77 "io"
8- "runtime"
98 "strings"
109 "sync"
1110 "testing"
@@ -393,19 +392,19 @@ func TestScrollbackEvictionFeedsCopyModeYank(t *testing.T) {
393392 d := newTestDaemon(t, newFactory(t, p), clk.clock)
394393 tr, sends, releaseConn := newConn(t, mustHello(ports.IntentNew, "work", domain.Size{Cols: 16, Rows: 5}))
395394 advanceRender := func() {
396- for range 4096 {
397- select {
398- case timer := <-clk.timers:
399- timer.ch <- time.Time{}
400- return
401- default:
402- runtime.Gosched()
403- }
395+ deadline := time.NewTimer(2 * time.Second)
396+ defer deadline.Stop()
397+ select {
398+ case timer := <-clk.timers:
399+ timer.ch <- time.Time{}
400+ case <-deadline.C:
401+ t.Fatal("coordinator did not arm a controllable render timer")
404402 }
405- t.Fatal("coordinator did not arm a controllable render timer")
406403 }
407404 awaitControlledOutput := func() ports.Frame {
408- for range 4096 {
405+ deadline := time.NewTimer(2 * time.Second)
406+ defer deadline.Stop()
407+ for {
409408 select {
410409 case frame := <-sends:
411410 if frame.Type == ports.MsgOutput {
@@ -414,12 +413,11 @@ func TestScrollbackEvictionFeedsCopyModeYank(t *testing.T) {
414413 t.Fatalf("unexpected frame type %d while advancing render clock", frame.Type)
415414 case timer := <-clk.timers:
416415 timer.ch <- time.Time{}
417- default:
418- runtime.Gosched()
416+ case <-deadline.C:
417+ t.Fatal("controllable timers did not produce an output frame")
418+ return ports.Frame{}
419419 }
420420 }
421- t.Fatal("controllable timers did not produce an output frame")
422- return ports.Frame{}
423421 }
424422 var hg sync.WaitGroup
425423 hg.Go(func() { d.handleConn(tr) })
@@ -5,7 +5,6 @@ import (
5 "errors"5 "errors"
6 "fmt"6 "fmt"
7 "io"7 "io"
8- "runtime"
9 "strings"8 "strings"
10 "sync"9 "sync"
11 "testing"10 "testing"
@@ -393,19 +392,19 @@ func TestScrollbackEvictionFeedsCopyModeYank(t *testing.T) {
393 d := newTestDaemon(t, newFactory(t, p), clk.clock)392 d := newTestDaemon(t, newFactory(t, p), clk.clock)
394 tr, sends, releaseConn := newConn(t, mustHello(ports.IntentNew, "work", domain.Size{Cols: 16, Rows: 5}))393 tr, sends, releaseConn := newConn(t, mustHello(ports.IntentNew, "work", domain.Size{Cols: 16, Rows: 5}))
395 advanceRender := func() {394 advanceRender := func() {
396- for range 4096 {395+ deadline := time.NewTimer(2 * time.Second)
397- select {396+ defer deadline.Stop()
398- case timer := <-clk.timers:397+ select {
399- timer.ch <- time.Time{}398+ case timer := <-clk.timers:
400- return399+ timer.ch <- time.Time{}
401- default:400+ case <-deadline.C:
402- runtime.Gosched()401+ t.Fatal("coordinator did not arm a controllable render timer")
403- }
404 }402 }
405- t.Fatal("coordinator did not arm a controllable render timer")
406 }403 }
407 awaitControlledOutput := func() ports.Frame {404 awaitControlledOutput := func() ports.Frame {
408- for range 4096 {405+ deadline := time.NewTimer(2 * time.Second)
406+ defer deadline.Stop()
407+ for {
409 select {408 select {
410 case frame := <-sends:409 case frame := <-sends:
411 if frame.Type == ports.MsgOutput {410 if frame.Type == ports.MsgOutput {
@@ -414,12 +413,11 @@ func TestScrollbackEvictionFeedsCopyModeYank(t *testing.T) {
414 t.Fatalf("unexpected frame type %d while advancing render clock", frame.Type)413 t.Fatalf("unexpected frame type %d while advancing render clock", frame.Type)
415 case timer := <-clk.timers:414 case timer := <-clk.timers:
416 timer.ch <- time.Time{}415 timer.ch <- time.Time{}
417- default:416+ case <-deadline.C:
418- runtime.Gosched()417+ t.Fatal("controllable timers did not produce an output frame")
418+ return ports.Frame{}
419 }419 }
420 }420 }
421- t.Fatal("controllable timers did not produce an output frame")
422- return ports.Frame{}
423 }421 }
424 var hg sync.WaitGroup422 var hg sync.WaitGroup
425 hg.Go(func() { d.handleConn(tr) })423 hg.Go(func() { d.handleConn(tr) })
M internal/usecase/daemon/daemon_test.go +2 -11
@@ -460,18 +460,9 @@ func TestHandleHelloDefersFreshOutputUntilWelcome(t *testing.T) {
460460 // Exercise the real producer fan-in after publication. The deadline worker
461461 // has completed before asserting the gate, so this is not a timing race.
462462 d.invalidateRender(sess, ac, true, "welcome-gate-test")
463- timer := awaitCoordinatorScheduledTimer(t, clock)
464- // attach-time theme setup may have armed an older bulk timer. The fresh
463+ // Attach-time theme setup may have armed an older bulk timer. The fresh
465464 // urgent producer replaces it, so advance the newest published deadline.
466- for {
467- select {
468- case timer = <-clock.timers:
469- default:
470- goto currentDeadline
471- }
472- }
473-
474-currentDeadline:
465+ timer := awaitLatestCoordinatorTimer(t, clock)
475466 rc := sess.renderCoordinator()
476467 rc.mu.Lock()
477468 workerDone := rc.normalWorkerDone
@@ -460,18 +460,9 @@ func TestHandleHelloDefersFreshOutputUntilWelcome(t *testing.T) {
460 // Exercise the real producer fan-in after publication. The deadline worker460 // Exercise the real producer fan-in after publication. The deadline worker
461 // has completed before asserting the gate, so this is not a timing race.461 // has completed before asserting the gate, so this is not a timing race.
462 d.invalidateRender(sess, ac, true, "welcome-gate-test")462 d.invalidateRender(sess, ac, true, "welcome-gate-test")
463- timer := awaitCoordinatorScheduledTimer(t, clock)463+ // Attach-time theme setup may have armed an older bulk timer. The fresh
464- // attach-time theme setup may have armed an older bulk timer. The fresh
465 // urgent producer replaces it, so advance the newest published deadline.464 // urgent producer replaces it, so advance the newest published deadline.
466- for {465+ timer := awaitLatestCoordinatorTimer(t, clock)
467- select {
468- case timer = <-clock.timers:
469- default:
470- goto currentDeadline
471- }
472- }
473-
474-currentDeadline:
475 rc := sess.renderCoordinator()466 rc := sess.renderCoordinator()
476 rc.mu.Lock()467 rc.mu.Lock()
477 workerDone := rc.normalWorkerDone468 workerDone := rc.normalWorkerDone
M internal/usecase/daemon/performance_benchmark_test.go +2 -2
@@ -57,7 +57,7 @@ func TestPerformanceFixtureCounters(t *testing.T) {
5757 require.Equal(t, uint64(2), metrics.coordinatorInvalidations)
5858 require.Equal(t, uint64(2), metrics.coordinatorWakes)
5959 require.Equal(t, uint64(2), metrics.coordinatorCoalesced)
60- require.Positive(t, metrics.coordinatorInvalidations, "coordinator metric ratios require a nonzero denominator")
60+ require.Positive(t, metrics.coordinatorWakes, "coordinator metric ratios require a nonzero denominator")
6161
6262 fixture.captureSnapshot()
6363 metrics = fixture.metrics()
@@ -104,7 +104,7 @@ func TestPerformanceFixtureCoordinatorMetricsForCanonicalTopologies(t *testing.T
104104 require.Equal(t, uint64(1), metrics.coordinatorInvalidations)
105105 require.Equal(t, uint64(1), metrics.coordinatorWakes)
106106 require.Equal(t, uint64(1), metrics.coordinatorCoalesced)
107- require.Positive(t, metrics.coordinatorInvalidations, "coalescing ratio denominator")
107+ require.Positive(t, metrics.coordinatorWakes, "coalescing ratio denominator")
108108 })
109109 }
110110 }
@@ -57,7 +57,7 @@ func TestPerformanceFixtureCounters(t *testing.T) {
57 require.Equal(t, uint64(2), metrics.coordinatorInvalidations)57 require.Equal(t, uint64(2), metrics.coordinatorInvalidations)
58 require.Equal(t, uint64(2), metrics.coordinatorWakes)58 require.Equal(t, uint64(2), metrics.coordinatorWakes)
59 require.Equal(t, uint64(2), metrics.coordinatorCoalesced)59 require.Equal(t, uint64(2), metrics.coordinatorCoalesced)
60- require.Positive(t, metrics.coordinatorInvalidations, "coordinator metric ratios require a nonzero denominator")60+ require.Positive(t, metrics.coordinatorWakes, "coordinator metric ratios require a nonzero denominator")
61 61
62 fixture.captureSnapshot()62 fixture.captureSnapshot()
63 metrics = fixture.metrics()63 metrics = fixture.metrics()
@@ -104,7 +104,7 @@ func TestPerformanceFixtureCoordinatorMetricsForCanonicalTopologies(t *testing.T
104 require.Equal(t, uint64(1), metrics.coordinatorInvalidations)104 require.Equal(t, uint64(1), metrics.coordinatorInvalidations)
105 require.Equal(t, uint64(1), metrics.coordinatorWakes)105 require.Equal(t, uint64(1), metrics.coordinatorWakes)
106 require.Equal(t, uint64(1), metrics.coordinatorCoalesced)106 require.Equal(t, uint64(1), metrics.coordinatorCoalesced)
107- require.Positive(t, metrics.coordinatorInvalidations, "coalescing ratio denominator")107+ require.Positive(t, metrics.coordinatorWakes, "coalescing ratio denominator")
108 })108 })
109 }109 }
110 }110 }
M internal/usecase/daemon/render_coordinator.go +6 -0
@@ -616,8 +616,11 @@ func (c *renderCoordinator) noteDetach(ac *attachedClient) {
616616 c.attachmentReady = false
617617 c.detached = true
618618 c.pending = false
619+ c.pendingReset = false
620+ c.pendingUrgent = false
619621 c.ackDeferred = false
620622 c.pendingPreview = false
623+ c.coalesced = 0
621624 c.generation++
622625 c.armed = false
623626 timer = c.detachNormalTimerLocked()
@@ -648,8 +651,11 @@ func (c *renderCoordinator) noteReplace(old, replacement *attachedClient, readin
648651 }
649652 c.detached = false
650653 c.pending = false
654+ c.pendingReset = false
655+ c.pendingUrgent = false
651656 c.ackDeferred = false
652657 c.pendingPreview = false
658+ c.coalesced = 0
653659 c.generation++
654660 c.armed = false
655661 timer = c.detachNormalTimerLocked()
@@ -616,8 +616,11 @@ func (c *renderCoordinator) noteDetach(ac *attachedClient) {
616 c.attachmentReady = false616 c.attachmentReady = false
617 c.detached = true617 c.detached = true
618 c.pending = false618 c.pending = false
619+ c.pendingReset = false
620+ c.pendingUrgent = false
619 c.ackDeferred = false621 c.ackDeferred = false
620 c.pendingPreview = false622 c.pendingPreview = false
623+ c.coalesced = 0
621 c.generation++624 c.generation++
622 c.armed = false625 c.armed = false
623 timer = c.detachNormalTimerLocked()626 timer = c.detachNormalTimerLocked()
@@ -648,8 +651,11 @@ func (c *renderCoordinator) noteReplace(old, replacement *attachedClient, readin
648 }651 }
649 c.detached = false652 c.detached = false
650 c.pending = false653 c.pending = false
654+ c.pendingReset = false
655+ c.pendingUrgent = false
651 c.ackDeferred = false656 c.ackDeferred = false
652 c.pendingPreview = false657 c.pendingPreview = false
658+ c.coalesced = 0
653 c.generation++659 c.generation++
654 c.armed = false660 c.armed = false
655 timer = c.detachNormalTimerLocked()661 timer = c.detachNormalTimerLocked()
M internal/usecase/daemon/render_coordinator_test.go +12 -0
@@ -209,6 +209,18 @@ func awaitCoordinatorScheduledTimer(t *testing.T, clk *coordinatorMockClock) *co
209209 }
210210 }
211211
212+func awaitLatestCoordinatorTimer(t *testing.T, clk *coordinatorMockClock) *coordinatorMockTimer {
213+ t.Helper()
214+ timer := awaitCoordinatorScheduledTimer(t, clk)
215+ for {
216+ select {
217+ case timer = <-clk.timers:
218+ default:
219+ return timer
220+ }
221+ }
222+}
223+
212224 func requireNoCoordinatorOutputFrame(t *testing.T, sends chan ports.Frame) {
213225 t.Helper()
214226 select {
@@ -209,6 +209,18 @@ func awaitCoordinatorScheduledTimer(t *testing.T, clk *coordinatorMockClock) *co
209 }209 }
210 }210 }
211 211
212+func awaitLatestCoordinatorTimer(t *testing.T, clk *coordinatorMockClock) *coordinatorMockTimer {
213+ t.Helper()
214+ timer := awaitCoordinatorScheduledTimer(t, clk)
215+ for {
216+ select {
217+ case timer = <-clk.timers:
218+ default:
219+ return timer
220+ }
221+ }
222+}
223+
212 func requireNoCoordinatorOutputFrame(t *testing.T, sends chan ports.Frame) {224 func requireNoCoordinatorOutputFrame(t *testing.T, sends chan ports.Frame) {
213 t.Helper()225 t.Helper()
214 select {226 select {
M internal/usecase/daemon/render_test.go +2 -2
@@ -288,7 +288,7 @@ func TestPTYReaderRepublishesSynchronizedCompletionAfterAttachmentLifecycle(t *t
288288 fireCoordinatorTimer(t, rc, drainCoordinatorTimers(clock), urgentRenderDeadline)
289289 wake := <-previews
290290 require.True(t, wake.urgent)
291- require.Equal(t, 2, wake.coalesced, "completion retains coordinator coalescing across the cleared attachment work")
291+ require.Equal(t, 1, wake.coalesced, "completion starts a fresh headless preview batch after detach")
292292 select {
293293 case duplicate := <-previews:
294294 t.Fatalf("sync completion must publish exactly one preview wake: %#v", duplicate)
@@ -331,7 +331,7 @@ func TestPTYReaderRepublishesSynchronizedCompletionAfterAttachmentLifecycle(t *t
331331 wake := <-wakes
332332 require.True(t, wake.urgent)
333333 require.True(t, wake.reset, "the replacement's cleared batch must repaint a complete frame")
334- require.Equal(t, 3, wake.coalesced, "completion must retain the replacement reset and coordinator coalescing")
334+ require.Equal(t, 2, wake.coalesced, "completion coalesces only with the replacement's fresh reset batch")
335335 require.Same(t, replacement, wake.attachment)
336336 select {
337337 case duplicate := <-wakes:
@@ -288,7 +288,7 @@ func TestPTYReaderRepublishesSynchronizedCompletionAfterAttachmentLifecycle(t *t
288 fireCoordinatorTimer(t, rc, drainCoordinatorTimers(clock), urgentRenderDeadline)288 fireCoordinatorTimer(t, rc, drainCoordinatorTimers(clock), urgentRenderDeadline)
289 wake := <-previews289 wake := <-previews
290 require.True(t, wake.urgent)290 require.True(t, wake.urgent)
291- require.Equal(t, 2, wake.coalesced, "completion retains coordinator coalescing across the cleared attachment work")291+ require.Equal(t, 1, wake.coalesced, "completion starts a fresh headless preview batch after detach")
292 select {292 select {
293 case duplicate := <-previews:293 case duplicate := <-previews:
294 t.Fatalf("sync completion must publish exactly one preview wake: %#v", duplicate)294 t.Fatalf("sync completion must publish exactly one preview wake: %#v", duplicate)
@@ -331,7 +331,7 @@ func TestPTYReaderRepublishesSynchronizedCompletionAfterAttachmentLifecycle(t *t
331 wake := <-wakes331 wake := <-wakes
332 require.True(t, wake.urgent)332 require.True(t, wake.urgent)
333 require.True(t, wake.reset, "the replacement's cleared batch must repaint a complete frame")333 require.True(t, wake.reset, "the replacement's cleared batch must repaint a complete frame")
334- require.Equal(t, 3, wake.coalesced, "completion must retain the replacement reset and coordinator coalescing")334+ require.Equal(t, 2, wake.coalesced, "completion coalesces only with the replacement's fresh reset batch")
335 require.Same(t, replacement, wake.attachment)335 require.Same(t, replacement, wake.attachment)
336 select {336 select {
337 case duplicate := <-wakes:337 case duplicate := <-wakes:
M internal/usecase/daemon/resume.go +3 -3
@@ -48,9 +48,9 @@ func (d *Daemon) parkAttachment(sess *session, ac *attachedClient) bool {
4848 if !d.prepareParkAttachment(sess, ac) {
4949 return false
5050 }
51- // A parked attachment has no valid visible destination. Retire both the
52- // deferred diff and any resize-idle callback before registering its grace
53- // timer; resume performs a fresh full paint.
51+ // A parked attachment has no valid visible destination. Cancel its
52+ // resize-idle callback before registering the grace timer; resume performs
53+ // a fresh full paint.
5454 ac.cancelResizePaint()
5555 d.mu.Lock()
5656 if d.closing {
@@ -48,9 +48,9 @@ func (d *Daemon) parkAttachment(sess *session, ac *attachedClient) bool {
48 if !d.prepareParkAttachment(sess, ac) {48 if !d.prepareParkAttachment(sess, ac) {
49 return false49 return false
50 }50 }
51- // A parked attachment has no valid visible destination. Retire both the51+ // A parked attachment has no valid visible destination. Cancel its
52- // deferred diff and any resize-idle callback before registering its grace52+ // resize-idle callback before registering the grace timer; resume performs
53- // timer; resume performs a fresh full paint.53+ // a fresh full paint.
54 ac.cancelResizePaint()54 ac.cancelResizePaint()
55 d.mu.Lock()55 d.mu.Lock()
56 if d.closing {56 if d.closing {
M internal/usecase/daemon/resume_test.go +1 -10
@@ -105,16 +105,7 @@ func TestHandleHelloResumeDefersFreshOutputUntilWelcome(t *testing.T) {
105105 require.Same(t, ac, resumed)
106106
107107 d.invalidateRender(sess, resumed, true, "resume-welcome-gate-test")
108- timer := awaitCoordinatorScheduledTimer(t, clock)
109- for {
110- select {
111- case timer = <-clock.timers:
112- default:
113- goto currentDeadline
114- }
115- }
116-
117-currentDeadline:
108+ timer := awaitLatestCoordinatorTimer(t, clock)
118109 rc := sess.renderCoordinator()
119110 rc.mu.Lock()
120111 workerDone := rc.normalWorkerDone
@@ -105,16 +105,7 @@ func TestHandleHelloResumeDefersFreshOutputUntilWelcome(t *testing.T) {
105 require.Same(t, ac, resumed)105 require.Same(t, ac, resumed)
106 106
107 d.invalidateRender(sess, resumed, true, "resume-welcome-gate-test")107 d.invalidateRender(sess, resumed, true, "resume-welcome-gate-test")
108- timer := awaitCoordinatorScheduledTimer(t, clock)108+ timer := awaitLatestCoordinatorTimer(t, clock)
109- for {
110- select {
111- case timer = <-clock.timers:
112- default:
113- goto currentDeadline
114- }
115- }
116-
117-currentDeadline:
118 rc := sess.renderCoordinator()109 rc := sess.renderCoordinator()
119 rc.mu.Lock()110 rc.mu.Lock()
120 workerDone := rc.normalWorkerDone111 workerDone := rc.normalWorkerDone
M internal/usecase/daemon/status_test.go +7 -0
@@ -1335,12 +1335,19 @@ func TestStatusCoalescesCreateSwitchAndResize(t *testing.T) {
13351335 // latest-state wake. Advance every retained PR #71/coordinator deadline
13361336 // until that coalesced output is observable.
13371337 var resized ports.Frame
1338+ deadline := time.NewTimer(2 * time.Second)
1339+ defer deadline.Stop()
13381340 for resized.Type != ports.MsgOutput {
13391341 select {
13401342 case f := <-sends:
1343+ if f.Type != ports.MsgOutput {
1344+ t.Fatalf("unexpected frame type %d while awaiting resized output", f.Type)
1345+ }
13411346 resized = f
13421347 case timer := <-clock.timers:
13431348 timer.ch <- time.Time{}
1349+ case <-deadline.C:
1350+ t.Fatal("timed out awaiting resized output frame")
13441351 }
13451352 }
13461353
@@ -1335,12 +1335,19 @@ func TestStatusCoalescesCreateSwitchAndResize(t *testing.T) {
1335 // latest-state wake. Advance every retained PR #71/coordinator deadline1335 // latest-state wake. Advance every retained PR #71/coordinator deadline
1336 // until that coalesced output is observable.1336 // until that coalesced output is observable.
1337 var resized ports.Frame1337 var resized ports.Frame
1338+ deadline := time.NewTimer(2 * time.Second)
1339+ defer deadline.Stop()
1338 for resized.Type != ports.MsgOutput {1340 for resized.Type != ports.MsgOutput {
1339 select {1341 select {
1340 case f := <-sends:1342 case f := <-sends:
1343+ if f.Type != ports.MsgOutput {
1344+ t.Fatalf("unexpected frame type %d while awaiting resized output", f.Type)
1345+ }
1341 resized = f1346 resized = f
1342 case timer := <-clock.timers:1347 case timer := <-clock.timers:
1343 timer.ch <- time.Time{}1348 timer.ch <- time.Time{}
1349+ case <-deadline.C:
1350+ t.Fatal("timed out awaiting resized output frame")
1344 }1351 }
1345 }1352 }
1346 1353

Keyboard shortcuts

Single-key shortcuts are on. ctrl k and esc work whatever this says.

ctrl k :
go to a section, branch, tag, file or repository
?
this list
j k
move the row cursor down / up
enter
open the row under the cursor; on Files, a directory opens or closes in place
↑ ↓ ← → home end
in the file tree (focus on a row): previous / next row, close / open a directory or go to its parent / first entry, first / last row. They never open a file
l h
on Files: open a directory or a file / close a directory or go up to its parent, then to the parent page. Elsewhere: into the side panel (on a merge request, its files) / back to the content; there j k move between its controls
g g G
first / last row
/
filter the list; on Files, find a file anywhere in the repository (esc clears, enter opens the best match)
b
switch branch or tag
g s g f g c g b g m g i g r g a
tabs: summary, files, commits, branches & tags, merge requests, issues, releases, actions (when available)
g h
home
w
wide mode: use the whole width of the screen, or not (remembered)
y
copy a permalink pinned to the commit
d u x
diff: split / unified / collapse or expand all
e E
diff: the whole file in view / every file, with the lines the hunks hide
n
merge request: next note in the diff
esc
close a dialog, leave a field

Branches 12

Tags 11

↑↓ moveenter openesc close