fix(daemon): retire lease on attachment rebind
- commit
- 308292f76a0dc69938923742e41a237c1c1029f1
- author
- brice · · 2026-08-03 05:54 UTC
- committer
- brice · · 2026-08-03 20:53 UTC
- parent
- 800c63b
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email
3 files · +62 -1
M internal/usecase/daemon/attachment_transition_publish.go +5 -1
| @@ -200,7 +200,11 @@ func buildAttachmentPostcommitPlanLocked(publication *attachmentPublication) att | |||
| 200 | 200 | } | |
| 201 | 201 | var lease *attachmentLease | |
| 202 | 202 | if publication.targetCoordinator != nil && !req.preserveAttachment { | |
| 203 | - | lease = publication.targetCoordinator.attachWithReadinessLocked(req.next, req.ready) | |
| 203 | + | if publication.source == req.target { | |
| 204 | + | lease = publication.targetCoordinator.rebindAttachmentWithReadinessLocked(req.next, req.ready) | |
| 205 | + | } else { | |
| 206 | + | lease = publication.targetCoordinator.attachWithReadinessLocked(req.next, req.ready) | |
| 207 | + | } | |
| 204 | 208 | } | |
| 205 | 209 | result.published = attachmentConnectionToken{ | |
| 206 | 210 | sess: req.target, ac: req.next, | |
| @@ -200,7 +200,11 @@ func buildAttachmentPostcommitPlanLocked(publication *attachmentPublication) att | |||||
| 200 | } | 200 | } | ||
| 201 | var lease *attachmentLease | 201 | var lease *attachmentLease | ||
| 202 | if publication.targetCoordinator != nil && !req.preserveAttachment { | 202 | if publication.targetCoordinator != nil && !req.preserveAttachment { | ||
| 203 | - | lease = publication.targetCoordinator.attachWithReadinessLocked(req.next, req.ready) | 203 | + | if publication.source == req.target { |
| 204 | + | lease = publication.targetCoordinator.rebindAttachmentWithReadinessLocked(req.next, req.ready) | |||
| 205 | + | } else { | |||
| 206 | + | lease = publication.targetCoordinator.attachWithReadinessLocked(req.next, req.ready) | |||
| 207 | + | } | |||
| 204 | } | 208 | } | ||
| 205 | result.published = attachmentConnectionToken{ | 209 | result.published = attachmentConnectionToken{ | ||
| 206 | sess: req.target, ac: req.next, | 210 | sess: req.target, ac: req.next, | ||
M internal/usecase/daemon/render_coordinator.go +15 -0
| @@ -593,6 +593,21 @@ func (c *renderCoordinator) attachWithReadinessLocked(ac *attachedClient, ready | |||
| 593 | 593 | return c.installLeaseLocked(ac, ready) | |
| 594 | 594 | } | |
| 595 | 595 | ||
| 596 | + | // rebindAttachmentWithReadinessLocked retires the previous lease before | |
| 597 | + | // publishing a new one for the same attachment object. A queued wake carries | |
| 598 | + | // the old lease pointer, so reusing it would let that wake paint a rebound | |
| 599 | + | // transport. | |
| 600 | + | func (c *renderCoordinator) rebindAttachmentWithReadinessLocked(ac *attachedClient, ready bool) *attachmentLease { | |
| 601 | + | if c.torndown { | |
| 602 | + | return nil | |
| 603 | + | } | |
| 604 | + | if lease := c.leases[ac]; lease != nil { | |
| 605 | + | lease.active = false | |
| 606 | + | delete(c.leases, ac) | |
| 607 | + | } | |
| 608 | + | return c.installLeaseLocked(ac, ready) | |
| 609 | + | } | |
| 610 | + | ||
| 596 | 611 | // markAttachmentReady completes only the captured attachment incarnation. | |
| 597 | 612 | func (c *renderCoordinator) markAttachmentReady(lease *attachmentLease) bool { | |
| 598 | 613 | c.mu.Lock() | |
| @@ -593,6 +593,21 @@ func (c *renderCoordinator) attachWithReadinessLocked(ac *attachedClient, ready | |||||
| 593 | return c.installLeaseLocked(ac, ready) | 593 | return c.installLeaseLocked(ac, ready) | ||
| 594 | } | 594 | } | ||
| 595 | 595 | ||||
| 596 | + | // rebindAttachmentWithReadinessLocked retires the previous lease before | |||
| 597 | + | // publishing a new one for the same attachment object. A queued wake carries | |||
| 598 | + | // the old lease pointer, so reusing it would let that wake paint a rebound | |||
| 599 | + | // transport. | |||
| 600 | + | func (c *renderCoordinator) rebindAttachmentWithReadinessLocked(ac *attachedClient, ready bool) *attachmentLease { | |||
| 601 | + | if c.torndown { | |||
| 602 | + | return nil | |||
| 603 | + | } | |||
| 604 | + | if lease := c.leases[ac]; lease != nil { | |||
| 605 | + | lease.active = false | |||
| 606 | + | delete(c.leases, ac) | |||
| 607 | + | } | |||
| 608 | + | return c.installLeaseLocked(ac, ready) | |||
| 609 | + | } | |||
| 610 | + | ||||
| 596 | // markAttachmentReady completes only the captured attachment incarnation. | 611 | // markAttachmentReady completes only the captured attachment incarnation. | ||
| 597 | func (c *renderCoordinator) markAttachmentReady(lease *attachmentLease) bool { | 612 | func (c *renderCoordinator) markAttachmentReady(lease *attachmentLease) bool { | ||
| 598 | c.mu.Lock() | 613 | c.mu.Lock() | ||
M internal/usecase/daemon/resume_wake_incarnation_test.go +42 -0
| @@ -66,3 +66,45 @@ func TestParkedResumeRejectsDispatchedWakeFromPriorAttachment(t *testing.T) { | |||
| 66 | 66 | d.firstPaint(sess, ac, ac.size) | |
| 67 | 67 | require.NotEmpty(t, newTransport.Sends()) | |
| 68 | 68 | } | |
| 69 | + | ||
| 70 | + | func TestSameAttachmentRebindRejectsQueuedWakeAndFansOutFreshGeneration(t *testing.T) { | |
| 71 | + | d, sess, ac, sends := newManualSessionWithPTYs(t, newQuietPTY()) | |
| 72 | + | rc := d.attachCoordinator(sess, nil, ac, true) | |
| 73 | + | oldLease := rc.attachmentLease(ac) | |
| 74 | + | require.NotNil(t, oldLease) | |
| 75 | + | ||
| 76 | + | originalWake := rc.opts.wake | |
| 77 | + | wakeDispatched := make(chan struct{}) | |
| 78 | + | releaseWake := make(chan struct{}) | |
| 79 | + | rc.opts.wake = func(w renderWake) { | |
| 80 | + | close(wakeDispatched) | |
| 81 | + | <-releaseWake | |
| 82 | + | originalWake(w) | |
| 83 | + | } | |
| 84 | + | wakeDone := make(chan struct{}) | |
| 85 | + | go func() { | |
| 86 | + | d.invalidateRenderNow(sess, ac, true, "same-attachment-rebind") | |
| 87 | + | close(wakeDone) | |
| 88 | + | }() | |
| 89 | + | <-wakeDispatched | |
| 90 | + | ||
| 91 | + | oldGeneration := ac.connectionGeneration.Load() | |
| 92 | + | result, err := d.transitionAttachment(attachmentTransitionRequest{ | |
| 93 | + | source: sess, target: sess, next: ac, | |
| 94 | + | ||
| 95 | + | expectedTransport: ac.transportSnapshot(), ready: true, | |
| 96 | + | }) | |
| 97 | + | require.NoError(t, err) | |
| 98 | + | d.deferAttachmentTransitionCleanups(result) | |
| 99 | + | require.Greater(t, ac.connectionGeneration.Load(), oldGeneration) | |
| 100 | + | freshLease := rc.attachmentLease(ac) | |
| 101 | + | require.NotSame(t, oldLease, freshLease) | |
| 102 | + | ||
| 103 | + | close(releaseWake) | |
| 104 | + | <-wakeDone | |
| 105 | + | require.Empty(t, sends, "a queued wake from the prior generation must not paint the rebound transport") | |
| 106 | + | ||
| 107 | + | rc.opts.wake = originalWake | |
| 108 | + | d.invalidateRenderNow(sess, ac, true, "fresh-generation-fanout") | |
| 109 | + | require.Greater(t, len(sends), 0, "the current generation must receive fanout output") | |
| 110 | + | } | |
| @@ -66,3 +66,45 @@ func TestParkedResumeRejectsDispatchedWakeFromPriorAttachment(t *testing.T) { | |||||
| 66 | d.firstPaint(sess, ac, ac.size) | 66 | d.firstPaint(sess, ac, ac.size) | ||
| 67 | require.NotEmpty(t, newTransport.Sends()) | 67 | require.NotEmpty(t, newTransport.Sends()) | ||
| 68 | } | 68 | } | ||
| 69 | + | ||||
| 70 | + | func TestSameAttachmentRebindRejectsQueuedWakeAndFansOutFreshGeneration(t *testing.T) { | |||
| 71 | + | d, sess, ac, sends := newManualSessionWithPTYs(t, newQuietPTY()) | |||
| 72 | + | rc := d.attachCoordinator(sess, nil, ac, true) | |||
| 73 | + | oldLease := rc.attachmentLease(ac) | |||
| 74 | + | require.NotNil(t, oldLease) | |||
| 75 | + | ||||
| 76 | + | originalWake := rc.opts.wake | |||
| 77 | + | wakeDispatched := make(chan struct{}) | |||
| 78 | + | releaseWake := make(chan struct{}) | |||
| 79 | + | rc.opts.wake = func(w renderWake) { | |||
| 80 | + | close(wakeDispatched) | |||
| 81 | + | <-releaseWake | |||
| 82 | + | originalWake(w) | |||
| 83 | + | } | |||
| 84 | + | wakeDone := make(chan struct{}) | |||
| 85 | + | go func() { | |||
| 86 | + | d.invalidateRenderNow(sess, ac, true, "same-attachment-rebind") | |||
| 87 | + | close(wakeDone) | |||
| 88 | + | }() | |||
| 89 | + | <-wakeDispatched | |||
| 90 | + | ||||
| 91 | + | oldGeneration := ac.connectionGeneration.Load() | |||
| 92 | + | result, err := d.transitionAttachment(attachmentTransitionRequest{ | |||
| 93 | + | source: sess, target: sess, next: ac, | |||
| 94 | + | ||||
| 95 | + | expectedTransport: ac.transportSnapshot(), ready: true, | |||
| 96 | + | }) | |||
| 97 | + | require.NoError(t, err) | |||
| 98 | + | d.deferAttachmentTransitionCleanups(result) | |||
| 99 | + | require.Greater(t, ac.connectionGeneration.Load(), oldGeneration) | |||
| 100 | + | freshLease := rc.attachmentLease(ac) | |||
| 101 | + | require.NotSame(t, oldLease, freshLease) | |||
| 102 | + | ||||
| 103 | + | close(releaseWake) | |||
| 104 | + | <-wakeDone | |||
| 105 | + | require.Empty(t, sends, "a queued wake from the prior generation must not paint the rebound transport") | |||
| 106 | + | ||||
| 107 | + | rc.opts.wake = originalWake | |||
| 108 | + | d.invalidateRenderNow(sess, ac, true, "fresh-generation-fanout") | |||
| 109 | + | require.Greater(t, len(sends), 0, "the current generation must receive fanout output") | |||
| 110 | + | } | |||