fix(daemon): gate output until client welcome
- commit
- 35dbd6b939d8275374788802a3432e7ac6565d85
- author
- brice · · 2026-07-11 21:27 UTC
- parent
- 97fb27a
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email
11 files · +69 -20
M internal/usecase/daemon/client.go +9 -6
| @@ -26,7 +26,10 @@ type attachedClient struct { | |||
| 26 | 26 | // coordinatorEpoch is published atomically by renderCoordinator lifecycle | |
| 27 | 27 | // transitions. A wake verifies it again after acquiring sendMu, so a parked | |
| 28 | 28 | // attachment reused by resume cannot carry an old wake to its new transport. | |
| 29 | - | coordinatorEpoch atomic.Uint64 | |
| 29 | + | coordinatorEpoch atomic.Uint64 | |
| 30 | + | // coordinatorReadyEpoch is non-zero only after Welcome completed for this | |
| 31 | + | // exact coordinator incarnation. paint rechecks it under sendMu. | |
| 32 | + | coordinatorReadyEpoch atomic.Uint64 | |
| 30 | 33 | echoAck atomic.Uint64 | |
| 31 | 34 | bars barCache // only touched while sendMu is held | |
| 32 | 35 | composed composedFrameCache // only touched while sendMu is held | |
| @@ -361,7 +364,7 @@ func (d *Daemon) attachClient(sess *session, tr ports.Transport, sz domain.Size, | |||
| 361 | 364 | old := sess.client | |
| 362 | 365 | name := sess.name | |
| 363 | 366 | sess.mu.Unlock() | |
| 364 | - | d.attachCoordinator(sess, old, ac) | |
| 367 | + | d.attachCoordinator(sess, old, ac, false) | |
| 365 | 368 | sess.mu.Lock() | |
| 366 | 369 | sess.client = ac | |
| 367 | 370 | sess.mu.Unlock() | |
| @@ -381,13 +384,13 @@ func (d *Daemon) handoffCoordinator(from, target *session, old, current *attache | |||
| 381 | 384 | current.sendMu.Lock() | |
| 382 | 385 | current.output.rebase() | |
| 383 | 386 | current.sendMu.Unlock() | |
| 384 | - | d.attachCoordinator(target, old, current) | |
| 387 | + | d.attachCoordinator(target, old, current, true) | |
| 385 | 388 | } | |
| 386 | 389 | ||
| 387 | 390 | // attachCoordinator is the sole direct attachment handoff. It creates at | |
| 388 | 391 | // most one coordinator for sess and changes its identity before any caller | |
| 389 | 392 | // can publish resize or render state for the new client. | |
| 390 | - | func (d *Daemon) attachCoordinator(sess *session, old, current *attachedClient) *renderCoordinator { | |
| 393 | + | func (d *Daemon) attachCoordinator(sess *session, old, current *attachedClient, ready bool) *renderCoordinator { | |
| 391 | 394 | sess.mu.Lock() | |
| 392 | 395 | rc := sess.renderCoordinator() | |
| 393 | 396 | if rc == nil { | |
| @@ -419,9 +422,9 @@ func (d *Daemon) attachCoordinator(sess *session, old, current *attachedClient) | |||
| 419 | 422 | } | |
| 420 | 423 | sess.mu.Unlock() | |
| 421 | 424 | if old != nil { | |
| 422 | - | rc.noteReplace(old, current) | |
| 425 | + | rc.noteReplace(old, current, ready) | |
| 423 | 426 | } else if current != nil { | |
| 424 | - | rc.attach(current) | |
| 427 | + | rc.attachWithReadiness(current, ready) | |
| 425 | 428 | } | |
| 426 | 429 | return rc | |
| 427 | 430 | } | |
| @@ -26,7 +26,10 @@ type attachedClient struct { | |||||
| 26 | // coordinatorEpoch is published atomically by renderCoordinator lifecycle | 26 | // coordinatorEpoch is published atomically by renderCoordinator lifecycle | ||
| 27 | // transitions. A wake verifies it again after acquiring sendMu, so a parked | 27 | // transitions. A wake verifies it again after acquiring sendMu, so a parked | ||
| 28 | // attachment reused by resume cannot carry an old wake to its new transport. | 28 | // attachment reused by resume cannot carry an old wake to its new transport. | ||
| 29 | - | coordinatorEpoch atomic.Uint64 | 29 | + | coordinatorEpoch atomic.Uint64 |
| 30 | + | // coordinatorReadyEpoch is non-zero only after Welcome completed for this | |||
| 31 | + | // exact coordinator incarnation. paint rechecks it under sendMu. | |||
| 32 | + | coordinatorReadyEpoch atomic.Uint64 | |||
| 30 | echoAck atomic.Uint64 | 33 | echoAck atomic.Uint64 | ||
| 31 | bars barCache // only touched while sendMu is held | 34 | bars barCache // only touched while sendMu is held | ||
| 32 | composed composedFrameCache // only touched while sendMu is held | 35 | composed composedFrameCache // only touched while sendMu is held | ||
| @@ -361,7 +364,7 @@ func (d *Daemon) attachClient(sess *session, tr ports.Transport, sz domain.Size, | |||||
| 361 | old := sess.client | 364 | old := sess.client | ||
| 362 | name := sess.name | 365 | name := sess.name | ||
| 363 | sess.mu.Unlock() | 366 | sess.mu.Unlock() | ||
| 364 | - | d.attachCoordinator(sess, old, ac) | 367 | + | d.attachCoordinator(sess, old, ac, false) |
| 365 | sess.mu.Lock() | 368 | sess.mu.Lock() | ||
| 366 | sess.client = ac | 369 | sess.client = ac | ||
| 367 | sess.mu.Unlock() | 370 | sess.mu.Unlock() | ||
| @@ -381,13 +384,13 @@ func (d *Daemon) handoffCoordinator(from, target *session, old, current *attache | |||||
| 381 | current.sendMu.Lock() | 384 | current.sendMu.Lock() | ||
| 382 | current.output.rebase() | 385 | current.output.rebase() | ||
| 383 | current.sendMu.Unlock() | 386 | current.sendMu.Unlock() | ||
| 384 | - | d.attachCoordinator(target, old, current) | 387 | + | d.attachCoordinator(target, old, current, true) |
| 385 | } | 388 | } | ||
| 386 | 389 | ||||
| 387 | // attachCoordinator is the sole direct attachment handoff. It creates at | 390 | // attachCoordinator is the sole direct attachment handoff. It creates at | ||
| 388 | // most one coordinator for sess and changes its identity before any caller | 391 | // most one coordinator for sess and changes its identity before any caller | ||
| 389 | // can publish resize or render state for the new client. | 392 | // can publish resize or render state for the new client. | ||
| 390 | - | func (d *Daemon) attachCoordinator(sess *session, old, current *attachedClient) *renderCoordinator { | 393 | + | func (d *Daemon) attachCoordinator(sess *session, old, current *attachedClient, ready bool) *renderCoordinator { |
| 391 | sess.mu.Lock() | 394 | sess.mu.Lock() | ||
| 392 | rc := sess.renderCoordinator() | 395 | rc := sess.renderCoordinator() | ||
| 393 | if rc == nil { | 396 | if rc == nil { | ||
| @@ -419,9 +422,9 @@ func (d *Daemon) attachCoordinator(sess *session, old, current *attachedClient) | |||||
| 419 | } | 422 | } | ||
| 420 | sess.mu.Unlock() | 423 | sess.mu.Unlock() | ||
| 421 | if old != nil { | 424 | if old != nil { | ||
| 422 | - | rc.noteReplace(old, current) | 425 | + | rc.noteReplace(old, current, ready) |
| 423 | } else if current != nil { | 426 | } else if current != nil { | ||
| 424 | - | rc.attach(current) | 427 | + | rc.attachWithReadiness(current, ready) |
| 425 | } | 428 | } | ||
| 426 | return rc | 429 | return rc | ||
| 427 | } | 430 | } | ||
M internal/usecase/daemon/daemon.go +6 -0
| @@ -598,6 +598,12 @@ func (d *Daemon) handleHello(tr ports.Transport, f ports.Frame) { | |||
| 598 | 598 | d.clientGone(sess, ac, tr, false) | |
| 599 | 599 | return | |
| 600 | 600 | } | |
| 601 | + | if rc := sess.renderCoordinator(); rc == nil || !rc.markAttachmentReady(ac) { | |
| 602 | + | // The attachment was displaced or detached while Welcome was in flight; | |
| 603 | + | // never let this stale handshake emit an Output frame. | |
| 604 | + | d.clientGone(sess, ac, tr, false) | |
| 605 | + | return | |
| 606 | + | } | |
| 601 | 607 | d.firstPaint(sess, ac, h.Size) | |
| 602 | 608 | d.runConnLoop(ac) | |
| 603 | 609 | _ = tr.Close() | |
| @@ -598,6 +598,12 @@ func (d *Daemon) handleHello(tr ports.Transport, f ports.Frame) { | |||||
| 598 | d.clientGone(sess, ac, tr, false) | 598 | d.clientGone(sess, ac, tr, false) | ||
| 599 | return | 599 | return | ||
| 600 | } | 600 | } | ||
| 601 | + | if rc := sess.renderCoordinator(); rc == nil || !rc.markAttachmentReady(ac) { | |||
| 602 | + | // The attachment was displaced or detached while Welcome was in flight; | |||
| 603 | + | // never let this stale handshake emit an Output frame. | |||
| 604 | + | d.clientGone(sess, ac, tr, false) | |||
| 605 | + | return | |||
| 606 | + | } | |||
| 601 | d.firstPaint(sess, ac, h.Size) | 607 | d.firstPaint(sess, ac, h.Size) | ||
| 602 | d.runConnLoop(ac) | 608 | d.runConnLoop(ac) | ||
| 603 | _ = tr.Close() | 609 | _ = tr.Close() | ||
M internal/usecase/daemon/output_state_sync_test.go +1 -0
| @@ -378,6 +378,7 @@ func TestDatagramWindowOneCoalescesPaintsUntilMsgAck(t *testing.T) { | |||
| 378 | 378 | MaxOutputInFlight: 1, | |
| 379 | 379 | }, tr) | |
| 380 | 380 | require.NoError(t, err) | |
| 381 | + | require.True(t, sess.renderCoordinator().markAttachmentReady(ac)) | |
| 381 | 382 | ||
| 382 | 383 | pane := sess.tabs[0].focusedPane() | |
| 383 | 384 | pane.screen.Write([]byte("A")) | |
| @@ -378,6 +378,7 @@ func TestDatagramWindowOneCoalescesPaintsUntilMsgAck(t *testing.T) { | |||||
| 378 | MaxOutputInFlight: 1, | 378 | MaxOutputInFlight: 1, | ||
| 379 | }, tr) | 379 | }, tr) | ||
| 380 | require.NoError(t, err) | 380 | require.NoError(t, err) | ||
| 381 | + | require.True(t, sess.renderCoordinator().markAttachmentReady(ac)) | |||
| 381 | 382 | ||||
| 382 | pane := sess.tabs[0].focusedPane() | 383 | pane := sess.tabs[0].focusedPane() | ||
| 383 | pane.screen.Write([]byte("A")) | 384 | pane.screen.Write([]byte("A")) | ||
M internal/usecase/daemon/performance_benchmark_test.go +1 -1
| @@ -292,7 +292,7 @@ func newPerformanceFixture(t testing.TB, config performanceConfig) *performanceF | |||
| 292 | 292 | // The fixture uses the production coordinator. stubClock's inert timer is | |
| 293 | 293 | // completed synchronously by the coordinator, keeping benchmark operations | |
| 294 | 294 | // deterministic while retaining their real invalidation path. | |
| 295 | - | d.attachCoordinator(sess, nil, ac) | |
| 295 | + | d.attachCoordinator(sess, nil, ac, true) | |
| 296 | 296 | ||
| 297 | 297 | // Prime the real renderer shadow before measurements. Subsequent paints use | |
| 298 | 298 | // actual production diffs rather than the initial full frame. | |
| @@ -292,7 +292,7 @@ func newPerformanceFixture(t testing.TB, config performanceConfig) *performanceF | |||||
| 292 | // The fixture uses the production coordinator. stubClock's inert timer is | 292 | // The fixture uses the production coordinator. stubClock's inert timer is | ||
| 293 | // completed synchronously by the coordinator, keeping benchmark operations | 293 | // completed synchronously by the coordinator, keeping benchmark operations | ||
| 294 | // deterministic while retaining their real invalidation path. | 294 | // deterministic while retaining their real invalidation path. | ||
| 295 | - | d.attachCoordinator(sess, nil, ac) | 295 | + | d.attachCoordinator(sess, nil, ac, true) |
| 296 | 296 | ||||
| 297 | // Prime the real renderer shadow before measurements. Subsequent paints use | 297 | // Prime the real renderer shadow before measurements. Subsequent paints use | ||
| 298 | // actual production diffs rather than the initial full frame. | 298 | // actual production diffs rather than the initial full frame. | ||
M internal/usecase/daemon/picker.go +1 -1
| @@ -254,7 +254,7 @@ func (d *Daemon) registerPreviewForSelection(ac *attachedClient) { | |||
| 254 | 254 | if targetSess == ac.currentSession() { | |
| 255 | 255 | return | |
| 256 | 256 | } | |
| 257 | - | rc := d.attachCoordinator(targetSess, nil, nil) | |
| 257 | + | rc := d.attachCoordinator(targetSess, nil, nil, false) | |
| 258 | 258 | rc.subscribePreviewFor(ac, func(renderWake) { | |
| 259 | 259 | ac.overlays.pickerMu.Lock() | |
| 260 | 260 | valid := ac.overlays.pickerPreviewSession == targetSess | |
| @@ -254,7 +254,7 @@ func (d *Daemon) registerPreviewForSelection(ac *attachedClient) { | |||||
| 254 | if targetSess == ac.currentSession() { | 254 | if targetSess == ac.currentSession() { | ||
| 255 | return | 255 | return | ||
| 256 | } | 256 | } | ||
| 257 | - | rc := d.attachCoordinator(targetSess, nil, nil) | 257 | + | rc := d.attachCoordinator(targetSess, nil, nil, false) |
| 258 | rc.subscribePreviewFor(ac, func(renderWake) { | 258 | rc.subscribePreviewFor(ac, func(renderWake) { | ||
| 259 | ac.overlays.pickerMu.Lock() | 259 | ac.overlays.pickerMu.Lock() | ||
| 260 | valid := ac.overlays.pickerPreviewSession == targetSess | 260 | valid := ac.overlays.pickerPreviewSession == targetSess | ||
M internal/usecase/daemon/render.go +1 -1
| @@ -300,7 +300,7 @@ func (d *Daemon) paintForResizeGeneration(sess *session, ac *attachedClient, res | |||
| 300 | 300 | sess.mu.Lock() | |
| 301 | 301 | owned := sess.client == ac | |
| 302 | 302 | sess.mu.Unlock() | |
| 303 | - | if !owned || ac.currentSession() != sess || (attachmentEpoch != 0 && ac.coordinatorEpoch.Load() != attachmentEpoch) { | |
| 303 | + | if !owned || ac.currentSession() != sess || (attachmentEpoch != 0 && (ac.coordinatorEpoch.Load() != attachmentEpoch || ac.coordinatorReadyEpoch.Load() != attachmentEpoch)) { | |
| 304 | 304 | ac.sendMu.Unlock() | |
| 305 | 305 | return | |
| 306 | 306 | } | |
| @@ -300,7 +300,7 @@ func (d *Daemon) paintForResizeGeneration(sess *session, ac *attachedClient, res | |||||
| 300 | sess.mu.Lock() | 300 | sess.mu.Lock() | ||
| 301 | owned := sess.client == ac | 301 | owned := sess.client == ac | ||
| 302 | sess.mu.Unlock() | 302 | sess.mu.Unlock() | ||
| 303 | - | if !owned || ac.currentSession() != sess || (attachmentEpoch != 0 && ac.coordinatorEpoch.Load() != attachmentEpoch) { | 303 | + | if !owned || ac.currentSession() != sess || (attachmentEpoch != 0 && (ac.coordinatorEpoch.Load() != attachmentEpoch || ac.coordinatorReadyEpoch.Load() != attachmentEpoch)) { |
| 304 | ac.sendMu.Unlock() | 304 | ac.sendMu.Unlock() | ||
| 305 | return | 305 | return | ||
| 306 | } | 306 | } | ||
M internal/usecase/daemon/render_coordinator.go +42 -7
| @@ -151,6 +151,9 @@ type renderCoordinator struct { | |||
| 151 | 151 | // attachmentEpoch distinguishes consecutive bindings of the same client | |
| 152 | 152 | // object (notably park/resume, which replaces its transport in place). | |
| 153 | 153 | attachmentEpoch uint64 | |
| 154 | + | // attachmentReady is true only after Welcome succeeded for this exact | |
| 155 | + | // attachment incarnation. Internal cross-session handoffs bind ready. | |
| 156 | + | attachmentReady bool | |
| 154 | 157 | // detached rejects invalidations after detach/park until a new attachment. | |
| 155 | 158 | detached bool | |
| 156 | 159 | torndown bool | |
| @@ -551,28 +554,50 @@ func (c *renderCoordinator) advanceAttachmentEpochLocked(clients ...*attachedCli | |||
| 551 | 554 | for _, ac := range clients { | |
| 552 | 555 | if ac != nil { | |
| 553 | 556 | ac.coordinatorEpoch.Store(c.attachmentEpoch) | |
| 557 | + | ac.coordinatorReadyEpoch.Store(0) | |
| 554 | 558 | } | |
| 555 | 559 | } | |
| 556 | 560 | } | |
| 557 | 561 | ||
| 558 | - | // attach binds ac as the coordinator's current attachment identity. | |
| 559 | - | func (c *renderCoordinator) attach(ac *attachedClient) { | |
| 562 | + | // attach binds an already-handshaken internal attachment. Route/resume use | |
| 563 | + | // attachWithReadiness(..., false) until their Welcome frame is accepted. | |
| 564 | + | func (c *renderCoordinator) attach(ac *attachedClient) { c.attachWithReadiness(ac, true) } | |
| 565 | + | ||
| 566 | + | func (c *renderCoordinator) attachWithReadiness(ac *attachedClient, ready bool) { | |
| 560 | 567 | c.mu.Lock() | |
| 561 | - | if !c.torndown { | |
| 568 | + | if !c.torndown && c.attachment != ac { | |
| 562 | 569 | c.advanceAttachmentEpochLocked(ac) | |
| 563 | 570 | c.attachment = ac | |
| 564 | 571 | c.detached = false | |
| 572 | + | c.attachmentReady = ready | |
| 573 | + | if ready && ac != nil { | |
| 574 | + | ac.coordinatorReadyEpoch.Store(c.attachmentEpoch) | |
| 575 | + | } | |
| 565 | 576 | } | |
| 566 | 577 | c.mu.Unlock() | |
| 567 | 578 | } | |
| 568 | 579 | ||
| 580 | + | // markAttachmentReady completes the transport handshake for exactly the | |
| 581 | + | // current attachment incarnation. A stale Welcome can never revive a parked, | |
| 582 | + | // replaced, or detached attachment. | |
| 583 | + | func (c *renderCoordinator) markAttachmentReady(ac *attachedClient) bool { | |
| 584 | + | c.mu.Lock() | |
| 585 | + | defer c.mu.Unlock() | |
| 586 | + | if c.torndown || c.detached || c.attachment != ac || ac == nil || ac.coordinatorEpoch.Load() != c.attachmentEpoch { | |
| 587 | + | return false | |
| 588 | + | } | |
| 589 | + | c.attachmentReady = true | |
| 590 | + | ac.coordinatorReadyEpoch.Store(c.attachmentEpoch) | |
| 591 | + | return true | |
| 592 | + | } | |
| 593 | + | ||
| 569 | 594 | // wakeCurrent validates a dispatched wake without entering sendMu. The paint | |
| 570 | 595 | // path repeats the epoch check after taking sendMu, closing the remaining | |
| 571 | 596 | // lifecycle handoff window without holding c.mu across composition. | |
| 572 | 597 | func (c *renderCoordinator) wakeCurrent(w renderWake) bool { | |
| 573 | 598 | c.mu.Lock() | |
| 574 | 599 | defer c.mu.Unlock() | |
| 575 | - | return !c.torndown && !c.detached && c.attachment == w.attachment && c.attachmentEpoch == w.attachmentEpoch | |
| 600 | + | return !c.torndown && !c.detached && c.attachmentReady && c.attachment == w.attachment && c.attachmentEpoch == w.attachmentEpoch | |
| 576 | 601 | } | |
| 577 | 602 | ||
| 578 | 603 | // noteDetach invalidates pending wakes and stale callbacks for a detaching | |
| @@ -584,6 +609,7 @@ func (c *renderCoordinator) noteDetach(ac *attachedClient) { | |||
| 584 | 609 | if c.attachment == ac { | |
| 585 | 610 | c.advanceAttachmentEpochLocked(ac) | |
| 586 | 611 | c.attachment = nil | |
| 612 | + | c.attachmentReady = false | |
| 587 | 613 | c.detached = true | |
| 588 | 614 | c.pending = false | |
| 589 | 615 | c.ackDeferred = false | |
| @@ -600,7 +626,11 @@ func (c *renderCoordinator) noteDetach(ac *attachedClient) { | |||
| 600 | 626 | ||
| 601 | 627 | // noteReplace hands the coordinator from old to replacement; callbacks | |
| 602 | 628 | // captured by old become stale. | |
| 603 | - | func (c *renderCoordinator) noteReplace(old, replacement *attachedClient) { | |
| 629 | + | func (c *renderCoordinator) noteReplace(old, replacement *attachedClient, readiness ...bool) { | |
| 630 | + | ready := true | |
| 631 | + | if len(readiness) != 0 { | |
| 632 | + | ready = readiness[0] | |
| 633 | + | } | |
| 604 | 634 | c.mu.Lock() | |
| 605 | 635 | // A coordinator may be installed while replacing a legacy attachment | |
| 606 | 636 | // which predated coordinator ownership. In that case nil has no pending | |
| @@ -611,6 +641,10 @@ func (c *renderCoordinator) noteReplace(old, replacement *attachedClient) { | |||
| 611 | 641 | if c.attachment == old || c.attachment == nil { | |
| 612 | 642 | c.advanceAttachmentEpochLocked(old, replacement) | |
| 613 | 643 | c.attachment = replacement | |
| 644 | + | c.attachmentReady = ready | |
| 645 | + | if ready && replacement != nil { | |
| 646 | + | replacement.coordinatorReadyEpoch.Store(c.attachmentEpoch) | |
| 647 | + | } | |
| 614 | 648 | c.detached = false | |
| 615 | 649 | c.pending = false | |
| 616 | 650 | c.ackDeferred = false | |
| @@ -634,6 +668,7 @@ func (c *renderCoordinator) noteSessionTeardown() { | |||
| 634 | 668 | c.torndown = true | |
| 635 | 669 | c.advanceAttachmentEpochLocked(c.attachment) | |
| 636 | 670 | c.attachment = nil | |
| 671 | + | c.attachmentReady = false | |
| 637 | 672 | c.previewWake = nil | |
| 638 | 673 | c.previewWakes = nil | |
| 639 | 674 | c.pending = false | |
| @@ -757,8 +792,8 @@ func (c *renderCoordinator) fire(gen uint64, watchdog, deadline bool) { | |||
| 757 | 792 | attachmentEpoch = c.attachmentEpoch | |
| 758 | 793 | } | |
| 759 | 794 | w := renderWake{reset: c.pendingReset, urgent: c.pendingUrgent, coalesced: c.coalesced, watchdog: watchdog, attachment: c.attachment, attachmentEpoch: attachmentEpoch} | |
| 760 | - | if !ready { | |
| 761 | - | if deadline { | |
| 795 | + | if !ready || (c.attachment != nil && !c.attachmentReady) { | |
| 796 | + | if !ready && deadline { | |
| 762 | 797 | c.ackDeferred = true | |
| 763 | 798 | } | |
| 764 | 799 | preview, previews := c.takePendingPreviewsLocked() | |
| @@ -151,6 +151,9 @@ type renderCoordinator struct { | |||||
| 151 | // attachmentEpoch distinguishes consecutive bindings of the same client | 151 | // attachmentEpoch distinguishes consecutive bindings of the same client | ||
| 152 | // object (notably park/resume, which replaces its transport in place). | 152 | // object (notably park/resume, which replaces its transport in place). | ||
| 153 | attachmentEpoch uint64 | 153 | attachmentEpoch uint64 | ||
| 154 | + | // attachmentReady is true only after Welcome succeeded for this exact | |||
| 155 | + | // attachment incarnation. Internal cross-session handoffs bind ready. | |||
| 156 | + | attachmentReady bool | |||
| 154 | // detached rejects invalidations after detach/park until a new attachment. | 157 | // detached rejects invalidations after detach/park until a new attachment. | ||
| 155 | detached bool | 158 | detached bool | ||
| 156 | torndown bool | 159 | torndown bool | ||
| @@ -551,28 +554,50 @@ func (c *renderCoordinator) advanceAttachmentEpochLocked(clients ...*attachedCli | |||||
| 551 | for _, ac := range clients { | 554 | for _, ac := range clients { | ||
| 552 | if ac != nil { | 555 | if ac != nil { | ||
| 553 | ac.coordinatorEpoch.Store(c.attachmentEpoch) | 556 | ac.coordinatorEpoch.Store(c.attachmentEpoch) | ||
| 557 | + | ac.coordinatorReadyEpoch.Store(0) | |||
| 554 | } | 558 | } | ||
| 555 | } | 559 | } | ||
| 556 | } | 560 | } | ||
| 557 | 561 | ||||
| 558 | - | // attach binds ac as the coordinator's current attachment identity. | 562 | + | // attach binds an already-handshaken internal attachment. Route/resume use |
| 559 | - | func (c *renderCoordinator) attach(ac *attachedClient) { | 563 | + | // attachWithReadiness(..., false) until their Welcome frame is accepted. |
| 564 | + | func (c *renderCoordinator) attach(ac *attachedClient) { c.attachWithReadiness(ac, true) } | |||
| 565 | + | ||||
| 566 | + | func (c *renderCoordinator) attachWithReadiness(ac *attachedClient, ready bool) { | |||
| 560 | c.mu.Lock() | 567 | c.mu.Lock() | ||
| 561 | - | if !c.torndown { | 568 | + | if !c.torndown && c.attachment != ac { |
| 562 | c.advanceAttachmentEpochLocked(ac) | 569 | c.advanceAttachmentEpochLocked(ac) | ||
| 563 | c.attachment = ac | 570 | c.attachment = ac | ||
| 564 | c.detached = false | 571 | c.detached = false | ||
| 572 | + | c.attachmentReady = ready | |||
| 573 | + | if ready && ac != nil { | |||
| 574 | + | ac.coordinatorReadyEpoch.Store(c.attachmentEpoch) | |||
| 575 | + | } | |||
| 565 | } | 576 | } | ||
| 566 | c.mu.Unlock() | 577 | c.mu.Unlock() | ||
| 567 | } | 578 | } | ||
| 568 | 579 | ||||
| 580 | + | // markAttachmentReady completes the transport handshake for exactly the | |||
| 581 | + | // current attachment incarnation. A stale Welcome can never revive a parked, | |||
| 582 | + | // replaced, or detached attachment. | |||
| 583 | + | func (c *renderCoordinator) markAttachmentReady(ac *attachedClient) bool { | |||
| 584 | + | c.mu.Lock() | |||
| 585 | + | defer c.mu.Unlock() | |||
| 586 | + | if c.torndown || c.detached || c.attachment != ac || ac == nil || ac.coordinatorEpoch.Load() != c.attachmentEpoch { | |||
| 587 | + | return false | |||
| 588 | + | } | |||
| 589 | + | c.attachmentReady = true | |||
| 590 | + | ac.coordinatorReadyEpoch.Store(c.attachmentEpoch) | |||
| 591 | + | return true | |||
| 592 | + | } | |||
| 593 | + | ||||
| 569 | // wakeCurrent validates a dispatched wake without entering sendMu. The paint | 594 | // wakeCurrent validates a dispatched wake without entering sendMu. The paint | ||
| 570 | // path repeats the epoch check after taking sendMu, closing the remaining | 595 | // path repeats the epoch check after taking sendMu, closing the remaining | ||
| 571 | // lifecycle handoff window without holding c.mu across composition. | 596 | // lifecycle handoff window without holding c.mu across composition. | ||
| 572 | func (c *renderCoordinator) wakeCurrent(w renderWake) bool { | 597 | func (c *renderCoordinator) wakeCurrent(w renderWake) bool { | ||
| 573 | c.mu.Lock() | 598 | c.mu.Lock() | ||
| 574 | defer c.mu.Unlock() | 599 | defer c.mu.Unlock() | ||
| 575 | - | return !c.torndown && !c.detached && c.attachment == w.attachment && c.attachmentEpoch == w.attachmentEpoch | 600 | + | return !c.torndown && !c.detached && c.attachmentReady && c.attachment == w.attachment && c.attachmentEpoch == w.attachmentEpoch |
| 576 | } | 601 | } | ||
| 577 | 602 | ||||
| 578 | // noteDetach invalidates pending wakes and stale callbacks for a detaching | 603 | // noteDetach invalidates pending wakes and stale callbacks for a detaching | ||
| @@ -584,6 +609,7 @@ func (c *renderCoordinator) noteDetach(ac *attachedClient) { | |||||
| 584 | if c.attachment == ac { | 609 | if c.attachment == ac { | ||
| 585 | c.advanceAttachmentEpochLocked(ac) | 610 | c.advanceAttachmentEpochLocked(ac) | ||
| 586 | c.attachment = nil | 611 | c.attachment = nil | ||
| 612 | + | c.attachmentReady = false | |||
| 587 | c.detached = true | 613 | c.detached = true | ||
| 588 | c.pending = false | 614 | c.pending = false | ||
| 589 | c.ackDeferred = false | 615 | c.ackDeferred = false | ||
| @@ -600,7 +626,11 @@ func (c *renderCoordinator) noteDetach(ac *attachedClient) { | |||||
| 600 | 626 | ||||
| 601 | // noteReplace hands the coordinator from old to replacement; callbacks | 627 | // noteReplace hands the coordinator from old to replacement; callbacks | ||
| 602 | // captured by old become stale. | 628 | // captured by old become stale. | ||
| 603 | - | func (c *renderCoordinator) noteReplace(old, replacement *attachedClient) { | 629 | + | func (c *renderCoordinator) noteReplace(old, replacement *attachedClient, readiness ...bool) { |
| 630 | + | ready := true | |||
| 631 | + | if len(readiness) != 0 { | |||
| 632 | + | ready = readiness[0] | |||
| 633 | + | } | |||
| 604 | c.mu.Lock() | 634 | c.mu.Lock() | ||
| 605 | // A coordinator may be installed while replacing a legacy attachment | 635 | // A coordinator may be installed while replacing a legacy attachment | ||
| 606 | // which predated coordinator ownership. In that case nil has no pending | 636 | // which predated coordinator ownership. In that case nil has no pending | ||
| @@ -611,6 +641,10 @@ func (c *renderCoordinator) noteReplace(old, replacement *attachedClient) { | |||||
| 611 | if c.attachment == old || c.attachment == nil { | 641 | if c.attachment == old || c.attachment == nil { | ||
| 612 | c.advanceAttachmentEpochLocked(old, replacement) | 642 | c.advanceAttachmentEpochLocked(old, replacement) | ||
| 613 | c.attachment = replacement | 643 | c.attachment = replacement | ||
| 644 | + | c.attachmentReady = ready | |||
| 645 | + | if ready && replacement != nil { | |||
| 646 | + | replacement.coordinatorReadyEpoch.Store(c.attachmentEpoch) | |||
| 647 | + | } | |||
| 614 | c.detached = false | 648 | c.detached = false | ||
| 615 | c.pending = false | 649 | c.pending = false | ||
| 616 | c.ackDeferred = false | 650 | c.ackDeferred = false | ||
| @@ -634,6 +668,7 @@ func (c *renderCoordinator) noteSessionTeardown() { | |||||
| 634 | c.torndown = true | 668 | c.torndown = true | ||
| 635 | c.advanceAttachmentEpochLocked(c.attachment) | 669 | c.advanceAttachmentEpochLocked(c.attachment) | ||
| 636 | c.attachment = nil | 670 | c.attachment = nil | ||
| 671 | + | c.attachmentReady = false | |||
| 637 | c.previewWake = nil | 672 | c.previewWake = nil | ||
| 638 | c.previewWakes = nil | 673 | c.previewWakes = nil | ||
| 639 | c.pending = false | 674 | c.pending = false | ||
| @@ -757,8 +792,8 @@ func (c *renderCoordinator) fire(gen uint64, watchdog, deadline bool) { | |||||
| 757 | attachmentEpoch = c.attachmentEpoch | 792 | attachmentEpoch = c.attachmentEpoch | ||
| 758 | } | 793 | } | ||
| 759 | w := renderWake{reset: c.pendingReset, urgent: c.pendingUrgent, coalesced: c.coalesced, watchdog: watchdog, attachment: c.attachment, attachmentEpoch: attachmentEpoch} | 794 | w := renderWake{reset: c.pendingReset, urgent: c.pendingUrgent, coalesced: c.coalesced, watchdog: watchdog, attachment: c.attachment, attachmentEpoch: attachmentEpoch} | ||
| 760 | - | if !ready { | 795 | + | if !ready || (c.attachment != nil && !c.attachmentReady) { |
| 761 | - | if deadline { | 796 | + | if !ready && deadline { |
| 762 | c.ackDeferred = true | 797 | c.ackDeferred = true | ||
| 763 | } | 798 | } | ||
| 764 | preview, previews := c.takePendingPreviewsLocked() | 799 | preview, previews := c.takePendingPreviewsLocked() | ||
M internal/usecase/daemon/render_test.go +2 -2
| @@ -178,7 +178,7 @@ func TestPTYReaderSyncVisibilityTransitions(t *testing.T) { | |||
| 178 | 178 | d, sess, ac, sends := newManualSessionWithPTYs(t, activePTY, inactivePTY) | |
| 179 | 179 | clock := newCoordinatorMockClock(t, 8) | |
| 180 | 180 | d.clock = clock.clock | |
| 181 | - | d.attachCoordinator(sess, nil, ac) | |
| 181 | + | d.attachCoordinator(sess, nil, ac, true) | |
| 182 | 182 | ||
| 183 | 183 | d.sessWg.Add(1) | |
| 184 | 184 | go d.ptyReader(sess, sess.tabs[1], sess.tabs[1].focusedPane()) | |
| @@ -213,7 +213,7 @@ func TestPTYReaderSyncVisibilityTransitions(t *testing.T) { | |||
| 213 | 213 | d, sess, ac, sends := newManualSessionWithPTYs(t, oldPTY, newPTY, parkedPTY) | |
| 214 | 214 | clock := newCoordinatorMockClock(t, 8) | |
| 215 | 215 | d.clock = clock.clock | |
| 216 | - | d.attachCoordinator(sess, nil, ac) | |
| 216 | + | d.attachCoordinator(sess, nil, ac, true) | |
| 217 | 217 | ||
| 218 | 218 | d.sessWg.Add(2) | |
| 219 | 219 | go d.ptyReader(sess, sess.tabs[0], sess.tabs[0].focusedPane()) | |
| @@ -178,7 +178,7 @@ func TestPTYReaderSyncVisibilityTransitions(t *testing.T) { | |||||
| 178 | d, sess, ac, sends := newManualSessionWithPTYs(t, activePTY, inactivePTY) | 178 | d, sess, ac, sends := newManualSessionWithPTYs(t, activePTY, inactivePTY) | ||
| 179 | clock := newCoordinatorMockClock(t, 8) | 179 | clock := newCoordinatorMockClock(t, 8) | ||
| 180 | d.clock = clock.clock | 180 | d.clock = clock.clock | ||
| 181 | - | d.attachCoordinator(sess, nil, ac) | 181 | + | d.attachCoordinator(sess, nil, ac, true) |
| 182 | 182 | ||||
| 183 | d.sessWg.Add(1) | 183 | d.sessWg.Add(1) | ||
| 184 | go d.ptyReader(sess, sess.tabs[1], sess.tabs[1].focusedPane()) | 184 | go d.ptyReader(sess, sess.tabs[1], sess.tabs[1].focusedPane()) | ||
| @@ -213,7 +213,7 @@ func TestPTYReaderSyncVisibilityTransitions(t *testing.T) { | |||||
| 213 | d, sess, ac, sends := newManualSessionWithPTYs(t, oldPTY, newPTY, parkedPTY) | 213 | d, sess, ac, sends := newManualSessionWithPTYs(t, oldPTY, newPTY, parkedPTY) | ||
| 214 | clock := newCoordinatorMockClock(t, 8) | 214 | clock := newCoordinatorMockClock(t, 8) | ||
| 215 | d.clock = clock.clock | 215 | d.clock = clock.clock | ||
| 216 | - | d.attachCoordinator(sess, nil, ac) | 216 | + | d.attachCoordinator(sess, nil, ac, true) |
| 217 | 217 | ||||
| 218 | d.sessWg.Add(2) | 218 | d.sessWg.Add(2) | ||
| 219 | go d.ptyReader(sess, sess.tabs[0], sess.tabs[0].focusedPane()) | 219 | go d.ptyReader(sess, sess.tabs[0], sess.tabs[0].focusedPane()) | ||
M internal/usecase/daemon/resume.go +1 -1
| @@ -187,7 +187,7 @@ func (d *Daemon) resumeParkedLocked(h ports.Hello, tr ports.Transport, sz domain | |||
| 187 | 187 | sess.client = ac | |
| 188 | 188 | sess.terminal = terminalEnv{TrueColor: h.TrueColor} | |
| 189 | 189 | sess.mu.Unlock() | |
| 190 | - | d.attachCoordinator(sess, nil, ac) | |
| 190 | + | d.attachCoordinator(sess, nil, ac, false) | |
| 191 | 191 | d.touchMRU(sess) | |
| 192 | 192 | d.log.Info("client resumed", "session", sess.name) | |
| 193 | 193 | return sess, ac, true, nil | |
| @@ -187,7 +187,7 @@ func (d *Daemon) resumeParkedLocked(h ports.Hello, tr ports.Transport, sz domain | |||||
| 187 | sess.client = ac | 187 | sess.client = ac | ||
| 188 | sess.terminal = terminalEnv{TrueColor: h.TrueColor} | 188 | sess.terminal = terminalEnv{TrueColor: h.TrueColor} | ||
| 189 | sess.mu.Unlock() | 189 | sess.mu.Unlock() | ||
| 190 | - | d.attachCoordinator(sess, nil, ac) | 190 | + | d.attachCoordinator(sess, nil, ac, false) |
| 191 | d.touchMRU(sess) | 191 | d.touchMRU(sess) | ||
| 192 | d.log.Info("client resumed", "session", sess.name) | 192 | d.log.Info("client resumed", "session", sess.name) | ||
| 193 | return sess, ac, true, nil | 193 | return sess, ac, true, nil | ||
M internal/usecase/daemon/resume_test.go +1 -0
| @@ -228,6 +228,7 @@ func TestResumeRebasesFullOutputWindowBeforeFirstPaint(t *testing.T) { | |||
| 228 | 228 | resumedSess, resumedAC, err := d.route(helloResumeCapable(ports.IntentResume, "work", token), newTr) | |
| 229 | 229 | require.NoError(t, err) | |
| 230 | 230 | require.Same(t, ac, resumedAC) | |
| 231 | + | require.True(t, resumedSess.renderCoordinator().markAttachmentReady(resumedAC)) | |
| 231 | 232 | d.firstPaint(resumedSess, resumedAC, resumedAC.size) | |
| 232 | 233 | ||
| 233 | 234 | sends := newTr.Sends() | |
| @@ -228,6 +228,7 @@ func TestResumeRebasesFullOutputWindowBeforeFirstPaint(t *testing.T) { | |||||
| 228 | resumedSess, resumedAC, err := d.route(helloResumeCapable(ports.IntentResume, "work", token), newTr) | 228 | resumedSess, resumedAC, err := d.route(helloResumeCapable(ports.IntentResume, "work", token), newTr) | ||
| 229 | require.NoError(t, err) | 229 | require.NoError(t, err) | ||
| 230 | require.Same(t, ac, resumedAC) | 230 | require.Same(t, ac, resumedAC) | ||
| 231 | + | require.True(t, resumedSess.renderCoordinator().markAttachmentReady(resumedAC)) | |||
| 231 | d.firstPaint(resumedSess, resumedAC, resumedAC.size) | 232 | d.firstPaint(resumedSess, resumedAC, resumedAC.size) | ||
| 232 | 233 | ||||
| 233 | sends := newTr.Sends() | 234 | sends := newTr.Sends() | ||
M internal/usecase/daemon/resume_wake_incarnation_test.go +4 -1
| @@ -24,6 +24,7 @@ func TestParkedResumeRejectsDispatchedWakeFromPriorAttachment(t *testing.T) { | |||
| 24 | 24 | token := ac.resumeToken | |
| 25 | 25 | rc := sess.renderCoordinator() | |
| 26 | 26 | require.NotNil(t, rc) | |
| 27 | + | require.True(t, rc.markAttachmentReady(ac)) | |
| 27 | 28 | ||
| 28 | 29 | // Pause after dispatch but before the coordinator's normal wake callback | |
| 29 | 30 | // reaches paint/sendMu. This is the precise stale-wake window. | |
| @@ -58,8 +59,10 @@ func TestParkedResumeRejectsDispatchedWakeFromPriorAttachment(t *testing.T) { | |||
| 58 | 59 | ac.sendMu.Unlock() | |
| 59 | 60 | require.False(t, staleWakeComposed, "a stale wake must not mutate the resumed attachment shadow") | |
| 60 | 61 | ||
| 61 | - | // The new attachment incarnation still accepts its normal required paint. | |
| 62 | + | // The new attachment incarnation accepts the required paint only after its | |
| 63 | + | // replacement transport has completed Welcome. | |
| 62 | 64 | rc.opts.wake = originalWake | |
| 65 | + | require.True(t, rc.markAttachmentReady(ac)) | |
| 63 | 66 | d.firstPaint(sess, ac, ac.size) | |
| 64 | 67 | require.NotEmpty(t, newTransport.Sends()) | |
| 65 | 68 | } | |
| @@ -24,6 +24,7 @@ func TestParkedResumeRejectsDispatchedWakeFromPriorAttachment(t *testing.T) { | |||||
| 24 | token := ac.resumeToken | 24 | token := ac.resumeToken | ||
| 25 | rc := sess.renderCoordinator() | 25 | rc := sess.renderCoordinator() | ||
| 26 | require.NotNil(t, rc) | 26 | require.NotNil(t, rc) | ||
| 27 | + | require.True(t, rc.markAttachmentReady(ac)) | |||
| 27 | 28 | ||||
| 28 | // Pause after dispatch but before the coordinator's normal wake callback | 29 | // Pause after dispatch but before the coordinator's normal wake callback | ||
| 29 | // reaches paint/sendMu. This is the precise stale-wake window. | 30 | // reaches paint/sendMu. This is the precise stale-wake window. | ||
| @@ -58,8 +59,10 @@ func TestParkedResumeRejectsDispatchedWakeFromPriorAttachment(t *testing.T) { | |||||
| 58 | ac.sendMu.Unlock() | 59 | ac.sendMu.Unlock() | ||
| 59 | require.False(t, staleWakeComposed, "a stale wake must not mutate the resumed attachment shadow") | 60 | require.False(t, staleWakeComposed, "a stale wake must not mutate the resumed attachment shadow") | ||
| 60 | 61 | ||||
| 61 | - | // The new attachment incarnation still accepts its normal required paint. | 62 | + | // The new attachment incarnation accepts the required paint only after its |
| 63 | + | // replacement transport has completed Welcome. | |||
| 62 | rc.opts.wake = originalWake | 64 | rc.opts.wake = originalWake | ||
| 65 | + | require.True(t, rc.markAttachmentReady(ac)) | |||
| 63 | d.firstPaint(sess, ac, ac.size) | 66 | d.firstPaint(sess, ac, ac.size) | ||
| 64 | require.NotEmpty(t, newTransport.Sends()) | 67 | require.NotEmpty(t, newTransport.Sends()) | ||
| 65 | } | 68 | } | ||