Merge pull request #86 from bnema/fix/render-timer-generation-deadlock
fix(render): eliminate timer worker self-joins
17 files · +480 -472
M internal/usecase/daemon/daemon_test.go +1 -1
| @@ -472,7 +472,7 @@ func TestHandleHelloDefersFreshOutputUntilWelcome(t *testing.T) { | |||
| 472 | 472 | timer := awaitLatestCoordinatorTimer(t, clock) | |
| 473 | 473 | rc := sess.renderCoordinator() | |
| 474 | 474 | rc.mu.Lock() | |
| 475 | - | workerDone := rc.normalWorkerDone | |
| 475 | + | workerDone := rc.normalLane.token.done | |
| 476 | 476 | rc.mu.Unlock() | |
| 477 | 477 | require.NotNil(t, workerDone) | |
| 478 | 478 | timer.ch <- time.Time{} | |
| @@ -472,7 +472,7 @@ func TestHandleHelloDefersFreshOutputUntilWelcome(t *testing.T) { | |||||
| 472 | timer := awaitLatestCoordinatorTimer(t, clock) | 472 | timer := awaitLatestCoordinatorTimer(t, clock) | ||
| 473 | rc := sess.renderCoordinator() | 473 | rc := sess.renderCoordinator() | ||
| 474 | rc.mu.Lock() | 474 | rc.mu.Lock() | ||
| 475 | - | workerDone := rc.normalWorkerDone | 475 | + | workerDone := rc.normalLane.token.done |
| 476 | rc.mu.Unlock() | 476 | rc.mu.Unlock() | ||
| 477 | require.NotNil(t, workerDone) | 477 | require.NotNil(t, workerDone) | ||
| 478 | timer.ch <- time.Time{} | 478 | timer.ch <- time.Time{} | ||
M internal/usecase/daemon/lease_callback_guard_test.go +10 -6
| @@ -14,14 +14,14 @@ func TestResizeAndRetryCallbacksRejectSameObjectLeaseReplacement(t *testing.T) { | |||
| 14 | 14 | for _, tc := range []struct { | |
| 15 | 15 | name string | |
| 16 | 16 | schedule func(*renderCoordinator, *attachedClient, *attachmentLease, func()) | |
| 17 | - | done func(*renderCoordinator) <-chan struct{} | |
| 17 | + | done func(*testing.T, *renderCoordinator) <-chan struct{} | |
| 18 | 18 | }{ | |
| 19 | 19 | { | |
| 20 | 20 | name: "resize", | |
| 21 | 21 | schedule: func(rc *renderCoordinator, ac *attachedClient, lease *attachmentLease, run func()) { | |
| 22 | 22 | rc.scheduleResizeForLease(domain.Size{Cols: 100, Rows: 30}, ac, lease, func(uint64) { run() }) | |
| 23 | 23 | }, | |
| 24 | - | done: func(rc *renderCoordinator) <-chan struct{} { return rc.resizeCallbackDone() }, | |
| 24 | + | done: captureResizeCallbackDone, | |
| 25 | 25 | }, | |
| 26 | 26 | { | |
| 27 | 27 | name: "retry", | |
| @@ -31,10 +31,14 @@ func TestResizeAndRetryCallbacksRejectSameObjectLeaseReplacement(t *testing.T) { | |||
| 31 | 31 | require.True(t, rc.resizeCurrentForLease(epoch, ac, lease, true)) | |
| 32 | 32 | rc.scheduleResizeRetryForLease(epoch, ac, lease, run) | |
| 33 | 33 | }, | |
| 34 | - | done: func(rc *renderCoordinator) <-chan struct{} { | |
| 34 | + | done: func(t *testing.T, rc *renderCoordinator) <-chan struct{} { | |
| 35 | + | t.Helper() | |
| 35 | 36 | rc.mu.Lock() | |
| 36 | 37 | defer rc.mu.Unlock() | |
| 37 | - | return rc.retryDone | |
| 38 | + | if rc.retryLane.token == nil { | |
| 39 | + | t.Fatal("coordinator did not publish a retry callback completion") | |
| 40 | + | } | |
| 41 | + | return rc.retryLane.token.done | |
| 38 | 42 | }, | |
| 39 | 43 | }, | |
| 40 | 44 | } { | |
| @@ -47,13 +51,13 @@ func TestResizeAndRetryCallbacksRejectSameObjectLeaseReplacement(t *testing.T) { | |||
| 47 | 51 | ran := false | |
| 48 | 52 | tc.schedule(rc, ac, lease, func() { ran = true }) | |
| 49 | 53 | timer := awaitCoordinatorScheduledTimer(t, clock) | |
| 50 | - | done := tc.done(rc) | |
| 54 | + | done := tc.done(t, rc) | |
| 51 | 55 | require.NotNil(t, done) | |
| 52 | 56 | ||
| 53 | 57 | rc.notePark(ac) | |
| 54 | 58 | rc.attach(ac) // resume the exact same object under a new incarnation | |
| 55 | 59 | timer.ch <- time.Time{} | |
| 56 | - | <-done | |
| 60 | + | awaitTestCompletion(t, done, "stale callback did not complete") | |
| 57 | 61 | for range 32 { | |
| 58 | 62 | runtime.Gosched() | |
| 59 | 63 | } | |
| @@ -14,14 +14,14 @@ func TestResizeAndRetryCallbacksRejectSameObjectLeaseReplacement(t *testing.T) { | |||||
| 14 | for _, tc := range []struct { | 14 | for _, tc := range []struct { | ||
| 15 | name string | 15 | name string | ||
| 16 | schedule func(*renderCoordinator, *attachedClient, *attachmentLease, func()) | 16 | schedule func(*renderCoordinator, *attachedClient, *attachmentLease, func()) | ||
| 17 | - | done func(*renderCoordinator) <-chan struct{} | 17 | + | done func(*testing.T, *renderCoordinator) <-chan struct{} |
| 18 | }{ | 18 | }{ | ||
| 19 | { | 19 | { | ||
| 20 | name: "resize", | 20 | name: "resize", | ||
| 21 | schedule: func(rc *renderCoordinator, ac *attachedClient, lease *attachmentLease, run func()) { | 21 | schedule: func(rc *renderCoordinator, ac *attachedClient, lease *attachmentLease, run func()) { | ||
| 22 | rc.scheduleResizeForLease(domain.Size{Cols: 100, Rows: 30}, ac, lease, func(uint64) { run() }) | 22 | rc.scheduleResizeForLease(domain.Size{Cols: 100, Rows: 30}, ac, lease, func(uint64) { run() }) | ||
| 23 | }, | 23 | }, | ||
| 24 | - | done: func(rc *renderCoordinator) <-chan struct{} { return rc.resizeCallbackDone() }, | 24 | + | done: captureResizeCallbackDone, |
| 25 | }, | 25 | }, | ||
| 26 | { | 26 | { | ||
| 27 | name: "retry", | 27 | name: "retry", | ||
| @@ -31,10 +31,14 @@ func TestResizeAndRetryCallbacksRejectSameObjectLeaseReplacement(t *testing.T) { | |||||
| 31 | require.True(t, rc.resizeCurrentForLease(epoch, ac, lease, true)) | 31 | require.True(t, rc.resizeCurrentForLease(epoch, ac, lease, true)) | ||
| 32 | rc.scheduleResizeRetryForLease(epoch, ac, lease, run) | 32 | rc.scheduleResizeRetryForLease(epoch, ac, lease, run) | ||
| 33 | }, | 33 | }, | ||
| 34 | - | done: func(rc *renderCoordinator) <-chan struct{} { | 34 | + | done: func(t *testing.T, rc *renderCoordinator) <-chan struct{} { |
| 35 | + | t.Helper() | |||
| 35 | rc.mu.Lock() | 36 | rc.mu.Lock() | ||
| 36 | defer rc.mu.Unlock() | 37 | defer rc.mu.Unlock() | ||
| 37 | - | return rc.retryDone | 38 | + | if rc.retryLane.token == nil { |
| 39 | + | t.Fatal("coordinator did not publish a retry callback completion") | |||
| 40 | + | } | |||
| 41 | + | return rc.retryLane.token.done | |||
| 38 | }, | 42 | }, | ||
| 39 | }, | 43 | }, | ||
| 40 | } { | 44 | } { | ||
| @@ -47,13 +51,13 @@ func TestResizeAndRetryCallbacksRejectSameObjectLeaseReplacement(t *testing.T) { | |||||
| 47 | ran := false | 51 | ran := false | ||
| 48 | tc.schedule(rc, ac, lease, func() { ran = true }) | 52 | tc.schedule(rc, ac, lease, func() { ran = true }) | ||
| 49 | timer := awaitCoordinatorScheduledTimer(t, clock) | 53 | timer := awaitCoordinatorScheduledTimer(t, clock) | ||
| 50 | - | done := tc.done(rc) | 54 | + | done := tc.done(t, rc) |
| 51 | require.NotNil(t, done) | 55 | require.NotNil(t, done) | ||
| 52 | 56 | ||||
| 53 | rc.notePark(ac) | 57 | rc.notePark(ac) | ||
| 54 | rc.attach(ac) // resume the exact same object under a new incarnation | 58 | rc.attach(ac) // resume the exact same object under a new incarnation | ||
| 55 | timer.ch <- time.Time{} | 59 | timer.ch <- time.Time{} | ||
| 56 | - | <-done | 60 | + | awaitTestCompletion(t, done, "stale callback did not complete") |
| 57 | for range 32 { | 61 | for range 32 { | ||
| 58 | runtime.Gosched() | 62 | runtime.Gosched() | ||
| 59 | } | 63 | } | ||
M internal/usecase/daemon/output_state_sync_test.go +8 -1
| @@ -308,8 +308,9 @@ func TestResizeGrowthFirstFrameIncludesConcurrentPTYRedraw(t *testing.T) { | |||
| 308 | 308 | d.clock = clock | |
| 309 | 309 | d.resize(sess, ac, domain.Size{Cols: 120, Rows: 24}) | |
| 310 | 310 | timer := <-clock.timers | |
| 311 | + | done := captureResizeCallbackDone(t, sess.renderCoordinator()) | |
| 311 | 312 | timer.ch <- time.Time{} | |
| 312 | - | awaitResizeCallback(t, sess.renderCoordinator()) | |
| 313 | + | awaitTestCompletion(t, done, "resize callback did not complete") | |
| 313 | 314 | require.True(t, p.deliveredWhileApplying(), "redraw must arrive while PTY.Resize owns the apply gate") | |
| 314 | 315 | require.Equal(t, 'B', pane.screen.Frame.At(100, 0).Rune, "replay must parse the redraw before commit") | |
| 315 | 316 | ||
| @@ -342,7 +343,9 @@ func TestResizeWithoutPTYOutputFlushesOneFullFrameAtDeadline(t *testing.T) { | |||
| 342 | 343 | } | |
| 343 | 344 | requireNoOutputFrame(t, sends) | |
| 344 | 345 | ||
| 346 | + | done := captureResizeCallbackDone(t, sess.renderCoordinator()) | |
| 345 | 347 | timer.ch <- time.Now() | |
| 348 | + | awaitTestCompletion(t, done, "resize callback did not complete") | |
| 346 | 349 | frame := awaitFrame(t, sends, ports.MsgOutput) | |
| 347 | 350 | client := vt.NewScreen(120, 24) | |
| 348 | 351 | out := mustApplyOutput(t, client, frame) | |
| @@ -367,6 +370,7 @@ func TestResizeBurstFlushesOnlyLatestGeometry(t *testing.T) { | |||
| 367 | 370 | case <-time.After(time.Second): | |
| 368 | 371 | t.Fatal("first resize did not schedule a bounded paint") | |
| 369 | 372 | } | |
| 373 | + | firstDone := captureResizeCallbackDone(t, sess.renderCoordinator()) | |
| 370 | 374 | d.resize(sess, ac, domain.Size{Cols: 120, Rows: 24}) | |
| 371 | 375 | var latest *signalTimer | |
| 372 | 376 | select { | |
| @@ -375,9 +379,12 @@ func TestResizeBurstFlushesOnlyLatestGeometry(t *testing.T) { | |||
| 375 | 379 | t.Fatal("latest resize did not replace the bounded paint") | |
| 376 | 380 | } | |
| 377 | 381 | ||
| 382 | + | latestDone := captureResizeCallbackDone(t, sess.renderCoordinator()) | |
| 378 | 383 | first.ch <- time.Now() | |
| 384 | + | awaitTestCompletion(t, firstDone, "obsolete resize callback did not complete") | |
| 379 | 385 | requireNoOutputFrame(t, sends) | |
| 380 | 386 | latest.ch <- time.Now() | |
| 387 | + | awaitTestCompletion(t, latestDone, "latest resize callback did not complete") | |
| 381 | 388 | awaitFrame(t, sends, ports.MsgOutput) | |
| 382 | 389 | require.Equal(t, domain.Size{Cols: 120, Rows: 24}, ac.size) | |
| 383 | 390 | require.Equal(t, 120, sess.activeTab().focusedPane().screen.Frame.Width) | |
| @@ -308,8 +308,9 @@ func TestResizeGrowthFirstFrameIncludesConcurrentPTYRedraw(t *testing.T) { | |||||
| 308 | d.clock = clock | 308 | d.clock = clock | ||
| 309 | d.resize(sess, ac, domain.Size{Cols: 120, Rows: 24}) | 309 | d.resize(sess, ac, domain.Size{Cols: 120, Rows: 24}) | ||
| 310 | timer := <-clock.timers | 310 | timer := <-clock.timers | ||
| 311 | + | done := captureResizeCallbackDone(t, sess.renderCoordinator()) | |||
| 311 | timer.ch <- time.Time{} | 312 | timer.ch <- time.Time{} | ||
| 312 | - | awaitResizeCallback(t, sess.renderCoordinator()) | 313 | + | awaitTestCompletion(t, done, "resize callback did not complete") |
| 313 | require.True(t, p.deliveredWhileApplying(), "redraw must arrive while PTY.Resize owns the apply gate") | 314 | require.True(t, p.deliveredWhileApplying(), "redraw must arrive while PTY.Resize owns the apply gate") | ||
| 314 | require.Equal(t, 'B', pane.screen.Frame.At(100, 0).Rune, "replay must parse the redraw before commit") | 315 | require.Equal(t, 'B', pane.screen.Frame.At(100, 0).Rune, "replay must parse the redraw before commit") | ||
| 315 | 316 | ||||
| @@ -342,7 +343,9 @@ func TestResizeWithoutPTYOutputFlushesOneFullFrameAtDeadline(t *testing.T) { | |||||
| 342 | } | 343 | } | ||
| 343 | requireNoOutputFrame(t, sends) | 344 | requireNoOutputFrame(t, sends) | ||
| 344 | 345 | ||||
| 346 | + | done := captureResizeCallbackDone(t, sess.renderCoordinator()) | |||
| 345 | timer.ch <- time.Now() | 347 | timer.ch <- time.Now() | ||
| 348 | + | awaitTestCompletion(t, done, "resize callback did not complete") | |||
| 346 | frame := awaitFrame(t, sends, ports.MsgOutput) | 349 | frame := awaitFrame(t, sends, ports.MsgOutput) | ||
| 347 | client := vt.NewScreen(120, 24) | 350 | client := vt.NewScreen(120, 24) | ||
| 348 | out := mustApplyOutput(t, client, frame) | 351 | out := mustApplyOutput(t, client, frame) | ||
| @@ -367,6 +370,7 @@ func TestResizeBurstFlushesOnlyLatestGeometry(t *testing.T) { | |||||
| 367 | case <-time.After(time.Second): | 370 | case <-time.After(time.Second): | ||
| 368 | t.Fatal("first resize did not schedule a bounded paint") | 371 | t.Fatal("first resize did not schedule a bounded paint") | ||
| 369 | } | 372 | } | ||
| 373 | + | firstDone := captureResizeCallbackDone(t, sess.renderCoordinator()) | |||
| 370 | d.resize(sess, ac, domain.Size{Cols: 120, Rows: 24}) | 374 | d.resize(sess, ac, domain.Size{Cols: 120, Rows: 24}) | ||
| 371 | var latest *signalTimer | 375 | var latest *signalTimer | ||
| 372 | select { | 376 | select { | ||
| @@ -375,9 +379,12 @@ func TestResizeBurstFlushesOnlyLatestGeometry(t *testing.T) { | |||||
| 375 | t.Fatal("latest resize did not replace the bounded paint") | 379 | t.Fatal("latest resize did not replace the bounded paint") | ||
| 376 | } | 380 | } | ||
| 377 | 381 | ||||
| 382 | + | latestDone := captureResizeCallbackDone(t, sess.renderCoordinator()) | |||
| 378 | first.ch <- time.Now() | 383 | first.ch <- time.Now() | ||
| 384 | + | awaitTestCompletion(t, firstDone, "obsolete resize callback did not complete") | |||
| 379 | requireNoOutputFrame(t, sends) | 385 | requireNoOutputFrame(t, sends) | ||
| 380 | latest.ch <- time.Now() | 386 | latest.ch <- time.Now() | ||
| 387 | + | awaitTestCompletion(t, latestDone, "latest resize callback did not complete") | |||
| 381 | awaitFrame(t, sends, ports.MsgOutput) | 388 | awaitFrame(t, sends, ports.MsgOutput) | ||
| 382 | require.Equal(t, domain.Size{Cols: 120, Rows: 24}, ac.size) | 389 | require.Equal(t, domain.Size{Cols: 120, Rows: 24}, ac.size) | ||
| 383 | require.Equal(t, 120, sess.activeTab().focusedPane().screen.Frame.Width) | 390 | require.Equal(t, 120, sess.activeTab().focusedPane().screen.Frame.Width) | ||
M internal/usecase/daemon/picker_test.go +2 -0
| @@ -324,6 +324,7 @@ func TestPickerDisplacementCancelsSupersededResize(t *testing.T) { | |||
| 324 | 324 | ||
| 325 | 325 | d.resize(sess2, ac2, domain.Size{Cols: 100, Rows: 24}) | |
| 326 | 326 | timer := <-clock.timers | |
| 327 | + | done := captureResizeCallbackDone(t, sess2.renderCoordinator()) | |
| 327 | 328 | before := sess2.renderCoordinator().resizeSnapshot().epoch | |
| 328 | 329 | ||
| 329 | 330 | require.Same(t, ac2, d.stealClientForTarget(sess1, ac1, sess2, picker.Target{Session: sess2.id})) | |
| @@ -331,6 +332,7 @@ func TestPickerDisplacementCancelsSupersededResize(t *testing.T) { | |||
| 331 | 332 | // no attachment-local timer survives the transfer. | |
| 332 | 333 | require.Equal(t, before, sess2.renderCoordinator().resizeSnapshot().epoch) | |
| 333 | 334 | timer.ch <- time.Time{} | |
| 335 | + | awaitTestCompletion(t, done, "superseded resize callback did not complete") | |
| 334 | 336 | } | |
| 335 | 337 | ||
| 336 | 338 | func TestPickerStalePaintAfterSessionSwitchSendsNoFrame(t *testing.T) { | |
| @@ -324,6 +324,7 @@ func TestPickerDisplacementCancelsSupersededResize(t *testing.T) { | |||||
| 324 | 324 | ||||
| 325 | d.resize(sess2, ac2, domain.Size{Cols: 100, Rows: 24}) | 325 | d.resize(sess2, ac2, domain.Size{Cols: 100, Rows: 24}) | ||
| 326 | timer := <-clock.timers | 326 | timer := <-clock.timers | ||
| 327 | + | done := captureResizeCallbackDone(t, sess2.renderCoordinator()) | |||
| 327 | before := sess2.renderCoordinator().resizeSnapshot().epoch | 328 | before := sess2.renderCoordinator().resizeSnapshot().epoch | ||
| 328 | 329 | ||||
| 329 | require.Same(t, ac2, d.stealClientForTarget(sess1, ac1, sess2, picker.Target{Session: sess2.id})) | 330 | require.Same(t, ac2, d.stealClientForTarget(sess1, ac1, sess2, picker.Target{Session: sess2.id})) | ||
| @@ -331,6 +332,7 @@ func TestPickerDisplacementCancelsSupersededResize(t *testing.T) { | |||||
| 331 | // no attachment-local timer survives the transfer. | 332 | // no attachment-local timer survives the transfer. | ||
| 332 | require.Equal(t, before, sess2.renderCoordinator().resizeSnapshot().epoch) | 333 | require.Equal(t, before, sess2.renderCoordinator().resizeSnapshot().epoch) | ||
| 333 | timer.ch <- time.Time{} | 334 | timer.ch <- time.Time{} | ||
| 335 | + | awaitTestCompletion(t, done, "superseded resize callback did not complete") | |||
| 334 | } | 336 | } | ||
| 335 | 337 | ||||
| 336 | func TestPickerStalePaintAfterSessionSwitchSendsNoFrame(t *testing.T) { | 338 | func TestPickerStalePaintAfterSessionSwitchSendsNoFrame(t *testing.T) { | ||
M internal/usecase/daemon/render_coordinator.go +144 -158
| @@ -53,32 +53,16 @@ type renderCoordinator struct { | |||
| 53 | 53 | torndown bool | |
| 54 | 54 | ||
| 55 | 55 | resize resizeRequestMetadata | |
| 56 | - | // resizeTimer is independent from render invalidation deadlines. It owns | |
| 57 | - | // resize burst coalescing and is cancelled by the same lifecycle changes. | |
| 58 | - | resizeTimer ports.Timer | |
| 59 | - | resizeCancel chan struct{} | |
| 60 | - | // resizeDone closes after the latest timer callback has either been | |
| 61 | - | // rejected or completed its transaction. It provides a completion edge | |
| 62 | - | // without polling scheduler state. | |
| 63 | - | resizeDone chan struct{} | |
| 64 | - | resizeGen uint64 | |
| 65 | - | // retry is a separate, latest-committed-epoch retry lane. It is cancelled | |
| 66 | - | // with resize requests and attachment lifecycle changes; callbacks also | |
| 67 | - | // validate their epoch before touching a pane. | |
| 68 | - | retryTimer ports.Timer | |
| 69 | - | retryCancel chan struct{} | |
| 70 | - | retryDone chan struct{} | |
| 71 | - | retryGen uint64 | |
| 72 | - | metrics renderCoordinatorBurstMetrics | |
| 73 | - | // generation invalidates callbacks from superseded deadline/watchdog timers. | |
| 74 | - | // Each worker also owns an explicit cancellation channel: fake timers may | |
| 75 | - | // expose a nil or never-firing C channel, so Stop alone cannot release a | |
| 76 | - | // worker waiting in select. | |
| 77 | - | generation uint64 | |
| 78 | - | armed bool | |
| 79 | - | normalTimer ports.Timer | |
| 80 | - | normalCancel chan struct{} | |
| 81 | - | normalWorkerDone chan struct{} | |
| 56 | + | // Each lane owns one complete immutable worker token at a time. | |
| 57 | + | resizeLane timerLane | |
| 58 | + | retryLane timerLane | |
| 59 | + | metrics renderCoordinatorBurstMetrics | |
| 60 | + | // normalLane owns render invalidation deadlines. The supervisor is only | |
| 61 | + | // waited by the session owner after terminal teardown has rejected future | |
| 62 | + | // registrations. | |
| 63 | + | normalLane timerLane | |
| 64 | + | armed bool | |
| 65 | + | supervisor timerSupervisor | |
| 82 | 66 | // syncBatches is keyed by the stable pane owner. Each pane owns an | |
| 83 | 67 | // independent watchdog; composition remains gated until every batch ends. | |
| 84 | 68 | syncBatches map[*pane]*syncBatch | |
| @@ -94,25 +78,10 @@ func newRenderCoordinator(opts renderCoordinatorOptions) *renderCoordinator { | |||
| 94 | 78 | return &renderCoordinator{opts: opts} | |
| 95 | 79 | } | |
| 96 | 80 | ||
| 97 | - | // normalTimerOwner owns normal deadline cancellation and completion tickets. | |
| 98 | - | // Timer methods remain unlocked at its call sites. | |
| 99 | - | func (c *renderCoordinator) normalTimerOwner() timerOwnership { | |
| 100 | - | return timerOwner(&c.normalTimer, &c.normalCancel, &c.normalWorkerDone, &c.generation) | |
| 101 | - | } | |
| 102 | - | ||
| 103 | - | func (c *renderCoordinator) resizeTimerOwner() timerOwnership { | |
| 104 | - | return timerOwner(&c.resizeTimer, &c.resizeCancel, &c.resizeDone, &c.resizeGen) | |
| 105 | - | } | |
| 106 | - | ||
| 107 | - | func (c *renderCoordinator) retryTimerOwner() timerOwnership { | |
| 108 | - | return timerOwner(&c.retryTimer, &c.retryCancel, &c.retryDone, &c.retryGen) | |
| 109 | - | } | |
| 110 | - | ||
| 111 | - | // detachNormalTimerLocked releases coordinator ownership. The caller must stop | |
| 112 | - | // and join the returned worker after dropping c.mu: timer methods and worker | |
| 113 | - | // callbacks are external coordinator re-entry points. | |
| 114 | - | func (c *renderCoordinator) detachNormalTimerLocked() timerWorker { | |
| 115 | - | return c.normalTimerOwner().detachLocked() | |
| 81 | + | // detachNormalTimerLocked removes the complete current token. The caller may | |
| 82 | + | // stop it only after dropping c.mu and must never wait for its worker. | |
| 83 | + | func (c *renderCoordinator) detachNormalTimerLocked() *timerToken { | |
| 84 | + | return c.normalLane.detachLocked() | |
| 116 | 85 | } | |
| 117 | 86 | ||
| 118 | 87 | type syncBatch struct { | |
| @@ -121,19 +90,13 @@ type syncBatch struct { | |||
| 121 | 90 | // acquired by this predicate. | |
| 122 | 91 | renderable func() bool | |
| 123 | 92 | force func() | |
| 124 | - | timer ports.Timer | |
| 125 | - | cancel chan struct{} | |
| 126 | - | done chan struct{} | |
| 127 | - | } | |
| 128 | - | ||
| 129 | - | func (batch *syncBatch) timerOwner() timerOwnership { | |
| 130 | - | return timerOwner(&batch.timer, &batch.cancel, &batch.done, &batch.generation) | |
| 93 | + | lane timerLane | |
| 131 | 94 | } | |
| 132 | 95 | ||
| 133 | - | func detachSyncBatchLocked(batch *syncBatch) timerWorker { return batch.timerOwner().detachLocked() } | |
| 96 | + | func detachSyncBatchLocked(batch *syncBatch) *timerToken { return batch.lane.detachLocked() } | |
| 134 | 97 | ||
| 135 | - | func (c *renderCoordinator) detachSyncTimersLocked() []timerWorker { | |
| 136 | - | workers := make([]timerWorker, 0, len(c.syncBatches)) | |
| 98 | + | func (c *renderCoordinator) detachSyncTimersLocked() []*timerToken { | |
| 99 | + | workers := make([]*timerToken, 0, len(c.syncBatches)) | |
| 137 | 100 | for pane, batch := range c.syncBatches { | |
| 138 | 101 | workers = append(workers, detachSyncBatchLocked(batch)) | |
| 139 | 102 | delete(c.syncBatches, pane) | |
| @@ -209,11 +172,11 @@ func (c *renderCoordinator) invalidateForLeaseAtResizeEpoch(source *attachedClie | |||
| 209 | 172 | // pending work. Completion republishes urgently and must reserve a fresh | |
| 210 | 173 | // deadline rather than leaving that already-fired timer as the only arm. | |
| 211 | 174 | arm := !wasPending || (!wasUrgent && c.pendingUrgent) || (!wasPreviewPending && c.pendingPreview) || c.deadlineDue | |
| 212 | - | var old timerWorker | |
| 175 | + | var old *timerToken | |
| 213 | 176 | if arm { | |
| 214 | - | _, old = c.normalTimerOwner().replaceLocked() | |
| 177 | + | _, old = c.normalLane.replaceLocked() | |
| 215 | 178 | } | |
| 216 | - | gen := c.generation | |
| 179 | + | gen := c.normalLane.generation | |
| 217 | 180 | delay := minOutputRenderDeadline + time.Duration(c.outputPressure)*time.Millisecond | |
| 218 | 181 | if c.pendingUrgent { | |
| 219 | 182 | delay = urgentRenderDeadline | |
| @@ -225,7 +188,7 @@ func (c *renderCoordinator) invalidateForLeaseAtResizeEpoch(source *attachedClie | |||
| 225 | 188 | if queueStart && observer != nil { | |
| 226 | 189 | observer.ObserveRuntime(ports.NewRuntimeMarkWithCorrelation("daemon", queueCorrelation, ports.RuntimeQueueEnqueued, 0, true)) | |
| 227 | 190 | } | |
| 228 | - | stopAndJoinTimerWorker(old, nil) | |
| 191 | + | stopDetachedTimer(old) | |
| 229 | 192 | if !arm || clock == nil { | |
| 230 | 193 | if onInvalidate != nil { | |
| 231 | 194 | onInvalidate(inv) | |
| @@ -238,31 +201,34 @@ func (c *renderCoordinator) invalidateForLeaseAtResizeEpoch(source *attachedClie | |||
| 238 | 201 | timer := clock.NewTimer(delay) | |
| 239 | 202 | timerC := timer.C() | |
| 240 | 203 | c.mu.Lock() | |
| 241 | - | valid := !c.torndown && c.pending && c.generation == gen | |
| 242 | - | var cancel, done chan struct{} | |
| 204 | + | valid := !c.torndown && c.pending && c.normalLane.generation == gen | |
| 205 | + | var token *timerToken | |
| 243 | 206 | if valid && timerC != nil { | |
| 244 | - | cancel, done, valid = c.normalTimerOwner().publishLocked(gen, timer) | |
| 207 | + | token = c.normalLane.publishLocked(gen, timer) | |
| 208 | + | valid = token != nil | |
| 209 | + | if valid { | |
| 210 | + | c.supervisor.startLocked(token, timerC, func() { | |
| 211 | + | c.markDeadlineDue(token) | |
| 212 | + | c.fireFromTimer(token, false, true) | |
| 213 | + | }) | |
| 214 | + | } | |
| 245 | 215 | } | |
| 246 | 216 | c.mu.Unlock() | |
| 247 | 217 | if !valid { | |
| 248 | - | stopTimer(timer) | |
| 218 | + | timer.Stop() | |
| 249 | 219 | if onInvalidate != nil { | |
| 250 | 220 | onInvalidate(inv) | |
| 251 | 221 | } | |
| 252 | 222 | return true | |
| 253 | 223 | } | |
| 254 | 224 | if timerC == nil { | |
| 255 | - | stopTimer(timer) | |
| 225 | + | timer.Stop() | |
| 256 | 226 | c.fire(gen, false, true) | |
| 257 | 227 | if onInvalidate != nil { | |
| 258 | 228 | onInvalidate(inv) | |
| 259 | 229 | } | |
| 260 | 230 | return true | |
| 261 | 231 | } | |
| 262 | - | runTimerWorker(timerC, cancel, done, func() { | |
| 263 | - | c.markDeadlineDue(gen) | |
| 264 | - | c.fireFromTimer(gen, false, true, done) | |
| 265 | - | }) | |
| 266 | 232 | // Test-visible observation follows deadline publication, so a producer | |
| 267 | 233 | // callback cannot observe a successful invalidation before its timer owns | |
| 268 | 234 | // a worker (and race tests cannot finish while a mock expectation remains). | |
| @@ -282,7 +248,7 @@ func (c *renderCoordinator) notifyAck() { | |||
| 282 | 248 | c.mu.Unlock() | |
| 283 | 249 | return | |
| 284 | 250 | } | |
| 285 | - | gen := c.generation | |
| 251 | + | gen := c.normalLane.generation | |
| 286 | 252 | blocked := c.ackBlocked | |
| 287 | 253 | c.ackBlocked = nil | |
| 288 | 254 | c.mu.Unlock() | |
| @@ -296,9 +262,9 @@ func (c *renderCoordinator) notifyAck() { | |||
| 296 | 262 | // arms that pane's watchdog. Overlapping pane batches are independent. | |
| 297 | 263 | // markDeadlineDue records a received timer tick before the worker probes ACK | |
| 298 | 264 | // readiness, so a concurrent ACK cannot miss an already-expired deadline. | |
| 299 | - | func (c *renderCoordinator) markDeadlineDue(gen uint64) { | |
| 265 | + | func (c *renderCoordinator) markDeadlineDue(token *timerToken) { | |
| 300 | 266 | c.mu.Lock() | |
| 301 | - | if c.fireValidLocked(gen, false) { | |
| 267 | + | if c.fireValidLocked(token, token.generation, false) { | |
| 302 | 268 | c.deadlineDue = true | |
| 303 | 269 | } | |
| 304 | 270 | c.mu.Unlock() | |
| @@ -323,7 +289,7 @@ func (c *renderCoordinator) noteSyncBeginWithRenderability(p *pane, gen uint64, | |||
| 323 | 289 | if c.syncBatches == nil { | |
| 324 | 290 | c.syncBatches = make(map[*pane]*syncBatch) | |
| 325 | 291 | } | |
| 326 | - | var old timerWorker | |
| 292 | + | var old *timerToken | |
| 327 | 293 | if previous := c.syncBatches[p]; previous != nil { | |
| 328 | 294 | old = detachSyncBatchLocked(previous) | |
| 329 | 295 | } | |
| @@ -335,7 +301,7 @@ func (c *renderCoordinator) noteSyncBeginWithRenderability(p *pane, gen uint64, | |||
| 335 | 301 | c.syncRegistryVersion++ | |
| 336 | 302 | clock := c.opts.clock | |
| 337 | 303 | c.mu.Unlock() | |
| 338 | - | stopAndJoinTimerWorker(old, nil) | |
| 304 | + | stopDetachedTimer(old) | |
| 339 | 305 | if clock == nil { | |
| 340 | 306 | return | |
| 341 | 307 | } | |
| @@ -345,53 +311,59 @@ func (c *renderCoordinator) noteSyncBeginWithRenderability(p *pane, gen uint64, | |||
| 345 | 311 | timer := clock.NewTimer(maxSyncUpdateDuration) | |
| 346 | 312 | timerC := timer.C() | |
| 347 | 313 | c.mu.Lock() | |
| 348 | - | valid := !c.torndown && c.syncBatches[p] == batch && batch.generation == gen | |
| 349 | - | var cancel, done chan struct{} | |
| 350 | - | if valid { | |
| 351 | - | cancel, done, valid = batch.timerOwner().publishLocked(gen, timer) | |
| 314 | + | valid := !c.torndown && c.syncBatches[p] == batch && batch.generation == gen && batch.lane.generation == 0 | |
| 315 | + | var token *timerToken | |
| 316 | + | if valid && timerC != nil { | |
| 317 | + | batch.lane.generation = gen | |
| 318 | + | token = batch.lane.publishLocked(gen, timer) | |
| 319 | + | valid = token != nil | |
| 320 | + | if valid { | |
| 321 | + | c.supervisor.startLocked(token, timerC, func() { c.runSyncWatchdog(p, batch, token) }) | |
| 322 | + | } | |
| 352 | 323 | } | |
| 353 | 324 | c.mu.Unlock() | |
| 325 | + | if !valid || timerC == nil { | |
| 326 | + | stopDetachedTimer(&timerToken{timer: timer}) | |
| 327 | + | return | |
| 328 | + | } | |
| 329 | + | } | |
| 330 | + | ||
| 331 | + | func (c *renderCoordinator) runSyncWatchdog(p *pane, batch *syncBatch, token *timerToken) { | |
| 332 | + | // Keep the batch registered while force runs. A concurrent render must | |
| 333 | + | // continue to observe the synchronized-output gate until the VT has | |
| 334 | + | // authoritatively closed the batch. | |
| 335 | + | c.mu.Lock() | |
| 336 | + | current := c.syncBatches[p] | |
| 337 | + | valid := !c.torndown && current == batch && current.generation == token.generation && current.lane.token == token | |
| 338 | + | c.mu.Unlock() | |
| 354 | 339 | if !valid { | |
| 355 | - | stopTimer(timer) | |
| 356 | 340 | return | |
| 357 | 341 | } | |
| 358 | - | runTimerWorker(timerC, cancel, done, func() { | |
| 359 | - | // Keep the batch registered while force runs. A concurrent render must | |
| 360 | - | // continue to observe the synchronized-output gate until the VT has | |
| 361 | - | // authoritatively closed the batch. | |
| 362 | - | c.mu.Lock() | |
| 363 | - | current := c.syncBatches[p] | |
| 364 | - | valid := !c.torndown && current == batch && current.generation == gen | |
| 365 | - | c.mu.Unlock() | |
| 366 | - | if !valid { | |
| 367 | - | return | |
| 368 | - | } | |
| 369 | - | if batch.force != nil { | |
| 370 | - | batch.force() | |
| 371 | - | } | |
| 342 | + | if batch.force != nil { | |
| 343 | + | batch.force() | |
| 344 | + | } | |
| 372 | 345 | ||
| 373 | - | // force may synchronously end this batch, replace it with a newer | |
| 374 | - | // generation, or tear down the coordinator. Remove only the exact | |
| 375 | - | // snapshot that expired, then publish one urgent completion wake. | |
| 376 | - | c.mu.Lock() | |
| 377 | - | current = c.syncBatches[p] | |
| 378 | - | valid = !c.torndown && current == batch && current.generation == gen | |
| 379 | - | var stopped timerWorker | |
| 380 | - | if valid { | |
| 381 | - | delete(c.syncBatches, p) | |
| 382 | - | c.syncRegistryVersion++ | |
| 383 | - | stopped = detachSyncBatchLocked(batch) | |
| 384 | - | if len(c.syncBatches) == 0 && c.pending { | |
| 385 | - | c.pendingUrgent = true | |
| 386 | - | } | |
| 387 | - | } | |
| 388 | - | c.mu.Unlock() | |
| 389 | - | if valid { | |
| 390 | - | stopAndJoinTimerWorker(stopped, done) | |
| 391 | - | // Reevaluate the aggregate gate only after forcing this pane. | |
| 392 | - | c.fireCurrent(true) | |
| 346 | + | // force may synchronously end this batch, replace it with a newer | |
| 347 | + | // generation, or tear down the coordinator. Remove only the exact | |
| 348 | + | // snapshot that expired, then publish one urgent completion wake. | |
| 349 | + | c.mu.Lock() | |
| 350 | + | current = c.syncBatches[p] | |
| 351 | + | valid = !c.torndown && current == batch && current.generation == token.generation && current.lane.token == token | |
| 352 | + | var stopped *timerToken | |
| 353 | + | if valid { | |
| 354 | + | delete(c.syncBatches, p) | |
| 355 | + | c.syncRegistryVersion++ | |
| 356 | + | stopped = detachSyncBatchLocked(batch) | |
| 357 | + | if len(c.syncBatches) == 0 && c.pending { | |
| 358 | + | c.pendingUrgent = true | |
| 393 | 359 | } | |
| 394 | - | }) | |
| 360 | + | } | |
| 361 | + | c.mu.Unlock() | |
| 362 | + | if valid { | |
| 363 | + | stopDetachedTimer(stopped) | |
| 364 | + | // Reevaluate the aggregate gate only after forcing this pane. | |
| 365 | + | c.fireCurrent(true) | |
| 366 | + | } | |
| 395 | 367 | } | |
| 396 | 368 | ||
| 397 | 369 | // noteSyncEnd records completion for exactly one pane batch. It only flushes | |
| @@ -430,21 +402,21 @@ func (c *renderCoordinator) removeSyncEnd(p *pane, gen uint64, renderable bool) | |||
| 430 | 402 | c.pendingUrgent = true | |
| 431 | 403 | } | |
| 432 | 404 | c.mu.Unlock() | |
| 433 | - | stopAndJoinTimerWorker(worker, nil) | |
| 405 | + | stopDetachedTimer(worker) | |
| 434 | 406 | return renderable | |
| 435 | 407 | } | |
| 436 | 408 | ||
| 437 | 409 | // noteSyncPaneRemoved releases a pane watchdog when pane lifecycle ends. | |
| 438 | 410 | func (c *renderCoordinator) noteSyncPaneRemoved(p *pane) { | |
| 439 | 411 | c.mu.Lock() | |
| 440 | - | var worker timerWorker | |
| 412 | + | var worker *timerToken | |
| 441 | 413 | if batch := c.syncBatches[p]; batch != nil { | |
| 442 | 414 | delete(c.syncBatches, p) | |
| 443 | 415 | c.syncRegistryVersion++ | |
| 444 | 416 | worker = detachSyncBatchLocked(batch) | |
| 445 | 417 | } | |
| 446 | 418 | c.mu.Unlock() | |
| 447 | - | stopAndJoinTimerWorker(worker, nil) | |
| 419 | + | stopDetachedTimer(worker) | |
| 448 | 420 | } | |
| 449 | 421 | ||
| 450 | 422 | // subscribePreview installs fn as the preview observer for coalesced wakes. | |
| @@ -564,7 +536,7 @@ func (c *renderCoordinator) wakeCurrent(w renderWake) bool { | |||
| 564 | 536 | } | |
| 565 | 537 | ||
| 566 | 538 | type renderLifecycleCleanup struct { | |
| 567 | - | workers []timerWorker | |
| 539 | + | tokens []*timerToken | |
| 568 | 540 | observer ports.RuntimeObserver | |
| 569 | 541 | queueCorrelation ports.RuntimeCorrelation | |
| 570 | 542 | queueMarked bool | |
| @@ -572,7 +544,9 @@ type renderLifecycleCleanup struct { | |||
| 572 | 544 | } | |
| 573 | 545 | ||
| 574 | 546 | func (cleanup renderLifecycleCleanup) finish() { | |
| 575 | - | stopAndJoinTimerWorkers(cleanup.workers) | |
| 547 | + | for _, token := range cleanup.tokens { | |
| 548 | + | stopDetachedTimer(token) | |
| 549 | + | } | |
| 576 | 550 | if cleanup.queueMarked && cleanup.observer != nil { | |
| 577 | 551 | cleanup.observer.ObserveRuntime(ports.NewRuntimeMarkWithCorrelation("daemon", cleanup.queueCorrelation, ports.RuntimeQueueDequeued, 0, false)) | |
| 578 | 552 | } | |
| @@ -581,9 +555,7 @@ func (cleanup renderLifecycleCleanup) finish() { | |||
| 581 | 555 | } | |
| 582 | 556 | } | |
| 583 | 557 | ||
| 584 | - | // beginDetach invalidates pending work while leaving timer joins to the caller. | |
| 585 | - | // Callers holding daemon/session locks must publish the lifecycle transition, | |
| 586 | - | // release those locks, and only then call cleanup.finish(). | |
| 558 | + | // beginDetach invalidates pending work. It never waits for detached workers. | |
| 587 | 559 | func (c *renderCoordinator) beginDetach(ac *attachedClient) renderLifecycleCleanup { | |
| 588 | 560 | c.mu.Lock() | |
| 589 | 561 | var cleanup renderLifecycleCleanup | |
| @@ -595,12 +567,12 @@ func (c *renderCoordinator) beginDetach(ac *attachedClient) renderLifecycleClean | |||
| 595 | 567 | c.ackDeferred = false | |
| 596 | 568 | c.pendingPreview = false | |
| 597 | 569 | c.coalesced = 0 | |
| 598 | - | _, timer := c.normalTimerOwner().replaceLocked() | |
| 570 | + | _, timer := c.normalLane.replaceLocked() | |
| 599 | 571 | c.armed = false | |
| 600 | - | _, resizeTimer := c.resizeTimerOwner().replaceLocked() | |
| 601 | - | _, retryTimer := c.retryTimerOwner().replaceLocked() | |
| 572 | + | _, resizeTimer := c.resizeLane.replaceLocked() | |
| 573 | + | _, retryTimer := c.retryLane.replaceLocked() | |
| 602 | 574 | cleanup = renderLifecycleCleanup{ | |
| 603 | - | workers: []timerWorker{timer, resizeTimer, retryTimer}, | |
| 575 | + | tokens: []*timerToken{timer, resizeTimer, retryTimer}, | |
| 604 | 576 | observer: c.opts.observer, | |
| 605 | 577 | queueCorrelation: c.queueCorrelation, | |
| 606 | 578 | queueMarked: c.queueMarked, | |
| @@ -617,8 +589,8 @@ func (c *renderCoordinator) noteDetach(ac *attachedClient) { | |||
| 617 | 589 | c.beginDetach(ac).finish() | |
| 618 | 590 | } | |
| 619 | 591 | ||
| 620 | - | // beginReplace hands the coordinator from old to replacement while leaving | |
| 621 | - | // detached timer joins to the caller. | |
| 592 | + | // beginReplace hands the coordinator from old to replacement without waiting | |
| 593 | + | // for detached workers. | |
| 622 | 594 | func (c *renderCoordinator) beginReplace(old, replacement *attachedClient, ready bool) renderLifecycleCleanup { | |
| 623 | 595 | c.mu.Lock() | |
| 624 | 596 | var cleanup renderLifecycleCleanup | |
| @@ -634,12 +606,12 @@ func (c *renderCoordinator) beginReplace(old, replacement *attachedClient, ready | |||
| 634 | 606 | c.ackDeferred = false | |
| 635 | 607 | c.pendingPreview = false | |
| 636 | 608 | c.coalesced = 0 | |
| 637 | - | _, timer := c.normalTimerOwner().replaceLocked() | |
| 609 | + | _, timer := c.normalLane.replaceLocked() | |
| 638 | 610 | c.armed = false | |
| 639 | - | _, resizeTimer := c.resizeTimerOwner().replaceLocked() | |
| 640 | - | _, retryTimer := c.retryTimerOwner().replaceLocked() | |
| 611 | + | _, resizeTimer := c.resizeLane.replaceLocked() | |
| 612 | + | _, retryTimer := c.retryLane.replaceLocked() | |
| 641 | 613 | cleanup = renderLifecycleCleanup{ | |
| 642 | - | workers: []timerWorker{timer, resizeTimer, retryTimer}, | |
| 614 | + | tokens: []*timerToken{timer, resizeTimer, retryTimer}, | |
| 643 | 615 | observer: c.opts.observer, | |
| 644 | 616 | queueCorrelation: c.queueCorrelation, | |
| 645 | 617 | queueMarked: c.queueMarked, | |
| @@ -663,9 +635,14 @@ func (c *renderCoordinator) noteReplace(old, replacement *attachedClient, readin | |||
| 663 | 635 | // notePark invalidates pending wakes when the attachment parks for resume. | |
| 664 | 636 | func (c *renderCoordinator) notePark(ac *attachedClient) { c.noteDetach(ac) } | |
| 665 | 637 | ||
| 666 | - | // noteSessionTeardown terminally invalidates the coordinator. | |
| 667 | - | func (c *renderCoordinator) noteSessionTeardown() { | |
| 638 | + | // beginSessionTeardown is terminal phase one. It rejects future worker | |
| 639 | + | // registration and atomically detaches every complete lane token. | |
| 640 | + | func (c *renderCoordinator) beginSessionTeardown() renderLifecycleCleanup { | |
| 668 | 641 | c.mu.Lock() | |
| 642 | + | if c.torndown { | |
| 643 | + | c.mu.Unlock() | |
| 644 | + | return renderLifecycleCleanup{} | |
| 645 | + | } | |
| 669 | 646 | c.torndown = true | |
| 670 | 647 | if c.lease != nil { | |
| 671 | 648 | c.lease.active = false | |
| @@ -673,29 +650,30 @@ func (c *renderCoordinator) noteSessionTeardown() { | |||
| 673 | 650 | c.previewWake = nil | |
| 674 | 651 | c.previewWakes = nil | |
| 675 | 652 | c.pending = false | |
| 676 | - | _, resizeTimer := c.resizeTimerOwner().replaceLocked() | |
| 677 | - | _, retryTimer := c.retryTimerOwner().replaceLocked() | |
| 653 | + | _, resizeTimer := c.resizeLane.replaceLocked() | |
| 654 | + | _, retryTimer := c.retryLane.replaceLocked() | |
| 678 | 655 | c.ackDeferred = false | |
| 679 | 656 | c.pendingPreview = false | |
| 680 | - | _, timer := c.normalTimerOwner().replaceLocked() | |
| 657 | + | _, timer := c.normalLane.replaceLocked() | |
| 681 | 658 | c.armed = false | |
| 682 | 659 | workers := c.detachSyncTimersLocked() | |
| 683 | 660 | observer, queueCorrelation, queueMarked := c.opts.observer, c.queueCorrelation, c.queueMarked | |
| 684 | 661 | ackBlocked := c.ackBlocked | |
| 685 | 662 | c.queueMarked, c.ackBlocked = false, nil | |
| 686 | 663 | c.mu.Unlock() | |
| 687 | - | if queueMarked && observer != nil { | |
| 688 | - | observer.ObserveRuntime(ports.NewRuntimeMarkWithCorrelation("daemon", queueCorrelation, ports.RuntimeQueueDequeued, 0, false)) | |
| 664 | + | return renderLifecycleCleanup{ | |
| 665 | + | tokens: append([]*timerToken{timer, resizeTimer, retryTimer}, workers...), | |
| 666 | + | observer: observer, | |
| 667 | + | queueCorrelation: queueCorrelation, | |
| 668 | + | queueMarked: queueMarked, | |
| 669 | + | ackBlocked: ackBlocked, | |
| 689 | 670 | } | |
| 690 | - | if ackBlocked != nil { | |
| 691 | - | ackBlocked.finish(false) | |
| 692 | - | } | |
| 693 | - | stopAndJoinTimerWorker(timer, nil) | |
| 694 | - | stopAndJoinTimerWorker(resizeTimer, nil) | |
| 695 | - | stopAndJoinTimerWorker(retryTimer, nil) | |
| 696 | - | stopAndJoinTimerWorkers(workers) | |
| 697 | 671 | } | |
| 698 | 672 | ||
| 673 | + | // waitForTimerWorkers is terminal phase two. Only the session owner invokes it, | |
| 674 | + | // after beginSessionTeardown has made future supervisor registration impossible. | |
| 675 | + | func (c *renderCoordinator) waitForTimerWorkers() { c.supervisor.wait() } | |
| 676 | + | ||
| 699 | 677 | // invalidateRender is the sole producer fan-in. In tests and transitional | |
| 700 | 678 | // headless paths without an attached coordinator it retains the old private | |
| 701 | 679 | // compositor; attached sessions always schedule through their coordinator. | |
| @@ -728,21 +706,26 @@ func (d *Daemon) invalidateRenderNow(sess *session, ac *attachedClient, reset bo | |||
| 728 | 706 | } | |
| 729 | 707 | ||
| 730 | 708 | func (c *renderCoordinator) fire(gen uint64, watchdog, deadline bool) { | |
| 731 | - | c.fireWithTimerWorker(gen, watchdog, deadline, nil) | |
| 709 | + | c.mu.Lock() | |
| 710 | + | if !watchdog && c.normalLane.generation != gen { | |
| 711 | + | c.mu.Unlock() | |
| 712 | + | return | |
| 713 | + | } | |
| 714 | + | token := c.normalLane.token | |
| 715 | + | c.mu.Unlock() | |
| 716 | + | c.fireWithTimerToken(token, gen, watchdog, deadline) | |
| 732 | 717 | } | |
| 733 | 718 | ||
| 734 | - | // fireFromTimer consumes a normal deadline from its own worker. It must not | |
| 735 | - | // wait for that worker's done channel because runTimerWorker closes it only | |
| 736 | - | // after this callback returns. | |
| 737 | - | func (c *renderCoordinator) fireFromTimer(gen uint64, watchdog, deadline bool, self <-chan struct{}) { | |
| 738 | - | c.fireWithTimerWorker(gen, watchdog, deadline, self) | |
| 719 | + | // fireFromTimer consumes a normal deadline using its exact immutable token. | |
| 720 | + | func (c *renderCoordinator) fireFromTimer(token *timerToken, watchdog, deadline bool) { | |
| 721 | + | c.fireWithTimerToken(token, token.generation, watchdog, deadline) | |
| 739 | 722 | } | |
| 740 | 723 | ||
| 741 | - | func (c *renderCoordinator) fireWithTimerWorker(gen uint64, watchdog, deadline bool, self <-chan struct{}) { | |
| 724 | + | func (c *renderCoordinator) fireWithTimerToken(token *timerToken, gen uint64, watchdog, deadline bool) { | |
| 742 | 725 | // Readiness probes enter the attachment send path, which may in turn read | |
| 743 | 726 | // coordinator metadata. Never hold c.mu across that external callback. | |
| 744 | 727 | c.mu.Lock() | |
| 745 | - | if !c.fireValidLocked(gen, watchdog) { | |
| 728 | + | if !c.fireValidLocked(token, gen, watchdog) { | |
| 746 | 729 | c.mu.Unlock() | |
| 747 | 730 | return | |
| 748 | 731 | } | |
| @@ -759,7 +742,7 @@ func (c *renderCoordinator) fireWithTimerWorker(gen uint64, watchdog, deadline b | |||
| 759 | 742 | // syncGateOpenLocked retries predicate evaluation on every registry change | |
| 760 | 743 | // and returns holding c.mu, so pending consumption has no intervening gap. | |
| 761 | 744 | c.mu.Lock() | |
| 762 | - | if !c.fireValidLocked(gen, watchdog) { | |
| 745 | + | if !c.fireValidLocked(token, gen, watchdog) { | |
| 763 | 746 | c.mu.Unlock() | |
| 764 | 747 | return | |
| 765 | 748 | } | |
| @@ -767,7 +750,7 @@ func (c *renderCoordinator) fireWithTimerWorker(gen uint64, watchdog, deadline b | |||
| 767 | 750 | c.mu.Unlock() | |
| 768 | 751 | return | |
| 769 | 752 | } | |
| 770 | - | if !c.fireValidLocked(gen, watchdog) { | |
| 753 | + | if !c.fireValidLocked(token, gen, watchdog) { | |
| 771 | 754 | c.mu.Unlock() | |
| 772 | 755 | return | |
| 773 | 756 | } | |
| @@ -821,7 +804,7 @@ func (c *renderCoordinator) fireWithTimerWorker(gen uint64, watchdog, deadline b | |||
| 821 | 804 | c.queueMarked, c.ackBlocked = false, nil | |
| 822 | 805 | wake := c.opts.wake | |
| 823 | 806 | c.mu.Unlock() | |
| 824 | - | stopAndJoinTimerWorker(worker, self) | |
| 807 | + | stopDetachedTimer(worker) | |
| 825 | 808 | if queueMarked && observer != nil { | |
| 826 | 809 | observer.ObserveRuntime(ports.NewRuntimeMarkWithCorrelation("daemon", queueCorrelation, ports.RuntimeQueueDequeued, 0, true)) | |
| 827 | 810 | } | |
| @@ -834,8 +817,11 @@ func (c *renderCoordinator) fireWithTimerWorker(gen uint64, watchdog, deadline b | |||
| 834 | 817 | c.notifyPreviews(w, preview, previews) | |
| 835 | 818 | } | |
| 836 | 819 | ||
| 837 | - | func (c *renderCoordinator) fireValidLocked(gen uint64, watchdog bool) bool { | |
| 838 | - | return !c.torndown && c.pending && (watchdog || gen == c.generation) | |
| 820 | + | func (c *renderCoordinator) fireValidLocked(token *timerToken, gen uint64, watchdog bool) bool { | |
| 821 | + | if c.torndown || !c.pending || watchdog { | |
| 822 | + | return !c.torndown && c.pending | |
| 823 | + | } | |
| 824 | + | return c.normalLane.generation == gen && (token == nil || c.normalLane.token == token) | |
| 839 | 825 | } | |
| 840 | 826 | ||
| 841 | 827 | // syncGateOpen evaluates pane visibility without c.mu, then reacquires and | |
| @@ -53,32 +53,16 @@ type renderCoordinator struct { | |||||
| 53 | torndown bool | 53 | torndown bool | ||
| 54 | 54 | ||||
| 55 | resize resizeRequestMetadata | 55 | resize resizeRequestMetadata | ||
| 56 | - | // resizeTimer is independent from render invalidation deadlines. It owns | 56 | + | // Each lane owns one complete immutable worker token at a time. |
| 57 | - | // resize burst coalescing and is cancelled by the same lifecycle changes. | 57 | + | resizeLane timerLane |
| 58 | - | resizeTimer ports.Timer | 58 | + | retryLane timerLane |
| 59 | - | resizeCancel chan struct{} | 59 | + | metrics renderCoordinatorBurstMetrics |
| 60 | - | // resizeDone closes after the latest timer callback has either been | 60 | + | // normalLane owns render invalidation deadlines. The supervisor is only |
| 61 | - | // rejected or completed its transaction. It provides a completion edge | 61 | + | // waited by the session owner after terminal teardown has rejected future |
| 62 | - | // without polling scheduler state. | 62 | + | // registrations. |
| 63 | - | resizeDone chan struct{} | 63 | + | normalLane timerLane |
| 64 | - | resizeGen uint64 | 64 | + | armed bool |
| 65 | - | // retry is a separate, latest-committed-epoch retry lane. It is cancelled | 65 | + | supervisor timerSupervisor |
| 66 | - | // with resize requests and attachment lifecycle changes; callbacks also | |||
| 67 | - | // validate their epoch before touching a pane. | |||
| 68 | - | retryTimer ports.Timer | |||
| 69 | - | retryCancel chan struct{} | |||
| 70 | - | retryDone chan struct{} | |||
| 71 | - | retryGen uint64 | |||
| 72 | - | metrics renderCoordinatorBurstMetrics | |||
| 73 | - | // generation invalidates callbacks from superseded deadline/watchdog timers. | |||
| 74 | - | // Each worker also owns an explicit cancellation channel: fake timers may | |||
| 75 | - | // expose a nil or never-firing C channel, so Stop alone cannot release a | |||
| 76 | - | // worker waiting in select. | |||
| 77 | - | generation uint64 | |||
| 78 | - | armed bool | |||
| 79 | - | normalTimer ports.Timer | |||
| 80 | - | normalCancel chan struct{} | |||
| 81 | - | normalWorkerDone chan struct{} | |||
| 82 | // syncBatches is keyed by the stable pane owner. Each pane owns an | 66 | // syncBatches is keyed by the stable pane owner. Each pane owns an | ||
| 83 | // independent watchdog; composition remains gated until every batch ends. | 67 | // independent watchdog; composition remains gated until every batch ends. | ||
| 84 | syncBatches map[*pane]*syncBatch | 68 | syncBatches map[*pane]*syncBatch | ||
| @@ -94,25 +78,10 @@ func newRenderCoordinator(opts renderCoordinatorOptions) *renderCoordinator { | |||||
| 94 | return &renderCoordinator{opts: opts} | 78 | return &renderCoordinator{opts: opts} | ||
| 95 | } | 79 | } | ||
| 96 | 80 | ||||
| 97 | - | // normalTimerOwner owns normal deadline cancellation and completion tickets. | 81 | + | // detachNormalTimerLocked removes the complete current token. The caller may |
| 98 | - | // Timer methods remain unlocked at its call sites. | 82 | + | // stop it only after dropping c.mu and must never wait for its worker. |
| 99 | - | func (c *renderCoordinator) normalTimerOwner() timerOwnership { | 83 | + | func (c *renderCoordinator) detachNormalTimerLocked() *timerToken { |
| 100 | - | return timerOwner(&c.normalTimer, &c.normalCancel, &c.normalWorkerDone, &c.generation) | 84 | + | return c.normalLane.detachLocked() |
| 101 | - | } | |||
| 102 | - | ||||
| 103 | - | func (c *renderCoordinator) resizeTimerOwner() timerOwnership { | |||
| 104 | - | return timerOwner(&c.resizeTimer, &c.resizeCancel, &c.resizeDone, &c.resizeGen) | |||
| 105 | - | } | |||
| 106 | - | ||||
| 107 | - | func (c *renderCoordinator) retryTimerOwner() timerOwnership { | |||
| 108 | - | return timerOwner(&c.retryTimer, &c.retryCancel, &c.retryDone, &c.retryGen) | |||
| 109 | - | } | |||
| 110 | - | ||||
| 111 | - | // detachNormalTimerLocked releases coordinator ownership. The caller must stop | |||
| 112 | - | // and join the returned worker after dropping c.mu: timer methods and worker | |||
| 113 | - | // callbacks are external coordinator re-entry points. | |||
| 114 | - | func (c *renderCoordinator) detachNormalTimerLocked() timerWorker { | |||
| 115 | - | return c.normalTimerOwner().detachLocked() | |||
| 116 | } | 85 | } | ||
| 117 | 86 | ||||
| 118 | type syncBatch struct { | 87 | type syncBatch struct { | ||
| @@ -121,19 +90,13 @@ type syncBatch struct { | |||||
| 121 | // acquired by this predicate. | 90 | // acquired by this predicate. | ||
| 122 | renderable func() bool | 91 | renderable func() bool | ||
| 123 | force func() | 92 | force func() | ||
| 124 | - | timer ports.Timer | 93 | + | lane timerLane |
| 125 | - | cancel chan struct{} | |||
| 126 | - | done chan struct{} | |||
| 127 | - | } | |||
| 128 | - | ||||
| 129 | - | func (batch *syncBatch) timerOwner() timerOwnership { | |||
| 130 | - | return timerOwner(&batch.timer, &batch.cancel, &batch.done, &batch.generation) | |||
| 131 | } | 94 | } | ||
| 132 | 95 | ||||
| 133 | - | func detachSyncBatchLocked(batch *syncBatch) timerWorker { return batch.timerOwner().detachLocked() } | 96 | + | func detachSyncBatchLocked(batch *syncBatch) *timerToken { return batch.lane.detachLocked() } |
| 134 | 97 | ||||
| 135 | - | func (c *renderCoordinator) detachSyncTimersLocked() []timerWorker { | 98 | + | func (c *renderCoordinator) detachSyncTimersLocked() []*timerToken { |
| 136 | - | workers := make([]timerWorker, 0, len(c.syncBatches)) | 99 | + | workers := make([]*timerToken, 0, len(c.syncBatches)) |
| 137 | for pane, batch := range c.syncBatches { | 100 | for pane, batch := range c.syncBatches { | ||
| 138 | workers = append(workers, detachSyncBatchLocked(batch)) | 101 | workers = append(workers, detachSyncBatchLocked(batch)) | ||
| 139 | delete(c.syncBatches, pane) | 102 | delete(c.syncBatches, pane) | ||
| @@ -209,11 +172,11 @@ func (c *renderCoordinator) invalidateForLeaseAtResizeEpoch(source *attachedClie | |||||
| 209 | // pending work. Completion republishes urgently and must reserve a fresh | 172 | // pending work. Completion republishes urgently and must reserve a fresh | ||
| 210 | // deadline rather than leaving that already-fired timer as the only arm. | 173 | // deadline rather than leaving that already-fired timer as the only arm. | ||
| 211 | arm := !wasPending || (!wasUrgent && c.pendingUrgent) || (!wasPreviewPending && c.pendingPreview) || c.deadlineDue | 174 | arm := !wasPending || (!wasUrgent && c.pendingUrgent) || (!wasPreviewPending && c.pendingPreview) || c.deadlineDue | ||
| 212 | - | var old timerWorker | 175 | + | var old *timerToken |
| 213 | if arm { | 176 | if arm { | ||
| 214 | - | _, old = c.normalTimerOwner().replaceLocked() | 177 | + | _, old = c.normalLane.replaceLocked() |
| 215 | } | 178 | } | ||
| 216 | - | gen := c.generation | 179 | + | gen := c.normalLane.generation |
| 217 | delay := minOutputRenderDeadline + time.Duration(c.outputPressure)*time.Millisecond | 180 | delay := minOutputRenderDeadline + time.Duration(c.outputPressure)*time.Millisecond | ||
| 218 | if c.pendingUrgent { | 181 | if c.pendingUrgent { | ||
| 219 | delay = urgentRenderDeadline | 182 | delay = urgentRenderDeadline | ||
| @@ -225,7 +188,7 @@ func (c *renderCoordinator) invalidateForLeaseAtResizeEpoch(source *attachedClie | |||||
| 225 | if queueStart && observer != nil { | 188 | if queueStart && observer != nil { | ||
| 226 | observer.ObserveRuntime(ports.NewRuntimeMarkWithCorrelation("daemon", queueCorrelation, ports.RuntimeQueueEnqueued, 0, true)) | 189 | observer.ObserveRuntime(ports.NewRuntimeMarkWithCorrelation("daemon", queueCorrelation, ports.RuntimeQueueEnqueued, 0, true)) | ||
| 227 | } | 190 | } | ||
| 228 | - | stopAndJoinTimerWorker(old, nil) | 191 | + | stopDetachedTimer(old) |
| 229 | if !arm || clock == nil { | 192 | if !arm || clock == nil { | ||
| 230 | if onInvalidate != nil { | 193 | if onInvalidate != nil { | ||
| 231 | onInvalidate(inv) | 194 | onInvalidate(inv) | ||
| @@ -238,31 +201,34 @@ func (c *renderCoordinator) invalidateForLeaseAtResizeEpoch(source *attachedClie | |||||
| 238 | timer := clock.NewTimer(delay) | 201 | timer := clock.NewTimer(delay) | ||
| 239 | timerC := timer.C() | 202 | timerC := timer.C() | ||
| 240 | c.mu.Lock() | 203 | c.mu.Lock() | ||
| 241 | - | valid := !c.torndown && c.pending && c.generation == gen | 204 | + | valid := !c.torndown && c.pending && c.normalLane.generation == gen |
| 242 | - | var cancel, done chan struct{} | 205 | + | var token *timerToken |
| 243 | if valid && timerC != nil { | 206 | if valid && timerC != nil { | ||
| 244 | - | cancel, done, valid = c.normalTimerOwner().publishLocked(gen, timer) | 207 | + | token = c.normalLane.publishLocked(gen, timer) |
| 208 | + | valid = token != nil | |||
| 209 | + | if valid { | |||
| 210 | + | c.supervisor.startLocked(token, timerC, func() { | |||
| 211 | + | c.markDeadlineDue(token) | |||
| 212 | + | c.fireFromTimer(token, false, true) | |||
| 213 | + | }) | |||
| 214 | + | } | |||
| 245 | } | 215 | } | ||
| 246 | c.mu.Unlock() | 216 | c.mu.Unlock() | ||
| 247 | if !valid { | 217 | if !valid { | ||
| 248 | - | stopTimer(timer) | 218 | + | timer.Stop() |
| 249 | if onInvalidate != nil { | 219 | if onInvalidate != nil { | ||
| 250 | onInvalidate(inv) | 220 | onInvalidate(inv) | ||
| 251 | } | 221 | } | ||
| 252 | return true | 222 | return true | ||
| 253 | } | 223 | } | ||
| 254 | if timerC == nil { | 224 | if timerC == nil { | ||
| 255 | - | stopTimer(timer) | 225 | + | timer.Stop() |
| 256 | c.fire(gen, false, true) | 226 | c.fire(gen, false, true) | ||
| 257 | if onInvalidate != nil { | 227 | if onInvalidate != nil { | ||
| 258 | onInvalidate(inv) | 228 | onInvalidate(inv) | ||
| 259 | } | 229 | } | ||
| 260 | return true | 230 | return true | ||
| 261 | } | 231 | } | ||
| 262 | - | runTimerWorker(timerC, cancel, done, func() { | |||
| 263 | - | c.markDeadlineDue(gen) | |||
| 264 | - | c.fireFromTimer(gen, false, true, done) | |||
| 265 | - | }) | |||
| 266 | // Test-visible observation follows deadline publication, so a producer | 232 | // Test-visible observation follows deadline publication, so a producer | ||
| 267 | // callback cannot observe a successful invalidation before its timer owns | 233 | // callback cannot observe a successful invalidation before its timer owns | ||
| 268 | // a worker (and race tests cannot finish while a mock expectation remains). | 234 | // a worker (and race tests cannot finish while a mock expectation remains). | ||
| @@ -282,7 +248,7 @@ func (c *renderCoordinator) notifyAck() { | |||||
| 282 | c.mu.Unlock() | 248 | c.mu.Unlock() | ||
| 283 | return | 249 | return | ||
| 284 | } | 250 | } | ||
| 285 | - | gen := c.generation | 251 | + | gen := c.normalLane.generation |
| 286 | blocked := c.ackBlocked | 252 | blocked := c.ackBlocked | ||
| 287 | c.ackBlocked = nil | 253 | c.ackBlocked = nil | ||
| 288 | c.mu.Unlock() | 254 | c.mu.Unlock() | ||
| @@ -296,9 +262,9 @@ func (c *renderCoordinator) notifyAck() { | |||||
| 296 | // arms that pane's watchdog. Overlapping pane batches are independent. | 262 | // arms that pane's watchdog. Overlapping pane batches are independent. | ||
| 297 | // markDeadlineDue records a received timer tick before the worker probes ACK | 263 | // markDeadlineDue records a received timer tick before the worker probes ACK | ||
| 298 | // readiness, so a concurrent ACK cannot miss an already-expired deadline. | 264 | // readiness, so a concurrent ACK cannot miss an already-expired deadline. | ||
| 299 | - | func (c *renderCoordinator) markDeadlineDue(gen uint64) { | 265 | + | func (c *renderCoordinator) markDeadlineDue(token *timerToken) { |
| 300 | c.mu.Lock() | 266 | c.mu.Lock() | ||
| 301 | - | if c.fireValidLocked(gen, false) { | 267 | + | if c.fireValidLocked(token, token.generation, false) { |
| 302 | c.deadlineDue = true | 268 | c.deadlineDue = true | ||
| 303 | } | 269 | } | ||
| 304 | c.mu.Unlock() | 270 | c.mu.Unlock() | ||
| @@ -323,7 +289,7 @@ func (c *renderCoordinator) noteSyncBeginWithRenderability(p *pane, gen uint64, | |||||
| 323 | if c.syncBatches == nil { | 289 | if c.syncBatches == nil { | ||
| 324 | c.syncBatches = make(map[*pane]*syncBatch) | 290 | c.syncBatches = make(map[*pane]*syncBatch) | ||
| 325 | } | 291 | } | ||
| 326 | - | var old timerWorker | 292 | + | var old *timerToken |
| 327 | if previous := c.syncBatches[p]; previous != nil { | 293 | if previous := c.syncBatches[p]; previous != nil { | ||
| 328 | old = detachSyncBatchLocked(previous) | 294 | old = detachSyncBatchLocked(previous) | ||
| 329 | } | 295 | } | ||
| @@ -335,7 +301,7 @@ func (c *renderCoordinator) noteSyncBeginWithRenderability(p *pane, gen uint64, | |||||
| 335 | c.syncRegistryVersion++ | 301 | c.syncRegistryVersion++ | ||
| 336 | clock := c.opts.clock | 302 | clock := c.opts.clock | ||
| 337 | c.mu.Unlock() | 303 | c.mu.Unlock() | ||
| 338 | - | stopAndJoinTimerWorker(old, nil) | 304 | + | stopDetachedTimer(old) |
| 339 | if clock == nil { | 305 | if clock == nil { | ||
| 340 | return | 306 | return | ||
| 341 | } | 307 | } | ||
| @@ -345,53 +311,59 @@ func (c *renderCoordinator) noteSyncBeginWithRenderability(p *pane, gen uint64, | |||||
| 345 | timer := clock.NewTimer(maxSyncUpdateDuration) | 311 | timer := clock.NewTimer(maxSyncUpdateDuration) | ||
| 346 | timerC := timer.C() | 312 | timerC := timer.C() | ||
| 347 | c.mu.Lock() | 313 | c.mu.Lock() | ||
| 348 | - | valid := !c.torndown && c.syncBatches[p] == batch && batch.generation == gen | 314 | + | valid := !c.torndown && c.syncBatches[p] == batch && batch.generation == gen && batch.lane.generation == 0 |
| 349 | - | var cancel, done chan struct{} | 315 | + | var token *timerToken |
| 350 | - | if valid { | 316 | + | if valid && timerC != nil { |
| 351 | - | cancel, done, valid = batch.timerOwner().publishLocked(gen, timer) | 317 | + | batch.lane.generation = gen |
| 318 | + | token = batch.lane.publishLocked(gen, timer) | |||
| 319 | + | valid = token != nil | |||
| 320 | + | if valid { | |||
| 321 | + | c.supervisor.startLocked(token, timerC, func() { c.runSyncWatchdog(p, batch, token) }) | |||
| 322 | + | } | |||
| 352 | } | 323 | } | ||
| 353 | c.mu.Unlock() | 324 | c.mu.Unlock() | ||
| 325 | + | if !valid || timerC == nil { | |||
| 326 | + | stopDetachedTimer(&timerToken{timer: timer}) | |||
| 327 | + | return | |||
| 328 | + | } | |||
| 329 | + | } | |||
| 330 | + | ||||
| 331 | + | func (c *renderCoordinator) runSyncWatchdog(p *pane, batch *syncBatch, token *timerToken) { | |||
| 332 | + | // Keep the batch registered while force runs. A concurrent render must | |||
| 333 | + | // continue to observe the synchronized-output gate until the VT has | |||
| 334 | + | // authoritatively closed the batch. | |||
| 335 | + | c.mu.Lock() | |||
| 336 | + | current := c.syncBatches[p] | |||
| 337 | + | valid := !c.torndown && current == batch && current.generation == token.generation && current.lane.token == token | |||
| 338 | + | c.mu.Unlock() | |||
| 354 | if !valid { | 339 | if !valid { | ||
| 355 | - | stopTimer(timer) | |||
| 356 | return | 340 | return | ||
| 357 | } | 341 | } | ||
| 358 | - | runTimerWorker(timerC, cancel, done, func() { | 342 | + | if batch.force != nil { |
| 359 | - | // Keep the batch registered while force runs. A concurrent render must | 343 | + | batch.force() |
| 360 | - | // continue to observe the synchronized-output gate until the VT has | 344 | + | } |
| 361 | - | // authoritatively closed the batch. | |||
| 362 | - | c.mu.Lock() | |||
| 363 | - | current := c.syncBatches[p] | |||
| 364 | - | valid := !c.torndown && current == batch && current.generation == gen | |||
| 365 | - | c.mu.Unlock() | |||
| 366 | - | if !valid { | |||
| 367 | - | return | |||
| 368 | - | } | |||
| 369 | - | if batch.force != nil { | |||
| 370 | - | batch.force() | |||
| 371 | - | } | |||
| 372 | 345 | ||||
| 373 | - | // force may synchronously end this batch, replace it with a newer | 346 | + | // force may synchronously end this batch, replace it with a newer |
| 374 | - | // generation, or tear down the coordinator. Remove only the exact | 347 | + | // generation, or tear down the coordinator. Remove only the exact |
| 375 | - | // snapshot that expired, then publish one urgent completion wake. | 348 | + | // snapshot that expired, then publish one urgent completion wake. |
| 376 | - | c.mu.Lock() | 349 | + | c.mu.Lock() |
| 377 | - | current = c.syncBatches[p] | 350 | + | current = c.syncBatches[p] |
| 378 | - | valid = !c.torndown && current == batch && current.generation == gen | 351 | + | valid = !c.torndown && current == batch && current.generation == token.generation && current.lane.token == token |
| 379 | - | var stopped timerWorker | 352 | + | var stopped *timerToken |
| 380 | - | if valid { | 353 | + | if valid { |
| 381 | - | delete(c.syncBatches, p) | 354 | + | delete(c.syncBatches, p) |
| 382 | - | c.syncRegistryVersion++ | 355 | + | c.syncRegistryVersion++ |
| 383 | - | stopped = detachSyncBatchLocked(batch) | 356 | + | stopped = detachSyncBatchLocked(batch) |
| 384 | - | if len(c.syncBatches) == 0 && c.pending { | 357 | + | if len(c.syncBatches) == 0 && c.pending { |
| 385 | - | c.pendingUrgent = true | 358 | + | c.pendingUrgent = true |
| 386 | - | } | |||
| 387 | - | } | |||
| 388 | - | c.mu.Unlock() | |||
| 389 | - | if valid { | |||
| 390 | - | stopAndJoinTimerWorker(stopped, done) | |||
| 391 | - | // Reevaluate the aggregate gate only after forcing this pane. | |||
| 392 | - | c.fireCurrent(true) | |||
| 393 | } | 359 | } | ||
| 394 | - | }) | 360 | + | } |
| 361 | + | c.mu.Unlock() | |||
| 362 | + | if valid { | |||
| 363 | + | stopDetachedTimer(stopped) | |||
| 364 | + | // Reevaluate the aggregate gate only after forcing this pane. | |||
| 365 | + | c.fireCurrent(true) | |||
| 366 | + | } | |||
| 395 | } | 367 | } | ||
| 396 | 368 | ||||
| 397 | // noteSyncEnd records completion for exactly one pane batch. It only flushes | 369 | // noteSyncEnd records completion for exactly one pane batch. It only flushes | ||
| @@ -430,21 +402,21 @@ func (c *renderCoordinator) removeSyncEnd(p *pane, gen uint64, renderable bool) | |||||
| 430 | c.pendingUrgent = true | 402 | c.pendingUrgent = true | ||
| 431 | } | 403 | } | ||
| 432 | c.mu.Unlock() | 404 | c.mu.Unlock() | ||
| 433 | - | stopAndJoinTimerWorker(worker, nil) | 405 | + | stopDetachedTimer(worker) |
| 434 | return renderable | 406 | return renderable | ||
| 435 | } | 407 | } | ||
| 436 | 408 | ||||
| 437 | // noteSyncPaneRemoved releases a pane watchdog when pane lifecycle ends. | 409 | // noteSyncPaneRemoved releases a pane watchdog when pane lifecycle ends. | ||
| 438 | func (c *renderCoordinator) noteSyncPaneRemoved(p *pane) { | 410 | func (c *renderCoordinator) noteSyncPaneRemoved(p *pane) { | ||
| 439 | c.mu.Lock() | 411 | c.mu.Lock() | ||
| 440 | - | var worker timerWorker | 412 | + | var worker *timerToken |
| 441 | if batch := c.syncBatches[p]; batch != nil { | 413 | if batch := c.syncBatches[p]; batch != nil { | ||
| 442 | delete(c.syncBatches, p) | 414 | delete(c.syncBatches, p) | ||
| 443 | c.syncRegistryVersion++ | 415 | c.syncRegistryVersion++ | ||
| 444 | worker = detachSyncBatchLocked(batch) | 416 | worker = detachSyncBatchLocked(batch) | ||
| 445 | } | 417 | } | ||
| 446 | c.mu.Unlock() | 418 | c.mu.Unlock() | ||
| 447 | - | stopAndJoinTimerWorker(worker, nil) | 419 | + | stopDetachedTimer(worker) |
| 448 | } | 420 | } | ||
| 449 | 421 | ||||
| 450 | // subscribePreview installs fn as the preview observer for coalesced wakes. | 422 | // subscribePreview installs fn as the preview observer for coalesced wakes. | ||
| @@ -564,7 +536,7 @@ func (c *renderCoordinator) wakeCurrent(w renderWake) bool { | |||||
| 564 | } | 536 | } | ||
| 565 | 537 | ||||
| 566 | type renderLifecycleCleanup struct { | 538 | type renderLifecycleCleanup struct { | ||
| 567 | - | workers []timerWorker | 539 | + | tokens []*timerToken |
| 568 | observer ports.RuntimeObserver | 540 | observer ports.RuntimeObserver | ||
| 569 | queueCorrelation ports.RuntimeCorrelation | 541 | queueCorrelation ports.RuntimeCorrelation | ||
| 570 | queueMarked bool | 542 | queueMarked bool | ||
| @@ -572,7 +544,9 @@ type renderLifecycleCleanup struct { | |||||
| 572 | } | 544 | } | ||
| 573 | 545 | ||||
| 574 | func (cleanup renderLifecycleCleanup) finish() { | 546 | func (cleanup renderLifecycleCleanup) finish() { | ||
| 575 | - | stopAndJoinTimerWorkers(cleanup.workers) | 547 | + | for _, token := range cleanup.tokens { |
| 548 | + | stopDetachedTimer(token) | |||
| 549 | + | } | |||
| 576 | if cleanup.queueMarked && cleanup.observer != nil { | 550 | if cleanup.queueMarked && cleanup.observer != nil { | ||
| 577 | cleanup.observer.ObserveRuntime(ports.NewRuntimeMarkWithCorrelation("daemon", cleanup.queueCorrelation, ports.RuntimeQueueDequeued, 0, false)) | 551 | cleanup.observer.ObserveRuntime(ports.NewRuntimeMarkWithCorrelation("daemon", cleanup.queueCorrelation, ports.RuntimeQueueDequeued, 0, false)) | ||
| 578 | } | 552 | } | ||
| @@ -581,9 +555,7 @@ func (cleanup renderLifecycleCleanup) finish() { | |||||
| 581 | } | 555 | } | ||
| 582 | } | 556 | } | ||
| 583 | 557 | ||||
| 584 | - | // beginDetach invalidates pending work while leaving timer joins to the caller. | 558 | + | // beginDetach invalidates pending work. It never waits for detached workers. |
| 585 | - | // Callers holding daemon/session locks must publish the lifecycle transition, | |||
| 586 | - | // release those locks, and only then call cleanup.finish(). | |||
| 587 | func (c *renderCoordinator) beginDetach(ac *attachedClient) renderLifecycleCleanup { | 559 | func (c *renderCoordinator) beginDetach(ac *attachedClient) renderLifecycleCleanup { | ||
| 588 | c.mu.Lock() | 560 | c.mu.Lock() | ||
| 589 | var cleanup renderLifecycleCleanup | 561 | var cleanup renderLifecycleCleanup | ||
| @@ -595,12 +567,12 @@ func (c *renderCoordinator) beginDetach(ac *attachedClient) renderLifecycleClean | |||||
| 595 | c.ackDeferred = false | 567 | c.ackDeferred = false | ||
| 596 | c.pendingPreview = false | 568 | c.pendingPreview = false | ||
| 597 | c.coalesced = 0 | 569 | c.coalesced = 0 | ||
| 598 | - | _, timer := c.normalTimerOwner().replaceLocked() | 570 | + | _, timer := c.normalLane.replaceLocked() |
| 599 | c.armed = false | 571 | c.armed = false | ||
| 600 | - | _, resizeTimer := c.resizeTimerOwner().replaceLocked() | 572 | + | _, resizeTimer := c.resizeLane.replaceLocked() |
| 601 | - | _, retryTimer := c.retryTimerOwner().replaceLocked() | 573 | + | _, retryTimer := c.retryLane.replaceLocked() |
| 602 | cleanup = renderLifecycleCleanup{ | 574 | cleanup = renderLifecycleCleanup{ | ||
| 603 | - | workers: []timerWorker{timer, resizeTimer, retryTimer}, | 575 | + | tokens: []*timerToken{timer, resizeTimer, retryTimer}, |
| 604 | observer: c.opts.observer, | 576 | observer: c.opts.observer, | ||
| 605 | queueCorrelation: c.queueCorrelation, | 577 | queueCorrelation: c.queueCorrelation, | ||
| 606 | queueMarked: c.queueMarked, | 578 | queueMarked: c.queueMarked, | ||
| @@ -617,8 +589,8 @@ func (c *renderCoordinator) noteDetach(ac *attachedClient) { | |||||
| 617 | c.beginDetach(ac).finish() | 589 | c.beginDetach(ac).finish() | ||
| 618 | } | 590 | } | ||
| 619 | 591 | ||||
| 620 | - | // beginReplace hands the coordinator from old to replacement while leaving | 592 | + | // beginReplace hands the coordinator from old to replacement without waiting |
| 621 | - | // detached timer joins to the caller. | 593 | + | // for detached workers. |
| 622 | func (c *renderCoordinator) beginReplace(old, replacement *attachedClient, ready bool) renderLifecycleCleanup { | 594 | func (c *renderCoordinator) beginReplace(old, replacement *attachedClient, ready bool) renderLifecycleCleanup { | ||
| 623 | c.mu.Lock() | 595 | c.mu.Lock() | ||
| 624 | var cleanup renderLifecycleCleanup | 596 | var cleanup renderLifecycleCleanup | ||
| @@ -634,12 +606,12 @@ func (c *renderCoordinator) beginReplace(old, replacement *attachedClient, ready | |||||
| 634 | c.ackDeferred = false | 606 | c.ackDeferred = false | ||
| 635 | c.pendingPreview = false | 607 | c.pendingPreview = false | ||
| 636 | c.coalesced = 0 | 608 | c.coalesced = 0 | ||
| 637 | - | _, timer := c.normalTimerOwner().replaceLocked() | 609 | + | _, timer := c.normalLane.replaceLocked() |
| 638 | c.armed = false | 610 | c.armed = false | ||
| 639 | - | _, resizeTimer := c.resizeTimerOwner().replaceLocked() | 611 | + | _, resizeTimer := c.resizeLane.replaceLocked() |
| 640 | - | _, retryTimer := c.retryTimerOwner().replaceLocked() | 612 | + | _, retryTimer := c.retryLane.replaceLocked() |
| 641 | cleanup = renderLifecycleCleanup{ | 613 | cleanup = renderLifecycleCleanup{ | ||
| 642 | - | workers: []timerWorker{timer, resizeTimer, retryTimer}, | 614 | + | tokens: []*timerToken{timer, resizeTimer, retryTimer}, |
| 643 | observer: c.opts.observer, | 615 | observer: c.opts.observer, | ||
| 644 | queueCorrelation: c.queueCorrelation, | 616 | queueCorrelation: c.queueCorrelation, | ||
| 645 | queueMarked: c.queueMarked, | 617 | queueMarked: c.queueMarked, | ||
| @@ -663,9 +635,14 @@ func (c *renderCoordinator) noteReplace(old, replacement *attachedClient, readin | |||||
| 663 | // notePark invalidates pending wakes when the attachment parks for resume. | 635 | // notePark invalidates pending wakes when the attachment parks for resume. | ||
| 664 | func (c *renderCoordinator) notePark(ac *attachedClient) { c.noteDetach(ac) } | 636 | func (c *renderCoordinator) notePark(ac *attachedClient) { c.noteDetach(ac) } | ||
| 665 | 637 | ||||
| 666 | - | // noteSessionTeardown terminally invalidates the coordinator. | 638 | + | // beginSessionTeardown is terminal phase one. It rejects future worker |
| 667 | - | func (c *renderCoordinator) noteSessionTeardown() { | 639 | + | // registration and atomically detaches every complete lane token. |
| 640 | + | func (c *renderCoordinator) beginSessionTeardown() renderLifecycleCleanup { | |||
| 668 | c.mu.Lock() | 641 | c.mu.Lock() | ||
| 642 | + | if c.torndown { | |||
| 643 | + | c.mu.Unlock() | |||
| 644 | + | return renderLifecycleCleanup{} | |||
| 645 | + | } | |||
| 669 | c.torndown = true | 646 | c.torndown = true | ||
| 670 | if c.lease != nil { | 647 | if c.lease != nil { | ||
| 671 | c.lease.active = false | 648 | c.lease.active = false | ||
| @@ -673,29 +650,30 @@ func (c *renderCoordinator) noteSessionTeardown() { | |||||
| 673 | c.previewWake = nil | 650 | c.previewWake = nil | ||
| 674 | c.previewWakes = nil | 651 | c.previewWakes = nil | ||
| 675 | c.pending = false | 652 | c.pending = false | ||
| 676 | - | _, resizeTimer := c.resizeTimerOwner().replaceLocked() | 653 | + | _, resizeTimer := c.resizeLane.replaceLocked() |
| 677 | - | _, retryTimer := c.retryTimerOwner().replaceLocked() | 654 | + | _, retryTimer := c.retryLane.replaceLocked() |
| 678 | c.ackDeferred = false | 655 | c.ackDeferred = false | ||
| 679 | c.pendingPreview = false | 656 | c.pendingPreview = false | ||
| 680 | - | _, timer := c.normalTimerOwner().replaceLocked() | 657 | + | _, timer := c.normalLane.replaceLocked() |
| 681 | c.armed = false | 658 | c.armed = false | ||
| 682 | workers := c.detachSyncTimersLocked() | 659 | workers := c.detachSyncTimersLocked() | ||
| 683 | observer, queueCorrelation, queueMarked := c.opts.observer, c.queueCorrelation, c.queueMarked | 660 | observer, queueCorrelation, queueMarked := c.opts.observer, c.queueCorrelation, c.queueMarked | ||
| 684 | ackBlocked := c.ackBlocked | 661 | ackBlocked := c.ackBlocked | ||
| 685 | c.queueMarked, c.ackBlocked = false, nil | 662 | c.queueMarked, c.ackBlocked = false, nil | ||
| 686 | c.mu.Unlock() | 663 | c.mu.Unlock() | ||
| 687 | - | if queueMarked && observer != nil { | 664 | + | return renderLifecycleCleanup{ |
| 688 | - | observer.ObserveRuntime(ports.NewRuntimeMarkWithCorrelation("daemon", queueCorrelation, ports.RuntimeQueueDequeued, 0, false)) | 665 | + | tokens: append([]*timerToken{timer, resizeTimer, retryTimer}, workers...), |
| 666 | + | observer: observer, | |||
| 667 | + | queueCorrelation: queueCorrelation, | |||
| 668 | + | queueMarked: queueMarked, | |||
| 669 | + | ackBlocked: ackBlocked, | |||
| 689 | } | 670 | } | ||
| 690 | - | if ackBlocked != nil { | |||
| 691 | - | ackBlocked.finish(false) | |||
| 692 | - | } | |||
| 693 | - | stopAndJoinTimerWorker(timer, nil) | |||
| 694 | - | stopAndJoinTimerWorker(resizeTimer, nil) | |||
| 695 | - | stopAndJoinTimerWorker(retryTimer, nil) | |||
| 696 | - | stopAndJoinTimerWorkers(workers) | |||
| 697 | } | 671 | } | ||
| 698 | 672 | ||||
| 673 | + | // waitForTimerWorkers is terminal phase two. Only the session owner invokes it, | |||
| 674 | + | // after beginSessionTeardown has made future supervisor registration impossible. | |||
| 675 | + | func (c *renderCoordinator) waitForTimerWorkers() { c.supervisor.wait() } | |||
| 676 | + | ||||
| 699 | // invalidateRender is the sole producer fan-in. In tests and transitional | 677 | // invalidateRender is the sole producer fan-in. In tests and transitional | ||
| 700 | // headless paths without an attached coordinator it retains the old private | 678 | // headless paths without an attached coordinator it retains the old private | ||
| 701 | // compositor; attached sessions always schedule through their coordinator. | 679 | // compositor; attached sessions always schedule through their coordinator. | ||
| @@ -728,21 +706,26 @@ func (d *Daemon) invalidateRenderNow(sess *session, ac *attachedClient, reset bo | |||||
| 728 | } | 706 | } | ||
| 729 | 707 | ||||
| 730 | func (c *renderCoordinator) fire(gen uint64, watchdog, deadline bool) { | 708 | func (c *renderCoordinator) fire(gen uint64, watchdog, deadline bool) { | ||
| 731 | - | c.fireWithTimerWorker(gen, watchdog, deadline, nil) | 709 | + | c.mu.Lock() |
| 710 | + | if !watchdog && c.normalLane.generation != gen { | |||
| 711 | + | c.mu.Unlock() | |||
| 712 | + | return | |||
| 713 | + | } | |||
| 714 | + | token := c.normalLane.token | |||
| 715 | + | c.mu.Unlock() | |||
| 716 | + | c.fireWithTimerToken(token, gen, watchdog, deadline) | |||
| 732 | } | 717 | } | ||
| 733 | 718 | ||||
| 734 | - | // fireFromTimer consumes a normal deadline from its own worker. It must not | 719 | + | // fireFromTimer consumes a normal deadline using its exact immutable token. |
| 735 | - | // wait for that worker's done channel because runTimerWorker closes it only | 720 | + | func (c *renderCoordinator) fireFromTimer(token *timerToken, watchdog, deadline bool) { |
| 736 | - | // after this callback returns. | 721 | + | c.fireWithTimerToken(token, token.generation, watchdog, deadline) |
| 737 | - | func (c *renderCoordinator) fireFromTimer(gen uint64, watchdog, deadline bool, self <-chan struct{}) { | |||
| 738 | - | c.fireWithTimerWorker(gen, watchdog, deadline, self) | |||
| 739 | } | 722 | } | ||
| 740 | 723 | ||||
| 741 | - | func (c *renderCoordinator) fireWithTimerWorker(gen uint64, watchdog, deadline bool, self <-chan struct{}) { | 724 | + | func (c *renderCoordinator) fireWithTimerToken(token *timerToken, gen uint64, watchdog, deadline bool) { |
| 742 | // Readiness probes enter the attachment send path, which may in turn read | 725 | // Readiness probes enter the attachment send path, which may in turn read | ||
| 743 | // coordinator metadata. Never hold c.mu across that external callback. | 726 | // coordinator metadata. Never hold c.mu across that external callback. | ||
| 744 | c.mu.Lock() | 727 | c.mu.Lock() | ||
| 745 | - | if !c.fireValidLocked(gen, watchdog) { | 728 | + | if !c.fireValidLocked(token, gen, watchdog) { |
| 746 | c.mu.Unlock() | 729 | c.mu.Unlock() | ||
| 747 | return | 730 | return | ||
| 748 | } | 731 | } | ||
| @@ -759,7 +742,7 @@ func (c *renderCoordinator) fireWithTimerWorker(gen uint64, watchdog, deadline b | |||||
| 759 | // syncGateOpenLocked retries predicate evaluation on every registry change | 742 | // syncGateOpenLocked retries predicate evaluation on every registry change | ||
| 760 | // and returns holding c.mu, so pending consumption has no intervening gap. | 743 | // and returns holding c.mu, so pending consumption has no intervening gap. | ||
| 761 | c.mu.Lock() | 744 | c.mu.Lock() | ||
| 762 | - | if !c.fireValidLocked(gen, watchdog) { | 745 | + | if !c.fireValidLocked(token, gen, watchdog) { |
| 763 | c.mu.Unlock() | 746 | c.mu.Unlock() | ||
| 764 | return | 747 | return | ||
| 765 | } | 748 | } | ||
| @@ -767,7 +750,7 @@ func (c *renderCoordinator) fireWithTimerWorker(gen uint64, watchdog, deadline b | |||||
| 767 | c.mu.Unlock() | 750 | c.mu.Unlock() | ||
| 768 | return | 751 | return | ||
| 769 | } | 752 | } | ||
| 770 | - | if !c.fireValidLocked(gen, watchdog) { | 753 | + | if !c.fireValidLocked(token, gen, watchdog) { |
| 771 | c.mu.Unlock() | 754 | c.mu.Unlock() | ||
| 772 | return | 755 | return | ||
| 773 | } | 756 | } | ||
| @@ -821,7 +804,7 @@ func (c *renderCoordinator) fireWithTimerWorker(gen uint64, watchdog, deadline b | |||||
| 821 | c.queueMarked, c.ackBlocked = false, nil | 804 | c.queueMarked, c.ackBlocked = false, nil | ||
| 822 | wake := c.opts.wake | 805 | wake := c.opts.wake | ||
| 823 | c.mu.Unlock() | 806 | c.mu.Unlock() | ||
| 824 | - | stopAndJoinTimerWorker(worker, self) | 807 | + | stopDetachedTimer(worker) |
| 825 | if queueMarked && observer != nil { | 808 | if queueMarked && observer != nil { | ||
| 826 | observer.ObserveRuntime(ports.NewRuntimeMarkWithCorrelation("daemon", queueCorrelation, ports.RuntimeQueueDequeued, 0, true)) | 809 | observer.ObserveRuntime(ports.NewRuntimeMarkWithCorrelation("daemon", queueCorrelation, ports.RuntimeQueueDequeued, 0, true)) | ||
| 827 | } | 810 | } | ||
| @@ -834,8 +817,11 @@ func (c *renderCoordinator) fireWithTimerWorker(gen uint64, watchdog, deadline b | |||||
| 834 | c.notifyPreviews(w, preview, previews) | 817 | c.notifyPreviews(w, preview, previews) | ||
| 835 | } | 818 | } | ||
| 836 | 819 | ||||
| 837 | - | func (c *renderCoordinator) fireValidLocked(gen uint64, watchdog bool) bool { | 820 | + | func (c *renderCoordinator) fireValidLocked(token *timerToken, gen uint64, watchdog bool) bool { |
| 838 | - | return !c.torndown && c.pending && (watchdog || gen == c.generation) | 821 | + | if c.torndown || !c.pending || watchdog { |
| 822 | + | return !c.torndown && c.pending | |||
| 823 | + | } | |||
| 824 | + | return c.normalLane.generation == gen && (token == nil || c.normalLane.token == token) | |||
| 839 | } | 825 | } | ||
| 840 | 826 | ||||
| 841 | // syncGateOpen evaluates pane visibility without c.mu, then reacquires and | 827 | // syncGateOpen evaluates pane visibility without c.mu, then reacquires and | ||
M internal/usecase/daemon/render_coordinator_handshake_test.go +2 -2
| @@ -20,7 +20,7 @@ func TestRenderCoordinatorFreshWakeWaitsForWelcome(t *testing.T) { | |||
| 20 | 20 | h.rc.invalidate(renderInvalidation{class: invalidateOutput, reset: true, producer: "pty"}) | |
| 21 | 21 | timer := awaitCoordinatorScheduledTimer(t, h.clk) | |
| 22 | 22 | h.rc.mu.Lock() | |
| 23 | - | workerDone := h.rc.normalWorkerDone | |
| 23 | + | workerDone := h.rc.normalLane.token.done | |
| 24 | 24 | h.rc.mu.Unlock() | |
| 25 | 25 | require.NotNil(t, workerDone) | |
| 26 | 26 | timer.ch <- time.Time{} | |
| @@ -55,7 +55,7 @@ func TestRenderCoordinatorParkedResumeRequiresNewWelcome(t *testing.T) { | |||
| 55 | 55 | h.rc.invalidate(renderInvalidation{class: invalidateUrgent, reset: true, producer: "session"}) | |
| 56 | 56 | timer := awaitCoordinatorScheduledTimer(t, h.clk) | |
| 57 | 57 | h.rc.mu.Lock() | |
| 58 | - | workerDone := h.rc.normalWorkerDone | |
| 58 | + | workerDone := h.rc.normalLane.token.done | |
| 59 | 59 | h.rc.mu.Unlock() | |
| 60 | 60 | require.NotNil(t, workerDone) | |
| 61 | 61 | timer.ch <- time.Time{} | |
| @@ -20,7 +20,7 @@ func TestRenderCoordinatorFreshWakeWaitsForWelcome(t *testing.T) { | |||||
| 20 | h.rc.invalidate(renderInvalidation{class: invalidateOutput, reset: true, producer: "pty"}) | 20 | h.rc.invalidate(renderInvalidation{class: invalidateOutput, reset: true, producer: "pty"}) | ||
| 21 | timer := awaitCoordinatorScheduledTimer(t, h.clk) | 21 | timer := awaitCoordinatorScheduledTimer(t, h.clk) | ||
| 22 | h.rc.mu.Lock() | 22 | h.rc.mu.Lock() | ||
| 23 | - | workerDone := h.rc.normalWorkerDone | 23 | + | workerDone := h.rc.normalLane.token.done |
| 24 | h.rc.mu.Unlock() | 24 | h.rc.mu.Unlock() | ||
| 25 | require.NotNil(t, workerDone) | 25 | require.NotNil(t, workerDone) | ||
| 26 | timer.ch <- time.Time{} | 26 | timer.ch <- time.Time{} | ||
| @@ -55,7 +55,7 @@ func TestRenderCoordinatorParkedResumeRequiresNewWelcome(t *testing.T) { | |||||
| 55 | h.rc.invalidate(renderInvalidation{class: invalidateUrgent, reset: true, producer: "session"}) | 55 | h.rc.invalidate(renderInvalidation{class: invalidateUrgent, reset: true, producer: "session"}) | ||
| 56 | timer := awaitCoordinatorScheduledTimer(t, h.clk) | 56 | timer := awaitCoordinatorScheduledTimer(t, h.clk) | ||
| 57 | h.rc.mu.Lock() | 57 | h.rc.mu.Lock() | ||
| 58 | - | workerDone := h.rc.normalWorkerDone | 58 | + | workerDone := h.rc.normalLane.token.done |
| 59 | h.rc.mu.Unlock() | 59 | h.rc.mu.Unlock() | ||
| 60 | require.NotNil(t, workerDone) | 60 | require.NotNil(t, workerDone) | ||
| 61 | timer.ch <- time.Time{} | 61 | timer.ch <- time.Time{} | ||
M internal/usecase/daemon/render_coordinator_support.go +1 -1
| @@ -21,7 +21,7 @@ func (s *session) installRenderCoordinator(rc *renderCoordinator) { s.coordinato | |||
| 21 | 21 | ||
| 22 | 22 | func (c *renderCoordinator) fireCurrent(watchdog bool) { | |
| 23 | 23 | c.mu.Lock() | |
| 24 | - | gen := c.generation | |
| 24 | + | gen := c.normalLane.generation | |
| 25 | 25 | c.mu.Unlock() | |
| 26 | 26 | c.fire(gen, watchdog, watchdog) | |
| 27 | 27 | } | |
| @@ -21,7 +21,7 @@ func (s *session) installRenderCoordinator(rc *renderCoordinator) { s.coordinato | |||||
| 21 | 21 | ||||
| 22 | func (c *renderCoordinator) fireCurrent(watchdog bool) { | 22 | func (c *renderCoordinator) fireCurrent(watchdog bool) { | ||
| 23 | c.mu.Lock() | 23 | c.mu.Lock() | ||
| 24 | - | gen := c.generation | 24 | + | gen := c.normalLane.generation |
| 25 | c.mu.Unlock() | 25 | c.mu.Unlock() | ||
| 26 | c.fire(gen, watchdog, watchdog) | 26 | c.fire(gen, watchdog, watchdog) | ||
| 27 | } | 27 | } | ||
M internal/usecase/daemon/render_coordinator_test.go +67 -72
| @@ -25,7 +25,6 @@ import ( | |||
| 25 | 25 | type coordinatorMockClock struct { | |
| 26 | 26 | clock *portsmocks.MockClock | |
| 27 | 27 | timers chan *coordinatorMockTimer | |
| 28 | - | inert bool | |
| 29 | 28 | } | |
| 30 | 29 | ||
| 31 | 30 | type coordinatorMockTimer struct { | |
| @@ -52,9 +51,7 @@ func newCoordinatorMockClock(t *testing.T, capacity int) *coordinatorMockClock { | |||
| 52 | 51 | mock: portsmocks.NewMockTimer(t), | |
| 53 | 52 | duration: d, | |
| 54 | 53 | } | |
| 55 | - | if !clk.inert { | |
| 56 | - | timer.ch = make(chan time.Time, 1) | |
| 57 | - | } | |
| 54 | + | timer.ch = make(chan time.Time, 1) | |
| 58 | 55 | // A normal deadline reads C both in its worker and in the inert-clock | |
| 59 | 56 | // guard; a watchdog reads it in its worker. The generated expectation | |
| 60 | 57 | // makes every channel access observable without a hand-written Timer. | |
| @@ -66,22 +63,14 @@ func newCoordinatorMockClock(t *testing.T, capacity int) *coordinatorMockClock { | |||
| 66 | 63 | return clk | |
| 67 | 64 | } | |
| 68 | 65 | ||
| 69 | - | func newInertCoordinatorMockClock(t *testing.T, capacity int) *coordinatorMockClock { | |
| 70 | - | clk := newCoordinatorMockClock(t, capacity) | |
| 71 | - | clk.inert = true | |
| 72 | - | return clk | |
| 73 | - | } | |
| 74 | - | ||
| 75 | 66 | // TestRenderCoordinatorTimerCallbacksReenterCoordinator is deliberately a | |
| 76 | 67 | // deadlock regression: every external timer method synchronously reads | |
| 77 | 68 | // coordinator metadata. Before two-phase timer ownership this blocked while | |
| 78 | 69 | // c.mu was held during normal arming, urgent replacement, sync arming, and | |
| 79 | 70 | // detach cancellation. | |
| 80 | - | // TestRenderCoordinatorDetachJoinsSelectedDeadlineWorker reproduces the | |
| 81 | - | // lifecycle race exposed by timerOwnership extraction. A timer may select its | |
| 82 | - | // tick just before detach cancels it; cancellation alone does not prevent the | |
| 83 | - | // worker from entering an external readiness callback after lifecycle cleanup. | |
| 84 | - | func TestRenderCoordinatorDetachJoinsSelectedDeadlineWorker(t *testing.T) { | |
| 71 | + | // Detach cancels and stops a selected worker but never synchronously joins it; | |
| 72 | + | // the stale worker must be rejected when its external readiness probe returns. | |
| 73 | + | func TestRenderCoordinatorDetachDoesNotJoinSelectedDeadlineWorker(t *testing.T) { | |
| 85 | 74 | clock := portsmocks.NewMockClock(t) | |
| 86 | 75 | timer := portsmocks.NewMockTimer(t) | |
| 87 | 76 | timerC := make(chan time.Time, 1) | |
| @@ -112,14 +101,8 @@ func TestRenderCoordinatorDetachJoinsSelectedDeadlineWorker(t *testing.T) { | |||
| 112 | 101 | close(detached) | |
| 113 | 102 | }() | |
| 114 | 103 | <-stopCalled | |
| 115 | - | select { | |
| 116 | - | case <-detached: | |
| 117 | - | t.Fatal("detach returned before the selected deadline worker completed") | |
| 118 | - | default: | |
| 119 | - | } | |
| 120 | - | ||
| 104 | + | awaitTestCompletion(t, detached, "detach synchronously waited for a selected timer worker") | |
| 121 | 105 | close(release) | |
| 122 | - | awaitTestCompletion(t, detached, "detach did not complete after the deadline worker was released") | |
| 123 | 106 | } | |
| 124 | 107 | ||
| 125 | 108 | func TestRenderCoordinatorTimerCallbacksReenterCoordinator(t *testing.T) { | |
| @@ -224,13 +207,14 @@ func requireNoInvalidation(t *testing.T, ch chan renderInvalidation) { | |||
| 224 | 207 | } | |
| 225 | 208 | } | |
| 226 | 209 | ||
| 227 | - | func awaitResizeCallback(t *testing.T, rc *renderCoordinator) { | |
| 210 | + | func captureResizeCallbackDone(t *testing.T, rc *renderCoordinator) <-chan struct{} { | |
| 228 | 211 | t.Helper() | |
| 229 | - | done := rc.resizeCallbackDone() | |
| 230 | - | if done == nil { | |
| 212 | + | rc.mu.Lock() | |
| 213 | + | defer rc.mu.Unlock() | |
| 214 | + | if rc.resizeLane.token == nil { | |
| 231 | 215 | t.Fatal("coordinator did not publish a resize callback completion") | |
| 232 | 216 | } | |
| 233 | - | <-done | |
| 217 | + | return rc.resizeLane.token.done | |
| 234 | 218 | } | |
| 235 | 219 | ||
| 236 | 220 | func awaitCoordinatorScheduledTimer(t *testing.T, clk *coordinatorMockClock) *coordinatorMockTimer { | |
| @@ -474,7 +458,7 @@ func TestRenderCoordinatorAckFlushesOnlyExpiredAckDeferredWork(t *testing.T) { | |||
| 474 | 458 | timer := awaitCoordinatorScheduledTimer(t, h.clk) | |
| 475 | 459 | timer.ch <- time.Time{} | |
| 476 | 460 | requireNoWake(t, h.wakes) | |
| 477 | - | h.rc.noteSessionTeardown() | |
| 461 | + | h.rc.beginSessionTeardown().finish() | |
| 478 | 462 | h.ackReady.Store(true) | |
| 479 | 463 | h.rc.notifyAck() | |
| 480 | 464 | requireNoWake(t, h.wakes) | |
| @@ -673,7 +657,7 @@ func TestRenderCoordinatorSynchronizedOutput(t *testing.T) { | |||
| 673 | 657 | timers := h.armedTimers(t) | |
| 674 | 658 | require.Len(t, timers, 1) | |
| 675 | 659 | h.rc.mu.Lock() | |
| 676 | - | normalWorkerDone := h.rc.normalWorkerDone | |
| 660 | + | normalWorkerDone := h.rc.normalLane.token.done | |
| 677 | 661 | h.rc.mu.Unlock() | |
| 678 | 662 | require.NotNil(t, normalWorkerDone) | |
| 679 | 663 | timers[0].ch <- time.Time{} | |
| @@ -728,6 +712,25 @@ func TestRenderCoordinatorSynchronizedOutput(t *testing.T) { | |||
| 728 | 712 | requireNoWake(t, h.wakes) | |
| 729 | 713 | }) | |
| 730 | 714 | ||
| 715 | + | t.Run("watchdog flushes after the snapshotted generation is replaced", func(t *testing.T) { | |
| 716 | + | h := newCoordinatorHarness(t) | |
| 717 | + | h.rc.invalidate(renderInvalidation{class: invalidateOutput}) | |
| 718 | + | h.rc.mu.Lock() | |
| 719 | + | staleGeneration := h.rc.normalLane.generation | |
| 720 | + | h.rc.mu.Unlock() | |
| 721 | + | ||
| 722 | + | // Simulate a new urgent publication after fireCurrent(true) snapshots its | |
| 723 | + | // generation but before fire validates it. | |
| 724 | + | h.rc.invalidate(renderInvalidation{class: invalidateUrgent}) | |
| 725 | + | h.rc.fire(staleGeneration, true, true) | |
| 726 | + | ||
| 727 | + | w := awaitWake(t, h.wakes) | |
| 728 | + | require.True(t, w.watchdog) | |
| 729 | + | require.True(t, w.urgent) | |
| 730 | + | require.Equal(t, 2, w.coalesced) | |
| 731 | + | requireNoWake(t, h.wakes) | |
| 732 | + | }) | |
| 733 | + | ||
| 731 | 734 | t.Run("stale watchdog generation cannot wake a completed batch", func(t *testing.T) { | |
| 732 | 735 | h := newCoordinatorHarness(t) | |
| 733 | 736 | h.syncActive.Store(true) | |
| @@ -870,7 +873,7 @@ func TestRenderCoordinatorPreviewLifecycleDropsStaleTargetWakes(t *testing.T) { | |||
| 870 | 873 | }{ | |
| 871 | 874 | {"target detach", func(rc *renderCoordinator, target, _ *attachedClient) { rc.noteDetach(target) }}, | |
| 872 | 875 | {"target replacement", func(rc *renderCoordinator, target, replacement *attachedClient) { rc.noteReplace(target, replacement) }}, | |
| 873 | - | {"target teardown", func(rc *renderCoordinator, _ *attachedClient, _ *attachedClient) { rc.noteSessionTeardown() }}, | |
| 876 | + | {"target teardown", func(rc *renderCoordinator, _ *attachedClient, _ *attachedClient) { rc.beginSessionTeardown().finish() }}, | |
| 874 | 877 | } | |
| 875 | 878 | for _, tc := range cases { | |
| 876 | 879 | t.Run(tc.name, func(t *testing.T) { | |
| @@ -918,7 +921,7 @@ func TestRenderCoordinatorLifecycleDropsStaleWakes(t *testing.T) { | |||
| 918 | 921 | }{ | |
| 919 | 922 | {"detach", func(rc *renderCoordinator, owner *attachedClient) { rc.noteDetach(owner) }}, | |
| 920 | 923 | {"park", func(rc *renderCoordinator, owner *attachedClient) { rc.notePark(owner) }}, | |
| 921 | - | {"session teardown", func(rc *renderCoordinator, _ *attachedClient) { rc.noteSessionTeardown() }}, | |
| 924 | + | {"session teardown", func(rc *renderCoordinator, _ *attachedClient) { rc.beginSessionTeardown().finish() }}, | |
| 922 | 925 | } | |
| 923 | 926 | for _, tc := range cases { | |
| 924 | 927 | t.Run(tc.name, func(t *testing.T) { | |
| @@ -1034,7 +1037,7 @@ func TestRenderCoordinatorResizeMetadata(t *testing.T) { | |||
| 1034 | 1037 | case "park": | |
| 1035 | 1038 | h.rc.notePark(staleOwner) | |
| 1036 | 1039 | case "session teardown": | |
| 1037 | - | h.rc.noteSessionTeardown() | |
| 1040 | + | h.rc.beginSessionTeardown().finish() | |
| 1038 | 1041 | case "replacement": | |
| 1039 | 1042 | h.rc.noteReplace(staleOwner, freshOwner) | |
| 1040 | 1043 | h.rc.attach(freshOwner) | |
| @@ -1168,8 +1171,10 @@ func TestProducerInvalidations(t *testing.T) { | |||
| 1168 | 1171 | // Daemon's construction-time clock afterwards. | |
| 1169 | 1172 | sess.renderCoordinator().opts.clock = clk.clock | |
| 1170 | 1173 | d.resize(sess, ac, domain.Size{Cols: 100, Rows: 26}) | |
| 1171 | - | awaitCoordinatorScheduledTimer(t, clk).ch <- time.Time{} | |
| 1172 | - | awaitResizeCallback(t, sess.renderCoordinator()) | |
| 1174 | + | timer := awaitCoordinatorScheduledTimer(t, clk) | |
| 1175 | + | done := captureResizeCallbackDone(t, sess.renderCoordinator()) | |
| 1176 | + | timer.ch <- time.Time{} | |
| 1177 | + | awaitTestCompletion(t, done, "resize callback did not complete") | |
| 1173 | 1178 | }, | |
| 1174 | 1179 | }, | |
| 1175 | 1180 | { | |
| @@ -1314,26 +1319,18 @@ func TestCoordinatorDeadlineCannotPaintPublishedReplacementBeforeOwnershipInstal | |||
| 1314 | 1319 | requireNoCoordinatorOutputFrame(t, ownerSends) | |
| 1315 | 1320 | } | |
| 1316 | 1321 | ||
| 1317 | - | func TestRenderCoordinatorClearResizeTimerKeepsNewerTimer(t *testing.T) { | |
| 1322 | + | func TestRenderCoordinatorResizeLaneRejectsStaleToken(t *testing.T) { | |
| 1318 | 1323 | rc := newRenderCoordinator(renderCoordinatorOptions{}) | |
| 1319 | - | newer := &portsmocks.MockTimer{} | |
| 1320 | - | ||
| 1321 | - | rc.mu.Lock() | |
| 1322 | - | rc.resizeGen = 2 | |
| 1323 | - | rc.resizeTimer = newer | |
| 1324 | - | rc.resizeCancel = make(chan struct{}) | |
| 1325 | - | rc.mu.Unlock() | |
| 1324 | + | newer := &timerToken{generation: 2, timer: &portsmocks.MockTimer{}, cancel: make(chan struct{}), done: make(chan struct{})} | |
| 1325 | + | stale := &timerToken{generation: 1} | |
| 1326 | 1326 | ||
| 1327 | - | rc.clearResizeTimer(1) | |
| 1328 | 1327 | rc.mu.Lock() | |
| 1329 | - | require.Same(t, newer, rc.resizeTimer) | |
| 1330 | - | require.NotNil(t, rc.resizeCancel) | |
| 1331 | - | rc.mu.Unlock() | |
| 1332 | - | ||
| 1333 | - | rc.clearResizeTimer(2) | |
| 1334 | - | rc.mu.Lock() | |
| 1335 | - | require.Nil(t, rc.resizeTimer) | |
| 1336 | - | require.Nil(t, rc.resizeCancel) | |
| 1328 | + | rc.resizeLane.generation = 2 | |
| 1329 | + | rc.resizeLane.token = newer | |
| 1330 | + | require.False(t, rc.resizeLane.clearLocked(stale)) | |
| 1331 | + | require.Same(t, newer, rc.resizeLane.token) | |
| 1332 | + | require.True(t, rc.resizeLane.clearLocked(newer)) | |
| 1333 | + | require.Nil(t, rc.resizeLane.token) | |
| 1337 | 1334 | rc.mu.Unlock() | |
| 1338 | 1335 | } | |
| 1339 | 1336 | ||
| @@ -1350,16 +1347,9 @@ func TestRenderCoordinatorNilClockRetryCompletesWithoutSynchronousRun(t *testing | |||
| 1350 | 1347 | rc.scheduleResizeRetryForLease(epoch, owner, lease, func() { called = true }) | |
| 1351 | 1348 | require.False(t, called, "a nil clock is a disabled retry lane, not a synchronous retry") | |
| 1352 | 1349 | rc.mu.Lock() | |
| 1353 | - | retryTimer, retryCancel, retryDone := rc.retryTimer, rc.retryCancel, rc.retryDone | |
| 1350 | + | retryToken := rc.retryLane.token | |
| 1354 | 1351 | rc.mu.Unlock() | |
| 1355 | - | require.Nil(t, retryTimer) | |
| 1356 | - | require.Nil(t, retryCancel) | |
| 1357 | - | require.NotNil(t, retryDone) | |
| 1358 | - | select { | |
| 1359 | - | case <-retryDone: | |
| 1360 | - | default: | |
| 1361 | - | t.Fatal("nil-clock retry did not publish its completion edge") | |
| 1362 | - | } | |
| 1352 | + | require.Nil(t, retryToken, "a disabled retry lane must not retain stale completion ownership") | |
| 1363 | 1353 | } | |
| 1364 | 1354 | ||
| 1365 | 1355 | func TestRenderCoordinatorInertTimerFiresSynchronouslyWithoutWorker(t *testing.T) { | |
| @@ -1380,9 +1370,7 @@ func TestRenderCoordinatorInertTimerFiresSynchronouslyWithoutWorker(t *testing.T | |||
| 1380 | 1370 | require.Equal(t, renderWake{coalesced: 1}, <-wakes) | |
| 1381 | 1371 | require.Equal(t, renderCoordinatorBurstMetricsSnapshot{invalidations: 1, wakes: 1, coalesced: 1}, rc.burstMetricsSnapshot()) | |
| 1382 | 1372 | rc.mu.Lock() | |
| 1383 | - | require.Nil(t, rc.normalTimer) | |
| 1384 | - | require.Nil(t, rc.normalCancel) | |
| 1385 | - | require.Nil(t, rc.normalWorkerDone) | |
| 1373 | + | require.Nil(t, rc.normalLane.token) | |
| 1386 | 1374 | rc.mu.Unlock() | |
| 1387 | 1375 | } | |
| 1388 | 1376 | ||
| @@ -1410,7 +1398,7 @@ func TestRenderCoordinatorSyncBatchSurvivesAttachmentLifecycle(t *testing.T) { | |||
| 1410 | 1398 | timers := h.armedTimers(t) | |
| 1411 | 1399 | require.Len(t, timers, 1) | |
| 1412 | 1400 | h.rc.mu.Lock() | |
| 1413 | - | normalWorkerDone := h.rc.normalWorkerDone | |
| 1401 | + | normalWorkerDone := h.rc.normalLane.token.done | |
| 1414 | 1402 | h.rc.mu.Unlock() | |
| 1415 | 1403 | require.NotNil(t, normalWorkerDone) | |
| 1416 | 1404 | timers[0].ch <- time.Time{} | |
| @@ -1418,7 +1406,7 @@ func TestRenderCoordinatorSyncBatchSurvivesAttachmentLifecycle(t *testing.T) { | |||
| 1418 | 1406 | requireNoWake(t, h.previews) | |
| 1419 | 1407 | ||
| 1420 | 1408 | h.rc.mu.Lock() | |
| 1421 | - | require.Same(t, h.rc.syncBatches[p].timer, watchdog.mock) | |
| 1409 | + | require.Same(t, h.rc.syncBatches[p].lane.token.timer, watchdog.mock) | |
| 1422 | 1410 | h.rc.mu.Unlock() | |
| 1423 | 1411 | h.rc.noteSyncEnd(p, 1) | |
| 1424 | 1412 | preview := awaitWake(t, h.previews) | |
| @@ -1446,7 +1434,7 @@ func TestRenderCoordinatorSyncBatchSurvivesAttachmentLifecycle(t *testing.T) { | |||
| 1446 | 1434 | timers := h.armedTimers(t) | |
| 1447 | 1435 | require.Len(t, timers, 1) | |
| 1448 | 1436 | h.rc.mu.Lock() | |
| 1449 | - | normalWorkerDone := h.rc.normalWorkerDone | |
| 1437 | + | normalWorkerDone := h.rc.normalLane.token.done | |
| 1450 | 1438 | h.rc.mu.Unlock() | |
| 1451 | 1439 | require.NotNil(t, normalWorkerDone) | |
| 1452 | 1440 | timers[0].ch <- time.Time{} | |
| @@ -1454,7 +1442,7 @@ func TestRenderCoordinatorSyncBatchSurvivesAttachmentLifecycle(t *testing.T) { | |||
| 1454 | 1442 | requireNoWake(t, h.wakes) | |
| 1455 | 1443 | ||
| 1456 | 1444 | h.rc.mu.Lock() | |
| 1457 | - | require.Same(t, h.rc.syncBatches[p].timer, watchdog.mock) | |
| 1445 | + | require.Same(t, h.rc.syncBatches[p].lane.token.timer, watchdog.mock) | |
| 1458 | 1446 | h.rc.mu.Unlock() | |
| 1459 | 1447 | h.rc.noteSyncEnd(p, 1) | |
| 1460 | 1448 | wake := awaitWake(t, h.wakes) | |
| @@ -1470,20 +1458,23 @@ func TestRenderCoordinatorSyncBatchSurvivesAttachmentLifecycle(t *testing.T) { | |||
| 1470 | 1458 | end func(*renderCoordinator, *pane) | |
| 1471 | 1459 | }{ | |
| 1472 | 1460 | {"pane removal", func(rc *renderCoordinator, p *pane) { rc.noteSyncPaneRemoved(p) }}, | |
| 1473 | - | {"session teardown", func(rc *renderCoordinator, _ *pane) { rc.noteSessionTeardown() }}, | |
| 1461 | + | {"session teardown", func(rc *renderCoordinator, _ *pane) { rc.beginSessionTeardown().finish() }}, | |
| 1474 | 1462 | } { | |
| 1475 | 1463 | t.Run(lifecycle.name, func(t *testing.T) { | |
| 1476 | - | clk := newInertCoordinatorMockClock(t, 2) | |
| 1464 | + | clk := newCoordinatorMockClock(t, 2) | |
| 1477 | 1465 | rc := newRenderCoordinator(renderCoordinatorOptions{clock: clk.clock}) | |
| 1478 | 1466 | p := &pane{} | |
| 1479 | 1467 | rc.noteSyncBegin(p, 1) | |
| 1480 | 1468 | syncTimer := <-clk.timers | |
| 1481 | 1469 | rc.mu.Lock() | |
| 1482 | - | syncDone := rc.syncBatches[p].done | |
| 1470 | + | syncDone := rc.syncBatches[p].lane.token.done | |
| 1483 | 1471 | rc.mu.Unlock() | |
| 1484 | 1472 | lifecycle.end(rc, p) | |
| 1485 | 1473 | syncTimer.mock.AssertNumberOfCalls(t, "Stop", 1) | |
| 1486 | 1474 | requireWorkerExit(t, syncDone) | |
| 1475 | + | if lifecycle.name == "session teardown" { | |
| 1476 | + | rc.waitForTimerWorkers() | |
| 1477 | + | } | |
| 1487 | 1478 | }) | |
| 1488 | 1479 | } | |
| 1489 | 1480 | }) | |
| @@ -1522,8 +1513,9 @@ func TestRenderCoordinatorResizeEpochDispatch(t *testing.T) { | |||
| 1522 | 1513 | require.Equal(t, uint64(1), snap.epoch) | |
| 1523 | 1514 | requireNoCoordinatorOutputFrame(t, sends) | |
| 1524 | 1515 | ||
| 1516 | + | done := captureResizeCallbackDone(t, sess.renderCoordinator()) | |
| 1525 | 1517 | timer.ch <- time.Time{} | |
| 1526 | - | awaitResizeCallback(t, sess.renderCoordinator()) | |
| 1518 | + | awaitTestCompletion(t, done, "resize callback did not complete") | |
| 1527 | 1519 | inv := awaitInvalidation(t, invs) | |
| 1528 | 1520 | require.True(t, inv.reset, "the resize dispatch must request a full-redraw invalidation") | |
| 1529 | 1521 | requireNoInvalidation(t, invs) | |
| @@ -1540,12 +1532,13 @@ func TestRenderCoordinatorResizeEpochDispatch(t *testing.T) { | |||
| 1540 | 1532 | require.Equal(t, uint64(2), sess.renderCoordinator().resizeSnapshot().epoch, | |
| 1541 | 1533 | "every resize request must advance the coordinator epoch") | |
| 1542 | 1534 | ||
| 1535 | + | done := captureResizeCallbackDone(t, sess.renderCoordinator()) | |
| 1543 | 1536 | first.ch <- time.Time{} | |
| 1544 | 1537 | requireNoInvalidation(t, invs) | |
| 1545 | 1538 | requireNoCoordinatorOutputFrame(t, sends) | |
| 1546 | 1539 | ||
| 1547 | 1540 | latest.ch <- time.Time{} | |
| 1548 | - | awaitResizeCallback(t, sess.renderCoordinator()) | |
| 1541 | + | awaitTestCompletion(t, done, "latest resize callback did not complete") | |
| 1549 | 1542 | inv := awaitInvalidation(t, invs) | |
| 1550 | 1543 | require.True(t, inv.reset) | |
| 1551 | 1544 | requireNoInvalidation(t, invs) | |
| @@ -1558,9 +1551,11 @@ func TestRenderCoordinatorResizeEpochDispatch(t *testing.T) { | |||
| 1558 | 1551 | ||
| 1559 | 1552 | d.resize(sess, ac, domain.Size{Cols: 90, Rows: 30}) | |
| 1560 | 1553 | timer := awaitCoordinatorScheduledTimer(t, clk) | |
| 1554 | + | done := captureResizeCallbackDone(t, sess.renderCoordinator()) | |
| 1561 | 1555 | sess.renderCoordinator().noteDetach(ac) | |
| 1562 | 1556 | ||
| 1563 | 1557 | timer.ch <- time.Time{} | |
| 1558 | + | awaitTestCompletion(t, done, "cancelled resize callback did not complete") | |
| 1564 | 1559 | requireNoInvalidation(t, invs) | |
| 1565 | 1560 | requireNoCoordinatorOutputFrame(t, sends) | |
| 1566 | 1561 | }) | |
| @@ -1612,7 +1607,7 @@ func TestRenderCoordinatorResizeEpochDispatch(t *testing.T) { | |||
| 1612 | 1607 | awaitCoordinatorScheduledTimer(t, clk) | |
| 1613 | 1608 | ||
| 1614 | 1609 | // A torn-down coordinator cannot accept a stale attachment's schedule. | |
| 1615 | - | sess.renderCoordinator().noteSessionTeardown() | |
| 1610 | + | sess.renderCoordinator().beginSessionTeardown().finish() | |
| 1616 | 1611 | require.False(t, d.requestTransactionalResize(sess, ac, domain.Size{Cols: 140, Rows: 30}, false)) | |
| 1617 | 1612 | }) | |
| 1618 | 1613 | }) | |
| @@ -25,7 +25,6 @@ import ( | |||||
| 25 | type coordinatorMockClock struct { | 25 | type coordinatorMockClock struct { | ||
| 26 | clock *portsmocks.MockClock | 26 | clock *portsmocks.MockClock | ||
| 27 | timers chan *coordinatorMockTimer | 27 | timers chan *coordinatorMockTimer | ||
| 28 | - | inert bool | |||
| 29 | } | 28 | } | ||
| 30 | 29 | ||||
| 31 | type coordinatorMockTimer struct { | 30 | type coordinatorMockTimer struct { | ||
| @@ -52,9 +51,7 @@ func newCoordinatorMockClock(t *testing.T, capacity int) *coordinatorMockClock { | |||||
| 52 | mock: portsmocks.NewMockTimer(t), | 51 | mock: portsmocks.NewMockTimer(t), | ||
| 53 | duration: d, | 52 | duration: d, | ||
| 54 | } | 53 | } | ||
| 55 | - | if !clk.inert { | 54 | + | timer.ch = make(chan time.Time, 1) |
| 56 | - | timer.ch = make(chan time.Time, 1) | |||
| 57 | - | } | |||
| 58 | // A normal deadline reads C both in its worker and in the inert-clock | 55 | // A normal deadline reads C both in its worker and in the inert-clock | ||
| 59 | // guard; a watchdog reads it in its worker. The generated expectation | 56 | // guard; a watchdog reads it in its worker. The generated expectation | ||
| 60 | // makes every channel access observable without a hand-written Timer. | 57 | // makes every channel access observable without a hand-written Timer. | ||
| @@ -66,22 +63,14 @@ func newCoordinatorMockClock(t *testing.T, capacity int) *coordinatorMockClock { | |||||
| 66 | return clk | 63 | return clk | ||
| 67 | } | 64 | } | ||
| 68 | 65 | ||||
| 69 | - | func newInertCoordinatorMockClock(t *testing.T, capacity int) *coordinatorMockClock { | |||
| 70 | - | clk := newCoordinatorMockClock(t, capacity) | |||
| 71 | - | clk.inert = true | |||
| 72 | - | return clk | |||
| 73 | - | } | |||
| 74 | - | ||||
| 75 | // TestRenderCoordinatorTimerCallbacksReenterCoordinator is deliberately a | 66 | // TestRenderCoordinatorTimerCallbacksReenterCoordinator is deliberately a | ||
| 76 | // deadlock regression: every external timer method synchronously reads | 67 | // deadlock regression: every external timer method synchronously reads | ||
| 77 | // coordinator metadata. Before two-phase timer ownership this blocked while | 68 | // coordinator metadata. Before two-phase timer ownership this blocked while | ||
| 78 | // c.mu was held during normal arming, urgent replacement, sync arming, and | 69 | // c.mu was held during normal arming, urgent replacement, sync arming, and | ||
| 79 | // detach cancellation. | 70 | // detach cancellation. | ||
| 80 | - | // TestRenderCoordinatorDetachJoinsSelectedDeadlineWorker reproduces the | 71 | + | // Detach cancels and stops a selected worker but never synchronously joins it; |
| 81 | - | // lifecycle race exposed by timerOwnership extraction. A timer may select its | 72 | + | // the stale worker must be rejected when its external readiness probe returns. |
| 82 | - | // tick just before detach cancels it; cancellation alone does not prevent the | 73 | + | func TestRenderCoordinatorDetachDoesNotJoinSelectedDeadlineWorker(t *testing.T) { |
| 83 | - | // worker from entering an external readiness callback after lifecycle cleanup. | |||
| 84 | - | func TestRenderCoordinatorDetachJoinsSelectedDeadlineWorker(t *testing.T) { | |||
| 85 | clock := portsmocks.NewMockClock(t) | 74 | clock := portsmocks.NewMockClock(t) | ||
| 86 | timer := portsmocks.NewMockTimer(t) | 75 | timer := portsmocks.NewMockTimer(t) | ||
| 87 | timerC := make(chan time.Time, 1) | 76 | timerC := make(chan time.Time, 1) | ||
| @@ -112,14 +101,8 @@ func TestRenderCoordinatorDetachJoinsSelectedDeadlineWorker(t *testing.T) { | |||||
| 112 | close(detached) | 101 | close(detached) | ||
| 113 | }() | 102 | }() | ||
| 114 | <-stopCalled | 103 | <-stopCalled | ||
| 115 | - | select { | 104 | + | awaitTestCompletion(t, detached, "detach synchronously waited for a selected timer worker") |
| 116 | - | case <-detached: | |||
| 117 | - | t.Fatal("detach returned before the selected deadline worker completed") | |||
| 118 | - | default: | |||
| 119 | - | } | |||
| 120 | - | ||||
| 121 | close(release) | 105 | close(release) | ||
| 122 | - | awaitTestCompletion(t, detached, "detach did not complete after the deadline worker was released") | |||
| 123 | } | 106 | } | ||
| 124 | 107 | ||||
| 125 | func TestRenderCoordinatorTimerCallbacksReenterCoordinator(t *testing.T) { | 108 | func TestRenderCoordinatorTimerCallbacksReenterCoordinator(t *testing.T) { | ||
| @@ -224,13 +207,14 @@ func requireNoInvalidation(t *testing.T, ch chan renderInvalidation) { | |||||
| 224 | } | 207 | } | ||
| 225 | } | 208 | } | ||
| 226 | 209 | ||||
| 227 | - | func awaitResizeCallback(t *testing.T, rc *renderCoordinator) { | 210 | + | func captureResizeCallbackDone(t *testing.T, rc *renderCoordinator) <-chan struct{} { |
| 228 | t.Helper() | 211 | t.Helper() | ||
| 229 | - | done := rc.resizeCallbackDone() | 212 | + | rc.mu.Lock() |
| 230 | - | if done == nil { | 213 | + | defer rc.mu.Unlock() |
| 214 | + | if rc.resizeLane.token == nil { | |||
| 231 | t.Fatal("coordinator did not publish a resize callback completion") | 215 | t.Fatal("coordinator did not publish a resize callback completion") | ||
| 232 | } | 216 | } | ||
| 233 | - | <-done | 217 | + | return rc.resizeLane.token.done |
| 234 | } | 218 | } | ||
| 235 | 219 | ||||
| 236 | func awaitCoordinatorScheduledTimer(t *testing.T, clk *coordinatorMockClock) *coordinatorMockTimer { | 220 | func awaitCoordinatorScheduledTimer(t *testing.T, clk *coordinatorMockClock) *coordinatorMockTimer { | ||
| @@ -474,7 +458,7 @@ func TestRenderCoordinatorAckFlushesOnlyExpiredAckDeferredWork(t *testing.T) { | |||||
| 474 | timer := awaitCoordinatorScheduledTimer(t, h.clk) | 458 | timer := awaitCoordinatorScheduledTimer(t, h.clk) | ||
| 475 | timer.ch <- time.Time{} | 459 | timer.ch <- time.Time{} | ||
| 476 | requireNoWake(t, h.wakes) | 460 | requireNoWake(t, h.wakes) | ||
| 477 | - | h.rc.noteSessionTeardown() | 461 | + | h.rc.beginSessionTeardown().finish() |
| 478 | h.ackReady.Store(true) | 462 | h.ackReady.Store(true) | ||
| 479 | h.rc.notifyAck() | 463 | h.rc.notifyAck() | ||
| 480 | requireNoWake(t, h.wakes) | 464 | requireNoWake(t, h.wakes) | ||
| @@ -673,7 +657,7 @@ func TestRenderCoordinatorSynchronizedOutput(t *testing.T) { | |||||
| 673 | timers := h.armedTimers(t) | 657 | timers := h.armedTimers(t) | ||
| 674 | require.Len(t, timers, 1) | 658 | require.Len(t, timers, 1) | ||
| 675 | h.rc.mu.Lock() | 659 | h.rc.mu.Lock() | ||
| 676 | - | normalWorkerDone := h.rc.normalWorkerDone | 660 | + | normalWorkerDone := h.rc.normalLane.token.done |
| 677 | h.rc.mu.Unlock() | 661 | h.rc.mu.Unlock() | ||
| 678 | require.NotNil(t, normalWorkerDone) | 662 | require.NotNil(t, normalWorkerDone) | ||
| 679 | timers[0].ch <- time.Time{} | 663 | timers[0].ch <- time.Time{} | ||
| @@ -728,6 +712,25 @@ func TestRenderCoordinatorSynchronizedOutput(t *testing.T) { | |||||
| 728 | requireNoWake(t, h.wakes) | 712 | requireNoWake(t, h.wakes) | ||
| 729 | }) | 713 | }) | ||
| 730 | 714 | ||||
| 715 | + | t.Run("watchdog flushes after the snapshotted generation is replaced", func(t *testing.T) { | |||
| 716 | + | h := newCoordinatorHarness(t) | |||
| 717 | + | h.rc.invalidate(renderInvalidation{class: invalidateOutput}) | |||
| 718 | + | h.rc.mu.Lock() | |||
| 719 | + | staleGeneration := h.rc.normalLane.generation | |||
| 720 | + | h.rc.mu.Unlock() | |||
| 721 | + | ||||
| 722 | + | // Simulate a new urgent publication after fireCurrent(true) snapshots its | |||
| 723 | + | // generation but before fire validates it. | |||
| 724 | + | h.rc.invalidate(renderInvalidation{class: invalidateUrgent}) | |||
| 725 | + | h.rc.fire(staleGeneration, true, true) | |||
| 726 | + | ||||
| 727 | + | w := awaitWake(t, h.wakes) | |||
| 728 | + | require.True(t, w.watchdog) | |||
| 729 | + | require.True(t, w.urgent) | |||
| 730 | + | require.Equal(t, 2, w.coalesced) | |||
| 731 | + | requireNoWake(t, h.wakes) | |||
| 732 | + | }) | |||
| 733 | + | ||||
| 731 | t.Run("stale watchdog generation cannot wake a completed batch", func(t *testing.T) { | 734 | t.Run("stale watchdog generation cannot wake a completed batch", func(t *testing.T) { | ||
| 732 | h := newCoordinatorHarness(t) | 735 | h := newCoordinatorHarness(t) | ||
| 733 | h.syncActive.Store(true) | 736 | h.syncActive.Store(true) | ||
| @@ -870,7 +873,7 @@ func TestRenderCoordinatorPreviewLifecycleDropsStaleTargetWakes(t *testing.T) { | |||||
| 870 | }{ | 873 | }{ | ||
| 871 | {"target detach", func(rc *renderCoordinator, target, _ *attachedClient) { rc.noteDetach(target) }}, | 874 | {"target detach", func(rc *renderCoordinator, target, _ *attachedClient) { rc.noteDetach(target) }}, | ||
| 872 | {"target replacement", func(rc *renderCoordinator, target, replacement *attachedClient) { rc.noteReplace(target, replacement) }}, | 875 | {"target replacement", func(rc *renderCoordinator, target, replacement *attachedClient) { rc.noteReplace(target, replacement) }}, | ||
| 873 | - | {"target teardown", func(rc *renderCoordinator, _ *attachedClient, _ *attachedClient) { rc.noteSessionTeardown() }}, | 876 | + | {"target teardown", func(rc *renderCoordinator, _ *attachedClient, _ *attachedClient) { rc.beginSessionTeardown().finish() }}, |
| 874 | } | 877 | } | ||
| 875 | for _, tc := range cases { | 878 | for _, tc := range cases { | ||
| 876 | t.Run(tc.name, func(t *testing.T) { | 879 | t.Run(tc.name, func(t *testing.T) { | ||
| @@ -918,7 +921,7 @@ func TestRenderCoordinatorLifecycleDropsStaleWakes(t *testing.T) { | |||||
| 918 | }{ | 921 | }{ | ||
| 919 | {"detach", func(rc *renderCoordinator, owner *attachedClient) { rc.noteDetach(owner) }}, | 922 | {"detach", func(rc *renderCoordinator, owner *attachedClient) { rc.noteDetach(owner) }}, | ||
| 920 | {"park", func(rc *renderCoordinator, owner *attachedClient) { rc.notePark(owner) }}, | 923 | {"park", func(rc *renderCoordinator, owner *attachedClient) { rc.notePark(owner) }}, | ||
| 921 | - | {"session teardown", func(rc *renderCoordinator, _ *attachedClient) { rc.noteSessionTeardown() }}, | 924 | + | {"session teardown", func(rc *renderCoordinator, _ *attachedClient) { rc.beginSessionTeardown().finish() }}, |
| 922 | } | 925 | } | ||
| 923 | for _, tc := range cases { | 926 | for _, tc := range cases { | ||
| 924 | t.Run(tc.name, func(t *testing.T) { | 927 | t.Run(tc.name, func(t *testing.T) { | ||
| @@ -1034,7 +1037,7 @@ func TestRenderCoordinatorResizeMetadata(t *testing.T) { | |||||
| 1034 | case "park": | 1037 | case "park": | ||
| 1035 | h.rc.notePark(staleOwner) | 1038 | h.rc.notePark(staleOwner) | ||
| 1036 | case "session teardown": | 1039 | case "session teardown": | ||
| 1037 | - | h.rc.noteSessionTeardown() | 1040 | + | h.rc.beginSessionTeardown().finish() |
| 1038 | case "replacement": | 1041 | case "replacement": | ||
| 1039 | h.rc.noteReplace(staleOwner, freshOwner) | 1042 | h.rc.noteReplace(staleOwner, freshOwner) | ||
| 1040 | h.rc.attach(freshOwner) | 1043 | h.rc.attach(freshOwner) | ||
| @@ -1168,8 +1171,10 @@ func TestProducerInvalidations(t *testing.T) { | |||||
| 1168 | // Daemon's construction-time clock afterwards. | 1171 | // Daemon's construction-time clock afterwards. | ||
| 1169 | sess.renderCoordinator().opts.clock = clk.clock | 1172 | sess.renderCoordinator().opts.clock = clk.clock | ||
| 1170 | d.resize(sess, ac, domain.Size{Cols: 100, Rows: 26}) | 1173 | d.resize(sess, ac, domain.Size{Cols: 100, Rows: 26}) | ||
| 1171 | - | awaitCoordinatorScheduledTimer(t, clk).ch <- time.Time{} | 1174 | + | timer := awaitCoordinatorScheduledTimer(t, clk) |
| 1172 | - | awaitResizeCallback(t, sess.renderCoordinator()) | 1175 | + | done := captureResizeCallbackDone(t, sess.renderCoordinator()) |
| 1176 | + | timer.ch <- time.Time{} | |||
| 1177 | + | awaitTestCompletion(t, done, "resize callback did not complete") | |||
| 1173 | }, | 1178 | }, | ||
| 1174 | }, | 1179 | }, | ||
| 1175 | { | 1180 | { | ||
| @@ -1314,26 +1319,18 @@ func TestCoordinatorDeadlineCannotPaintPublishedReplacementBeforeOwnershipInstal | |||||
| 1314 | requireNoCoordinatorOutputFrame(t, ownerSends) | 1319 | requireNoCoordinatorOutputFrame(t, ownerSends) | ||
| 1315 | } | 1320 | } | ||
| 1316 | 1321 | ||||
| 1317 | - | func TestRenderCoordinatorClearResizeTimerKeepsNewerTimer(t *testing.T) { | 1322 | + | func TestRenderCoordinatorResizeLaneRejectsStaleToken(t *testing.T) { |
| 1318 | rc := newRenderCoordinator(renderCoordinatorOptions{}) | 1323 | rc := newRenderCoordinator(renderCoordinatorOptions{}) | ||
| 1319 | - | newer := &portsmocks.MockTimer{} | 1324 | + | newer := &timerToken{generation: 2, timer: &portsmocks.MockTimer{}, cancel: make(chan struct{}), done: make(chan struct{})} |
| 1320 | - | 1325 | + | stale := &timerToken{generation: 1} | |
| 1321 | - | rc.mu.Lock() | |||
| 1322 | - | rc.resizeGen = 2 | |||
| 1323 | - | rc.resizeTimer = newer | |||
| 1324 | - | rc.resizeCancel = make(chan struct{}) | |||
| 1325 | - | rc.mu.Unlock() | |||
| 1326 | 1326 | ||||
| 1327 | - | rc.clearResizeTimer(1) | |||
| 1328 | rc.mu.Lock() | 1327 | rc.mu.Lock() | ||
| 1329 | - | require.Same(t, newer, rc.resizeTimer) | 1328 | + | rc.resizeLane.generation = 2 |
| 1330 | - | require.NotNil(t, rc.resizeCancel) | 1329 | + | rc.resizeLane.token = newer |
| 1331 | - | rc.mu.Unlock() | 1330 | + | require.False(t, rc.resizeLane.clearLocked(stale)) |
| 1332 | - | 1331 | + | require.Same(t, newer, rc.resizeLane.token) | |
| 1333 | - | rc.clearResizeTimer(2) | 1332 | + | require.True(t, rc.resizeLane.clearLocked(newer)) |
| 1334 | - | rc.mu.Lock() | 1333 | + | require.Nil(t, rc.resizeLane.token) |
| 1335 | - | require.Nil(t, rc.resizeTimer) | |||
| 1336 | - | require.Nil(t, rc.resizeCancel) | |||
| 1337 | rc.mu.Unlock() | 1334 | rc.mu.Unlock() | ||
| 1338 | } | 1335 | } | ||
| 1339 | 1336 | ||||
| @@ -1350,16 +1347,9 @@ func TestRenderCoordinatorNilClockRetryCompletesWithoutSynchronousRun(t *testing | |||||
| 1350 | rc.scheduleResizeRetryForLease(epoch, owner, lease, func() { called = true }) | 1347 | rc.scheduleResizeRetryForLease(epoch, owner, lease, func() { called = true }) | ||
| 1351 | require.False(t, called, "a nil clock is a disabled retry lane, not a synchronous retry") | 1348 | require.False(t, called, "a nil clock is a disabled retry lane, not a synchronous retry") | ||
| 1352 | rc.mu.Lock() | 1349 | rc.mu.Lock() | ||
| 1353 | - | retryTimer, retryCancel, retryDone := rc.retryTimer, rc.retryCancel, rc.retryDone | 1350 | + | retryToken := rc.retryLane.token |
| 1354 | rc.mu.Unlock() | 1351 | rc.mu.Unlock() | ||
| 1355 | - | require.Nil(t, retryTimer) | 1352 | + | require.Nil(t, retryToken, "a disabled retry lane must not retain stale completion ownership") |
| 1356 | - | require.Nil(t, retryCancel) | |||
| 1357 | - | require.NotNil(t, retryDone) | |||
| 1358 | - | select { | |||
| 1359 | - | case <-retryDone: | |||
| 1360 | - | default: | |||
| 1361 | - | t.Fatal("nil-clock retry did not publish its completion edge") | |||
| 1362 | - | } | |||
| 1363 | } | 1353 | } | ||
| 1364 | 1354 | ||||
| 1365 | func TestRenderCoordinatorInertTimerFiresSynchronouslyWithoutWorker(t *testing.T) { | 1355 | func TestRenderCoordinatorInertTimerFiresSynchronouslyWithoutWorker(t *testing.T) { | ||
| @@ -1380,9 +1370,7 @@ func TestRenderCoordinatorInertTimerFiresSynchronouslyWithoutWorker(t *testing.T | |||||
| 1380 | require.Equal(t, renderWake{coalesced: 1}, <-wakes) | 1370 | require.Equal(t, renderWake{coalesced: 1}, <-wakes) | ||
| 1381 | require.Equal(t, renderCoordinatorBurstMetricsSnapshot{invalidations: 1, wakes: 1, coalesced: 1}, rc.burstMetricsSnapshot()) | 1371 | require.Equal(t, renderCoordinatorBurstMetricsSnapshot{invalidations: 1, wakes: 1, coalesced: 1}, rc.burstMetricsSnapshot()) | ||
| 1382 | rc.mu.Lock() | 1372 | rc.mu.Lock() | ||
| 1383 | - | require.Nil(t, rc.normalTimer) | 1373 | + | require.Nil(t, rc.normalLane.token) |
| 1384 | - | require.Nil(t, rc.normalCancel) | |||
| 1385 | - | require.Nil(t, rc.normalWorkerDone) | |||
| 1386 | rc.mu.Unlock() | 1374 | rc.mu.Unlock() | ||
| 1387 | } | 1375 | } | ||
| 1388 | 1376 | ||||
| @@ -1410,7 +1398,7 @@ func TestRenderCoordinatorSyncBatchSurvivesAttachmentLifecycle(t *testing.T) { | |||||
| 1410 | timers := h.armedTimers(t) | 1398 | timers := h.armedTimers(t) | ||
| 1411 | require.Len(t, timers, 1) | 1399 | require.Len(t, timers, 1) | ||
| 1412 | h.rc.mu.Lock() | 1400 | h.rc.mu.Lock() | ||
| 1413 | - | normalWorkerDone := h.rc.normalWorkerDone | 1401 | + | normalWorkerDone := h.rc.normalLane.token.done |
| 1414 | h.rc.mu.Unlock() | 1402 | h.rc.mu.Unlock() | ||
| 1415 | require.NotNil(t, normalWorkerDone) | 1403 | require.NotNil(t, normalWorkerDone) | ||
| 1416 | timers[0].ch <- time.Time{} | 1404 | timers[0].ch <- time.Time{} | ||
| @@ -1418,7 +1406,7 @@ func TestRenderCoordinatorSyncBatchSurvivesAttachmentLifecycle(t *testing.T) { | |||||
| 1418 | requireNoWake(t, h.previews) | 1406 | requireNoWake(t, h.previews) | ||
| 1419 | 1407 | ||||
| 1420 | h.rc.mu.Lock() | 1408 | h.rc.mu.Lock() | ||
| 1421 | - | require.Same(t, h.rc.syncBatches[p].timer, watchdog.mock) | 1409 | + | require.Same(t, h.rc.syncBatches[p].lane.token.timer, watchdog.mock) |
| 1422 | h.rc.mu.Unlock() | 1410 | h.rc.mu.Unlock() | ||
| 1423 | h.rc.noteSyncEnd(p, 1) | 1411 | h.rc.noteSyncEnd(p, 1) | ||
| 1424 | preview := awaitWake(t, h.previews) | 1412 | preview := awaitWake(t, h.previews) | ||
| @@ -1446,7 +1434,7 @@ func TestRenderCoordinatorSyncBatchSurvivesAttachmentLifecycle(t *testing.T) { | |||||
| 1446 | timers := h.armedTimers(t) | 1434 | timers := h.armedTimers(t) | ||
| 1447 | require.Len(t, timers, 1) | 1435 | require.Len(t, timers, 1) | ||
| 1448 | h.rc.mu.Lock() | 1436 | h.rc.mu.Lock() | ||
| 1449 | - | normalWorkerDone := h.rc.normalWorkerDone | 1437 | + | normalWorkerDone := h.rc.normalLane.token.done |
| 1450 | h.rc.mu.Unlock() | 1438 | h.rc.mu.Unlock() | ||
| 1451 | require.NotNil(t, normalWorkerDone) | 1439 | require.NotNil(t, normalWorkerDone) | ||
| 1452 | timers[0].ch <- time.Time{} | 1440 | timers[0].ch <- time.Time{} | ||
| @@ -1454,7 +1442,7 @@ func TestRenderCoordinatorSyncBatchSurvivesAttachmentLifecycle(t *testing.T) { | |||||
| 1454 | requireNoWake(t, h.wakes) | 1442 | requireNoWake(t, h.wakes) | ||
| 1455 | 1443 | ||||
| 1456 | h.rc.mu.Lock() | 1444 | h.rc.mu.Lock() | ||
| 1457 | - | require.Same(t, h.rc.syncBatches[p].timer, watchdog.mock) | 1445 | + | require.Same(t, h.rc.syncBatches[p].lane.token.timer, watchdog.mock) |
| 1458 | h.rc.mu.Unlock() | 1446 | h.rc.mu.Unlock() | ||
| 1459 | h.rc.noteSyncEnd(p, 1) | 1447 | h.rc.noteSyncEnd(p, 1) | ||
| 1460 | wake := awaitWake(t, h.wakes) | 1448 | wake := awaitWake(t, h.wakes) | ||
| @@ -1470,20 +1458,23 @@ func TestRenderCoordinatorSyncBatchSurvivesAttachmentLifecycle(t *testing.T) { | |||||
| 1470 | end func(*renderCoordinator, *pane) | 1458 | end func(*renderCoordinator, *pane) | ||
| 1471 | }{ | 1459 | }{ | ||
| 1472 | {"pane removal", func(rc *renderCoordinator, p *pane) { rc.noteSyncPaneRemoved(p) }}, | 1460 | {"pane removal", func(rc *renderCoordinator, p *pane) { rc.noteSyncPaneRemoved(p) }}, | ||
| 1473 | - | {"session teardown", func(rc *renderCoordinator, _ *pane) { rc.noteSessionTeardown() }}, | 1461 | + | {"session teardown", func(rc *renderCoordinator, _ *pane) { rc.beginSessionTeardown().finish() }}, |
| 1474 | } { | 1462 | } { | ||
| 1475 | t.Run(lifecycle.name, func(t *testing.T) { | 1463 | t.Run(lifecycle.name, func(t *testing.T) { | ||
| 1476 | - | clk := newInertCoordinatorMockClock(t, 2) | 1464 | + | clk := newCoordinatorMockClock(t, 2) |
| 1477 | rc := newRenderCoordinator(renderCoordinatorOptions{clock: clk.clock}) | 1465 | rc := newRenderCoordinator(renderCoordinatorOptions{clock: clk.clock}) | ||
| 1478 | p := &pane{} | 1466 | p := &pane{} | ||
| 1479 | rc.noteSyncBegin(p, 1) | 1467 | rc.noteSyncBegin(p, 1) | ||
| 1480 | syncTimer := <-clk.timers | 1468 | syncTimer := <-clk.timers | ||
| 1481 | rc.mu.Lock() | 1469 | rc.mu.Lock() | ||
| 1482 | - | syncDone := rc.syncBatches[p].done | 1470 | + | syncDone := rc.syncBatches[p].lane.token.done |
| 1483 | rc.mu.Unlock() | 1471 | rc.mu.Unlock() | ||
| 1484 | lifecycle.end(rc, p) | 1472 | lifecycle.end(rc, p) | ||
| 1485 | syncTimer.mock.AssertNumberOfCalls(t, "Stop", 1) | 1473 | syncTimer.mock.AssertNumberOfCalls(t, "Stop", 1) | ||
| 1486 | requireWorkerExit(t, syncDone) | 1474 | requireWorkerExit(t, syncDone) | ||
| 1475 | + | if lifecycle.name == "session teardown" { | |||
| 1476 | + | rc.waitForTimerWorkers() | |||
| 1477 | + | } | |||
| 1487 | }) | 1478 | }) | ||
| 1488 | } | 1479 | } | ||
| 1489 | }) | 1480 | }) | ||
| @@ -1522,8 +1513,9 @@ func TestRenderCoordinatorResizeEpochDispatch(t *testing.T) { | |||||
| 1522 | require.Equal(t, uint64(1), snap.epoch) | 1513 | require.Equal(t, uint64(1), snap.epoch) | ||
| 1523 | requireNoCoordinatorOutputFrame(t, sends) | 1514 | requireNoCoordinatorOutputFrame(t, sends) | ||
| 1524 | 1515 | ||||
| 1516 | + | done := captureResizeCallbackDone(t, sess.renderCoordinator()) | |||
| 1525 | timer.ch <- time.Time{} | 1517 | timer.ch <- time.Time{} | ||
| 1526 | - | awaitResizeCallback(t, sess.renderCoordinator()) | 1518 | + | awaitTestCompletion(t, done, "resize callback did not complete") |
| 1527 | inv := awaitInvalidation(t, invs) | 1519 | inv := awaitInvalidation(t, invs) | ||
| 1528 | require.True(t, inv.reset, "the resize dispatch must request a full-redraw invalidation") | 1520 | require.True(t, inv.reset, "the resize dispatch must request a full-redraw invalidation") | ||
| 1529 | requireNoInvalidation(t, invs) | 1521 | requireNoInvalidation(t, invs) | ||
| @@ -1540,12 +1532,13 @@ func TestRenderCoordinatorResizeEpochDispatch(t *testing.T) { | |||||
| 1540 | require.Equal(t, uint64(2), sess.renderCoordinator().resizeSnapshot().epoch, | 1532 | require.Equal(t, uint64(2), sess.renderCoordinator().resizeSnapshot().epoch, | ||
| 1541 | "every resize request must advance the coordinator epoch") | 1533 | "every resize request must advance the coordinator epoch") | ||
| 1542 | 1534 | ||||
| 1535 | + | done := captureResizeCallbackDone(t, sess.renderCoordinator()) | |||
| 1543 | first.ch <- time.Time{} | 1536 | first.ch <- time.Time{} | ||
| 1544 | requireNoInvalidation(t, invs) | 1537 | requireNoInvalidation(t, invs) | ||
| 1545 | requireNoCoordinatorOutputFrame(t, sends) | 1538 | requireNoCoordinatorOutputFrame(t, sends) | ||
| 1546 | 1539 | ||||
| 1547 | latest.ch <- time.Time{} | 1540 | latest.ch <- time.Time{} | ||
| 1548 | - | awaitResizeCallback(t, sess.renderCoordinator()) | 1541 | + | awaitTestCompletion(t, done, "latest resize callback did not complete") |
| 1549 | inv := awaitInvalidation(t, invs) | 1542 | inv := awaitInvalidation(t, invs) | ||
| 1550 | require.True(t, inv.reset) | 1543 | require.True(t, inv.reset) | ||
| 1551 | requireNoInvalidation(t, invs) | 1544 | requireNoInvalidation(t, invs) | ||
| @@ -1558,9 +1551,11 @@ func TestRenderCoordinatorResizeEpochDispatch(t *testing.T) { | |||||
| 1558 | 1551 | ||||
| 1559 | d.resize(sess, ac, domain.Size{Cols: 90, Rows: 30}) | 1552 | d.resize(sess, ac, domain.Size{Cols: 90, Rows: 30}) | ||
| 1560 | timer := awaitCoordinatorScheduledTimer(t, clk) | 1553 | timer := awaitCoordinatorScheduledTimer(t, clk) | ||
| 1554 | + | done := captureResizeCallbackDone(t, sess.renderCoordinator()) | |||
| 1561 | sess.renderCoordinator().noteDetach(ac) | 1555 | sess.renderCoordinator().noteDetach(ac) | ||
| 1562 | 1556 | ||||
| 1563 | timer.ch <- time.Time{} | 1557 | timer.ch <- time.Time{} | ||
| 1558 | + | awaitTestCompletion(t, done, "cancelled resize callback did not complete") | |||
| 1564 | requireNoInvalidation(t, invs) | 1559 | requireNoInvalidation(t, invs) | ||
| 1565 | requireNoCoordinatorOutputFrame(t, sends) | 1560 | requireNoCoordinatorOutputFrame(t, sends) | ||
| 1566 | }) | 1561 | }) | ||
| @@ -1612,7 +1607,7 @@ func TestRenderCoordinatorResizeEpochDispatch(t *testing.T) { | |||||
| 1612 | awaitCoordinatorScheduledTimer(t, clk) | 1607 | awaitCoordinatorScheduledTimer(t, clk) | ||
| 1613 | 1608 | ||||
| 1614 | // A torn-down coordinator cannot accept a stale attachment's schedule. | 1609 | // A torn-down coordinator cannot accept a stale attachment's schedule. | ||
| 1615 | - | sess.renderCoordinator().noteSessionTeardown() | 1610 | + | sess.renderCoordinator().beginSessionTeardown().finish() |
| 1616 | require.False(t, d.requestTransactionalResize(sess, ac, domain.Size{Cols: 140, Rows: 30}, false)) | 1611 | require.False(t, d.requestTransactionalResize(sess, ac, domain.Size{Cols: 140, Rows: 30}, false)) | ||
| 1617 | }) | 1612 | }) | ||
| 1618 | }) | 1613 | }) | ||
M internal/usecase/daemon/render_resize_deadline.go +46 -91
| @@ -2,105 +2,79 @@ package daemon | |||
| 2 | 2 | ||
| 3 | 3 | import "github.com/bnema/vev/internal/domain" | |
| 4 | 4 | ||
| 5 | - | // Resize deadlines and retries own only scheduling mechanics. Attachment and | |
| 6 | - | // epoch validity stay with renderCoordinator, alongside the state they guard. | |
| 7 | - | ||
| 8 | - | // recordResizeRequest records the latest requested geometry for the attachment | |
| 9 | - | // currently owning source. Production callbacks use recordResizeRequestForLease | |
| 10 | - | // so a captured lease, rather than a reused attachedClient pointer, is checked. | |
| 5 | + | // Resize deadlines and retries retain coordinator-owned attachment, epoch, and | |
| 6 | + | // token validation at every callback effect boundary. | |
| 11 | 7 | func (c *renderCoordinator) recordResizeRequest(size domain.Size, source *attachedClient) uint64 { | |
| 12 | 8 | return c.recordResizeRequestForLease(size, source, c.attachmentLease(source)) | |
| 13 | 9 | } | |
| 14 | 10 | ||
| 15 | - | // recordResizeRequestForLease records a latest-wins request only for this exact | |
| 16 | - | // attachment incarnation. A same-object resume installs a different lease. | |
| 17 | 11 | func (c *renderCoordinator) recordResizeRequestForLease(size domain.Size, source *attachedClient, lease *attachmentLease) uint64 { | |
| 18 | 12 | c.mu.Lock() | |
| 19 | 13 | if !c.leaseCurrentLocked(lease, false) || lease.attachment != source { | |
| 20 | 14 | c.mu.Unlock() | |
| 21 | 15 | return 0 | |
| 22 | 16 | } | |
| 23 | - | // An intervening request supersedes any failed-pane retry, even if a fake | |
| 24 | - | // timer delivers a stopped callback afterwards. | |
| 25 | - | _, retryTimer := c.retryTimerOwner().replaceLocked() | |
| 17 | + | _, retry := c.retryLane.replaceLocked() | |
| 26 | 18 | c.resize.epoch++ | |
| 27 | - | c.resize.size = size | |
| 28 | - | c.resize.source = source | |
| 29 | - | c.resize.lease = lease | |
| 19 | + | c.resize.size, c.resize.source, c.resize.lease = size, source, lease | |
| 30 | 20 | epoch := c.resize.epoch | |
| 31 | 21 | c.mu.Unlock() | |
| 32 | - | stopAndJoinTimerWorker(retryTimer, nil) | |
| 22 | + | stopDetachedTimer(retry) | |
| 33 | 23 | return epoch | |
| 34 | 24 | } | |
| 35 | 25 | ||
| 36 | - | // scheduleResizeForLease runs apply after the bounded bulk window. The timer | |
| 37 | - | // callback validates the precise lease captured at request dispatch. | |
| 38 | 26 | func (c *renderCoordinator) scheduleResizeForLease(size domain.Size, source *attachedClient, lease *attachmentLease, run func(uint64)) uint64 { | |
| 39 | 27 | epoch := c.recordResizeRequestForLease(size, source, lease) | |
| 40 | 28 | if epoch == 0 { | |
| 41 | 29 | return 0 | |
| 42 | 30 | } | |
| 43 | 31 | c.mu.Lock() | |
| 44 | - | _, old := c.resizeTimerOwner().replaceLocked() | |
| 45 | - | gen := c.resizeGen | |
| 32 | + | gen, old := c.resizeLane.replaceLocked() | |
| 46 | 33 | clock := c.opts.clock | |
| 47 | 34 | c.mu.Unlock() | |
| 48 | - | stopAndJoinTimerWorker(old, nil) | |
| 35 | + | stopDetachedTimer(old) | |
| 49 | 36 | if clock == nil { | |
| 50 | 37 | if c.resizeCurrentForLease(epoch, source, lease, false) { | |
| 51 | 38 | run(epoch) | |
| 52 | 39 | } | |
| 53 | - | c.completeResizeTimer(gen) | |
| 54 | 40 | return epoch | |
| 55 | 41 | } | |
| 56 | 42 | timer := clock.NewTimer(minOutputRenderDeadline) | |
| 57 | 43 | timerC := timer.C() | |
| 58 | 44 | if timerC == nil { | |
| 59 | - | stopTimer(timer) | |
| 45 | + | timer.Stop() | |
| 60 | 46 | if c.resizeCurrentForLease(epoch, source, lease, false) { | |
| 61 | 47 | run(epoch) | |
| 62 | 48 | } | |
| 63 | - | c.completeResizeTimer(gen) | |
| 64 | 49 | return epoch | |
| 65 | 50 | } | |
| 66 | 51 | c.mu.Lock() | |
| 67 | - | valid := !c.torndown && c.resizeGen == gen && c.leaseCurrentLocked(lease, false) && c.resize.lease == lease && c.resize.epoch == epoch && c.resize.source == source | |
| 68 | - | var cancel, done chan struct{} | |
| 52 | + | valid := !c.torndown && c.resizeLane.generation == gen && c.leaseCurrentLocked(lease, false) && c.resize.lease == lease && c.resize.epoch == epoch && c.resize.source == source | |
| 53 | + | var token *timerToken | |
| 69 | 54 | if valid { | |
| 70 | - | cancel, done, valid = c.resizeTimerOwner().publishLocked(gen, timer) | |
| 55 | + | token = c.resizeLane.publishLocked(gen, timer) | |
| 56 | + | valid = token != nil | |
| 57 | + | if valid { | |
| 58 | + | c.supervisor.startLocked(token, timerC, func() { | |
| 59 | + | c.mu.Lock() | |
| 60 | + | valid := !c.torndown && c.resizeLane.token == token && c.resizeLane.generation == token.generation && c.leaseCurrentLocked(lease, false) && c.resize.lease == lease && c.resize.epoch == epoch && c.resize.source == source | |
| 61 | + | if valid { | |
| 62 | + | c.resizeLane.clearLocked(token) | |
| 63 | + | } | |
| 64 | + | c.mu.Unlock() | |
| 65 | + | if valid { | |
| 66 | + | run(epoch) | |
| 67 | + | } | |
| 68 | + | }) | |
| 69 | + | } | |
| 71 | 70 | } | |
| 72 | 71 | c.mu.Unlock() | |
| 73 | 72 | if !valid { | |
| 74 | - | stopTimer(timer) | |
| 75 | - | c.completeResizeTimer(gen) | |
| 76 | - | return epoch | |
| 73 | + | timer.Stop() | |
| 77 | 74 | } | |
| 78 | - | runTimerWorker(timerC, cancel, done, func() { | |
| 79 | - | c.mu.Lock() | |
| 80 | - | valid := !c.torndown && c.resizeGen == gen && c.leaseCurrentLocked(lease, false) && c.resize.lease == lease && c.resize.epoch == epoch && c.resize.source == source | |
| 81 | - | c.mu.Unlock() | |
| 82 | - | c.clearResizeTimer(gen) | |
| 83 | - | if valid { | |
| 84 | - | run(epoch) | |
| 85 | - | } | |
| 86 | - | }) | |
| 87 | 75 | return epoch | |
| 88 | 76 | } | |
| 89 | 77 | ||
| 90 | - | // clearResizeTimer releases timer ownership only for the matching resize generation. | |
| 91 | - | // A stale callback must not clear a newer request's timer or cancellation channel. | |
| 92 | - | func (c *renderCoordinator) clearResizeTimer(gen uint64) { | |
| 93 | - | c.mu.Lock() | |
| 94 | - | defer c.mu.Unlock() | |
| 95 | - | c.resizeTimerOwner().clearLocked(gen) | |
| 96 | - | } | |
| 97 | - | ||
| 98 | - | func (c *renderCoordinator) completeResizeTimer(gen uint64) { | |
| 99 | - | c.mu.Lock() | |
| 100 | - | defer c.mu.Unlock() | |
| 101 | - | c.resizeTimerOwner().completeLocked(gen) | |
| 102 | - | } | |
| 103 | - | ||
| 104 | 78 | func (c *renderCoordinator) resizeCurrentForLease(epoch uint64, source *attachedClient, lease *attachmentLease, commit bool) bool { | |
| 105 | 79 | c.mu.Lock() | |
| 106 | 80 | defer c.mu.Unlock() | |
| @@ -122,52 +96,42 @@ func (c *renderCoordinator) scheduleResizeRetryForLease(epoch uint64, source *at | |||
| 122 | 96 | c.mu.Unlock() | |
| 123 | 97 | return | |
| 124 | 98 | } | |
| 125 | - | _, old := c.retryTimerOwner().replaceLocked() | |
| 126 | - | gen := c.retryGen | |
| 99 | + | gen, old := c.retryLane.replaceLocked() | |
| 127 | 100 | clock := c.opts.clock | |
| 128 | 101 | c.mu.Unlock() | |
| 129 | - | stopAndJoinTimerWorker(old, nil) | |
| 102 | + | stopDetachedTimer(old) | |
| 130 | 103 | if clock == nil { | |
| 131 | - | // A nil clock disables deferred retries. Completing the generation keeps | |
| 132 | - | // retry bookkeeping consistent without recursively re-entering resize. | |
| 133 | - | c.mu.Lock() | |
| 134 | - | c.retryTimerOwner().completeLocked(gen) | |
| 135 | - | c.mu.Unlock() | |
| 136 | 104 | return | |
| 137 | 105 | } | |
| 138 | 106 | timer := clock.NewTimer(minOutputRenderDeadline) | |
| 139 | 107 | timerC := timer.C() | |
| 140 | 108 | if timerC == nil { | |
| 141 | - | // A nil timer channel is the deterministic disabled-clock contract used | |
| 142 | - | // by headless tests; do not spin retries synchronously. | |
| 143 | - | stopTimer(timer) | |
| 144 | - | c.mu.Lock() | |
| 145 | - | c.retryTimerOwner().completeLocked(gen) | |
| 146 | - | c.mu.Unlock() | |
| 109 | + | timer.Stop() | |
| 147 | 110 | return | |
| 148 | 111 | } | |
| 149 | 112 | c.mu.Lock() | |
| 150 | - | valid := !c.torndown && c.retryGen == gen && c.leaseCurrentLocked(lease, false) && c.resize.lease == lease && c.resize.epoch == epoch && c.resize.committed == epoch && c.resize.source == source | |
| 151 | - | var cancel, done chan struct{} | |
| 113 | + | valid := !c.torndown && c.retryLane.generation == gen && c.leaseCurrentLocked(lease, false) && c.resize.lease == lease && c.resize.epoch == epoch && c.resize.committed == epoch && c.resize.source == source | |
| 152 | 114 | if valid { | |
| 153 | - | cancel, done, valid = c.retryTimerOwner().publishLocked(gen, timer) | |
| 115 | + | token := c.retryLane.publishLocked(gen, timer) | |
| 116 | + | valid = token != nil | |
| 117 | + | if valid { | |
| 118 | + | c.supervisor.startLocked(token, timerC, func() { | |
| 119 | + | c.mu.Lock() | |
| 120 | + | valid := !c.torndown && c.retryLane.token == token && c.retryLane.generation == token.generation && c.leaseCurrentLocked(lease, false) && c.resize.lease == lease && c.resize.epoch == epoch && c.resize.committed == epoch && c.resize.source == source | |
| 121 | + | if valid { | |
| 122 | + | c.retryLane.clearLocked(token) | |
| 123 | + | } | |
| 124 | + | c.mu.Unlock() | |
| 125 | + | if valid { | |
| 126 | + | run() | |
| 127 | + | } | |
| 128 | + | }) | |
| 129 | + | } | |
| 154 | 130 | } | |
| 155 | 131 | c.mu.Unlock() | |
| 156 | 132 | if !valid { | |
| 157 | - | stopTimer(timer) | |
| 158 | - | return | |
| 133 | + | timer.Stop() | |
| 159 | 134 | } | |
| 160 | - | runTimerWorker(timerC, cancel, done, func() { | |
| 161 | - | c.mu.Lock() | |
| 162 | - | valid := !c.torndown && c.retryGen == gen && c.leaseCurrentLocked(lease, false) && c.resize.lease == lease && c.resize.epoch == epoch && c.resize.committed == epoch && c.resize.source == source | |
| 163 | - | if valid { | |
| 164 | - | c.retryTimerOwner().clearLocked(gen) | |
| 165 | - | } | |
| 166 | - | c.mu.Unlock() | |
| 167 | - | if valid { | |
| 168 | - | run() | |
| 169 | - | } | |
| 170 | - | }) | |
| 171 | 135 | } | |
| 172 | 136 | ||
| 173 | 137 | func (c *renderCoordinator) retryCurrentForLease(epoch uint64, source *attachedClient, lease *attachmentLease) bool { | |
| @@ -176,15 +140,6 @@ func (c *renderCoordinator) retryCurrentForLease(epoch uint64, source *attachedC | |||
| 176 | 140 | return c.leaseCurrentLocked(lease, false) && lease.attachment == source && c.resize.lease == lease && c.resize.source == source && c.resize.epoch == epoch && c.resize.committed == epoch | |
| 177 | 141 | } | |
| 178 | 142 | ||
| 179 | - | // resizeCallbackDone returns the completion edge for the latest scheduled | |
| 180 | - | // resize callback. A nil result means no resize deadline is pending. | |
| 181 | - | func (c *renderCoordinator) resizeCallbackDone() <-chan struct{} { | |
| 182 | - | c.mu.Lock() | |
| 183 | - | defer c.mu.Unlock() | |
| 184 | - | return c.resizeDone | |
| 185 | - | } | |
| 186 | - | ||
| 187 | - | // resizeSnapshot returns a locked copy of the latest resize metadata. | |
| 188 | 143 | func (c *renderCoordinator) resizeSnapshot() resizeRequestMetadata { | |
| 189 | 144 | c.mu.Lock() | |
| 190 | 145 | defer c.mu.Unlock() | |
| @@ -2,105 +2,79 @@ package daemon | |||||
| 2 | 2 | ||||
| 3 | import "github.com/bnema/vev/internal/domain" | 3 | import "github.com/bnema/vev/internal/domain" | ||
| 4 | 4 | ||||
| 5 | - | // Resize deadlines and retries own only scheduling mechanics. Attachment and | 5 | + | // Resize deadlines and retries retain coordinator-owned attachment, epoch, and |
| 6 | - | // epoch validity stay with renderCoordinator, alongside the state they guard. | 6 | + | // token validation at every callback effect boundary. |
| 7 | - | ||||
| 8 | - | // recordResizeRequest records the latest requested geometry for the attachment | |||
| 9 | - | // currently owning source. Production callbacks use recordResizeRequestForLease | |||
| 10 | - | // so a captured lease, rather than a reused attachedClient pointer, is checked. | |||
| 11 | func (c *renderCoordinator) recordResizeRequest(size domain.Size, source *attachedClient) uint64 { | 7 | func (c *renderCoordinator) recordResizeRequest(size domain.Size, source *attachedClient) uint64 { | ||
| 12 | return c.recordResizeRequestForLease(size, source, c.attachmentLease(source)) | 8 | return c.recordResizeRequestForLease(size, source, c.attachmentLease(source)) | ||
| 13 | } | 9 | } | ||
| 14 | 10 | ||||
| 15 | - | // recordResizeRequestForLease records a latest-wins request only for this exact | |||
| 16 | - | // attachment incarnation. A same-object resume installs a different lease. | |||
| 17 | func (c *renderCoordinator) recordResizeRequestForLease(size domain.Size, source *attachedClient, lease *attachmentLease) uint64 { | 11 | func (c *renderCoordinator) recordResizeRequestForLease(size domain.Size, source *attachedClient, lease *attachmentLease) uint64 { | ||
| 18 | c.mu.Lock() | 12 | c.mu.Lock() | ||
| 19 | if !c.leaseCurrentLocked(lease, false) || lease.attachment != source { | 13 | if !c.leaseCurrentLocked(lease, false) || lease.attachment != source { | ||
| 20 | c.mu.Unlock() | 14 | c.mu.Unlock() | ||
| 21 | return 0 | 15 | return 0 | ||
| 22 | } | 16 | } | ||
| 23 | - | // An intervening request supersedes any failed-pane retry, even if a fake | 17 | + | _, retry := c.retryLane.replaceLocked() |
| 24 | - | // timer delivers a stopped callback afterwards. | |||
| 25 | - | _, retryTimer := c.retryTimerOwner().replaceLocked() | |||
| 26 | c.resize.epoch++ | 18 | c.resize.epoch++ | ||
| 27 | - | c.resize.size = size | 19 | + | c.resize.size, c.resize.source, c.resize.lease = size, source, lease |
| 28 | - | c.resize.source = source | |||
| 29 | - | c.resize.lease = lease | |||
| 30 | epoch := c.resize.epoch | 20 | epoch := c.resize.epoch | ||
| 31 | c.mu.Unlock() | 21 | c.mu.Unlock() | ||
| 32 | - | stopAndJoinTimerWorker(retryTimer, nil) | 22 | + | stopDetachedTimer(retry) |
| 33 | return epoch | 23 | return epoch | ||
| 34 | } | 24 | } | ||
| 35 | 25 | ||||
| 36 | - | // scheduleResizeForLease runs apply after the bounded bulk window. The timer | |||
| 37 | - | // callback validates the precise lease captured at request dispatch. | |||
| 38 | func (c *renderCoordinator) scheduleResizeForLease(size domain.Size, source *attachedClient, lease *attachmentLease, run func(uint64)) uint64 { | 26 | func (c *renderCoordinator) scheduleResizeForLease(size domain.Size, source *attachedClient, lease *attachmentLease, run func(uint64)) uint64 { | ||
| 39 | epoch := c.recordResizeRequestForLease(size, source, lease) | 27 | epoch := c.recordResizeRequestForLease(size, source, lease) | ||
| 40 | if epoch == 0 { | 28 | if epoch == 0 { | ||
| 41 | return 0 | 29 | return 0 | ||
| 42 | } | 30 | } | ||
| 43 | c.mu.Lock() | 31 | c.mu.Lock() | ||
| 44 | - | _, old := c.resizeTimerOwner().replaceLocked() | 32 | + | gen, old := c.resizeLane.replaceLocked() |
| 45 | - | gen := c.resizeGen | |||
| 46 | clock := c.opts.clock | 33 | clock := c.opts.clock | ||
| 47 | c.mu.Unlock() | 34 | c.mu.Unlock() | ||
| 48 | - | stopAndJoinTimerWorker(old, nil) | 35 | + | stopDetachedTimer(old) |
| 49 | if clock == nil { | 36 | if clock == nil { | ||
| 50 | if c.resizeCurrentForLease(epoch, source, lease, false) { | 37 | if c.resizeCurrentForLease(epoch, source, lease, false) { | ||
| 51 | run(epoch) | 38 | run(epoch) | ||
| 52 | } | 39 | } | ||
| 53 | - | c.completeResizeTimer(gen) | |||
| 54 | return epoch | 40 | return epoch | ||
| 55 | } | 41 | } | ||
| 56 | timer := clock.NewTimer(minOutputRenderDeadline) | 42 | timer := clock.NewTimer(minOutputRenderDeadline) | ||
| 57 | timerC := timer.C() | 43 | timerC := timer.C() | ||
| 58 | if timerC == nil { | 44 | if timerC == nil { | ||
| 59 | - | stopTimer(timer) | 45 | + | timer.Stop() |
| 60 | if c.resizeCurrentForLease(epoch, source, lease, false) { | 46 | if c.resizeCurrentForLease(epoch, source, lease, false) { | ||
| 61 | run(epoch) | 47 | run(epoch) | ||
| 62 | } | 48 | } | ||
| 63 | - | c.completeResizeTimer(gen) | |||
| 64 | return epoch | 49 | return epoch | ||
| 65 | } | 50 | } | ||
| 66 | c.mu.Lock() | 51 | c.mu.Lock() | ||
| 67 | - | valid := !c.torndown && c.resizeGen == gen && c.leaseCurrentLocked(lease, false) && c.resize.lease == lease && c.resize.epoch == epoch && c.resize.source == source | 52 | + | valid := !c.torndown && c.resizeLane.generation == gen && c.leaseCurrentLocked(lease, false) && c.resize.lease == lease && c.resize.epoch == epoch && c.resize.source == source |
| 68 | - | var cancel, done chan struct{} | 53 | + | var token *timerToken |
| 69 | if valid { | 54 | if valid { | ||
| 70 | - | cancel, done, valid = c.resizeTimerOwner().publishLocked(gen, timer) | 55 | + | token = c.resizeLane.publishLocked(gen, timer) |
| 56 | + | valid = token != nil | |||
| 57 | + | if valid { | |||
| 58 | + | c.supervisor.startLocked(token, timerC, func() { | |||
| 59 | + | c.mu.Lock() | |||
| 60 | + | valid := !c.torndown && c.resizeLane.token == token && c.resizeLane.generation == token.generation && c.leaseCurrentLocked(lease, false) && c.resize.lease == lease && c.resize.epoch == epoch && c.resize.source == source | |||
| 61 | + | if valid { | |||
| 62 | + | c.resizeLane.clearLocked(token) | |||
| 63 | + | } | |||
| 64 | + | c.mu.Unlock() | |||
| 65 | + | if valid { | |||
| 66 | + | run(epoch) | |||
| 67 | + | } | |||
| 68 | + | }) | |||
| 69 | + | } | |||
| 71 | } | 70 | } | ||
| 72 | c.mu.Unlock() | 71 | c.mu.Unlock() | ||
| 73 | if !valid { | 72 | if !valid { | ||
| 74 | - | stopTimer(timer) | 73 | + | timer.Stop() |
| 75 | - | c.completeResizeTimer(gen) | |||
| 76 | - | return epoch | |||
| 77 | } | 74 | } | ||
| 78 | - | runTimerWorker(timerC, cancel, done, func() { | |||
| 79 | - | c.mu.Lock() | |||
| 80 | - | valid := !c.torndown && c.resizeGen == gen && c.leaseCurrentLocked(lease, false) && c.resize.lease == lease && c.resize.epoch == epoch && c.resize.source == source | |||
| 81 | - | c.mu.Unlock() | |||
| 82 | - | c.clearResizeTimer(gen) | |||
| 83 | - | if valid { | |||
| 84 | - | run(epoch) | |||
| 85 | - | } | |||
| 86 | - | }) | |||
| 87 | return epoch | 75 | return epoch | ||
| 88 | } | 76 | } | ||
| 89 | 77 | ||||
| 90 | - | // clearResizeTimer releases timer ownership only for the matching resize generation. | |||
| 91 | - | // A stale callback must not clear a newer request's timer or cancellation channel. | |||
| 92 | - | func (c *renderCoordinator) clearResizeTimer(gen uint64) { | |||
| 93 | - | c.mu.Lock() | |||
| 94 | - | defer c.mu.Unlock() | |||
| 95 | - | c.resizeTimerOwner().clearLocked(gen) | |||
| 96 | - | } | |||
| 97 | - | ||||
| 98 | - | func (c *renderCoordinator) completeResizeTimer(gen uint64) { | |||
| 99 | - | c.mu.Lock() | |||
| 100 | - | defer c.mu.Unlock() | |||
| 101 | - | c.resizeTimerOwner().completeLocked(gen) | |||
| 102 | - | } | |||
| 103 | - | ||||
| 104 | func (c *renderCoordinator) resizeCurrentForLease(epoch uint64, source *attachedClient, lease *attachmentLease, commit bool) bool { | 78 | func (c *renderCoordinator) resizeCurrentForLease(epoch uint64, source *attachedClient, lease *attachmentLease, commit bool) bool { | ||
| 105 | c.mu.Lock() | 79 | c.mu.Lock() | ||
| 106 | defer c.mu.Unlock() | 80 | defer c.mu.Unlock() | ||
| @@ -122,52 +96,42 @@ func (c *renderCoordinator) scheduleResizeRetryForLease(epoch uint64, source *at | |||||
| 122 | c.mu.Unlock() | 96 | c.mu.Unlock() | ||
| 123 | return | 97 | return | ||
| 124 | } | 98 | } | ||
| 125 | - | _, old := c.retryTimerOwner().replaceLocked() | 99 | + | gen, old := c.retryLane.replaceLocked() |
| 126 | - | gen := c.retryGen | |||
| 127 | clock := c.opts.clock | 100 | clock := c.opts.clock | ||
| 128 | c.mu.Unlock() | 101 | c.mu.Unlock() | ||
| 129 | - | stopAndJoinTimerWorker(old, nil) | 102 | + | stopDetachedTimer(old) |
| 130 | if clock == nil { | 103 | if clock == nil { | ||
| 131 | - | // A nil clock disables deferred retries. Completing the generation keeps | |||
| 132 | - | // retry bookkeeping consistent without recursively re-entering resize. | |||
| 133 | - | c.mu.Lock() | |||
| 134 | - | c.retryTimerOwner().completeLocked(gen) | |||
| 135 | - | c.mu.Unlock() | |||
| 136 | return | 104 | return | ||
| 137 | } | 105 | } | ||
| 138 | timer := clock.NewTimer(minOutputRenderDeadline) | 106 | timer := clock.NewTimer(minOutputRenderDeadline) | ||
| 139 | timerC := timer.C() | 107 | timerC := timer.C() | ||
| 140 | if timerC == nil { | 108 | if timerC == nil { | ||
| 141 | - | // A nil timer channel is the deterministic disabled-clock contract used | 109 | + | timer.Stop() |
| 142 | - | // by headless tests; do not spin retries synchronously. | |||
| 143 | - | stopTimer(timer) | |||
| 144 | - | c.mu.Lock() | |||
| 145 | - | c.retryTimerOwner().completeLocked(gen) | |||
| 146 | - | c.mu.Unlock() | |||
| 147 | return | 110 | return | ||
| 148 | } | 111 | } | ||
| 149 | c.mu.Lock() | 112 | c.mu.Lock() | ||
| 150 | - | valid := !c.torndown && c.retryGen == gen && c.leaseCurrentLocked(lease, false) && c.resize.lease == lease && c.resize.epoch == epoch && c.resize.committed == epoch && c.resize.source == source | 113 | + | valid := !c.torndown && c.retryLane.generation == gen && c.leaseCurrentLocked(lease, false) && c.resize.lease == lease && c.resize.epoch == epoch && c.resize.committed == epoch && c.resize.source == source |
| 151 | - | var cancel, done chan struct{} | |||
| 152 | if valid { | 114 | if valid { | ||
| 153 | - | cancel, done, valid = c.retryTimerOwner().publishLocked(gen, timer) | 115 | + | token := c.retryLane.publishLocked(gen, timer) |
| 116 | + | valid = token != nil | |||
| 117 | + | if valid { | |||
| 118 | + | c.supervisor.startLocked(token, timerC, func() { | |||
| 119 | + | c.mu.Lock() | |||
| 120 | + | valid := !c.torndown && c.retryLane.token == token && c.retryLane.generation == token.generation && c.leaseCurrentLocked(lease, false) && c.resize.lease == lease && c.resize.epoch == epoch && c.resize.committed == epoch && c.resize.source == source | |||
| 121 | + | if valid { | |||
| 122 | + | c.retryLane.clearLocked(token) | |||
| 123 | + | } | |||
| 124 | + | c.mu.Unlock() | |||
| 125 | + | if valid { | |||
| 126 | + | run() | |||
| 127 | + | } | |||
| 128 | + | }) | |||
| 129 | + | } | |||
| 154 | } | 130 | } | ||
| 155 | c.mu.Unlock() | 131 | c.mu.Unlock() | ||
| 156 | if !valid { | 132 | if !valid { | ||
| 157 | - | stopTimer(timer) | 133 | + | timer.Stop() |
| 158 | - | return | |||
| 159 | } | 134 | } | ||
| 160 | - | runTimerWorker(timerC, cancel, done, func() { | |||
| 161 | - | c.mu.Lock() | |||
| 162 | - | valid := !c.torndown && c.retryGen == gen && c.leaseCurrentLocked(lease, false) && c.resize.lease == lease && c.resize.epoch == epoch && c.resize.committed == epoch && c.resize.source == source | |||
| 163 | - | if valid { | |||
| 164 | - | c.retryTimerOwner().clearLocked(gen) | |||
| 165 | - | } | |||
| 166 | - | c.mu.Unlock() | |||
| 167 | - | if valid { | |||
| 168 | - | run() | |||
| 169 | - | } | |||
| 170 | - | }) | |||
| 171 | } | 135 | } | ||
| 172 | 136 | ||||
| 173 | func (c *renderCoordinator) retryCurrentForLease(epoch uint64, source *attachedClient, lease *attachmentLease) bool { | 137 | func (c *renderCoordinator) retryCurrentForLease(epoch uint64, source *attachedClient, lease *attachmentLease) bool { | ||
| @@ -176,15 +140,6 @@ func (c *renderCoordinator) retryCurrentForLease(epoch uint64, source *attachedC | |||||
| 176 | return c.leaseCurrentLocked(lease, false) && lease.attachment == source && c.resize.lease == lease && c.resize.source == source && c.resize.epoch == epoch && c.resize.committed == epoch | 140 | return c.leaseCurrentLocked(lease, false) && lease.attachment == source && c.resize.lease == lease && c.resize.source == source && c.resize.epoch == epoch && c.resize.committed == epoch | ||
| 177 | } | 141 | } | ||
| 178 | 142 | ||||
| 179 | - | // resizeCallbackDone returns the completion edge for the latest scheduled | |||
| 180 | - | // resize callback. A nil result means no resize deadline is pending. | |||
| 181 | - | func (c *renderCoordinator) resizeCallbackDone() <-chan struct{} { | |||
| 182 | - | c.mu.Lock() | |||
| 183 | - | defer c.mu.Unlock() | |||
| 184 | - | return c.resizeDone | |||
| 185 | - | } | |||
| 186 | - | ||||
| 187 | - | // resizeSnapshot returns a locked copy of the latest resize metadata. | |||
| 188 | func (c *renderCoordinator) resizeSnapshot() resizeRequestMetadata { | 143 | func (c *renderCoordinator) resizeSnapshot() resizeRequestMetadata { | ||
| 189 | c.mu.Lock() | 144 | c.mu.Lock() | ||
| 190 | defer c.mu.Unlock() | 145 | defer c.mu.Unlock() | ||
M internal/usecase/daemon/render_test.go +2 -2
| @@ -205,8 +205,8 @@ func fireCoordinatorTimer(t *testing.T, rc *renderCoordinator, timers []*coordin | |||
| 205 | 205 | } | |
| 206 | 206 | var done <-chan struct{} | |
| 207 | 207 | rc.mu.Lock() | |
| 208 | - | if rc.normalTimer == timer.mock { | |
| 209 | - | done = rc.normalWorkerDone | |
| 208 | + | if rc.normalLane.token.timer == timer.mock { | |
| 209 | + | done = rc.normalLane.token.done | |
| 210 | 210 | } | |
| 211 | 211 | rc.mu.Unlock() | |
| 212 | 212 | timer.ch <- time.Time{} | |
| @@ -205,8 +205,8 @@ func fireCoordinatorTimer(t *testing.T, rc *renderCoordinator, timers []*coordin | |||||
| 205 | } | 205 | } | ||
| 206 | var done <-chan struct{} | 206 | var done <-chan struct{} | ||
| 207 | rc.mu.Lock() | 207 | rc.mu.Lock() | ||
| 208 | - | if rc.normalTimer == timer.mock { | 208 | + | if rc.normalLane.token.timer == timer.mock { |
| 209 | - | done = rc.normalWorkerDone | 209 | + | done = rc.normalLane.token.done |
| 210 | } | 210 | } | ||
| 211 | rc.mu.Unlock() | 211 | rc.mu.Unlock() | ||
| 212 | timer.ch <- time.Time{} | 212 | timer.ch <- time.Time{} | ||
M internal/usecase/daemon/render_timer.go +53 -89
| @@ -1,126 +1,90 @@ | |||
| 1 | 1 | package daemon | |
| 2 | 2 | ||
| 3 | 3 | import ( | |
| 4 | + | "sync" | |
| 4 | 5 | "time" | |
| 5 | 6 | ||
| 6 | 7 | "github.com/bnema/vev/internal/ports" | |
| 7 | 8 | ) | |
| 8 | 9 | ||
| 9 | - | // timerOwnership is the coordinator-side ownership protocol for a timer lane. | |
| 10 | - | // Its methods ending in Locked require the owning coordinator lock. Timer.C, | |
| 11 | - | // Timer.Stop, and callbacks are deliberately kept out of this type and must | |
| 12 | - | // run after that lock is released: ports may synchronously re-enter the | |
| 13 | - | // coordinator. | |
| 14 | - | // | |
| 15 | - | // The pointers let each domain lane retain its own fields and predicates while | |
| 16 | - | // sharing generation tickets, cancellation, and worker completion ownership. | |
| 17 | - | type timerOwnership struct { | |
| 18 | - | timer *ports.Timer | |
| 19 | - | cancel *chan struct{} | |
| 20 | - | done *chan struct{} | |
| 21 | - | generation *uint64 | |
| 10 | + | // timerToken is the complete, immutable identity of one timer worker. A lane | |
| 11 | + | // owns either this token or nothing; cancellation and completion are never | |
| 12 | + | // retained separately where a stale callback could reacquire them. | |
| 13 | + | type timerToken struct { | |
| 14 | + | generation uint64 | |
| 15 | + | timer ports.Timer | |
| 16 | + | cancel chan struct{} | |
| 17 | + | done chan struct{} | |
| 22 | 18 | } | |
| 23 | 19 | ||
| 24 | - | func timerOwner(timer *ports.Timer, cancel, done *chan struct{}, generation *uint64) timerOwnership { | |
| 25 | - | return timerOwnership{timer: timer, cancel: cancel, done: done, generation: generation} | |
| 20 | + | // timerLane is coordinator-owned and must be accessed while renderCoordinator.mu | |
| 21 | + | // is held. Detach removes the complete token before any external Timer call. | |
| 22 | + | type timerLane struct { | |
| 23 | + | generation uint64 | |
| 24 | + | token *timerToken | |
| 26 | 25 | } | |
| 27 | 26 | ||
| 28 | - | // timerWorker is a detached timer and the completion edge of the worker | |
| 29 | - | // waiting on it. Stop and wait must happen after releasing the owner lock: | |
| 30 | - | // both Timer.Stop and a callback completing the worker may re-enter the | |
| 31 | - | // coordinator. | |
| 32 | - | type timerWorker struct { | |
| 33 | - | timer ports.Timer | |
| 34 | - | done <-chan struct{} | |
| 27 | + | func (l *timerLane) replaceLocked() (uint64, *timerToken) { | |
| 28 | + | l.generation++ | |
| 29 | + | return l.generation, l.detachLocked() | |
| 35 | 30 | } | |
| 36 | 31 | ||
| 37 | - | // replaceLocked invalidates the current ticket and releases its timer worker | |
| 38 | - | // for an unlocked cancellation and join. A worker is always released even when | |
| 39 | - | // a fake timer's channel is nil or never fires. | |
| 40 | - | func (o timerOwnership) replaceLocked() (uint64, timerWorker) { | |
| 41 | - | *o.generation = *o.generation + 1 | |
| 42 | - | return *o.generation, o.detachLocked() | |
| 43 | - | } | |
| 44 | - | ||
| 45 | - | // detachLocked releases the current worker and timer. The returned worker must | |
| 46 | - | // be stopped and joined after dropping the owner lock. | |
| 47 | - | func (o timerOwnership) detachLocked() timerWorker { | |
| 48 | - | worker := timerWorker{timer: *o.timer, done: *o.done} | |
| 49 | - | *o.timer = nil | |
| 50 | - | if *o.cancel != nil { | |
| 51 | - | close(*o.cancel) | |
| 52 | - | *o.cancel = nil | |
| 32 | + | func (l *timerLane) detachLocked() *timerToken { | |
| 33 | + | token := l.token | |
| 34 | + | l.token = nil | |
| 35 | + | if token != nil { | |
| 36 | + | close(token.cancel) | |
| 53 | 37 | } | |
| 54 | - | return worker | |
| 38 | + | return token | |
| 55 | 39 | } | |
| 56 | 40 | ||
| 57 | - | // publishLocked assigns a timer created and inspected outside the owner lock. | |
| 58 | - | // It rejects a stale ticket or an already-replaced timer without invoking a | |
| 59 | - | // port method. | |
| 60 | - | func (o timerOwnership) publishLocked(ticket uint64, timer ports.Timer) (cancel, done chan struct{}, ok bool) { | |
| 61 | - | if *o.generation != ticket || *o.timer != nil { | |
| 62 | - | return nil, nil, false | |
| 41 | + | func (l *timerLane) publishLocked(generation uint64, timer ports.Timer) *timerToken { | |
| 42 | + | if l.generation != generation || l.token != nil { | |
| 43 | + | return nil | |
| 63 | 44 | } | |
| 64 | - | cancel, done = make(chan struct{}), make(chan struct{}) | |
| 65 | - | *o.timer, *o.cancel, *o.done = timer, cancel, done | |
| 66 | - | return cancel, done, true | |
| 67 | - | } | |
| 68 | - | ||
| 69 | - | // clearLocked releases only the matching ticket after its worker has consumed | |
| 70 | - | // a tick. It cannot clear a replacement timer. | |
| 71 | - | func (o timerOwnership) clearLocked(ticket uint64) bool { | |
| 72 | - | if *o.generation != ticket { | |
| 73 | - | return false | |
| 45 | + | token := &timerToken{ | |
| 46 | + | generation: generation, | |
| 47 | + | timer: timer, | |
| 48 | + | cancel: make(chan struct{}), | |
| 49 | + | done: make(chan struct{}), | |
| 74 | 50 | } | |
| 75 | - | *o.timer, *o.cancel = nil, nil | |
| 76 | - | return true | |
| 51 | + | l.token = token | |
| 52 | + | return token | |
| 77 | 53 | } | |
| 78 | 54 | ||
| 79 | - | // completeLocked records a completed no-worker timer path (such as a disabled | |
| 80 | - | // clock) without overwriting a replacement lane's completion edge. | |
| 81 | - | func (o timerOwnership) completeLocked(ticket uint64) bool { | |
| 82 | - | if *o.generation != ticket { | |
| 55 | + | func (l *timerLane) clearLocked(token *timerToken) bool { | |
| 56 | + | if token == nil || l.generation != token.generation || l.token != token { | |
| 83 | 57 | return false | |
| 84 | 58 | } | |
| 85 | - | done := make(chan struct{}) | |
| 86 | - | close(done) | |
| 87 | - | *o.done = done | |
| 59 | + | l.token = nil | |
| 88 | 60 | return true | |
| 89 | 61 | } | |
| 90 | 62 | ||
| 91 | - | // runTimerWorker owns the completion edge for an armed timer. A nil timer | |
| 92 | - | // channel intentionally does not start a worker; callers apply their | |
| 93 | - | // domain-specific disabled-clock behavior synchronously. | |
| 94 | - | func runTimerWorker(timerC <-chan time.Time, cancel <-chan struct{}, done chan struct{}, fire func()) { | |
| 63 | + | // timerSupervisor owns worker accounting. Workers are registered only while | |
| 64 | + | // coordinator ownership is held and terminal teardown rejects registrations | |
| 65 | + | // before Wait begins, so Add cannot race the terminal Wait. | |
| 66 | + | type timerSupervisor struct{ workers sync.WaitGroup } | |
| 67 | + | ||
| 68 | + | func (s *timerSupervisor) startLocked(token *timerToken, timerC <-chan time.Time, fire func()) { | |
| 69 | + | s.workers.Add(1) | |
| 95 | 70 | go func() { | |
| 96 | - | defer close(done) | |
| 71 | + | defer s.workers.Done() | |
| 72 | + | defer close(token.done) | |
| 97 | 73 | select { | |
| 98 | 74 | case <-timerC: | |
| 99 | 75 | fire() | |
| 100 | - | case <-cancel: | |
| 76 | + | case <-token.cancel: | |
| 101 | 77 | } | |
| 102 | 78 | }() | |
| 103 | 79 | } | |
| 104 | 80 | ||
| 105 | - | func stopTimer(timer ports.Timer) { | |
| 106 | - | if timer != nil { | |
| 107 | - | timer.Stop() | |
| 108 | - | } | |
| 109 | - | } | |
| 110 | - | ||
| 111 | - | // stopAndJoinTimerWorker prevents a cancelled timer callback from escaping a | |
| 112 | - | // lifecycle boundary and touching transports or test-owned timer mocks later. | |
| 113 | - | // Callers must not hold coordinator locks here. self is supplied only by a | |
| 114 | - | // worker that consumed its own tick; waiting for that worker would deadlock. | |
| 115 | - | func stopAndJoinTimerWorker(worker timerWorker, self <-chan struct{}) { | |
| 116 | - | stopTimer(worker.timer) | |
| 117 | - | if worker.done != nil && worker.done != self { | |
| 118 | - | <-worker.done | |
| 119 | - | } | |
| 120 | - | } | |
| 81 | + | func (s *timerSupervisor) wait() { s.workers.Wait() } | |
| 121 | 82 | ||
| 122 | - | func stopAndJoinTimerWorkers(workers []timerWorker) { | |
| 123 | - | for _, worker := range workers { | |
| 124 | - | stopAndJoinTimerWorker(worker, nil) | |
| 83 | + | // stopDetachedTimer is intentionally non-blocking. Ordinary lifecycle and | |
| 84 | + | // replacement transitions may cancel/stop stale workers but callback stacks | |
| 85 | + | // never wait for a worker to finish. | |
| 86 | + | func stopDetachedTimer(token *timerToken) { | |
| 87 | + | if token != nil && token.timer != nil { | |
| 88 | + | token.timer.Stop() | |
| 125 | 89 | } | |
| 126 | 90 | } | |
| @@ -1,126 +1,90 @@ | |||||
| 1 | package daemon | 1 | package daemon | ||
| 2 | 2 | ||||
| 3 | import ( | 3 | import ( | ||
| 4 | + | "sync" | |||
| 4 | "time" | 5 | "time" | ||
| 5 | 6 | ||||
| 6 | "github.com/bnema/vev/internal/ports" | 7 | "github.com/bnema/vev/internal/ports" | ||
| 7 | ) | 8 | ) | ||
| 8 | 9 | ||||
| 9 | - | // timerOwnership is the coordinator-side ownership protocol for a timer lane. | 10 | + | // timerToken is the complete, immutable identity of one timer worker. A lane |
| 10 | - | // Its methods ending in Locked require the owning coordinator lock. Timer.C, | 11 | + | // owns either this token or nothing; cancellation and completion are never |
| 11 | - | // Timer.Stop, and callbacks are deliberately kept out of this type and must | 12 | + | // retained separately where a stale callback could reacquire them. |
| 12 | - | // run after that lock is released: ports may synchronously re-enter the | 13 | + | type timerToken struct { |
| 13 | - | // coordinator. | 14 | + | generation uint64 |
| 14 | - | // | 15 | + | timer ports.Timer |
| 15 | - | // The pointers let each domain lane retain its own fields and predicates while | 16 | + | cancel chan struct{} |
| 16 | - | // sharing generation tickets, cancellation, and worker completion ownership. | 17 | + | done chan struct{} |
| 17 | - | type timerOwnership struct { | |||
| 18 | - | timer *ports.Timer | |||
| 19 | - | cancel *chan struct{} | |||
| 20 | - | done *chan struct{} | |||
| 21 | - | generation *uint64 | |||
| 22 | } | 18 | } | ||
| 23 | 19 | ||||
| 24 | - | func timerOwner(timer *ports.Timer, cancel, done *chan struct{}, generation *uint64) timerOwnership { | 20 | + | // timerLane is coordinator-owned and must be accessed while renderCoordinator.mu |
| 25 | - | return timerOwnership{timer: timer, cancel: cancel, done: done, generation: generation} | 21 | + | // is held. Detach removes the complete token before any external Timer call. |
| 22 | + | type timerLane struct { | |||
| 23 | + | generation uint64 | |||
| 24 | + | token *timerToken | |||
| 26 | } | 25 | } | ||
| 27 | 26 | ||||
| 28 | - | // timerWorker is a detached timer and the completion edge of the worker | 27 | + | func (l *timerLane) replaceLocked() (uint64, *timerToken) { |
| 29 | - | // waiting on it. Stop and wait must happen after releasing the owner lock: | 28 | + | l.generation++ |
| 30 | - | // both Timer.Stop and a callback completing the worker may re-enter the | 29 | + | return l.generation, l.detachLocked() |
| 31 | - | // coordinator. | |||
| 32 | - | type timerWorker struct { | |||
| 33 | - | timer ports.Timer | |||
| 34 | - | done <-chan struct{} | |||
| 35 | } | 30 | } | ||
| 36 | 31 | ||||
| 37 | - | // replaceLocked invalidates the current ticket and releases its timer worker | 32 | + | func (l *timerLane) detachLocked() *timerToken { |
| 38 | - | // for an unlocked cancellation and join. A worker is always released even when | 33 | + | token := l.token |
| 39 | - | // a fake timer's channel is nil or never fires. | 34 | + | l.token = nil |
| 40 | - | func (o timerOwnership) replaceLocked() (uint64, timerWorker) { | 35 | + | if token != nil { |
| 41 | - | *o.generation = *o.generation + 1 | 36 | + | close(token.cancel) |
| 42 | - | return *o.generation, o.detachLocked() | |||
| 43 | - | } | |||
| 44 | - | ||||
| 45 | - | // detachLocked releases the current worker and timer. The returned worker must | |||
| 46 | - | // be stopped and joined after dropping the owner lock. | |||
| 47 | - | func (o timerOwnership) detachLocked() timerWorker { | |||
| 48 | - | worker := timerWorker{timer: *o.timer, done: *o.done} | |||
| 49 | - | *o.timer = nil | |||
| 50 | - | if *o.cancel != nil { | |||
| 51 | - | close(*o.cancel) | |||
| 52 | - | *o.cancel = nil | |||
| 53 | } | 37 | } | ||
| 54 | - | return worker | 38 | + | return token |
| 55 | } | 39 | } | ||
| 56 | 40 | ||||
| 57 | - | // publishLocked assigns a timer created and inspected outside the owner lock. | 41 | + | func (l *timerLane) publishLocked(generation uint64, timer ports.Timer) *timerToken { |
| 58 | - | // It rejects a stale ticket or an already-replaced timer without invoking a | 42 | + | if l.generation != generation || l.token != nil { |
| 59 | - | // port method. | 43 | + | return nil |
| 60 | - | func (o timerOwnership) publishLocked(ticket uint64, timer ports.Timer) (cancel, done chan struct{}, ok bool) { | |||
| 61 | - | if *o.generation != ticket || *o.timer != nil { | |||
| 62 | - | return nil, nil, false | |||
| 63 | } | 44 | } | ||
| 64 | - | cancel, done = make(chan struct{}), make(chan struct{}) | 45 | + | token := &timerToken{ |
| 65 | - | *o.timer, *o.cancel, *o.done = timer, cancel, done | 46 | + | generation: generation, |
| 66 | - | return cancel, done, true | 47 | + | timer: timer, |
| 67 | - | } | 48 | + | cancel: make(chan struct{}), |
| 68 | - | 49 | + | done: make(chan struct{}), | |
| 69 | - | // clearLocked releases only the matching ticket after its worker has consumed | |||
| 70 | - | // a tick. It cannot clear a replacement timer. | |||
| 71 | - | func (o timerOwnership) clearLocked(ticket uint64) bool { | |||
| 72 | - | if *o.generation != ticket { | |||
| 73 | - | return false | |||
| 74 | } | 50 | } | ||
| 75 | - | *o.timer, *o.cancel = nil, nil | 51 | + | l.token = token |
| 76 | - | return true | 52 | + | return token |
| 77 | } | 53 | } | ||
| 78 | 54 | ||||
| 79 | - | // completeLocked records a completed no-worker timer path (such as a disabled | 55 | + | func (l *timerLane) clearLocked(token *timerToken) bool { |
| 80 | - | // clock) without overwriting a replacement lane's completion edge. | 56 | + | if token == nil || l.generation != token.generation || l.token != token { |
| 81 | - | func (o timerOwnership) completeLocked(ticket uint64) bool { | |||
| 82 | - | if *o.generation != ticket { | |||
| 83 | return false | 57 | return false | ||
| 84 | } | 58 | } | ||
| 85 | - | done := make(chan struct{}) | 59 | + | l.token = nil |
| 86 | - | close(done) | |||
| 87 | - | *o.done = done | |||
| 88 | return true | 60 | return true | ||
| 89 | } | 61 | } | ||
| 90 | 62 | ||||
| 91 | - | // runTimerWorker owns the completion edge for an armed timer. A nil timer | 63 | + | // timerSupervisor owns worker accounting. Workers are registered only while |
| 92 | - | // channel intentionally does not start a worker; callers apply their | 64 | + | // coordinator ownership is held and terminal teardown rejects registrations |
| 93 | - | // domain-specific disabled-clock behavior synchronously. | 65 | + | // before Wait begins, so Add cannot race the terminal Wait. |
| 94 | - | func runTimerWorker(timerC <-chan time.Time, cancel <-chan struct{}, done chan struct{}, fire func()) { | 66 | + | type timerSupervisor struct{ workers sync.WaitGroup } |
| 67 | + | ||||
| 68 | + | func (s *timerSupervisor) startLocked(token *timerToken, timerC <-chan time.Time, fire func()) { | |||
| 69 | + | s.workers.Add(1) | |||
| 95 | go func() { | 70 | go func() { | ||
| 96 | - | defer close(done) | 71 | + | defer s.workers.Done() |
| 72 | + | defer close(token.done) | |||
| 97 | select { | 73 | select { | ||
| 98 | case <-timerC: | 74 | case <-timerC: | ||
| 99 | fire() | 75 | fire() | ||
| 100 | - | case <-cancel: | 76 | + | case <-token.cancel: |
| 101 | } | 77 | } | ||
| 102 | }() | 78 | }() | ||
| 103 | } | 79 | } | ||
| 104 | 80 | ||||
| 105 | - | func stopTimer(timer ports.Timer) { | 81 | + | func (s *timerSupervisor) wait() { s.workers.Wait() } |
| 106 | - | if timer != nil { | |||
| 107 | - | timer.Stop() | |||
| 108 | - | } | |||
| 109 | - | } | |||
| 110 | - | ||||
| 111 | - | // stopAndJoinTimerWorker prevents a cancelled timer callback from escaping a | |||
| 112 | - | // lifecycle boundary and touching transports or test-owned timer mocks later. | |||
| 113 | - | // Callers must not hold coordinator locks here. self is supplied only by a | |||
| 114 | - | // worker that consumed its own tick; waiting for that worker would deadlock. | |||
| 115 | - | func stopAndJoinTimerWorker(worker timerWorker, self <-chan struct{}) { | |||
| 116 | - | stopTimer(worker.timer) | |||
| 117 | - | if worker.done != nil && worker.done != self { | |||
| 118 | - | <-worker.done | |||
| 119 | - | } | |||
| 120 | - | } | |||
| 121 | 82 | ||||
| 122 | - | func stopAndJoinTimerWorkers(workers []timerWorker) { | 83 | + | // stopDetachedTimer is intentionally non-blocking. Ordinary lifecycle and |
| 123 | - | for _, worker := range workers { | 84 | + | // replacement transitions may cancel/stop stale workers but callback stacks |
| 124 | - | stopAndJoinTimerWorker(worker, nil) | 85 | + | // never wait for a worker to finish. |
| 86 | + | func stopDetachedTimer(token *timerToken) { | |||
| 87 | + | if token != nil && token.timer != nil { | |||
| 88 | + | token.timer.Stop() | |||
| 125 | } | 89 | } | ||
| 126 | } | 90 | } | ||
M internal/usecase/daemon/render_timer_test.go +129 -39
| @@ -6,49 +6,139 @@ import ( | |||
| 6 | 6 | ||
| 7 | 7 | "github.com/stretchr/testify/require" | |
| 8 | 8 | ||
| 9 | - | "github.com/bnema/vev/internal/ports" | |
| 9 | + | "github.com/bnema/vev/internal/domain" | |
| 10 | 10 | portsmocks "github.com/bnema/vev/internal/ports/mocks" | |
| 11 | 11 | ) | |
| 12 | 12 | ||
| 13 | - | func TestTimerOwnershipRejectsStaleTicketsAndCancelsWorkers(t *testing.T) { | |
| 14 | - | var timer ports.Timer | |
| 15 | - | var cancel, done chan struct{} | |
| 16 | - | var generation uint64 | |
| 17 | - | owner := timerOwner(&timer, &cancel, &done, &generation) | |
| 18 | - | ||
| 19 | - | ticket, stopped := owner.replaceLocked() | |
| 20 | - | require.Nil(t, stopped.timer) | |
| 21 | - | first := portsmocks.NewMockTimer(t) | |
| 22 | - | first.EXPECT().Stop().Return(true).Once() | |
| 23 | - | firstCancel, firstDone, ok := owner.publishLocked(ticket, first) | |
| 24 | - | require.True(t, ok) | |
| 25 | - | ||
| 26 | - | // A replacement releases a nil-channel worker through cancellation, rather | |
| 27 | - | // than relying on Timer.Stop to wake it. | |
| 28 | - | nilTimerC := (<-chan time.Time)(nil) | |
| 29 | - | runTimerWorker(nilTimerC, firstCancel, firstDone, func() { t.Fatal("cancelled worker fired") }) | |
| 30 | - | _, stopped = owner.replaceLocked() | |
| 31 | - | require.Same(t, first, stopped.timer) | |
| 32 | - | stopAndJoinTimerWorker(stopped, nil) | |
| 33 | - | ||
| 34 | - | _, _, ok = owner.publishLocked(ticket, portsmocks.NewMockTimer(t)) | |
| 35 | - | require.False(t, ok, "a stale generation must not reclaim a replacement lane") | |
| 36 | - | ||
| 37 | - | require.True(t, owner.completeLocked(generation)) | |
| 38 | - | awaitTestCompletion(t, done, "timer worker did not complete") | |
| 39 | - | } | |
| 40 | - | ||
| 41 | - | func TestRunTimerWorkerOwnsCompletionAfterTick(t *testing.T) { | |
| 42 | - | timerC := make(chan time.Time, 1) | |
| 43 | - | cancel := make(chan struct{}) | |
| 44 | - | done := make(chan struct{}) | |
| 45 | - | fired := make(chan struct{}, 1) | |
| 46 | - | runTimerWorker(timerC, cancel, done, func() { fired <- struct{}{} }) | |
| 47 | - | timerC <- time.Time{} | |
| 48 | - | awaitTestCompletion(t, done, "timer worker did not complete") | |
| 13 | + | func TestTimerLaneDetachRemovesCompleteToken(t *testing.T) { | |
| 14 | + | lane := timerLane{generation: 1} | |
| 15 | + | timer := portsmocks.NewMockTimer(t) | |
| 16 | + | token := lane.publishLocked(1, timer) | |
| 17 | + | require.NotNil(t, token) | |
| 18 | + | detached := lane.detachLocked() | |
| 19 | + | require.Same(t, token, detached) | |
| 20 | + | require.Nil(t, lane.token) | |
| 49 | 21 | select { | |
| 50 | - | case <-fired: | |
| 22 | + | case <-token.cancel: | |
| 51 | 23 | default: | |
| 52 | - | t.Fatal("timer worker did not run callback") | |
| 24 | + | t.Fatal("detach did not cancel token") | |
| 25 | + | } | |
| 26 | + | require.Nil(t, lane.publishLocked(0, timer), "stale generation reacquired detached lane") | |
| 27 | + | } | |
| 28 | + | ||
| 29 | + | func TestRenderDeadlineCallbackCanInvalidateAgainWithoutJoiningItself(t *testing.T) { | |
| 30 | + | clock := newCoordinatorMockClock(t, 2) | |
| 31 | + | ac := &attachedClient{} | |
| 32 | + | wakeReturned := make(chan struct{}) | |
| 33 | + | var rc *renderCoordinator | |
| 34 | + | rc = newRenderCoordinator(renderCoordinatorOptions{ | |
| 35 | + | clock: clock.clock, | |
| 36 | + | wake: func(renderWake) { | |
| 37 | + | require.True(t, rc.invalidateForAttachment(ac, renderInvalidation{class: invalidateUrgent, producer: "reentrant-wake-test"})) | |
| 38 | + | close(wakeReturned) | |
| 39 | + | }, | |
| 40 | + | }) | |
| 41 | + | rc.attach(ac) | |
| 42 | + | t.Cleanup(func() { | |
| 43 | + | rc.beginSessionTeardown().finish() | |
| 44 | + | rc.waitForTimerWorkers() | |
| 45 | + | }) | |
| 46 | + | require.True(t, rc.invalidateForAttachment(ac, renderInvalidation{class: invalidateUrgent, producer: "initial-wake-test"})) | |
| 47 | + | awaitCoordinatorScheduledTimer(t, clock).ch <- time.Time{} | |
| 48 | + | awaitTestCompletion(t, wakeReturned, "timer callback deadlocked while publishing its next invalidation") | |
| 49 | + | } | |
| 50 | + | ||
| 51 | + | func TestRenderTimerCallbacksCanReplaceTheirOwnLane(t *testing.T) { | |
| 52 | + | cases := []struct { | |
| 53 | + | name string | |
| 54 | + | run func(*testing.T, *renderCoordinator, *coordinatorMockClock, *attachedClient) | |
| 55 | + | }{ | |
| 56 | + | { | |
| 57 | + | name: "normal wake invalidates", run: func(t *testing.T, rc *renderCoordinator, clock *coordinatorMockClock, ac *attachedClient) { | |
| 58 | + | woke := make(chan struct{}) | |
| 59 | + | rc.opts.wake = func(renderWake) { | |
| 60 | + | rc.invalidateForAttachment(ac, renderInvalidation{class: invalidateUrgent, producer: "table-normal"}) | |
| 61 | + | close(woke) | |
| 62 | + | } | |
| 63 | + | rc.invalidateForAttachment(ac, renderInvalidation{class: invalidateUrgent, producer: "table-normal"}) | |
| 64 | + | awaitCoordinatorScheduledTimer(t, clock).ch <- time.Time{} | |
| 65 | + | awaitTestCompletion(t, woke, "normal callback did not reenter invalidation") | |
| 66 | + | }, | |
| 67 | + | }, | |
| 68 | + | { | |
| 69 | + | name: "resize callback schedules resize", run: func(t *testing.T, rc *renderCoordinator, clock *coordinatorMockClock, ac *attachedClient) { | |
| 70 | + | lease := rc.attachmentLease(ac) | |
| 71 | + | rc.scheduleResizeForLease(domain.Size{Cols: 80, Rows: 24}, ac, lease, func(uint64) { | |
| 72 | + | rc.scheduleResizeForLease(domain.Size{Cols: 81, Rows: 24}, ac, lease, func(uint64) {}) | |
| 73 | + | }) | |
| 74 | + | timer := awaitCoordinatorScheduledTimer(t, clock) | |
| 75 | + | done := captureResizeCallbackDone(t, rc) | |
| 76 | + | timer.ch <- time.Time{} | |
| 77 | + | _ = awaitTestValue(t, clock.timers, "resize callback did not schedule replacement") | |
| 78 | + | awaitTestCompletion(t, done, "resize callback did not complete") | |
| 79 | + | }, | |
| 80 | + | }, | |
| 81 | + | { | |
| 82 | + | name: "retry callback schedules retry", run: func(t *testing.T, rc *renderCoordinator, clock *coordinatorMockClock, ac *attachedClient) { | |
| 83 | + | lease := rc.attachmentLease(ac) | |
| 84 | + | epoch := rc.recordResizeRequestForLease(domain.Size{Cols: 80, Rows: 24}, ac, lease) | |
| 85 | + | require.True(t, rc.resizeCurrentForLease(epoch, ac, lease, true)) | |
| 86 | + | rc.scheduleResizeRetryForLease(epoch, ac, lease, func() { rc.scheduleResizeRetryForLease(epoch, ac, lease, func() {}) }) | |
| 87 | + | awaitCoordinatorScheduledTimer(t, clock).ch <- time.Time{} | |
| 88 | + | _ = awaitTestValue(t, clock.timers, "retry callback did not schedule replacement") | |
| 89 | + | }, | |
| 90 | + | }, | |
| 91 | + | { | |
| 92 | + | name: "sync force transitions sync generation", run: func(t *testing.T, rc *renderCoordinator, clock *coordinatorMockClock, _ *attachedClient) { | |
| 93 | + | p := &pane{} | |
| 94 | + | rc.noteSyncBegin(p, 1, func() { rc.noteSyncBegin(p, 2) }) | |
| 95 | + | awaitCoordinatorScheduledTimer(t, clock).ch <- time.Time{} | |
| 96 | + | _ = awaitTestValue(t, clock.timers, "sync force did not schedule replacement") | |
| 97 | + | }, | |
| 98 | + | }, | |
| 53 | 99 | } | |
| 100 | + | for _, tc := range cases { | |
| 101 | + | t.Run(tc.name, func(t *testing.T) { | |
| 102 | + | clock := newCoordinatorMockClock(t, 4) | |
| 103 | + | rc, ac := newRenderCoordinator(renderCoordinatorOptions{clock: clock.clock}), &attachedClient{} | |
| 104 | + | rc.attach(ac) | |
| 105 | + | tc.run(t, rc, clock, ac) | |
| 106 | + | rc.beginSessionTeardown().finish() | |
| 107 | + | rc.waitForTimerWorkers() | |
| 108 | + | }) | |
| 109 | + | } | |
| 110 | + | } | |
| 111 | + | ||
| 112 | + | func TestRenderTimerScheduleRacesTerminalTeardown(t *testing.T) { | |
| 113 | + | for range 32 { | |
| 114 | + | clock := newCoordinatorMockClock(t, 2) | |
| 115 | + | rc, ac := newRenderCoordinator(renderCoordinatorOptions{clock: clock.clock}), &attachedClient{} | |
| 116 | + | rc.attach(ac) | |
| 117 | + | start := make(chan struct{}) | |
| 118 | + | scheduled := make(chan struct{}) | |
| 119 | + | go func() { | |
| 120 | + | <-start | |
| 121 | + | rc.invalidateForAttachment(ac, renderInvalidation{class: invalidateUrgent, producer: "race"}) | |
| 122 | + | close(scheduled) | |
| 123 | + | }() | |
| 124 | + | close(start) | |
| 125 | + | cleanup := rc.beginSessionTeardown() | |
| 126 | + | awaitTestCompletion(t, scheduled, "schedule raced terminal teardown") | |
| 127 | + | cleanup.finish() | |
| 128 | + | rc.waitForTimerWorkers() | |
| 129 | + | rc.mu.Lock() | |
| 130 | + | require.True(t, rc.torndown) | |
| 131 | + | require.Nil(t, rc.normalLane.token) | |
| 132 | + | rc.mu.Unlock() | |
| 133 | + | } | |
| 134 | + | } | |
| 135 | + | ||
| 136 | + | func TestTimerSupervisorCancelsNeverFiringWorker(t *testing.T) { | |
| 137 | + | var lane timerLane | |
| 138 | + | generation, _ := lane.replaceLocked() | |
| 139 | + | token := lane.publishLocked(generation, portsmocks.NewMockTimer(t)) | |
| 140 | + | var supervisor timerSupervisor | |
| 141 | + | supervisor.startLocked(token, nil, func() { t.Fatal("cancelled worker fired") }) | |
| 142 | + | lane.detachLocked() | |
| 143 | + | supervisor.wait() | |
| 54 | 144 | } | |
| @@ -6,49 +6,139 @@ import ( | |||||
| 6 | 6 | ||||
| 7 | "github.com/stretchr/testify/require" | 7 | "github.com/stretchr/testify/require" | ||
| 8 | 8 | ||||
| 9 | - | "github.com/bnema/vev/internal/ports" | 9 | + | "github.com/bnema/vev/internal/domain" |
| 10 | portsmocks "github.com/bnema/vev/internal/ports/mocks" | 10 | portsmocks "github.com/bnema/vev/internal/ports/mocks" | ||
| 11 | ) | 11 | ) | ||
| 12 | 12 | ||||
| 13 | - | func TestTimerOwnershipRejectsStaleTicketsAndCancelsWorkers(t *testing.T) { | 13 | + | func TestTimerLaneDetachRemovesCompleteToken(t *testing.T) { |
| 14 | - | var timer ports.Timer | 14 | + | lane := timerLane{generation: 1} |
| 15 | - | var cancel, done chan struct{} | 15 | + | timer := portsmocks.NewMockTimer(t) |
| 16 | - | var generation uint64 | 16 | + | token := lane.publishLocked(1, timer) |
| 17 | - | owner := timerOwner(&timer, &cancel, &done, &generation) | 17 | + | require.NotNil(t, token) |
| 18 | - | 18 | + | detached := lane.detachLocked() | |
| 19 | - | ticket, stopped := owner.replaceLocked() | 19 | + | require.Same(t, token, detached) |
| 20 | - | require.Nil(t, stopped.timer) | 20 | + | require.Nil(t, lane.token) |
| 21 | - | first := portsmocks.NewMockTimer(t) | |||
| 22 | - | first.EXPECT().Stop().Return(true).Once() | |||
| 23 | - | firstCancel, firstDone, ok := owner.publishLocked(ticket, first) | |||
| 24 | - | require.True(t, ok) | |||
| 25 | - | ||||
| 26 | - | // A replacement releases a nil-channel worker through cancellation, rather | |||
| 27 | - | // than relying on Timer.Stop to wake it. | |||
| 28 | - | nilTimerC := (<-chan time.Time)(nil) | |||
| 29 | - | runTimerWorker(nilTimerC, firstCancel, firstDone, func() { t.Fatal("cancelled worker fired") }) | |||
| 30 | - | _, stopped = owner.replaceLocked() | |||
| 31 | - | require.Same(t, first, stopped.timer) | |||
| 32 | - | stopAndJoinTimerWorker(stopped, nil) | |||
| 33 | - | ||||
| 34 | - | _, _, ok = owner.publishLocked(ticket, portsmocks.NewMockTimer(t)) | |||
| 35 | - | require.False(t, ok, "a stale generation must not reclaim a replacement lane") | |||
| 36 | - | ||||
| 37 | - | require.True(t, owner.completeLocked(generation)) | |||
| 38 | - | awaitTestCompletion(t, done, "timer worker did not complete") | |||
| 39 | - | } | |||
| 40 | - | ||||
| 41 | - | func TestRunTimerWorkerOwnsCompletionAfterTick(t *testing.T) { | |||
| 42 | - | timerC := make(chan time.Time, 1) | |||
| 43 | - | cancel := make(chan struct{}) | |||
| 44 | - | done := make(chan struct{}) | |||
| 45 | - | fired := make(chan struct{}, 1) | |||
| 46 | - | runTimerWorker(timerC, cancel, done, func() { fired <- struct{}{} }) | |||
| 47 | - | timerC <- time.Time{} | |||
| 48 | - | awaitTestCompletion(t, done, "timer worker did not complete") | |||
| 49 | select { | 21 | select { | ||
| 50 | - | case <-fired: | 22 | + | case <-token.cancel: |
| 51 | default: | 23 | default: | ||
| 52 | - | t.Fatal("timer worker did not run callback") | 24 | + | t.Fatal("detach did not cancel token") |
| 25 | + | } | |||
| 26 | + | require.Nil(t, lane.publishLocked(0, timer), "stale generation reacquired detached lane") | |||
| 27 | + | } | |||
| 28 | + | ||||
| 29 | + | func TestRenderDeadlineCallbackCanInvalidateAgainWithoutJoiningItself(t *testing.T) { | |||
| 30 | + | clock := newCoordinatorMockClock(t, 2) | |||
| 31 | + | ac := &attachedClient{} | |||
| 32 | + | wakeReturned := make(chan struct{}) | |||
| 33 | + | var rc *renderCoordinator | |||
| 34 | + | rc = newRenderCoordinator(renderCoordinatorOptions{ | |||
| 35 | + | clock: clock.clock, | |||
| 36 | + | wake: func(renderWake) { | |||
| 37 | + | require.True(t, rc.invalidateForAttachment(ac, renderInvalidation{class: invalidateUrgent, producer: "reentrant-wake-test"})) | |||
| 38 | + | close(wakeReturned) | |||
| 39 | + | }, | |||
| 40 | + | }) | |||
| 41 | + | rc.attach(ac) | |||
| 42 | + | t.Cleanup(func() { | |||
| 43 | + | rc.beginSessionTeardown().finish() | |||
| 44 | + | rc.waitForTimerWorkers() | |||
| 45 | + | }) | |||
| 46 | + | require.True(t, rc.invalidateForAttachment(ac, renderInvalidation{class: invalidateUrgent, producer: "initial-wake-test"})) | |||
| 47 | + | awaitCoordinatorScheduledTimer(t, clock).ch <- time.Time{} | |||
| 48 | + | awaitTestCompletion(t, wakeReturned, "timer callback deadlocked while publishing its next invalidation") | |||
| 49 | + | } | |||
| 50 | + | ||||
| 51 | + | func TestRenderTimerCallbacksCanReplaceTheirOwnLane(t *testing.T) { | |||
| 52 | + | cases := []struct { | |||
| 53 | + | name string | |||
| 54 | + | run func(*testing.T, *renderCoordinator, *coordinatorMockClock, *attachedClient) | |||
| 55 | + | }{ | |||
| 56 | + | { | |||
| 57 | + | name: "normal wake invalidates", run: func(t *testing.T, rc *renderCoordinator, clock *coordinatorMockClock, ac *attachedClient) { | |||
| 58 | + | woke := make(chan struct{}) | |||
| 59 | + | rc.opts.wake = func(renderWake) { | |||
| 60 | + | rc.invalidateForAttachment(ac, renderInvalidation{class: invalidateUrgent, producer: "table-normal"}) | |||
| 61 | + | close(woke) | |||
| 62 | + | } | |||
| 63 | + | rc.invalidateForAttachment(ac, renderInvalidation{class: invalidateUrgent, producer: "table-normal"}) | |||
| 64 | + | awaitCoordinatorScheduledTimer(t, clock).ch <- time.Time{} | |||
| 65 | + | awaitTestCompletion(t, woke, "normal callback did not reenter invalidation") | |||
| 66 | + | }, | |||
| 67 | + | }, | |||
| 68 | + | { | |||
| 69 | + | name: "resize callback schedules resize", run: func(t *testing.T, rc *renderCoordinator, clock *coordinatorMockClock, ac *attachedClient) { | |||
| 70 | + | lease := rc.attachmentLease(ac) | |||
| 71 | + | rc.scheduleResizeForLease(domain.Size{Cols: 80, Rows: 24}, ac, lease, func(uint64) { | |||
| 72 | + | rc.scheduleResizeForLease(domain.Size{Cols: 81, Rows: 24}, ac, lease, func(uint64) {}) | |||
| 73 | + | }) | |||
| 74 | + | timer := awaitCoordinatorScheduledTimer(t, clock) | |||
| 75 | + | done := captureResizeCallbackDone(t, rc) | |||
| 76 | + | timer.ch <- time.Time{} | |||
| 77 | + | _ = awaitTestValue(t, clock.timers, "resize callback did not schedule replacement") | |||
| 78 | + | awaitTestCompletion(t, done, "resize callback did not complete") | |||
| 79 | + | }, | |||
| 80 | + | }, | |||
| 81 | + | { | |||
| 82 | + | name: "retry callback schedules retry", run: func(t *testing.T, rc *renderCoordinator, clock *coordinatorMockClock, ac *attachedClient) { | |||
| 83 | + | lease := rc.attachmentLease(ac) | |||
| 84 | + | epoch := rc.recordResizeRequestForLease(domain.Size{Cols: 80, Rows: 24}, ac, lease) | |||
| 85 | + | require.True(t, rc.resizeCurrentForLease(epoch, ac, lease, true)) | |||
| 86 | + | rc.scheduleResizeRetryForLease(epoch, ac, lease, func() { rc.scheduleResizeRetryForLease(epoch, ac, lease, func() {}) }) | |||
| 87 | + | awaitCoordinatorScheduledTimer(t, clock).ch <- time.Time{} | |||
| 88 | + | _ = awaitTestValue(t, clock.timers, "retry callback did not schedule replacement") | |||
| 89 | + | }, | |||
| 90 | + | }, | |||
| 91 | + | { | |||
| 92 | + | name: "sync force transitions sync generation", run: func(t *testing.T, rc *renderCoordinator, clock *coordinatorMockClock, _ *attachedClient) { | |||
| 93 | + | p := &pane{} | |||
| 94 | + | rc.noteSyncBegin(p, 1, func() { rc.noteSyncBegin(p, 2) }) | |||
| 95 | + | awaitCoordinatorScheduledTimer(t, clock).ch <- time.Time{} | |||
| 96 | + | _ = awaitTestValue(t, clock.timers, "sync force did not schedule replacement") | |||
| 97 | + | }, | |||
| 98 | + | }, | |||
| 53 | } | 99 | } | ||
| 100 | + | for _, tc := range cases { | |||
| 101 | + | t.Run(tc.name, func(t *testing.T) { | |||
| 102 | + | clock := newCoordinatorMockClock(t, 4) | |||
| 103 | + | rc, ac := newRenderCoordinator(renderCoordinatorOptions{clock: clock.clock}), &attachedClient{} | |||
| 104 | + | rc.attach(ac) | |||
| 105 | + | tc.run(t, rc, clock, ac) | |||
| 106 | + | rc.beginSessionTeardown().finish() | |||
| 107 | + | rc.waitForTimerWorkers() | |||
| 108 | + | }) | |||
| 109 | + | } | |||
| 110 | + | } | |||
| 111 | + | ||||
| 112 | + | func TestRenderTimerScheduleRacesTerminalTeardown(t *testing.T) { | |||
| 113 | + | for range 32 { | |||
| 114 | + | clock := newCoordinatorMockClock(t, 2) | |||
| 115 | + | rc, ac := newRenderCoordinator(renderCoordinatorOptions{clock: clock.clock}), &attachedClient{} | |||
| 116 | + | rc.attach(ac) | |||
| 117 | + | start := make(chan struct{}) | |||
| 118 | + | scheduled := make(chan struct{}) | |||
| 119 | + | go func() { | |||
| 120 | + | <-start | |||
| 121 | + | rc.invalidateForAttachment(ac, renderInvalidation{class: invalidateUrgent, producer: "race"}) | |||
| 122 | + | close(scheduled) | |||
| 123 | + | }() | |||
| 124 | + | close(start) | |||
| 125 | + | cleanup := rc.beginSessionTeardown() | |||
| 126 | + | awaitTestCompletion(t, scheduled, "schedule raced terminal teardown") | |||
| 127 | + | cleanup.finish() | |||
| 128 | + | rc.waitForTimerWorkers() | |||
| 129 | + | rc.mu.Lock() | |||
| 130 | + | require.True(t, rc.torndown) | |||
| 131 | + | require.Nil(t, rc.normalLane.token) | |||
| 132 | + | rc.mu.Unlock() | |||
| 133 | + | } | |||
| 134 | + | } | |||
| 135 | + | ||||
| 136 | + | func TestTimerSupervisorCancelsNeverFiringWorker(t *testing.T) { | |||
| 137 | + | var lane timerLane | |||
| 138 | + | generation, _ := lane.replaceLocked() | |||
| 139 | + | token := lane.publishLocked(generation, portsmocks.NewMockTimer(t)) | |||
| 140 | + | var supervisor timerSupervisor | |||
| 141 | + | supervisor.startLocked(token, nil, func() { t.Fatal("cancelled worker fired") }) | |||
| 142 | + | lane.detachLocked() | |||
| 143 | + | supervisor.wait() | |||
| 54 | } | 144 | } | ||
M internal/usecase/daemon/resume_test.go +1 -1
| @@ -167,7 +167,7 @@ func TestHandleHelloResumeDefersFreshOutputUntilWelcome(t *testing.T) { | |||
| 167 | 167 | timer := awaitLatestCoordinatorTimer(t, clock) | |
| 168 | 168 | rc := sess.renderCoordinator() | |
| 169 | 169 | rc.mu.Lock() | |
| 170 | - | workerDone := rc.normalWorkerDone | |
| 170 | + | workerDone := rc.normalLane.token.done | |
| 171 | 171 | rc.mu.Unlock() | |
| 172 | 172 | require.NotNil(t, workerDone) | |
| 173 | 173 | timer.ch <- time.Time{} | |
| @@ -167,7 +167,7 @@ func TestHandleHelloResumeDefersFreshOutputUntilWelcome(t *testing.T) { | |||||
| 167 | timer := awaitLatestCoordinatorTimer(t, clock) | 167 | timer := awaitLatestCoordinatorTimer(t, clock) | ||
| 168 | rc := sess.renderCoordinator() | 168 | rc := sess.renderCoordinator() | ||
| 169 | rc.mu.Lock() | 169 | rc.mu.Lock() | ||
| 170 | - | workerDone := rc.normalWorkerDone | 170 | + | workerDone := rc.normalLane.token.done |
| 171 | rc.mu.Unlock() | 171 | rc.mu.Unlock() | ||
| 172 | require.NotNil(t, workerDone) | 172 | require.NotNil(t, workerDone) | ||
| 173 | timer.ch <- time.Time{} | 173 | timer.ch <- time.Time{} | ||
M internal/usecase/daemon/session.go +5 -1
| @@ -749,7 +749,11 @@ func (d *Daemon) killSession(sess *session, reason uint8, purge bool) error { | |||
| 749 | 749 | sess.client = nil | |
| 750 | 750 | sess.mu.Unlock() | |
| 751 | 751 | if rc := sess.renderCoordinator(); rc != nil { | |
| 752 | - | rc.noteSessionTeardown() | |
| 752 | + | // Terminal teardown has two phases: this session owner first prevents any | |
| 753 | + | // new worker registration and detaches all tokens, then stops and waits | |
| 754 | + | // outside coordinator callbacks and session locks. | |
| 755 | + | rc.beginSessionTeardown().finish() | |
| 756 | + | rc.waitForTimerWorkers() | |
| 753 | 757 | } | |
| 754 | 758 | if ac != nil { | |
| 755 | 759 | d.unregisterPreview(ac) | |
| @@ -749,7 +749,11 @@ func (d *Daemon) killSession(sess *session, reason uint8, purge bool) error { | |||||
| 749 | sess.client = nil | 749 | sess.client = nil | ||
| 750 | sess.mu.Unlock() | 750 | sess.mu.Unlock() | ||
| 751 | if rc := sess.renderCoordinator(); rc != nil { | 751 | if rc := sess.renderCoordinator(); rc != nil { | ||
| 752 | - | rc.noteSessionTeardown() | 752 | + | // Terminal teardown has two phases: this session owner first prevents any |
| 753 | + | // new worker registration and detaches all tokens, then stops and waits | |||
| 754 | + | // outside coordinator callbacks and session locks. | |||
| 755 | + | rc.beginSessionTeardown().finish() | |||
| 756 | + | rc.waitForTimerWorkers() | |||
| 753 | } | 757 | } | ||
| 754 | if ac != nil { | 758 | if ac != nil { | ||
| 755 | d.unregisterPreview(ac) | 759 | d.unregisterPreview(ac) | ||
M internal/usecase/daemon/snapshot_test.go +1 -1
| @@ -103,7 +103,7 @@ func exerciseBlockedSnapshotWorker(t *testing.T, blockEncode bool) { | |||
| 103 | 103 | sess.installRenderCoordinator(rc) | |
| 104 | 104 | rc.attach(ac) | |
| 105 | 105 | require.True(t, rc.markAttachmentReady(rc.attachmentLease(ac))) | |
| 106 | - | t.Cleanup(rc.noteSessionTeardown) | |
| 106 | + | t.Cleanup(func() { rc.beginSessionTeardown().finish(); rc.waitForTimerWorkers() }) | |
| 107 | 107 | ||
| 108 | 108 | markSnapshotDirty(sess) | |
| 109 | 109 | require.True(t, d.scheduleSnapshot(sess)) | |
| @@ -103,7 +103,7 @@ func exerciseBlockedSnapshotWorker(t *testing.T, blockEncode bool) { | |||||
| 103 | sess.installRenderCoordinator(rc) | 103 | sess.installRenderCoordinator(rc) | ||
| 104 | rc.attach(ac) | 104 | rc.attach(ac) | ||
| 105 | require.True(t, rc.markAttachmentReady(rc.attachmentLease(ac))) | 105 | require.True(t, rc.markAttachmentReady(rc.attachmentLease(ac))) | ||
| 106 | - | t.Cleanup(rc.noteSessionTeardown) | 106 | + | t.Cleanup(func() { rc.beginSessionTeardown().finish(); rc.waitForTimerWorkers() }) |
| 107 | 107 | ||||
| 108 | markSnapshotDirty(sess) | 108 | markSnapshotDirty(sess) | ||
| 109 | require.True(t, d.scheduleSnapshot(sess)) | 109 | require.True(t, d.scheduleSnapshot(sess)) | ||
M internal/usecase/daemon/transactional_resize_test.go +2 -1
| @@ -344,10 +344,11 @@ func TestTransactionalResizeObsoleteTimerCallbacksCommitOnlyLatestEpoch(t *testi | |||
| 344 | 344 | for len(timers) < 3 { | |
| 345 | 345 | timers = append(timers, <-clock.timers) | |
| 346 | 346 | } | |
| 347 | + | done := captureResizeCallbackDone(t, sess.renderCoordinator()) | |
| 347 | 348 | for _, timer := range timers { | |
| 348 | 349 | timer.ch <- time.Time{} | |
| 349 | 350 | } | |
| 350 | - | awaitResizeCallback(t, sess.renderCoordinator()) | |
| 351 | + | awaitTestCompletion(t, done, "latest resize callback did not complete") | |
| 351 | 352 | require.Equal(t, []domain.Size{{Cols: 120, Rows: 30}}, pty.requested()) | |
| 352 | 353 | snapshot := sess.renderCoordinator().resizeSnapshot() | |
| 353 | 354 | require.Equal(t, uint64(3), snapshot.epoch) | |
| @@ -344,10 +344,11 @@ func TestTransactionalResizeObsoleteTimerCallbacksCommitOnlyLatestEpoch(t *testi | |||||
| 344 | for len(timers) < 3 { | 344 | for len(timers) < 3 { | ||
| 345 | timers = append(timers, <-clock.timers) | 345 | timers = append(timers, <-clock.timers) | ||
| 346 | } | 346 | } | ||
| 347 | + | done := captureResizeCallbackDone(t, sess.renderCoordinator()) | |||
| 347 | for _, timer := range timers { | 348 | for _, timer := range timers { | ||
| 348 | timer.ch <- time.Time{} | 349 | timer.ch <- time.Time{} | ||
| 349 | } | 350 | } | ||
| 350 | - | awaitResizeCallback(t, sess.renderCoordinator()) | 351 | + | awaitTestCompletion(t, done, "latest resize callback did not complete") |
| 351 | require.Equal(t, []domain.Size{{Cols: 120, Rows: 30}}, pty.requested()) | 352 | require.Equal(t, []domain.Size{{Cols: 120, Rows: 30}}, pty.requested()) | ||
| 352 | snapshot := sess.renderCoordinator().resizeSnapshot() | 353 | snapshot := sess.renderCoordinator().resizeSnapshot() | ||
| 353 | require.Equal(t, uint64(3), snapshot.epoch) | 354 | require.Equal(t, uint64(3), snapshot.epoch) | ||
M internal/usecase/daemon/transport_observability_test.go +6 -6
| @@ -178,13 +178,13 @@ func TestTransportObservabilityACKBlockedSpansEndExactlyOnce(t *testing.T) { | |||
| 178 | 178 | name: "successful consume", | |
| 179 | 179 | finish: func(rc *renderCoordinator, ready *bool) { | |
| 180 | 180 | *ready = true | |
| 181 | - | rc.fire(rc.generation, false, false) | |
| 181 | + | rc.fire(rc.normalLane.generation, false, false) | |
| 182 | 182 | }, | |
| 183 | 183 | }, | |
| 184 | 184 | { | |
| 185 | 185 | name: "teardown", | |
| 186 | 186 | finish: func(rc *renderCoordinator, ready *bool) { | |
| 187 | - | rc.noteSessionTeardown() | |
| 187 | + | rc.beginSessionTeardown().finish() | |
| 188 | 188 | }, | |
| 189 | 189 | }, | |
| 190 | 190 | } | |
| @@ -198,7 +198,7 @@ func TestTransportObservabilityACKBlockedSpansEndExactlyOnce(t *testing.T) { | |||
| 198 | 198 | }) | |
| 199 | 199 | rc.attach(&attachedClient{}) | |
| 200 | 200 | rc.invalidate(renderInvalidation{class: invalidateOutput}) | |
| 201 | - | rc.fire(rc.generation, false, true) | |
| 201 | + | rc.fire(rc.normalLane.generation, false, true) | |
| 202 | 202 | tt.finish(rc, &ready) | |
| 203 | 203 | // A late notification after consuming or tearing down must not emit a | |
| 204 | 204 | // second end for the already-cleared blocked interval. | |
| @@ -247,7 +247,7 @@ func TestTransportObservabilityBlockedACKEndDoesNotDelayNotifyAck(t *testing.T) | |||
| 247 | 247 | ||
| 248 | 248 | // Enter the ACK-blocked state and wait until Start has reached the injected | |
| 249 | 249 | // observer. End is the only deliberately blocked call in this test. | |
| 250 | - | rc.fire(rc.generation, false, true) | |
| 250 | + | rc.fire(rc.normalLane.generation, false, true) | |
| 251 | 251 | awaitDaemonObserver(t, observer.startObserved, "ACK-blocked start observer") | |
| 252 | 252 | ||
| 253 | 253 | ready.Store(true) | |
| @@ -295,7 +295,7 @@ func TestTransportObservabilityBlockedCoordinatorObserverReleasesLock(t *testing | |||
| 295 | 295 | rc.attach(&attachedClient{}) | |
| 296 | 296 | rc.mu.Lock() | |
| 297 | 297 | rc.pending = true | |
| 298 | - | gen := rc.generation | |
| 298 | + | gen := rc.normalLane.generation | |
| 299 | 299 | rc.mu.Unlock() | |
| 300 | 300 | ||
| 301 | 301 | fired := make(chan struct{}) | |
| @@ -357,7 +357,7 @@ func TestTransportObservabilityACKBlockedStartPublishesBeforeEnd(t *testing.T) { | |||
| 357 | 357 | }) | |
| 358 | 358 | rc.mu.Lock() | |
| 359 | 359 | rc.pending = true | |
| 360 | - | gen := rc.generation | |
| 360 | + | gen := rc.normalLane.generation | |
| 361 | 361 | rc.mu.Unlock() | |
| 362 | 362 | ||
| 363 | 363 | fireDone := make(chan struct{}) | |
| @@ -178,13 +178,13 @@ func TestTransportObservabilityACKBlockedSpansEndExactlyOnce(t *testing.T) { | |||||
| 178 | name: "successful consume", | 178 | name: "successful consume", | ||
| 179 | finish: func(rc *renderCoordinator, ready *bool) { | 179 | finish: func(rc *renderCoordinator, ready *bool) { | ||
| 180 | *ready = true | 180 | *ready = true | ||
| 181 | - | rc.fire(rc.generation, false, false) | 181 | + | rc.fire(rc.normalLane.generation, false, false) |
| 182 | }, | 182 | }, | ||
| 183 | }, | 183 | }, | ||
| 184 | { | 184 | { | ||
| 185 | name: "teardown", | 185 | name: "teardown", | ||
| 186 | finish: func(rc *renderCoordinator, ready *bool) { | 186 | finish: func(rc *renderCoordinator, ready *bool) { | ||
| 187 | - | rc.noteSessionTeardown() | 187 | + | rc.beginSessionTeardown().finish() |
| 188 | }, | 188 | }, | ||
| 189 | }, | 189 | }, | ||
| 190 | } | 190 | } | ||
| @@ -198,7 +198,7 @@ func TestTransportObservabilityACKBlockedSpansEndExactlyOnce(t *testing.T) { | |||||
| 198 | }) | 198 | }) | ||
| 199 | rc.attach(&attachedClient{}) | 199 | rc.attach(&attachedClient{}) | ||
| 200 | rc.invalidate(renderInvalidation{class: invalidateOutput}) | 200 | rc.invalidate(renderInvalidation{class: invalidateOutput}) | ||
| 201 | - | rc.fire(rc.generation, false, true) | 201 | + | rc.fire(rc.normalLane.generation, false, true) |
| 202 | tt.finish(rc, &ready) | 202 | tt.finish(rc, &ready) | ||
| 203 | // A late notification after consuming or tearing down must not emit a | 203 | // A late notification after consuming or tearing down must not emit a | ||
| 204 | // second end for the already-cleared blocked interval. | 204 | // second end for the already-cleared blocked interval. | ||
| @@ -247,7 +247,7 @@ func TestTransportObservabilityBlockedACKEndDoesNotDelayNotifyAck(t *testing.T) | |||||
| 247 | 247 | ||||
| 248 | // Enter the ACK-blocked state and wait until Start has reached the injected | 248 | // Enter the ACK-blocked state and wait until Start has reached the injected | ||
| 249 | // observer. End is the only deliberately blocked call in this test. | 249 | // observer. End is the only deliberately blocked call in this test. | ||
| 250 | - | rc.fire(rc.generation, false, true) | 250 | + | rc.fire(rc.normalLane.generation, false, true) |
| 251 | awaitDaemonObserver(t, observer.startObserved, "ACK-blocked start observer") | 251 | awaitDaemonObserver(t, observer.startObserved, "ACK-blocked start observer") | ||
| 252 | 252 | ||||
| 253 | ready.Store(true) | 253 | ready.Store(true) | ||
| @@ -295,7 +295,7 @@ func TestTransportObservabilityBlockedCoordinatorObserverReleasesLock(t *testing | |||||
| 295 | rc.attach(&attachedClient{}) | 295 | rc.attach(&attachedClient{}) | ||
| 296 | rc.mu.Lock() | 296 | rc.mu.Lock() | ||
| 297 | rc.pending = true | 297 | rc.pending = true | ||
| 298 | - | gen := rc.generation | 298 | + | gen := rc.normalLane.generation |
| 299 | rc.mu.Unlock() | 299 | rc.mu.Unlock() | ||
| 300 | 300 | ||||
| 301 | fired := make(chan struct{}) | 301 | fired := make(chan struct{}) | ||
| @@ -357,7 +357,7 @@ func TestTransportObservabilityACKBlockedStartPublishesBeforeEnd(t *testing.T) { | |||||
| 357 | }) | 357 | }) | ||
| 358 | rc.mu.Lock() | 358 | rc.mu.Lock() | ||
| 359 | rc.pending = true | 359 | rc.pending = true | ||
| 360 | - | gen := rc.generation | 360 | + | gen := rc.normalLane.generation |
| 361 | rc.mu.Unlock() | 361 | rc.mu.Unlock() | ||
| 362 | 362 | ||||
| 363 | fireDone := make(chan struct{}) | 363 | fireDone := make(chan struct{}) | ||