fix(daemon): fence remote resume parking races
- commit
- c6688370383ebf22d990b2df07a6d9ea220f15e1
- author
- brice · · 2026-07-29 21:53 UTC
- committer
- brice · · 2026-07-29 22:02 UTC
- parent
- fe43a19
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email
8 files · +1174 -8
M internal/usecase/daemon/client_transport_errors.go +26 -0
| @@ -12,10 +12,17 @@ func (d *Daemon) clientGone(sess *session, ac *attachedClient, failed ports.Tran | |||
| 12 | 12 | return // stale connection loop; a newer transport owns this client | |
| 13 | 13 | } | |
| 14 | 14 | } | |
| 15 | + | // Advertise parking before detach so IntentResume never observes both the | |
| 16 | + | // live seat and parking/parked registries empty for a still-valid token. | |
| 17 | + | var parkingToken uint64 | |
| 18 | + | if !explicit { | |
| 19 | + | parkingToken = d.markParkingInFlight(sess, ac) | |
| 20 | + | } | |
| 15 | 21 | if d.beforeClientGoneDetach != nil { | |
| 16 | 22 | d.beforeClientGoneDetach() | |
| 17 | 23 | } | |
| 18 | 24 | if !d.detachIfCurrentTransport(sess, ac, expected) { | |
| 25 | + | d.clearParkingInFlightIfAbandoned(sess, ac, parkingToken) | |
| 19 | 26 | return // displaced, or the link was rebound after the precheck | |
| 20 | 27 | } | |
| 21 | 28 | d.finishClientGone(sess, ac, failed, explicit) | |
| @@ -27,7 +34,12 @@ func (d *Daemon) clientGoneForRole(token attachmentRoleToken, explicit bool) boo | |||
| 27 | 34 | } | |
| 28 | 35 | token.effect.bindActionEnd(d, "detach") | |
| 29 | 36 | token.effect.End() | |
| 37 | + | var parkingToken uint64 | |
| 38 | + | if !explicit { | |
| 39 | + | parkingToken = d.markParkingInFlight(token.sess, token.ac) | |
| 40 | + | } | |
| 30 | 41 | if !d.detachIfRoleCurrent(token) { | |
| 42 | + | d.clearParkingInFlightIfAbandoned(token.sess, token.ac, parkingToken) | |
| 31 | 43 | return false | |
| 32 | 44 | } | |
| 33 | 45 | d.finishClientGone(token.sess, token.ac, token.transport.transport, explicit) | |
| @@ -35,6 +47,9 @@ func (d *Daemon) clientGoneForRole(token attachmentRoleToken, explicit bool) boo | |||
| 35 | 47 | } | |
| 36 | 48 | ||
| 37 | 49 | func (d *Daemon) finishClientGone(sess *session, ac *attachedClient, failed ports.Transport, explicit bool) { | |
| 50 | + | if d.afterClientGoneDetach != nil { | |
| 51 | + | d.afterClientGoneDetach() | |
| 52 | + | } | |
| 38 | 53 | if rc := sess.renderCoordinator(); rc != nil { | |
| 39 | 54 | rc.noteDetach(ac) | |
| 40 | 55 | } | |
| @@ -55,6 +70,10 @@ func (d *Daemon) finishClientGone(sess *session, ac *attachedClient, failed port | |||
| 55 | 70 | d.log.Info("client parked", "session", sess.name) | |
| 56 | 71 | return | |
| 57 | 72 | } | |
| 73 | + | // Explicit winners (and non-explicit park failures) must drop any same- | |
| 74 | + | // attachment parking marker left by a raced non-explicit teardown so | |
| 75 | + | // IntentResume waiters are not stranded on a never-published park. | |
| 76 | + | d.clearParkingInFlight(ac.resumeToken, ac) | |
| 58 | 77 | ||
| 59 | 78 | d.resetScreenDefaultColors(sess) | |
| 60 | 79 | ac.clearPreviousSession() | |
| @@ -79,9 +98,12 @@ func (d *Daemon) detachOnSendError(sess *session, ac *attachedClient, failed por | |||
| 79 | 98 | return | |
| 80 | 99 | } | |
| 81 | 100 | } | |
| 101 | + | parkingToken := d.markParkingInFlight(sess, ac) | |
| 82 | 102 | if d.detachIfCurrentTransport(sess, ac, expected) { | |
| 83 | 103 | d.finishSendErrorDetach(sess, ac, failed) | |
| 104 | + | return | |
| 84 | 105 | } | |
| 106 | + | d.clearParkingInFlightIfAbandoned(sess, ac, parkingToken) | |
| 85 | 107 | } | |
| 86 | 108 | ||
| 87 | 109 | func (d *Daemon) detachOnRoleSendError(token attachmentRoleToken, failed ports.Transport) { | |
| @@ -92,9 +114,12 @@ func (d *Daemon) detachOnRoleSendErrorUntil(token attachmentRoleToken, failed po | |||
| 92 | 114 | if failed != token.transport.transport { | |
| 93 | 115 | return | |
| 94 | 116 | } | |
| 117 | + | parkingToken := d.markParkingInFlight(token.sess, token.ac) | |
| 95 | 118 | if d.detachIfRoleCurrentUntil(token, done) { | |
| 96 | 119 | d.finishSendErrorDetach(token.sess, token.ac, failed) | |
| 120 | + | return | |
| 97 | 121 | } | |
| 122 | + | d.clearParkingInFlightIfAbandoned(token.sess, token.ac, parkingToken) | |
| 98 | 123 | } | |
| 99 | 124 | ||
| 100 | 125 | // reserveRoleSendErrorCleanup accounts for cleanup before End releases the | |
| @@ -128,6 +153,7 @@ func (d *Daemon) finishSendErrorDetach(sess *session, ac *attachedClient, failed | |||
| 128 | 153 | d.log.Warn("parked client after send error", "session", sess.name) | |
| 129 | 154 | return | |
| 130 | 155 | } | |
| 156 | + | d.clearParkingInFlight(ac.resumeToken, ac) | |
| 131 | 157 | d.resetScreenDefaultColors(sess) | |
| 132 | 158 | ac.clearPreviousSession() | |
| 133 | 159 | _ = ac.closeCapturedTransport(failed) | |
| @@ -12,10 +12,17 @@ func (d *Daemon) clientGone(sess *session, ac *attachedClient, failed ports.Tran | |||||
| 12 | return // stale connection loop; a newer transport owns this client | 12 | return // stale connection loop; a newer transport owns this client | ||
| 13 | } | 13 | } | ||
| 14 | } | 14 | } | ||
| 15 | + | // Advertise parking before detach so IntentResume never observes both the | |||
| 16 | + | // live seat and parking/parked registries empty for a still-valid token. | |||
| 17 | + | var parkingToken uint64 | |||
| 18 | + | if !explicit { | |||
| 19 | + | parkingToken = d.markParkingInFlight(sess, ac) | |||
| 20 | + | } | |||
| 15 | if d.beforeClientGoneDetach != nil { | 21 | if d.beforeClientGoneDetach != nil { | ||
| 16 | d.beforeClientGoneDetach() | 22 | d.beforeClientGoneDetach() | ||
| 17 | } | 23 | } | ||
| 18 | if !d.detachIfCurrentTransport(sess, ac, expected) { | 24 | if !d.detachIfCurrentTransport(sess, ac, expected) { | ||
| 25 | + | d.clearParkingInFlightIfAbandoned(sess, ac, parkingToken) | |||
| 19 | return // displaced, or the link was rebound after the precheck | 26 | return // displaced, or the link was rebound after the precheck | ||
| 20 | } | 27 | } | ||
| 21 | d.finishClientGone(sess, ac, failed, explicit) | 28 | d.finishClientGone(sess, ac, failed, explicit) | ||
| @@ -27,7 +34,12 @@ func (d *Daemon) clientGoneForRole(token attachmentRoleToken, explicit bool) boo | |||||
| 27 | } | 34 | } | ||
| 28 | token.effect.bindActionEnd(d, "detach") | 35 | token.effect.bindActionEnd(d, "detach") | ||
| 29 | token.effect.End() | 36 | token.effect.End() | ||
| 37 | + | var parkingToken uint64 | |||
| 38 | + | if !explicit { | |||
| 39 | + | parkingToken = d.markParkingInFlight(token.sess, token.ac) | |||
| 40 | + | } | |||
| 30 | if !d.detachIfRoleCurrent(token) { | 41 | if !d.detachIfRoleCurrent(token) { | ||
| 42 | + | d.clearParkingInFlightIfAbandoned(token.sess, token.ac, parkingToken) | |||
| 31 | return false | 43 | return false | ||
| 32 | } | 44 | } | ||
| 33 | d.finishClientGone(token.sess, token.ac, token.transport.transport, explicit) | 45 | d.finishClientGone(token.sess, token.ac, token.transport.transport, explicit) | ||
| @@ -35,6 +47,9 @@ func (d *Daemon) clientGoneForRole(token attachmentRoleToken, explicit bool) boo | |||||
| 35 | } | 47 | } | ||
| 36 | 48 | ||||
| 37 | func (d *Daemon) finishClientGone(sess *session, ac *attachedClient, failed ports.Transport, explicit bool) { | 49 | func (d *Daemon) finishClientGone(sess *session, ac *attachedClient, failed ports.Transport, explicit bool) { | ||
| 50 | + | if d.afterClientGoneDetach != nil { | |||
| 51 | + | d.afterClientGoneDetach() | |||
| 52 | + | } | |||
| 38 | if rc := sess.renderCoordinator(); rc != nil { | 53 | if rc := sess.renderCoordinator(); rc != nil { | ||
| 39 | rc.noteDetach(ac) | 54 | rc.noteDetach(ac) | ||
| 40 | } | 55 | } | ||
| @@ -55,6 +70,10 @@ func (d *Daemon) finishClientGone(sess *session, ac *attachedClient, failed port | |||||
| 55 | d.log.Info("client parked", "session", sess.name) | 70 | d.log.Info("client parked", "session", sess.name) | ||
| 56 | return | 71 | return | ||
| 57 | } | 72 | } | ||
| 73 | + | // Explicit winners (and non-explicit park failures) must drop any same- | |||
| 74 | + | // attachment parking marker left by a raced non-explicit teardown so | |||
| 75 | + | // IntentResume waiters are not stranded on a never-published park. | |||
| 76 | + | d.clearParkingInFlight(ac.resumeToken, ac) | |||
| 58 | 77 | ||||
| 59 | d.resetScreenDefaultColors(sess) | 78 | d.resetScreenDefaultColors(sess) | ||
| 60 | ac.clearPreviousSession() | 79 | ac.clearPreviousSession() | ||
| @@ -79,9 +98,12 @@ func (d *Daemon) detachOnSendError(sess *session, ac *attachedClient, failed por | |||||
| 79 | return | 98 | return | ||
| 80 | } | 99 | } | ||
| 81 | } | 100 | } | ||
| 101 | + | parkingToken := d.markParkingInFlight(sess, ac) | |||
| 82 | if d.detachIfCurrentTransport(sess, ac, expected) { | 102 | if d.detachIfCurrentTransport(sess, ac, expected) { | ||
| 83 | d.finishSendErrorDetach(sess, ac, failed) | 103 | d.finishSendErrorDetach(sess, ac, failed) | ||
| 104 | + | return | |||
| 84 | } | 105 | } | ||
| 106 | + | d.clearParkingInFlightIfAbandoned(sess, ac, parkingToken) | |||
| 85 | } | 107 | } | ||
| 86 | 108 | ||||
| 87 | func (d *Daemon) detachOnRoleSendError(token attachmentRoleToken, failed ports.Transport) { | 109 | func (d *Daemon) detachOnRoleSendError(token attachmentRoleToken, failed ports.Transport) { | ||
| @@ -92,9 +114,12 @@ func (d *Daemon) detachOnRoleSendErrorUntil(token attachmentRoleToken, failed po | |||||
| 92 | if failed != token.transport.transport { | 114 | if failed != token.transport.transport { | ||
| 93 | return | 115 | return | ||
| 94 | } | 116 | } | ||
| 117 | + | parkingToken := d.markParkingInFlight(token.sess, token.ac) | |||
| 95 | if d.detachIfRoleCurrentUntil(token, done) { | 118 | if d.detachIfRoleCurrentUntil(token, done) { | ||
| 96 | d.finishSendErrorDetach(token.sess, token.ac, failed) | 119 | d.finishSendErrorDetach(token.sess, token.ac, failed) | ||
| 120 | + | return | |||
| 97 | } | 121 | } | ||
| 122 | + | d.clearParkingInFlightIfAbandoned(token.sess, token.ac, parkingToken) | |||
| 98 | } | 123 | } | ||
| 99 | 124 | ||||
| 100 | // reserveRoleSendErrorCleanup accounts for cleanup before End releases the | 125 | // reserveRoleSendErrorCleanup accounts for cleanup before End releases the | ||
| @@ -128,6 +153,7 @@ func (d *Daemon) finishSendErrorDetach(sess *session, ac *attachedClient, failed | |||||
| 128 | d.log.Warn("parked client after send error", "session", sess.name) | 153 | d.log.Warn("parked client after send error", "session", sess.name) | ||
| 129 | return | 154 | return | ||
| 130 | } | 155 | } | ||
| 156 | + | d.clearParkingInFlight(ac.resumeToken, ac) | |||
| 131 | d.resetScreenDefaultColors(sess) | 157 | d.resetScreenDefaultColors(sess) | ||
| 132 | ac.clearPreviousSession() | 158 | ac.clearPreviousSession() | ||
| 133 | _ = ac.closeCapturedTransport(failed) | 159 | _ = ac.closeCapturedTransport(failed) | ||
M internal/usecase/daemon/daemon.go +43 -0
| @@ -108,6 +108,11 @@ type Daemon struct { | |||
| 108 | 108 | // with Wait. | |
| 109 | 109 | notifies []chan struct{} | |
| 110 | 110 | parked map[uint64]*parkedAttachment | |
| 111 | + | // parking tracks resume-capable attachments from before detach clears the | |
| 112 | + | // live seat until parkAttachment publishes the token into parked. | |
| 113 | + | // IntentResume waits on the matching entry instead of treating the live | |
| 114 | + | // credential as unknown across that gap. | |
| 115 | + | parking map[uint64]*parkingAttachment | |
| 111 | 116 | ||
| 112 | 117 | attnMu sync.Mutex | |
| 113 | 118 | animFrame int | |
| @@ -175,6 +180,25 @@ type Daemon struct { | |||
| 175 | 180 | // beforeClientGoneDetach pauses clientGone after the stale-transport | |
| 176 | 181 | // precheck and before exact transport/incarnation detach validation. | |
| 177 | 182 | beforeClientGoneDetach func() | |
| 183 | + | // beforeMarkParkingInFlight pauses resumeLiveAttachment after the live | |
| 184 | + | // credential matches and before markParkingInFlight, so tests can win | |
| 185 | + | // explicit detach or session removal before late marker publication. | |
| 186 | + | beforeMarkParkingInFlight func() | |
| 187 | + | // afterClientGoneDetach pauses finishClientGone after detach won and before | |
| 188 | + | // parkAttachment. The parking-in-flight marker must already be published. | |
| 189 | + | afterClientGoneDetach func() | |
| 190 | + | // afterResumeLiveDetach pauses resumeLiveAttachment after detach won and | |
| 191 | + | // before parkAttachment. The parking-in-flight marker must already be | |
| 192 | + | // published so a concurrent same-token resume can wait the gap. | |
| 193 | + | afterResumeLiveDetach func() | |
| 194 | + | // afterParkingWaitArmed observes waitParkingInFlight after it has resolved a | |
| 195 | + | // matching in-flight entry and before it blocks on done. | |
| 196 | + | afterParkingWaitArmed func() | |
| 197 | + | // afterSnatchedUnrouteBeforePark pauses parkSnatchedAttachment and | |
| 198 | + | // cleanupInterruptedSnatchedAttachment after exact snatched ownership is | |
| 199 | + | // removed and before parkAttachmentAs. The parking-in-flight marker must | |
| 200 | + | // already be published so a concurrent same-token resume can wait the gap. | |
| 201 | + | afterSnatchedUnrouteBeforePark func() | |
| 178 | 202 | // beforeResumeParkedSendMu pauses resumeParked after the initial parked | |
| 179 | 203 | // lookup and before the attachment send lock, so tests can consume or | |
| 180 | 204 | // replace the credential while the handshake waits. | |
| @@ -307,6 +331,23 @@ type parkedAttachment struct { | |||
| 307 | 331 | doneOnce sync.Once | |
| 308 | 332 | } | |
| 309 | 333 | ||
| 334 | + | // parkingAttachment is the observable detach→park lifecycle for one resume | |
| 335 | + | // credential. done closes when the token is published into parked or parking | |
| 336 | + | // is abandoned. | |
| 337 | + | type parkingAttachment struct { | |
| 338 | + | sess *session | |
| 339 | + | ac *attachedClient | |
| 340 | + | done chan struct{} | |
| 341 | + | doneOnce sync.Once | |
| 342 | + | } | |
| 343 | + | ||
| 344 | + | func (p *parkingAttachment) closeDone() { | |
| 345 | + | if p == nil || p.done == nil { | |
| 346 | + | return | |
| 347 | + | } | |
| 348 | + | p.doneOnce.Do(func() { close(p.done) }) | |
| 349 | + | } | |
| 350 | + | ||
| 310 | 351 | // session is a single multiplexed session. It owns one or more full-screen | |
| 311 | 352 | ||
| 312 | 353 | type stoppedSession struct { | |
| @@ -539,6 +580,7 @@ func New(ptys ports.PTYFactory, clock ports.Clock, log *slog.Logger, opts ...Opt | |||
| 539 | 580 | stopped: make(map[string]stoppedSession), | |
| 540 | 581 | creating: make(map[string]struct{}), | |
| 541 | 582 | parked: make(map[uint64]*parkedAttachment), | |
| 583 | + | parking: make(map[uint64]*parkingAttachment), | |
| 542 | 584 | paneProcessCtx: paneProcessCtx, | |
| 543 | 585 | paneProcessCancel: paneProcessCancel, | |
| 544 | 586 | ptys: ptys, | |
| @@ -731,6 +773,7 @@ func (d *Daemon) shutdownAllWithSnapshotDeadline(reason uint8, deadline *snapsho | |||
| 731 | 773 | d.closeMoveLifecycles() | |
| 732 | 774 | d.mu.Lock() | |
| 733 | 775 | d.closing = true | |
| 776 | + | d.purgeAllParkingLocked() | |
| 734 | 777 | parkedRetirements := d.purgeAllParkedLocked() | |
| 735 | 778 | snapshot := d.sessionsSnapshotLocked() | |
| 736 | 779 | empty := len(snapshot) == 0 | |
| @@ -108,6 +108,11 @@ type Daemon struct { | |||||
| 108 | // with Wait. | 108 | // with Wait. | ||
| 109 | notifies []chan struct{} | 109 | notifies []chan struct{} | ||
| 110 | parked map[uint64]*parkedAttachment | 110 | parked map[uint64]*parkedAttachment | ||
| 111 | + | // parking tracks resume-capable attachments from before detach clears the | |||
| 112 | + | // live seat until parkAttachment publishes the token into parked. | |||
| 113 | + | // IntentResume waits on the matching entry instead of treating the live | |||
| 114 | + | // credential as unknown across that gap. | |||
| 115 | + | parking map[uint64]*parkingAttachment | |||
| 111 | 116 | ||||
| 112 | attnMu sync.Mutex | 117 | attnMu sync.Mutex | ||
| 113 | animFrame int | 118 | animFrame int | ||
| @@ -175,6 +180,25 @@ type Daemon struct { | |||||
| 175 | // beforeClientGoneDetach pauses clientGone after the stale-transport | 180 | // beforeClientGoneDetach pauses clientGone after the stale-transport | ||
| 176 | // precheck and before exact transport/incarnation detach validation. | 181 | // precheck and before exact transport/incarnation detach validation. | ||
| 177 | beforeClientGoneDetach func() | 182 | beforeClientGoneDetach func() | ||
| 183 | + | // beforeMarkParkingInFlight pauses resumeLiveAttachment after the live | |||
| 184 | + | // credential matches and before markParkingInFlight, so tests can win | |||
| 185 | + | // explicit detach or session removal before late marker publication. | |||
| 186 | + | beforeMarkParkingInFlight func() | |||
| 187 | + | // afterClientGoneDetach pauses finishClientGone after detach won and before | |||
| 188 | + | // parkAttachment. The parking-in-flight marker must already be published. | |||
| 189 | + | afterClientGoneDetach func() | |||
| 190 | + | // afterResumeLiveDetach pauses resumeLiveAttachment after detach won and | |||
| 191 | + | // before parkAttachment. The parking-in-flight marker must already be | |||
| 192 | + | // published so a concurrent same-token resume can wait the gap. | |||
| 193 | + | afterResumeLiveDetach func() | |||
| 194 | + | // afterParkingWaitArmed observes waitParkingInFlight after it has resolved a | |||
| 195 | + | // matching in-flight entry and before it blocks on done. | |||
| 196 | + | afterParkingWaitArmed func() | |||
| 197 | + | // afterSnatchedUnrouteBeforePark pauses parkSnatchedAttachment and | |||
| 198 | + | // cleanupInterruptedSnatchedAttachment after exact snatched ownership is | |||
| 199 | + | // removed and before parkAttachmentAs. The parking-in-flight marker must | |||
| 200 | + | // already be published so a concurrent same-token resume can wait the gap. | |||
| 201 | + | afterSnatchedUnrouteBeforePark func() | |||
| 178 | // beforeResumeParkedSendMu pauses resumeParked after the initial parked | 202 | // beforeResumeParkedSendMu pauses resumeParked after the initial parked | ||
| 179 | // lookup and before the attachment send lock, so tests can consume or | 203 | // lookup and before the attachment send lock, so tests can consume or | ||
| 180 | // replace the credential while the handshake waits. | 204 | // replace the credential while the handshake waits. | ||
| @@ -307,6 +331,23 @@ type parkedAttachment struct { | |||||
| 307 | doneOnce sync.Once | 331 | doneOnce sync.Once | ||
| 308 | } | 332 | } | ||
| 309 | 333 | ||||
| 334 | + | // parkingAttachment is the observable detach→park lifecycle for one resume | |||
| 335 | + | // credential. done closes when the token is published into parked or parking | |||
| 336 | + | // is abandoned. | |||
| 337 | + | type parkingAttachment struct { | |||
| 338 | + | sess *session | |||
| 339 | + | ac *attachedClient | |||
| 340 | + | done chan struct{} | |||
| 341 | + | doneOnce sync.Once | |||
| 342 | + | } | |||
| 343 | + | ||||
| 344 | + | func (p *parkingAttachment) closeDone() { | |||
| 345 | + | if p == nil || p.done == nil { | |||
| 346 | + | return | |||
| 347 | + | } | |||
| 348 | + | p.doneOnce.Do(func() { close(p.done) }) | |||
| 349 | + | } | |||
| 350 | + | ||||
| 310 | // session is a single multiplexed session. It owns one or more full-screen | 351 | // session is a single multiplexed session. It owns one or more full-screen | ||
| 311 | 352 | ||||
| 312 | type stoppedSession struct { | 353 | type stoppedSession struct { | ||
| @@ -539,6 +580,7 @@ func New(ptys ports.PTYFactory, clock ports.Clock, log *slog.Logger, opts ...Opt | |||||
| 539 | stopped: make(map[string]stoppedSession), | 580 | stopped: make(map[string]stoppedSession), | ||
| 540 | creating: make(map[string]struct{}), | 581 | creating: make(map[string]struct{}), | ||
| 541 | parked: make(map[uint64]*parkedAttachment), | 582 | parked: make(map[uint64]*parkedAttachment), | ||
| 583 | + | parking: make(map[uint64]*parkingAttachment), | |||
| 542 | paneProcessCtx: paneProcessCtx, | 584 | paneProcessCtx: paneProcessCtx, | ||
| 543 | paneProcessCancel: paneProcessCancel, | 585 | paneProcessCancel: paneProcessCancel, | ||
| 544 | ptys: ptys, | 586 | ptys: ptys, | ||
| @@ -731,6 +773,7 @@ func (d *Daemon) shutdownAllWithSnapshotDeadline(reason uint8, deadline *snapsho | |||||
| 731 | d.closeMoveLifecycles() | 773 | d.closeMoveLifecycles() | ||
| 732 | d.mu.Lock() | 774 | d.mu.Lock() | ||
| 733 | d.closing = true | 775 | d.closing = true | ||
| 776 | + | d.purgeAllParkingLocked() | |||
| 734 | parkedRetirements := d.purgeAllParkedLocked() | 777 | parkedRetirements := d.purgeAllParkedLocked() | ||
| 735 | snapshot := d.sessionsSnapshotLocked() | 778 | snapshot := d.sessionsSnapshotLocked() | ||
| 736 | empty := len(snapshot) == 0 | 779 | empty := len(snapshot) == 0 | ||
M internal/usecase/daemon/move_pane_commit.go +1 -0
| @@ -158,6 +158,7 @@ func (c *movePaneCommit) publishLocked(d *Daemon) bool { | |||
| 158 | 158 | if candidate.removeSourceTab && len(c.source.tabs) == 0 { | |
| 159 | 159 | delete(d.sessions, c.source.id) | |
| 160 | 160 | c.retiredAttachments = retireEmptyMoveSessionLocked(c.source, retirement) | |
| 161 | + | d.purgeParkingForSessionLocked(c.source) | |
| 161 | 162 | c.retiredParked = d.purgeParkedForSessionLocked(c.source) | |
| 162 | 163 | } | |
| 163 | 164 | c.sourceEmpty = len(c.source.tabs) == 0 | |
| @@ -158,6 +158,7 @@ func (c *movePaneCommit) publishLocked(d *Daemon) bool { | |||||
| 158 | if candidate.removeSourceTab && len(c.source.tabs) == 0 { | 158 | if candidate.removeSourceTab && len(c.source.tabs) == 0 { | ||
| 159 | delete(d.sessions, c.source.id) | 159 | delete(d.sessions, c.source.id) | ||
| 160 | c.retiredAttachments = retireEmptyMoveSessionLocked(c.source, retirement) | 160 | c.retiredAttachments = retireEmptyMoveSessionLocked(c.source, retirement) | ||
| 161 | + | d.purgeParkingForSessionLocked(c.source) | |||
| 161 | c.retiredParked = d.purgeParkedForSessionLocked(c.source) | 162 | c.retiredParked = d.purgeParkedForSessionLocked(c.source) | ||
| 162 | } | 163 | } | ||
| 163 | c.sourceEmpty = len(c.source.tabs) == 0 | 164 | c.sourceEmpty = len(c.source.tabs) == 0 | ||
M internal/usecase/daemon/move_tab.go +1 -0
| @@ -386,6 +386,7 @@ func (c *moveTabCommit) publishLocked(d *Daemon, fencedPanes []*pane) bool { | |||
| 386 | 386 | if len(c.source.tabs) == 0 { | |
| 387 | 387 | delete(d.sessions, c.source.id) | |
| 388 | 388 | c.retiredAttachments = retireEmptyMoveSessionLocked(c.source, retirement) | |
| 389 | + | d.purgeParkingForSessionLocked(c.source) | |
| 389 | 390 | c.retiredParked = d.purgeParkedForSessionLocked(c.source) | |
| 390 | 391 | } | |
| 391 | 392 | c.sourceEmpty = len(c.source.tabs) == 0 | |
| @@ -386,6 +386,7 @@ func (c *moveTabCommit) publishLocked(d *Daemon, fencedPanes []*pane) bool { | |||||
| 386 | if len(c.source.tabs) == 0 { | 386 | if len(c.source.tabs) == 0 { | ||
| 387 | delete(d.sessions, c.source.id) | 387 | delete(d.sessions, c.source.id) | ||
| 388 | c.retiredAttachments = retireEmptyMoveSessionLocked(c.source, retirement) | 388 | c.retiredAttachments = retireEmptyMoveSessionLocked(c.source, retirement) | ||
| 389 | + | d.purgeParkingForSessionLocked(c.source) | |||
| 389 | c.retiredParked = d.purgeParkedForSessionLocked(c.source) | 390 | c.retiredParked = d.purgeParkedForSessionLocked(c.source) | ||
| 390 | } | 391 | } | ||
| 391 | c.sourceEmpty = len(c.source.tabs) == 0 | 392 | c.sourceEmpty = len(c.source.tabs) == 0 | ||
M internal/usecase/daemon/resume.go +178 -3
| @@ -26,11 +26,163 @@ func (d *Daemon) nextResumeTokenLocked() uint64 { | |||
| 26 | 26 | continue | |
| 27 | 27 | } | |
| 28 | 28 | if _, exists := d.parked[tok]; !exists { | |
| 29 | - | return tok | |
| 29 | + | if _, parking := d.parking[tok]; !parking { | |
| 30 | + | return tok | |
| 31 | + | } | |
| 32 | + | } | |
| 33 | + | } | |
| 34 | + | } | |
| 35 | + | ||
| 36 | + | // markParkingInFlight publishes an observable parking lifecycle so IntentResume | |
| 37 | + | // can wait out the detach→park gap. Callers must publish this before clearing | |
| 38 | + | // the live seat so resume never observes both registries empty for a still-valid | |
| 39 | + | // credential. Under d.mu then sess.mu it advertises only when the daemon still | |
| 40 | + | // registers the exact session and ac is still the exact active or snatched | |
| 41 | + | // owner; otherwise it returns 0 and never creates a marker after terminal | |
| 42 | + | // cleanup. Returns the resume token, or 0 when parking cannot be advertised. | |
| 43 | + | func (d *Daemon) markParkingInFlight(sess *session, ac *attachedClient) uint64 { | |
| 44 | + | if sess == nil || ac == nil || !ac.resumeCapable { | |
| 45 | + | return 0 | |
| 46 | + | } | |
| 47 | + | d.mu.Lock() | |
| 48 | + | defer d.mu.Unlock() | |
| 49 | + | if d.closing || d.sessions[sess.id] != sess { | |
| 50 | + | return 0 | |
| 51 | + | } | |
| 52 | + | sess.mu.Lock() | |
| 53 | + | role := sess.attachmentRoleLocked(ac) | |
| 54 | + | owner := role == attachmentActive || role == attachmentSnatched | |
| 55 | + | sess.mu.Unlock() | |
| 56 | + | if !owner { | |
| 57 | + | return 0 | |
| 58 | + | } | |
| 59 | + | if ac.resumeToken == 0 { | |
| 60 | + | ac.resumeToken = d.nextResumeTokenLocked() | |
| 61 | + | } | |
| 62 | + | d.ensureParkingInFlightLocked(sess, ac) | |
| 63 | + | d.log.Info("parking in flight", "session", sess.name) | |
| 64 | + | return ac.resumeToken | |
| 65 | + | } | |
| 66 | + | ||
| 67 | + | // ensureParkingInFlightLocked records or refreshes the in-flight marker for ac. | |
| 68 | + | // Caller holds d.mu and has verified the daemon is not closing. | |
| 69 | + | func (d *Daemon) ensureParkingInFlightLocked(sess *session, ac *attachedClient) { | |
| 70 | + | token := ac.resumeToken | |
| 71 | + | if token == 0 { | |
| 72 | + | return | |
| 73 | + | } | |
| 74 | + | if existing := d.parking[token]; existing != nil { | |
| 75 | + | if existing.ac == ac { | |
| 76 | + | existing.sess = sess | |
| 77 | + | return | |
| 78 | + | } | |
| 79 | + | delete(d.parking, token) | |
| 80 | + | existing.closeDone() | |
| 81 | + | } | |
| 82 | + | d.parking[token] = &parkingAttachment{ | |
| 83 | + | sess: sess, | |
| 84 | + | ac: ac, | |
| 85 | + | done: make(chan struct{}), | |
| 86 | + | } | |
| 87 | + | } | |
| 88 | + | ||
| 89 | + | func (d *Daemon) clearParkingInFlight(token uint64, ac *attachedClient) { | |
| 90 | + | if token == 0 { | |
| 91 | + | return | |
| 92 | + | } | |
| 93 | + | d.mu.Lock() | |
| 94 | + | d.clearParkingInFlightLocked(token, ac) | |
| 95 | + | d.mu.Unlock() | |
| 96 | + | } | |
| 97 | + | ||
| 98 | + | // clearParkingInFlightIfAbandoned drops a pre-detach parking marker when this | |
| 99 | + | // teardown lost the seat while still the live owner (stale transport fence for | |
| 100 | + | // active or snatched). If another path already detached/unrouted, that path | |
| 101 | + | // owns park publication. | |
| 102 | + | func (d *Daemon) clearParkingInFlightIfAbandoned(sess *session, ac *attachedClient, token uint64) { | |
| 103 | + | if token == 0 || ac == nil { | |
| 104 | + | return | |
| 105 | + | } | |
| 106 | + | d.mu.Lock() | |
| 107 | + | defer d.mu.Unlock() | |
| 108 | + | if d.parked[token] != nil { | |
| 109 | + | return | |
| 110 | + | } | |
| 111 | + | stillOwner := false | |
| 112 | + | if sess != nil { | |
| 113 | + | sess.mu.Lock() | |
| 114 | + | role := sess.attachmentRoleLocked(ac) | |
| 115 | + | stillOwner = role == attachmentActive || role == attachmentSnatched | |
| 116 | + | sess.mu.Unlock() | |
| 117 | + | } | |
| 118 | + | if !stillOwner { | |
| 119 | + | return | |
| 120 | + | } | |
| 121 | + | d.clearParkingInFlightLocked(token, ac) | |
| 122 | + | } | |
| 123 | + | ||
| 124 | + | func (d *Daemon) clearParkingInFlightLocked(token uint64, ac *attachedClient) { | |
| 125 | + | pending := d.parking[token] | |
| 126 | + | if pending == nil { | |
| 127 | + | return | |
| 128 | + | } | |
| 129 | + | if ac != nil && pending.ac != ac { | |
| 130 | + | return | |
| 131 | + | } | |
| 132 | + | delete(d.parking, token) | |
| 133 | + | pending.closeDone() | |
| 134 | + | } | |
| 135 | + | ||
| 136 | + | func (d *Daemon) purgeAllParkingLocked() { | |
| 137 | + | for token, pending := range d.parking { | |
| 138 | + | delete(d.parking, token) | |
| 139 | + | pending.closeDone() | |
| 140 | + | } | |
| 141 | + | } | |
| 142 | + | ||
| 143 | + | // purgeParkingForSessionLocked removes and closes every in-flight parking | |
| 144 | + | // marker for sess so same-token waiters fail closed instead of hanging when | |
| 145 | + | // the session is removed before park publication. Caller holds d.mu. | |
| 146 | + | func (d *Daemon) purgeParkingForSessionLocked(sess *session) { | |
| 147 | + | for token, pending := range d.parking { | |
| 148 | + | if pending.sess == sess { | |
| 149 | + | delete(d.parking, token) | |
| 150 | + | pending.closeDone() | |
| 30 | 151 | } | |
| 31 | 152 | } | |
| 32 | 153 | } | |
| 33 | 154 | ||
| 155 | + | // waitParkingInFlight waits for an identified same-client parking publication | |
| 156 | + | // to finish. Returns true when a matching in-flight entry was observed. | |
| 157 | + | // Unknown and mismatched credentials return false without waiting. | |
| 158 | + | func (d *Daemon) waitParkingInFlight(h ports.Hello) bool { | |
| 159 | + | if h.ResumeToken == 0 { | |
| 160 | + | return false | |
| 161 | + | } | |
| 162 | + | d.mu.Lock() | |
| 163 | + | pending := d.parking[h.ResumeToken] | |
| 164 | + | if pending == nil { | |
| 165 | + | d.mu.Unlock() | |
| 166 | + | return false | |
| 167 | + | } | |
| 168 | + | if pending.ac.clientID != h.ClientID { | |
| 169 | + | d.mu.Unlock() | |
| 170 | + | return false | |
| 171 | + | } | |
| 172 | + | if h.Name != "" && pending.sess != nil && pending.sess.name != h.Name { | |
| 173 | + | d.mu.Unlock() | |
| 174 | + | return false | |
| 175 | + | } | |
| 176 | + | done := pending.done | |
| 177 | + | d.mu.Unlock() | |
| 178 | + | d.log.Info("resume waiting for in-flight parking", "session", h.Name) | |
| 179 | + | if d.afterParkingWaitArmed != nil { | |
| 180 | + | d.afterParkingWaitArmed() | |
| 181 | + | } | |
| 182 | + | <-done | |
| 183 | + | return true | |
| 184 | + | } | |
| 185 | + | ||
| 34 | 186 | func (d *Daemon) prepareParkAttachment(sess *session, ac *attachedClient) bool { | |
| 35 | 187 | if !ac.resumeCapable { | |
| 36 | 188 | return false | |
| @@ -43,6 +195,10 @@ func (d *Daemon) prepareParkAttachment(sess *session, ac *attachedClient) bool { | |||
| 43 | 195 | if ac.resumeToken == 0 { | |
| 44 | 196 | ac.resumeToken = d.nextResumeTokenLocked() | |
| 45 | 197 | } | |
| 198 | + | // Do not recreate parking markers here: post-detach publication races | |
| 199 | + | // terminal session removal and can strand IntentResume waiters. Callers | |
| 200 | + | // that need the detach→park gap advertised must markParkingInFlight while | |
| 201 | + | // still the exact live owner. Direct parkAttachment still parks below. | |
| 46 | 202 | d.log.Info("parking client prepared", "session", sess.name) | |
| 47 | 203 | return true | |
| 48 | 204 | } | |
| @@ -62,6 +218,7 @@ func (d *Daemon) parkAttachmentAs(sess *session, ac *attachedClient, role attach | |||
| 62 | 218 | ac.clearCaptureFrames() | |
| 63 | 219 | d.mu.Lock() | |
| 64 | 220 | if d.closing || d.sessions[sess.id] != sess { | |
| 221 | + | d.clearParkingInFlightLocked(ac.resumeToken, ac) | |
| 65 | 222 | d.mu.Unlock() | |
| 66 | 223 | return false | |
| 67 | 224 | } | |
| @@ -90,6 +247,7 @@ func (d *Daemon) parkAttachmentAs(sess *session, ac *attachedClient, role attach | |||
| 90 | 247 | parked := &parkedAttachment{sess: sess, ac: ac, role: role, timer: timer, done: make(chan struct{})} | |
| 91 | 248 | ac.parked = true | |
| 92 | 249 | d.parked[token] = parked | |
| 250 | + | d.clearParkingInFlightLocked(token, ac) | |
| 93 | 251 | d.mu.Unlock() | |
| 94 | 252 | if oldRetirement != nil { | |
| 95 | 253 | finishParkedAttachmentRetirements([]parkedAttachmentRetirement{*oldRetirement}) | |
| @@ -255,13 +413,29 @@ func (d *Daemon) resumeLiveAttachment(h ports.Hello, tr ports.Transport, sz doma | |||
| 255 | 413 | return nil, nil, false, &protoErr{ports.ErrNoSuchSession, "resume token is no longer valid"} | |
| 256 | 414 | } | |
| 257 | 415 | if !credentialMatch { | |
| 258 | - | return nil, nil, false, nil | |
| 416 | + | // Detach may have won and not published parked yet; wait out that gap | |
| 417 | + | // for the exact same-client credential, then try the parked path. | |
| 418 | + | _ = d.waitParkingInFlight(h) | |
| 419 | + | return d.resumeParked(h, tr, sz) | |
| 259 | 420 | } | |
| 260 | 421 | ||
| 422 | + | if d.beforeMarkParkingInFlight != nil { | |
| 423 | + | d.beforeMarkParkingInFlight() | |
| 424 | + | } | |
| 425 | + | // Advertise parking before detach so a concurrent same-token resume never | |
| 426 | + | // observes both the live seat and parking/parked registries empty. Late | |
| 427 | + | // publication after terminal detach/session removal returns 0. | |
| 428 | + | parkingToken := d.markParkingInFlight(sess, ac) | |
| 261 | 429 | if !d.detachIfCurrentTransport(sess, ac, oldSnap) { | |
| 262 | - | // A newer owner, rebound link, or teardown won; try the parked path. | |
| 430 | + | d.clearParkingInFlightIfAbandoned(sess, ac, parkingToken) | |
| 431 | + | // A newer owner, rebound link, or teardown won; wait if teardown is | |
| 432 | + | // still publishing the park, then try the parked path. | |
| 433 | + | _ = d.waitParkingInFlight(h) | |
| 263 | 434 | return d.resumeParked(h, tr, sz) | |
| 264 | 435 | } | |
| 436 | + | if d.afterResumeLiveDetach != nil { | |
| 437 | + | d.afterResumeLiveDetach() | |
| 438 | + | } | |
| 265 | 439 | if rc := sess.renderCoordinator(); rc != nil { | |
| 266 | 440 | rc.noteDetach(ac) | |
| 267 | 441 | } | |
| @@ -269,6 +443,7 @@ func (d *Daemon) resumeLiveAttachment(h ports.Hello, tr ports.Transport, sz doma | |||
| 269 | 443 | if !d.parkAttachment(sess, ac) { | |
| 270 | 444 | // Detach already published; retire the captured link exactly once so | |
| 271 | 445 | // shutdown cannot leave an orphaned open transport with no owner. | |
| 446 | + | d.clearParkingInFlight(ac.resumeToken, ac) | |
| 272 | 447 | d.resetScreenDefaultColors(sess) | |
| 273 | 448 | ac.clearPreviousSession() | |
| 274 | 449 | _ = ac.closeCapturedTransport(ac.revokeTransport(oldSnap.transport)) | |
| @@ -26,11 +26,163 @@ func (d *Daemon) nextResumeTokenLocked() uint64 { | |||||
| 26 | continue | 26 | continue | ||
| 27 | } | 27 | } | ||
| 28 | if _, exists := d.parked[tok]; !exists { | 28 | if _, exists := d.parked[tok]; !exists { | ||
| 29 | - | return tok | 29 | + | if _, parking := d.parking[tok]; !parking { |
| 30 | + | return tok | |||
| 31 | + | } | |||
| 32 | + | } | |||
| 33 | + | } | |||
| 34 | + | } | |||
| 35 | + | ||||
| 36 | + | // markParkingInFlight publishes an observable parking lifecycle so IntentResume | |||
| 37 | + | // can wait out the detach→park gap. Callers must publish this before clearing | |||
| 38 | + | // the live seat so resume never observes both registries empty for a still-valid | |||
| 39 | + | // credential. Under d.mu then sess.mu it advertises only when the daemon still | |||
| 40 | + | // registers the exact session and ac is still the exact active or snatched | |||
| 41 | + | // owner; otherwise it returns 0 and never creates a marker after terminal | |||
| 42 | + | // cleanup. Returns the resume token, or 0 when parking cannot be advertised. | |||
| 43 | + | func (d *Daemon) markParkingInFlight(sess *session, ac *attachedClient) uint64 { | |||
| 44 | + | if sess == nil || ac == nil || !ac.resumeCapable { | |||
| 45 | + | return 0 | |||
| 46 | + | } | |||
| 47 | + | d.mu.Lock() | |||
| 48 | + | defer d.mu.Unlock() | |||
| 49 | + | if d.closing || d.sessions[sess.id] != sess { | |||
| 50 | + | return 0 | |||
| 51 | + | } | |||
| 52 | + | sess.mu.Lock() | |||
| 53 | + | role := sess.attachmentRoleLocked(ac) | |||
| 54 | + | owner := role == attachmentActive || role == attachmentSnatched | |||
| 55 | + | sess.mu.Unlock() | |||
| 56 | + | if !owner { | |||
| 57 | + | return 0 | |||
| 58 | + | } | |||
| 59 | + | if ac.resumeToken == 0 { | |||
| 60 | + | ac.resumeToken = d.nextResumeTokenLocked() | |||
| 61 | + | } | |||
| 62 | + | d.ensureParkingInFlightLocked(sess, ac) | |||
| 63 | + | d.log.Info("parking in flight", "session", sess.name) | |||
| 64 | + | return ac.resumeToken | |||
| 65 | + | } | |||
| 66 | + | ||||
| 67 | + | // ensureParkingInFlightLocked records or refreshes the in-flight marker for ac. | |||
| 68 | + | // Caller holds d.mu and has verified the daemon is not closing. | |||
| 69 | + | func (d *Daemon) ensureParkingInFlightLocked(sess *session, ac *attachedClient) { | |||
| 70 | + | token := ac.resumeToken | |||
| 71 | + | if token == 0 { | |||
| 72 | + | return | |||
| 73 | + | } | |||
| 74 | + | if existing := d.parking[token]; existing != nil { | |||
| 75 | + | if existing.ac == ac { | |||
| 76 | + | existing.sess = sess | |||
| 77 | + | return | |||
| 78 | + | } | |||
| 79 | + | delete(d.parking, token) | |||
| 80 | + | existing.closeDone() | |||
| 81 | + | } | |||
| 82 | + | d.parking[token] = &parkingAttachment{ | |||
| 83 | + | sess: sess, | |||
| 84 | + | ac: ac, | |||
| 85 | + | done: make(chan struct{}), | |||
| 86 | + | } | |||
| 87 | + | } | |||
| 88 | + | ||||
| 89 | + | func (d *Daemon) clearParkingInFlight(token uint64, ac *attachedClient) { | |||
| 90 | + | if token == 0 { | |||
| 91 | + | return | |||
| 92 | + | } | |||
| 93 | + | d.mu.Lock() | |||
| 94 | + | d.clearParkingInFlightLocked(token, ac) | |||
| 95 | + | d.mu.Unlock() | |||
| 96 | + | } | |||
| 97 | + | ||||
| 98 | + | // clearParkingInFlightIfAbandoned drops a pre-detach parking marker when this | |||
| 99 | + | // teardown lost the seat while still the live owner (stale transport fence for | |||
| 100 | + | // active or snatched). If another path already detached/unrouted, that path | |||
| 101 | + | // owns park publication. | |||
| 102 | + | func (d *Daemon) clearParkingInFlightIfAbandoned(sess *session, ac *attachedClient, token uint64) { | |||
| 103 | + | if token == 0 || ac == nil { | |||
| 104 | + | return | |||
| 105 | + | } | |||
| 106 | + | d.mu.Lock() | |||
| 107 | + | defer d.mu.Unlock() | |||
| 108 | + | if d.parked[token] != nil { | |||
| 109 | + | return | |||
| 110 | + | } | |||
| 111 | + | stillOwner := false | |||
| 112 | + | if sess != nil { | |||
| 113 | + | sess.mu.Lock() | |||
| 114 | + | role := sess.attachmentRoleLocked(ac) | |||
| 115 | + | stillOwner = role == attachmentActive || role == attachmentSnatched | |||
| 116 | + | sess.mu.Unlock() | |||
| 117 | + | } | |||
| 118 | + | if !stillOwner { | |||
| 119 | + | return | |||
| 120 | + | } | |||
| 121 | + | d.clearParkingInFlightLocked(token, ac) | |||
| 122 | + | } | |||
| 123 | + | ||||
| 124 | + | func (d *Daemon) clearParkingInFlightLocked(token uint64, ac *attachedClient) { | |||
| 125 | + | pending := d.parking[token] | |||
| 126 | + | if pending == nil { | |||
| 127 | + | return | |||
| 128 | + | } | |||
| 129 | + | if ac != nil && pending.ac != ac { | |||
| 130 | + | return | |||
| 131 | + | } | |||
| 132 | + | delete(d.parking, token) | |||
| 133 | + | pending.closeDone() | |||
| 134 | + | } | |||
| 135 | + | ||||
| 136 | + | func (d *Daemon) purgeAllParkingLocked() { | |||
| 137 | + | for token, pending := range d.parking { | |||
| 138 | + | delete(d.parking, token) | |||
| 139 | + | pending.closeDone() | |||
| 140 | + | } | |||
| 141 | + | } | |||
| 142 | + | ||||
| 143 | + | // purgeParkingForSessionLocked removes and closes every in-flight parking | |||
| 144 | + | // marker for sess so same-token waiters fail closed instead of hanging when | |||
| 145 | + | // the session is removed before park publication. Caller holds d.mu. | |||
| 146 | + | func (d *Daemon) purgeParkingForSessionLocked(sess *session) { | |||
| 147 | + | for token, pending := range d.parking { | |||
| 148 | + | if pending.sess == sess { | |||
| 149 | + | delete(d.parking, token) | |||
| 150 | + | pending.closeDone() | |||
| 30 | } | 151 | } | ||
| 31 | } | 152 | } | ||
| 32 | } | 153 | } | ||
| 33 | 154 | ||||
| 155 | + | // waitParkingInFlight waits for an identified same-client parking publication | |||
| 156 | + | // to finish. Returns true when a matching in-flight entry was observed. | |||
| 157 | + | // Unknown and mismatched credentials return false without waiting. | |||
| 158 | + | func (d *Daemon) waitParkingInFlight(h ports.Hello) bool { | |||
| 159 | + | if h.ResumeToken == 0 { | |||
| 160 | + | return false | |||
| 161 | + | } | |||
| 162 | + | d.mu.Lock() | |||
| 163 | + | pending := d.parking[h.ResumeToken] | |||
| 164 | + | if pending == nil { | |||
| 165 | + | d.mu.Unlock() | |||
| 166 | + | return false | |||
| 167 | + | } | |||
| 168 | + | if pending.ac.clientID != h.ClientID { | |||
| 169 | + | d.mu.Unlock() | |||
| 170 | + | return false | |||
| 171 | + | } | |||
| 172 | + | if h.Name != "" && pending.sess != nil && pending.sess.name != h.Name { | |||
| 173 | + | d.mu.Unlock() | |||
| 174 | + | return false | |||
| 175 | + | } | |||
| 176 | + | done := pending.done | |||
| 177 | + | d.mu.Unlock() | |||
| 178 | + | d.log.Info("resume waiting for in-flight parking", "session", h.Name) | |||
| 179 | + | if d.afterParkingWaitArmed != nil { | |||
| 180 | + | d.afterParkingWaitArmed() | |||
| 181 | + | } | |||
| 182 | + | <-done | |||
| 183 | + | return true | |||
| 184 | + | } | |||
| 185 | + | ||||
| 34 | func (d *Daemon) prepareParkAttachment(sess *session, ac *attachedClient) bool { | 186 | func (d *Daemon) prepareParkAttachment(sess *session, ac *attachedClient) bool { | ||
| 35 | if !ac.resumeCapable { | 187 | if !ac.resumeCapable { | ||
| 36 | return false | 188 | return false | ||
| @@ -43,6 +195,10 @@ func (d *Daemon) prepareParkAttachment(sess *session, ac *attachedClient) bool { | |||||
| 43 | if ac.resumeToken == 0 { | 195 | if ac.resumeToken == 0 { | ||
| 44 | ac.resumeToken = d.nextResumeTokenLocked() | 196 | ac.resumeToken = d.nextResumeTokenLocked() | ||
| 45 | } | 197 | } | ||
| 198 | + | // Do not recreate parking markers here: post-detach publication races | |||
| 199 | + | // terminal session removal and can strand IntentResume waiters. Callers | |||
| 200 | + | // that need the detach→park gap advertised must markParkingInFlight while | |||
| 201 | + | // still the exact live owner. Direct parkAttachment still parks below. | |||
| 46 | d.log.Info("parking client prepared", "session", sess.name) | 202 | d.log.Info("parking client prepared", "session", sess.name) | ||
| 47 | return true | 203 | return true | ||
| 48 | } | 204 | } | ||
| @@ -62,6 +218,7 @@ func (d *Daemon) parkAttachmentAs(sess *session, ac *attachedClient, role attach | |||||
| 62 | ac.clearCaptureFrames() | 218 | ac.clearCaptureFrames() | ||
| 63 | d.mu.Lock() | 219 | d.mu.Lock() | ||
| 64 | if d.closing || d.sessions[sess.id] != sess { | 220 | if d.closing || d.sessions[sess.id] != sess { | ||
| 221 | + | d.clearParkingInFlightLocked(ac.resumeToken, ac) | |||
| 65 | d.mu.Unlock() | 222 | d.mu.Unlock() | ||
| 66 | return false | 223 | return false | ||
| 67 | } | 224 | } | ||
| @@ -90,6 +247,7 @@ func (d *Daemon) parkAttachmentAs(sess *session, ac *attachedClient, role attach | |||||
| 90 | parked := &parkedAttachment{sess: sess, ac: ac, role: role, timer: timer, done: make(chan struct{})} | 247 | parked := &parkedAttachment{sess: sess, ac: ac, role: role, timer: timer, done: make(chan struct{})} | ||
| 91 | ac.parked = true | 248 | ac.parked = true | ||
| 92 | d.parked[token] = parked | 249 | d.parked[token] = parked | ||
| 250 | + | d.clearParkingInFlightLocked(token, ac) | |||
| 93 | d.mu.Unlock() | 251 | d.mu.Unlock() | ||
| 94 | if oldRetirement != nil { | 252 | if oldRetirement != nil { | ||
| 95 | finishParkedAttachmentRetirements([]parkedAttachmentRetirement{*oldRetirement}) | 253 | finishParkedAttachmentRetirements([]parkedAttachmentRetirement{*oldRetirement}) | ||
| @@ -255,13 +413,29 @@ func (d *Daemon) resumeLiveAttachment(h ports.Hello, tr ports.Transport, sz doma | |||||
| 255 | return nil, nil, false, &protoErr{ports.ErrNoSuchSession, "resume token is no longer valid"} | 413 | return nil, nil, false, &protoErr{ports.ErrNoSuchSession, "resume token is no longer valid"} | ||
| 256 | } | 414 | } | ||
| 257 | if !credentialMatch { | 415 | if !credentialMatch { | ||
| 258 | - | return nil, nil, false, nil | 416 | + | // Detach may have won and not published parked yet; wait out that gap |
| 417 | + | // for the exact same-client credential, then try the parked path. | |||
| 418 | + | _ = d.waitParkingInFlight(h) | |||
| 419 | + | return d.resumeParked(h, tr, sz) | |||
| 259 | } | 420 | } | ||
| 260 | 421 | ||||
| 422 | + | if d.beforeMarkParkingInFlight != nil { | |||
| 423 | + | d.beforeMarkParkingInFlight() | |||
| 424 | + | } | |||
| 425 | + | // Advertise parking before detach so a concurrent same-token resume never | |||
| 426 | + | // observes both the live seat and parking/parked registries empty. Late | |||
| 427 | + | // publication after terminal detach/session removal returns 0. | |||
| 428 | + | parkingToken := d.markParkingInFlight(sess, ac) | |||
| 261 | if !d.detachIfCurrentTransport(sess, ac, oldSnap) { | 429 | if !d.detachIfCurrentTransport(sess, ac, oldSnap) { | ||
| 262 | - | // A newer owner, rebound link, or teardown won; try the parked path. | 430 | + | d.clearParkingInFlightIfAbandoned(sess, ac, parkingToken) |
| 431 | + | // A newer owner, rebound link, or teardown won; wait if teardown is | |||
| 432 | + | // still publishing the park, then try the parked path. | |||
| 433 | + | _ = d.waitParkingInFlight(h) | |||
| 263 | return d.resumeParked(h, tr, sz) | 434 | return d.resumeParked(h, tr, sz) | ||
| 264 | } | 435 | } | ||
| 436 | + | if d.afterResumeLiveDetach != nil { | |||
| 437 | + | d.afterResumeLiveDetach() | |||
| 438 | + | } | |||
| 265 | if rc := sess.renderCoordinator(); rc != nil { | 439 | if rc := sess.renderCoordinator(); rc != nil { | ||
| 266 | rc.noteDetach(ac) | 440 | rc.noteDetach(ac) | ||
| 267 | } | 441 | } | ||
| @@ -269,6 +443,7 @@ func (d *Daemon) resumeLiveAttachment(h ports.Hello, tr ports.Transport, sz doma | |||||
| 269 | if !d.parkAttachment(sess, ac) { | 443 | if !d.parkAttachment(sess, ac) { | ||
| 270 | // Detach already published; retire the captured link exactly once so | 444 | // Detach already published; retire the captured link exactly once so | ||
| 271 | // shutdown cannot leave an orphaned open transport with no owner. | 445 | // shutdown cannot leave an orphaned open transport with no owner. | ||
| 446 | + | d.clearParkingInFlight(ac.resumeToken, ac) | |||
| 272 | d.resetScreenDefaultColors(sess) | 447 | d.resetScreenDefaultColors(sess) | ||
| 273 | ac.clearPreviousSession() | 448 | ac.clearPreviousSession() | ||
| 274 | _ = ac.closeCapturedTransport(ac.revokeTransport(oldSnap.transport)) | 449 | _ = ac.closeCapturedTransport(ac.revokeTransport(oldSnap.transport)) | ||
M internal/usecase/daemon/resume_test.go +881 -0
| @@ -5,6 +5,7 @@ import ( | |||
| 5 | 5 | "errors" | |
| 6 | 6 | "strings" | |
| 7 | 7 | "sync" | |
| 8 | + | "sync/atomic" | |
| 8 | 9 | "testing" | |
| 9 | 10 | "time" | |
| 10 | 11 | ||
| @@ -475,6 +476,816 @@ func TestResumeActiveTokenBeforeParkRecoversSameAttachment(t *testing.T) { | |||
| 475 | 476 | require.False(t, newTr.Closed()) | |
| 476 | 477 | } | |
| 477 | 478 | ||
| 479 | + | // TestResumeDuringTeardownBeforeParkRecoversSameAttachment covers the opposite | |
| 480 | + | // interleaving from live-before-park recovery: old-link teardown wins | |
| 481 | + | // detachIfCurrentTransport, then pauses before parkAttachment publishes the | |
| 482 | + | // resume token. The parking-in-flight marker must already be visible in that | |
| 483 | + | // gap (published before detach). Same-client IntentResume must wait it out and | |
| 484 | + | // recover; unknown tokens stay fail-closed. | |
| 485 | + | func TestResumeDuringTeardownBeforeParkRecoversSameAttachment(t *testing.T) { | |
| 486 | + | pty, release := newBlockingPTY(t) | |
| 487 | + | defer release() | |
| 488 | + | d := newTestDaemon(t, newFactory(t, pty), stubClock{}) | |
| 489 | + | ||
| 490 | + | oldTr := &closeTrackingTransport{} | |
| 491 | + | sess, ac, err := d.route(helloResumeCapable(ports.IntentNew, "work", 0), oldTr) | |
| 492 | + | require.NoError(t, err) | |
| 493 | + | token := ac.resumeToken | |
| 494 | + | require.NotZero(t, token) | |
| 495 | + | require.Same(t, ac, sess.client) | |
| 496 | + | ||
| 497 | + | reachedGap := make(chan struct{}) | |
| 498 | + | releaseGap := make(chan struct{}) | |
| 499 | + | var releaseOnce sync.Once | |
| 500 | + | t.Cleanup(func() { releaseOnce.Do(func() { close(releaseGap) }) }) | |
| 501 | + | d.afterClientGoneDetach = func() { | |
| 502 | + | close(reachedGap) | |
| 503 | + | <-releaseGap | |
| 504 | + | } | |
| 505 | + | ||
| 506 | + | goneDone := make(chan struct{}) | |
| 507 | + | go func() { | |
| 508 | + | d.clientGone(sess, ac, oldTr, false) | |
| 509 | + | close(goneDone) | |
| 510 | + | }() | |
| 511 | + | awaitTestCompletion(t, reachedGap, "teardown did not pause after detach before park") | |
| 512 | + | ||
| 513 | + | require.Nil(t, sess.client, "fixture: detach must have cleared the live owner") | |
| 514 | + | d.mu.Lock() | |
| 515 | + | parkingInGap := d.parking[token] | |
| 516 | + | _, parkedInGap := d.parked[token] | |
| 517 | + | d.mu.Unlock() | |
| 518 | + | require.NotNil(t, parkingInGap, "fixture: parking marker must precede detach publication") | |
| 519 | + | require.Same(t, ac, parkingInGap.ac) | |
| 520 | + | require.Same(t, sess, parkingInGap.sess) | |
| 521 | + | require.False(t, parkedInGap, "fixture: park must not have published yet") | |
| 522 | + | ||
| 523 | + | _, _, err = d.route(helloResumeCapable(ports.IntentResume, "work", 0xdecafbad), &closeTrackingTransport{}) | |
| 524 | + | require.Error(t, err) | |
| 525 | + | var unknownErr *protoErr | |
| 526 | + | require.ErrorAs(t, err, &unknownErr) | |
| 527 | + | require.Equal(t, ports.ErrNoSuchSession, unknownErr.code) | |
| 528 | + | require.Contains(t, unknownErr.Error(), "resume token is no longer valid") | |
| 529 | + | ||
| 530 | + | wrongClient := helloResumeCapable(ports.IntentResume, "work", token) | |
| 531 | + | wrongClient.ClientID = [16]byte{9, 9, 9, 9} | |
| 532 | + | _, _, err = d.route(wrongClient, &closeTrackingTransport{}) | |
| 533 | + | require.Error(t, err) | |
| 534 | + | var mismatchErr *protoErr | |
| 535 | + | require.ErrorAs(t, err, &mismatchErr) | |
| 536 | + | require.Equal(t, ports.ErrNoSuchSession, mismatchErr.code) | |
| 537 | + | require.Contains(t, mismatchErr.Error(), "resume token is no longer valid") | |
| 538 | + | ||
| 539 | + | newTr := &closeTrackingTransport{} | |
| 540 | + | type routeResult struct { | |
| 541 | + | sess *session | |
| 542 | + | ac *attachedClient | |
| 543 | + | err error | |
| 544 | + | } | |
| 545 | + | waiterArmed := make(chan struct{}) | |
| 546 | + | d.afterParkingWaitArmed = func() { close(waiterArmed) } | |
| 547 | + | result := make(chan routeResult, 1) | |
| 548 | + | go func() { | |
| 549 | + | routedSess, routedAC, routeErr := d.route(helloResumeCapable(ports.IntentResume, "work", token), newTr) | |
| 550 | + | result <- routeResult{sess: routedSess, ac: routedAC, err: routeErr} | |
| 551 | + | }() | |
| 552 | + | awaitTestCompletion(t, waiterArmed, "resume waiter did not arm on parking marker before park") | |
| 553 | + | ||
| 554 | + | releaseOnce.Do(func() { close(releaseGap) }) | |
| 555 | + | awaitTestCompletion(t, goneDone, "teardown did not finish after park release") | |
| 556 | + | got := awaitTestValue(t, result, "IntentResume did not finish after teardown-before-park gap") | |
| 557 | + | ||
| 558 | + | require.NoError(t, got.err, "same-client token must recover across detach-before-park") | |
| 559 | + | require.Same(t, sess, got.sess) | |
| 560 | + | require.Same(t, ac, got.ac) | |
| 561 | + | require.Same(t, ac, sess.client) | |
| 562 | + | require.Same(t, newTr, ac.transport()) | |
| 563 | + | require.NotEqual(t, token, ac.resumeToken, "successful resume rotates the credential") | |
| 564 | + | require.True(t, oldTr.Closed(), "teardown/resume must retire the old transport") | |
| 565 | + | require.False(t, newTr.Closed(), "rebound transport must survive") | |
| 566 | + | d.mu.Lock() | |
| 567 | + | _, oldTokenParked := d.parked[token] | |
| 568 | + | _, stillParking := d.parking[token] | |
| 569 | + | d.mu.Unlock() | |
| 570 | + | require.False(t, oldTokenParked, "consumed credential must not remain resumeable") | |
| 571 | + | require.False(t, stillParking, "parking marker must be consumed after park/resume") | |
| 572 | + | ||
| 573 | + | d.clientGone(sess, ac, oldTr, false) | |
| 574 | + | require.Same(t, ac, sess.client, "stale old-link cleanup must not detach the rebound attachment") | |
| 575 | + | require.Same(t, newTr, ac.transport()) | |
| 576 | + | require.False(t, newTr.Closed()) | |
| 577 | + | } | |
| 578 | + | ||
| 579 | + | // TestConcurrentLiveResumesWaitParkingMarkerBeforePark covers two same-token | |
| 580 | + | // IntentResume handshakes where the winner of detachIfCurrentTransport pauses | |
| 581 | + | // before parkAttachment. The parking marker must already be published so the | |
| 582 | + | // loser waits instead of treating the credential as unknown across that gap. | |
| 583 | + | func TestConcurrentLiveResumesWaitParkingMarkerBeforePark(t *testing.T) { | |
| 584 | + | pty, release := newBlockingPTY(t) | |
| 585 | + | defer release() | |
| 586 | + | d := newTestDaemon(t, newFactory(t, pty), stubClock{}) | |
| 587 | + | ||
| 588 | + | oldTr := &closeTrackingTransport{} | |
| 589 | + | sess, ac, err := d.route(helloResumeCapable(ports.IntentNew, "work", 0), oldTr) | |
| 590 | + | require.NoError(t, err) | |
| 591 | + | token := ac.resumeToken | |
| 592 | + | require.NotZero(t, token) | |
| 593 | + | require.Same(t, ac, sess.client) | |
| 594 | + | ||
| 595 | + | reachedGap := make(chan struct{}) | |
| 596 | + | releaseGap := make(chan struct{}) | |
| 597 | + | var releaseOnce sync.Once | |
| 598 | + | t.Cleanup(func() { releaseOnce.Do(func() { close(releaseGap) }) }) | |
| 599 | + | d.afterResumeLiveDetach = func() { | |
| 600 | + | close(reachedGap) | |
| 601 | + | <-releaseGap | |
| 602 | + | } | |
| 603 | + | ||
| 604 | + | type routeResult struct { | |
| 605 | + | sess *session | |
| 606 | + | ac *attachedClient | |
| 607 | + | err error | |
| 608 | + | } | |
| 609 | + | firstTr := &closeTrackingTransport{} | |
| 610 | + | firstResult := make(chan routeResult, 1) | |
| 611 | + | go func() { | |
| 612 | + | routedSess, routedAC, routeErr := d.route(helloResumeCapable(ports.IntentResume, "work", token), firstTr) | |
| 613 | + | firstResult <- routeResult{sess: routedSess, ac: routedAC, err: routeErr} | |
| 614 | + | }() | |
| 615 | + | awaitTestCompletion(t, reachedGap, "first live resume did not pause after detach before park") | |
| 616 | + | ||
| 617 | + | require.Nil(t, sess.client, "fixture: winning resume must have cleared the live owner") | |
| 618 | + | d.mu.Lock() | |
| 619 | + | parkingInGap := d.parking[token] | |
| 620 | + | _, parkedInGap := d.parked[token] | |
| 621 | + | d.mu.Unlock() | |
| 622 | + | require.NotNil(t, parkingInGap, "fixture: parking marker must precede detach for concurrent resumes") | |
| 623 | + | require.Same(t, ac, parkingInGap.ac) | |
| 624 | + | require.False(t, parkedInGap, "fixture: park must not have published yet") | |
| 625 | + | ||
| 626 | + | _, _, err = d.route(helloResumeCapable(ports.IntentResume, "work", 0xdecafbad), &closeTrackingTransport{}) | |
| 627 | + | require.Error(t, err) | |
| 628 | + | var unknownErr *protoErr | |
| 629 | + | require.ErrorAs(t, err, &unknownErr) | |
| 630 | + | require.Equal(t, ports.ErrNoSuchSession, unknownErr.code) | |
| 631 | + | ||
| 632 | + | secondTr := &closeTrackingTransport{} | |
| 633 | + | secondWaiting := make(chan struct{}) | |
| 634 | + | d.afterParkingWaitArmed = func() { | |
| 635 | + | close(secondWaiting) | |
| 636 | + | } | |
| 637 | + | secondResult := make(chan routeResult, 1) | |
| 638 | + | go func() { | |
| 639 | + | routedSess, routedAC, routeErr := d.route(helloResumeCapable(ports.IntentResume, "work", token), secondTr) | |
| 640 | + | secondResult <- routeResult{sess: routedSess, ac: routedAC, err: routeErr} | |
| 641 | + | }() | |
| 642 | + | awaitTestCompletion(t, secondWaiting, "second live resume did not arm parking wait") | |
| 643 | + | ||
| 644 | + | releaseOnce.Do(func() { close(releaseGap) }) | |
| 645 | + | first := awaitTestValue(t, firstResult, "first concurrent IntentResume did not finish") | |
| 646 | + | second := awaitTestValue(t, secondResult, "second concurrent IntentResume did not finish") | |
| 647 | + | ||
| 648 | + | var winner, loser routeResult | |
| 649 | + | var winnerTr, loserTr *closeTrackingTransport | |
| 650 | + | switch { | |
| 651 | + | case first.err == nil && second.err != nil: | |
| 652 | + | winner, loser = first, second | |
| 653 | + | winnerTr, loserTr = firstTr, secondTr | |
| 654 | + | case second.err == nil && first.err != nil: | |
| 655 | + | winner, loser = second, first | |
| 656 | + | winnerTr, loserTr = secondTr, firstTr | |
| 657 | + | default: | |
| 658 | + | t.Fatalf("expected exactly one concurrent resume to succeed; first=%v second=%v", first.err, second.err) | |
| 659 | + | } | |
| 660 | + | ||
| 661 | + | require.Same(t, sess, winner.sess) | |
| 662 | + | require.Same(t, ac, winner.ac) | |
| 663 | + | require.Same(t, ac, sess.client) | |
| 664 | + | require.Same(t, winnerTr, ac.transport()) | |
| 665 | + | require.NotEqual(t, token, ac.resumeToken, "successful resume rotates the credential") | |
| 666 | + | require.True(t, oldTr.Closed(), "winning resume must retire the old transport") | |
| 667 | + | require.False(t, winnerTr.Closed(), "winning rebound transport must survive") | |
| 668 | + | ||
| 669 | + | var loserErr *protoErr | |
| 670 | + | require.ErrorAs(t, loser.err, &loserErr) | |
| 671 | + | require.Equal(t, ports.ErrNoSuchSession, loserErr.code) | |
| 672 | + | require.Contains(t, loserErr.Error(), "resume token is no longer valid") | |
| 673 | + | require.True(t, loserTr.Closed() || len(loserTr.Sends()) == 0, "losing resume must not complete Welcome") | |
| 674 | + | ||
| 675 | + | d.mu.Lock() | |
| 676 | + | _, oldTokenParked := d.parked[token] | |
| 677 | + | _, stillParking := d.parking[token] | |
| 678 | + | d.mu.Unlock() | |
| 679 | + | require.False(t, oldTokenParked, "consumed credential must not remain resumeable") | |
| 680 | + | require.False(t, stillParking, "parking marker must be consumed after park/resume") | |
| 681 | + | ||
| 682 | + | d.clientGone(sess, ac, oldTr, false) | |
| 683 | + | require.Same(t, ac, sess.client, "stale old-link cleanup must not detach the rebound attachment") | |
| 684 | + | require.Same(t, winnerTr, ac.transport()) | |
| 685 | + | require.False(t, winnerTr.Closed()) | |
| 686 | + | } | |
| 687 | + | ||
| 688 | + | func setupResumableSnatchedAttachment(t *testing.T, d *Daemon) (*session, *attachedClient, *attachedClient, *closeTrackingTransport, uint64) { | |
| 689 | + | t.Helper() | |
| 690 | + | oldTr := &closeTrackingTransport{} | |
| 691 | + | sess, waiting, err := d.route(helloResumeCapable(ports.IntentNew, "work", 0), oldTr) | |
| 692 | + | require.NoError(t, err) | |
| 693 | + | token := waiting.resumeToken | |
| 694 | + | require.NotZero(t, token) | |
| 695 | + | ||
| 696 | + | _, active, err := d.route(helloResumeCapable(ports.IntentAttach, "work", 0), &closeTrackingTransport{}) | |
| 697 | + | require.NoError(t, err) | |
| 698 | + | d.attachmentCleanupWg.Wait() | |
| 699 | + | require.Equal(t, attachmentSnatched, sess.attachmentRole(waiting)) | |
| 700 | + | require.Same(t, active, sess.client) | |
| 701 | + | return sess, waiting, active, oldTr, token | |
| 702 | + | } | |
| 703 | + | ||
| 704 | + | // TestSnatchedResumeDuringTeardownBeforeParkRecoversSameAttachment covers the | |
| 705 | + | // snatched unroute→park gap: parkSnatchedAttachment publishes the parking | |
| 706 | + | // marker before ownership removal, then pauses before parkAttachmentAs. | |
| 707 | + | // Same-client IntentResume must wait it out and recover as snatched without | |
| 708 | + | // replacing the active owner; unknown/mismatched tokens stay fail-closed. | |
| 709 | + | func TestSnatchedResumeDuringTeardownBeforeParkRecoversSameAttachment(t *testing.T) { | |
| 710 | + | pty, release := newBlockingPTY(t) | |
| 711 | + | defer release() | |
| 712 | + | d := newTestDaemon(t, newFactory(t, pty), stubClock{}) | |
| 713 | + | ||
| 714 | + | sess, waiting, active, oldTr, token := setupResumableSnatchedAttachment(t, d) | |
| 715 | + | roleToken := sess.attachmentToken(waiting, oldTr) | |
| 716 | + | ||
| 717 | + | reachedGap := make(chan struct{}) | |
| 718 | + | releaseGap := make(chan struct{}) | |
| 719 | + | var releaseOnce sync.Once | |
| 720 | + | t.Cleanup(func() { releaseOnce.Do(func() { close(releaseGap) }) }) | |
| 721 | + | d.afterSnatchedUnrouteBeforePark = func() { | |
| 722 | + | close(reachedGap) | |
| 723 | + | <-releaseGap | |
| 724 | + | } | |
| 725 | + | ||
| 726 | + | parkDone := make(chan bool, 1) | |
| 727 | + | go func() { | |
| 728 | + | parkDone <- d.parkSnatchedAttachment(roleToken) | |
| 729 | + | }() | |
| 730 | + | awaitTestCompletion(t, reachedGap, "snatched park did not pause after unroute before park") | |
| 731 | + | ||
| 732 | + | require.Equal(t, attachmentDetached, sess.attachmentRole(waiting), "fixture: unroute must have cleared snatched ownership") | |
| 733 | + | d.mu.Lock() | |
| 734 | + | parkingInGap := d.parking[token] | |
| 735 | + | _, parkedInGap := d.parked[token] | |
| 736 | + | d.mu.Unlock() | |
| 737 | + | require.NotNil(t, parkingInGap, "fixture: parking marker must precede snatched ownership removal") | |
| 738 | + | require.Same(t, waiting, parkingInGap.ac) | |
| 739 | + | require.Same(t, sess, parkingInGap.sess) | |
| 740 | + | require.False(t, parkedInGap, "fixture: park must not have published yet") | |
| 741 | + | require.Same(t, active, sess.client, "fixture: active owner must remain undisturbed") | |
| 742 | + | ||
| 743 | + | _, _, err := d.route(helloResumeCapable(ports.IntentResume, "work", 0xdecafbad), &closeTrackingTransport{}) | |
| 744 | + | require.Error(t, err) | |
| 745 | + | var unknownErr *protoErr | |
| 746 | + | require.ErrorAs(t, err, &unknownErr) | |
| 747 | + | require.Equal(t, ports.ErrNoSuchSession, unknownErr.code) | |
| 748 | + | require.Contains(t, unknownErr.Error(), "resume token is no longer valid") | |
| 749 | + | ||
| 750 | + | wrongClient := helloResumeCapable(ports.IntentResume, "work", token) | |
| 751 | + | wrongClient.ClientID = [16]byte{9, 9, 9, 9} | |
| 752 | + | _, _, err = d.route(wrongClient, &closeTrackingTransport{}) | |
| 753 | + | require.Error(t, err) | |
| 754 | + | var mismatchErr *protoErr | |
| 755 | + | require.ErrorAs(t, err, &mismatchErr) | |
| 756 | + | require.Equal(t, ports.ErrNoSuchSession, mismatchErr.code) | |
| 757 | + | require.Contains(t, mismatchErr.Error(), "resume token is no longer valid") | |
| 758 | + | ||
| 759 | + | newTr := &closeTrackingTransport{} | |
| 760 | + | type routeResult struct { | |
| 761 | + | sess *session | |
| 762 | + | ac *attachedClient | |
| 763 | + | err error | |
| 764 | + | } | |
| 765 | + | waiterArmed := make(chan struct{}) | |
| 766 | + | d.afterParkingWaitArmed = func() { close(waiterArmed) } | |
| 767 | + | result := make(chan routeResult, 1) | |
| 768 | + | go func() { | |
| 769 | + | routedSess, routedAC, routeErr := d.route(helloResumeCapable(ports.IntentResume, "work", token), newTr) | |
| 770 | + | result <- routeResult{sess: routedSess, ac: routedAC, err: routeErr} | |
| 771 | + | }() | |
| 772 | + | awaitTestCompletion(t, waiterArmed, "resume waiter did not arm on snatched parking marker before park") | |
| 773 | + | ||
| 774 | + | releaseOnce.Do(func() { close(releaseGap) }) | |
| 775 | + | require.True(t, awaitTestValue(t, parkDone, "snatched park did not finish after release"), "parkSnatchedAttachment must succeed") | |
| 776 | + | got := awaitTestValue(t, result, "IntentResume did not finish after snatched teardown-before-park gap") | |
| 777 | + | ||
| 778 | + | require.NoError(t, got.err, "same-client snatched token must recover across unroute-before-park") | |
| 779 | + | require.Same(t, sess, got.sess) | |
| 780 | + | require.Same(t, waiting, got.ac) | |
| 781 | + | require.Same(t, active, sess.client, "snatched resume must not replace the active owner") | |
| 782 | + | require.Equal(t, attachmentSnatched, sess.attachmentRole(waiting)) | |
| 783 | + | require.Same(t, newTr, waiting.transport()) | |
| 784 | + | require.NotEqual(t, token, waiting.resumeToken, "successful resume rotates the credential") | |
| 785 | + | require.True(t, oldTr.Closed(), "teardown/resume must retire the old transport") | |
| 786 | + | require.False(t, newTr.Closed(), "rebound transport must survive") | |
| 787 | + | d.mu.Lock() | |
| 788 | + | _, oldTokenParked := d.parked[token] | |
| 789 | + | _, stillParking := d.parking[token] | |
| 790 | + | d.mu.Unlock() | |
| 791 | + | require.False(t, oldTokenParked, "consumed credential must not remain resumeable") | |
| 792 | + | require.False(t, stillParking, "parking marker must be consumed after park/resume") | |
| 793 | + | } | |
| 794 | + | ||
| 795 | + | // TestConcurrentSnatchedResumesWaitParkingMarkerBeforePark covers two same-token | |
| 796 | + | // IntentResume handshakes while parkSnatchedAttachment pauses after unroute and | |
| 797 | + | // before park. The parking marker must already be published so losers wait | |
| 798 | + | // instead of treating the credential as unknown across that gap. | |
| 799 | + | func TestConcurrentSnatchedResumesWaitParkingMarkerBeforePark(t *testing.T) { | |
| 800 | + | pty, release := newBlockingPTY(t) | |
| 801 | + | defer release() | |
| 802 | + | d := newTestDaemon(t, newFactory(t, pty), stubClock{}) | |
| 803 | + | ||
| 804 | + | sess, waiting, active, oldTr, token := setupResumableSnatchedAttachment(t, d) | |
| 805 | + | roleToken := sess.attachmentToken(waiting, oldTr) | |
| 806 | + | ||
| 807 | + | reachedGap := make(chan struct{}) | |
| 808 | + | releaseGap := make(chan struct{}) | |
| 809 | + | var releaseOnce sync.Once | |
| 810 | + | t.Cleanup(func() { releaseOnce.Do(func() { close(releaseGap) }) }) | |
| 811 | + | d.afterSnatchedUnrouteBeforePark = func() { | |
| 812 | + | close(reachedGap) | |
| 813 | + | <-releaseGap | |
| 814 | + | } | |
| 815 | + | ||
| 816 | + | parkDone := make(chan bool, 1) | |
| 817 | + | go func() { | |
| 818 | + | parkDone <- d.parkSnatchedAttachment(roleToken) | |
| 819 | + | }() | |
| 820 | + | awaitTestCompletion(t, reachedGap, "snatched park did not pause after unroute before park") | |
| 821 | + | ||
| 822 | + | require.Equal(t, attachmentDetached, sess.attachmentRole(waiting), "fixture: unroute must have cleared snatched ownership") | |
| 823 | + | d.mu.Lock() | |
| 824 | + | parkingInGap := d.parking[token] | |
| 825 | + | _, parkedInGap := d.parked[token] | |
| 826 | + | d.mu.Unlock() | |
| 827 | + | require.NotNil(t, parkingInGap, "fixture: parking marker must precede unroute for concurrent resumes") | |
| 828 | + | require.Same(t, waiting, parkingInGap.ac) | |
| 829 | + | require.False(t, parkedInGap, "fixture: park must not have published yet") | |
| 830 | + | ||
| 831 | + | _, _, err := d.route(helloResumeCapable(ports.IntentResume, "work", 0xdecafbad), &closeTrackingTransport{}) | |
| 832 | + | require.Error(t, err) | |
| 833 | + | var unknownErr *protoErr | |
| 834 | + | require.ErrorAs(t, err, &unknownErr) | |
| 835 | + | require.Equal(t, ports.ErrNoSuchSession, unknownErr.code) | |
| 836 | + | ||
| 837 | + | type routeResult struct { | |
| 838 | + | sess *session | |
| 839 | + | ac *attachedClient | |
| 840 | + | err error | |
| 841 | + | } | |
| 842 | + | firstTr := &closeTrackingTransport{} | |
| 843 | + | secondTr := &closeTrackingTransport{} | |
| 844 | + | bothWaitersArmed := make(chan struct{}) | |
| 845 | + | var armedCount atomic.Int32 | |
| 846 | + | d.afterParkingWaitArmed = func() { | |
| 847 | + | if armedCount.Add(1) == 2 { | |
| 848 | + | close(bothWaitersArmed) | |
| 849 | + | } | |
| 850 | + | } | |
| 851 | + | firstResult := make(chan routeResult, 1) | |
| 852 | + | secondResult := make(chan routeResult, 1) | |
| 853 | + | go func() { | |
| 854 | + | routedSess, routedAC, routeErr := d.route(helloResumeCapable(ports.IntentResume, "work", token), firstTr) | |
| 855 | + | firstResult <- routeResult{sess: routedSess, ac: routedAC, err: routeErr} | |
| 856 | + | }() | |
| 857 | + | go func() { | |
| 858 | + | routedSess, routedAC, routeErr := d.route(helloResumeCapable(ports.IntentResume, "work", token), secondTr) | |
| 859 | + | secondResult <- routeResult{sess: routedSess, ac: routedAC, err: routeErr} | |
| 860 | + | }() | |
| 861 | + | awaitTestCompletion(t, bothWaitersArmed, "both snatched resume waiters did not arm on parking marker before park") | |
| 862 | + | ||
| 863 | + | releaseOnce.Do(func() { close(releaseGap) }) | |
| 864 | + | require.True(t, awaitTestValue(t, parkDone, "snatched park did not finish after release")) | |
| 865 | + | first := awaitTestValue(t, firstResult, "first concurrent snatched IntentResume did not finish") | |
| 866 | + | second := awaitTestValue(t, secondResult, "second concurrent snatched IntentResume did not finish") | |
| 867 | + | ||
| 868 | + | var winner, loser routeResult | |
| 869 | + | var winnerTr, loserTr *closeTrackingTransport | |
| 870 | + | switch { | |
| 871 | + | case first.err == nil && second.err != nil: | |
| 872 | + | winner, loser = first, second | |
| 873 | + | winnerTr, loserTr = firstTr, secondTr | |
| 874 | + | case second.err == nil && first.err != nil: | |
| 875 | + | winner, loser = second, first | |
| 876 | + | winnerTr, loserTr = secondTr, firstTr | |
| 877 | + | default: | |
| 878 | + | t.Fatalf("expected exactly one concurrent snatched resume to succeed; first=%v second=%v", first.err, second.err) | |
| 879 | + | } | |
| 880 | + | ||
| 881 | + | require.Same(t, sess, winner.sess) | |
| 882 | + | require.Same(t, waiting, winner.ac) | |
| 883 | + | require.Same(t, active, sess.client, "snatched resume must not replace the active owner") | |
| 884 | + | require.Equal(t, attachmentSnatched, sess.attachmentRole(waiting)) | |
| 885 | + | require.Same(t, winnerTr, waiting.transport()) | |
| 886 | + | require.NotEqual(t, token, waiting.resumeToken, "successful resume rotates the credential") | |
| 887 | + | require.True(t, oldTr.Closed(), "winning resume must retire the old transport") | |
| 888 | + | require.False(t, winnerTr.Closed(), "winning rebound transport must survive") | |
| 889 | + | ||
| 890 | + | var loserErr *protoErr | |
| 891 | + | require.ErrorAs(t, loser.err, &loserErr) | |
| 892 | + | require.Equal(t, ports.ErrNoSuchSession, loserErr.code) | |
| 893 | + | require.Contains(t, loserErr.Error(), "resume token is no longer valid") | |
| 894 | + | require.True(t, loserTr.Closed() || len(loserTr.Sends()) == 0, "losing resume must not complete Welcome") | |
| 895 | + | ||
| 896 | + | d.mu.Lock() | |
| 897 | + | _, oldTokenParked := d.parked[token] | |
| 898 | + | _, stillParking := d.parking[token] | |
| 899 | + | d.mu.Unlock() | |
| 900 | + | require.False(t, oldTokenParked, "consumed credential must not remain resumeable") | |
| 901 | + | require.False(t, stillParking, "parking marker must be consumed after park/resume") | |
| 902 | + | } | |
| 903 | + | ||
| 904 | + | // TestStaleSnatchedParkClearsOnlyAbandonedParkingMarker covers a stale | |
| 905 | + | // incarnation that publishes a parking marker then loses exact unroute. Only | |
| 906 | + | // that attachment's abandoned marker is cleared; another attachment's marker | |
| 907 | + | // and the fresh snatched incarnation remain intact. | |
| 908 | + | func TestStaleSnatchedParkClearsOnlyAbandonedParkingMarker(t *testing.T) { | |
| 909 | + | workPTY, releaseWork := newBlockingPTY(t) | |
| 910 | + | defer releaseWork() | |
| 911 | + | otherPTY, releaseOther := newBlockingPTY(t) | |
| 912 | + | defer releaseOther() | |
| 913 | + | // Separate PTYs so work teardown cannot cascade-close the other session. | |
| 914 | + | d := newTestDaemon(t, newFactorySeq(t, workPTY, otherPTY), stubClock{}) | |
| 915 | + | ||
| 916 | + | sess, waiting, active, oldTr, token := setupResumableSnatchedAttachment(t, d) | |
| 917 | + | stale := sess.attachmentToken(waiting, oldTr) | |
| 918 | + | ||
| 919 | + | otherTr := &closeTrackingTransport{} | |
| 920 | + | otherSess, otherAC, err := d.route(helloResumeCapable(ports.IntentNew, "other", 0), otherTr) | |
| 921 | + | require.NoError(t, err) | |
| 922 | + | otherToken := d.markParkingInFlight(otherSess, otherAC) | |
| 923 | + | require.NotZero(t, otherToken) | |
| 924 | + | require.NotEqual(t, token, otherToken) | |
| 925 | + | ||
| 926 | + | fresh := &closeTrackingTransport{} | |
| 927 | + | waiting.replaceTransport(fresh) | |
| 928 | + | ||
| 929 | + | require.False(t, d.parkSnatchedAttachment(stale)) | |
| 930 | + | d.mu.Lock() | |
| 931 | + | _, parked := d.parked[token] | |
| 932 | + | _, stillParking := d.parking[token] | |
| 933 | + | otherMarker := d.parking[otherToken] | |
| 934 | + | d.mu.Unlock() | |
| 935 | + | require.False(t, parked) | |
| 936 | + | require.False(t, stillParking, "stale unroute must clear only the abandoned same-attachment marker") | |
| 937 | + | require.NotNil(t, otherMarker, "stale unroute must not clear another attachment's marker") | |
| 938 | + | require.Same(t, otherAC, otherMarker.ac) | |
| 939 | + | require.Equal(t, attachmentSnatched, sess.attachmentRole(waiting)) | |
| 940 | + | require.Same(t, fresh, waiting.transport()) | |
| 941 | + | require.False(t, fresh.Closed()) | |
| 942 | + | require.Same(t, active, sess.client) | |
| 943 | + | require.Equal(t, token, waiting.resumeToken) | |
| 944 | + | } | |
| 945 | + | ||
| 946 | + | // TestTerminalSnatchedRemovalClearsOrphanedParkingMarker covers the race where a | |
| 947 | + | // park path publishes a same-attachment parking-in-flight marker, then terminal | |
| 948 | + | // snatched removal wins before park completes. Only that attachment's marker is | |
| 949 | + | // retired and waiters unblock; other markers stay intact and the stale token | |
| 950 | + | // fails closed. A subsequent legitimate snatched park remains resumable. | |
| 951 | + | func TestTerminalSnatchedRemovalClearsOrphanedParkingMarker(t *testing.T) { | |
| 952 | + | workPTY, releaseWork := newBlockingPTY(t) | |
| 953 | + | defer releaseWork() | |
| 954 | + | otherPTY, releaseOther := newBlockingPTY(t) | |
| 955 | + | defer releaseOther() | |
| 956 | + | legitPTY, releaseLegit := newBlockingPTY(t) | |
| 957 | + | defer releaseLegit() | |
| 958 | + | // Separate PTYs so session teardown cannot cascade across marker fixtures. | |
| 959 | + | d := newTestDaemon(t, newFactorySeq(t, workPTY, otherPTY, legitPTY), stubClock{}) | |
| 960 | + | ||
| 961 | + | sess, waiting, active, oldTr, token := setupResumableSnatchedAttachment(t, d) | |
| 962 | + | roleToken := sess.attachmentToken(waiting, oldTr) | |
| 963 | + | ||
| 964 | + | otherTr := &closeTrackingTransport{} | |
| 965 | + | otherSess, otherAC, err := d.route(helloResumeCapable(ports.IntentNew, "other", 0), otherTr) | |
| 966 | + | require.NoError(t, err) | |
| 967 | + | otherToken := d.markParkingInFlight(otherSess, otherAC) | |
| 968 | + | require.NotZero(t, otherToken) | |
| 969 | + | require.NotEqual(t, token, otherToken) | |
| 970 | + | ||
| 971 | + | // Simulate the losing park path publishing before terminal unroute wins. | |
| 972 | + | require.Equal(t, token, d.markParkingInFlight(sess, waiting)) | |
| 973 | + | d.mu.Lock() | |
| 974 | + | orphaned := d.parking[token] | |
| 975 | + | otherMarker := d.parking[otherToken] | |
| 976 | + | d.mu.Unlock() | |
| 977 | + | require.NotNil(t, orphaned, "fixture: park path must publish the parking marker") | |
| 978 | + | require.Same(t, waiting, orphaned.ac) | |
| 979 | + | require.NotNil(t, otherMarker, "fixture: other attachment marker must remain published") | |
| 980 | + | require.Same(t, otherAC, otherMarker.ac) | |
| 981 | + | ||
| 982 | + | waiterArmed := make(chan struct{}) | |
| 983 | + | d.afterParkingWaitArmed = func() { close(waiterArmed) } | |
| 984 | + | waiterDone := make(chan bool, 1) | |
| 985 | + | go func() { | |
| 986 | + | waiterDone <- d.waitParkingInFlight(helloResumeCapable(ports.IntentResume, "work", token)) | |
| 987 | + | }() | |
| 988 | + | awaitTestCompletion(t, waiterArmed, "parking waiter did not arm on the orphaned snatched marker") | |
| 989 | + | ||
| 990 | + | require.True(t, d.removeSnatchedAttachment(roleToken), "terminal snatched removal must win exact unroute") | |
| 991 | + | require.Equal(t, attachmentDetached, sess.attachmentRole(waiting)) | |
| 992 | + | require.Same(t, active, sess.client, "terminal snatched removal must not disturb the active owner") | |
| 993 | + | ||
| 994 | + | waited := awaitTestValue(t, waiterDone, "parking waiter stayed blocked after terminal snatched removal") | |
| 995 | + | require.True(t, waited, "waiter must observe the same-attachment marker before it is cleared") | |
| 996 | + | ||
| 997 | + | d.mu.Lock() | |
| 998 | + | _, stillParking := d.parking[token] | |
| 999 | + | otherAfter := d.parking[otherToken] | |
| 1000 | + | _, parked := d.parked[token] | |
| 1001 | + | d.mu.Unlock() | |
| 1002 | + | require.False(t, stillParking, "terminal winner must clear the same attachment's orphaned marker") | |
| 1003 | + | require.False(t, parked, "terminal removal must not publish parked ownership") | |
| 1004 | + | require.NotNil(t, otherAfter, "terminal winner must not clear another attachment's marker") | |
| 1005 | + | require.Same(t, otherAC, otherAfter.ac) | |
| 1006 | + | ||
| 1007 | + | _, _, err = d.route(helloResumeCapable(ports.IntentResume, "work", 0xdecafbad), &closeTrackingTransport{}) | |
| 1008 | + | require.Error(t, err) | |
| 1009 | + | var unknownErr *protoErr | |
| 1010 | + | require.ErrorAs(t, err, &unknownErr) | |
| 1011 | + | require.Equal(t, ports.ErrNoSuchSession, unknownErr.code) | |
| 1012 | + | require.Contains(t, unknownErr.Error(), "resume token is no longer valid") | |
| 1013 | + | ||
| 1014 | + | _, _, err = d.route(helloResumeCapable(ports.IntentResume, "work", token), &closeTrackingTransport{}) | |
| 1015 | + | require.Error(t, err) | |
| 1016 | + | var retiredErr *protoErr | |
| 1017 | + | require.ErrorAs(t, err, &retiredErr) | |
| 1018 | + | require.Equal(t, ports.ErrNoSuchSession, retiredErr.code) | |
| 1019 | + | require.Contains(t, retiredErr.Error(), "resume token is no longer valid") | |
| 1020 | + | ||
| 1021 | + | // A later legitimate snatched park on a fresh attachment must still resume. | |
| 1022 | + | legitTr := &closeTrackingTransport{} | |
| 1023 | + | legitSess, legitWaiting, err := d.route(helloResumeCapable(ports.IntentNew, "legit", 0), legitTr) | |
| 1024 | + | require.NoError(t, err) | |
| 1025 | + | legitToken := legitWaiting.resumeToken | |
| 1026 | + | require.NotZero(t, legitToken) | |
| 1027 | + | _, _, err = d.route(helloResumeCapable(ports.IntentAttach, "legit", 0), &closeTrackingTransport{}) | |
| 1028 | + | require.NoError(t, err) | |
| 1029 | + | d.attachmentCleanupWg.Wait() | |
| 1030 | + | require.Equal(t, attachmentSnatched, legitSess.attachmentRole(legitWaiting)) | |
| 1031 | + | require.True(t, d.parkSnatchedAttachment(legitSess.attachmentToken(legitWaiting, legitTr))) | |
| 1032 | + | ||
| 1033 | + | resumeTr := &closeTrackingTransport{} | |
| 1034 | + | routedSess, routedAC, err := d.route(helloResumeCapable(ports.IntentResume, "legit", legitToken), resumeTr) | |
| 1035 | + | require.NoError(t, err) | |
| 1036 | + | require.Same(t, legitSess, routedSess) | |
| 1037 | + | require.Same(t, legitWaiting, routedAC) | |
| 1038 | + | require.Equal(t, attachmentSnatched, legitSess.attachmentRole(legitWaiting)) | |
| 1039 | + | require.Same(t, resumeTr, legitWaiting.transport()) | |
| 1040 | + | } | |
| 1041 | + | ||
| 1042 | + | // TestExplicitDetachClearsOrphanedSameAttachmentParkingMarker covers the race | |
| 1043 | + | // where non-explicit teardown publishes a parking-in-flight marker, then | |
| 1044 | + | // explicit detach wins the seat. The winner must clear only that attachment's | |
| 1045 | + | // orphaned marker and unblock waiters; other attachments' markers and a later | |
| 1046 | + | // parked lifecycle must stay intact. Unknown tokens stay fail-closed. | |
| 1047 | + | func TestExplicitDetachClearsOrphanedSameAttachmentParkingMarker(t *testing.T) { | |
| 1048 | + | workPTY, releaseWork := newBlockingPTY(t) | |
| 1049 | + | defer releaseWork() | |
| 1050 | + | otherPTY, releaseOther := newBlockingPTY(t) | |
| 1051 | + | defer releaseOther() | |
| 1052 | + | // Separate PTYs keep the other session alive while work detaches. | |
| 1053 | + | d := newTestDaemon(t, newFactorySeq(t, workPTY, otherPTY), stubClock{}) | |
| 1054 | + | ||
| 1055 | + | oldTr := &closeTrackingTransport{} | |
| 1056 | + | sess, ac, err := d.route(helloResumeCapable(ports.IntentNew, "work", 0), oldTr) | |
| 1057 | + | require.NoError(t, err) | |
| 1058 | + | token := ac.resumeToken | |
| 1059 | + | require.NotZero(t, token) | |
| 1060 | + | ||
| 1061 | + | otherTr := &closeTrackingTransport{} | |
| 1062 | + | otherSess, otherAC, err := d.route(helloResumeCapable(ports.IntentNew, "other", 0), otherTr) | |
| 1063 | + | require.NoError(t, err) | |
| 1064 | + | otherToken := d.markParkingInFlight(otherSess, otherAC) | |
| 1065 | + | require.NotZero(t, otherToken) | |
| 1066 | + | require.NotEqual(t, token, otherToken) | |
| 1067 | + | ||
| 1068 | + | // Pre-mark as a same-attachment non-explicit teardown would before detach. | |
| 1069 | + | require.Equal(t, token, d.markParkingInFlight(sess, ac)) | |
| 1070 | + | d.mu.Lock() | |
| 1071 | + | orphaned := d.parking[token] | |
| 1072 | + | otherMarker := d.parking[otherToken] | |
| 1073 | + | d.mu.Unlock() | |
| 1074 | + | require.NotNil(t, orphaned, "fixture: non-explicit teardown must publish the parking marker") | |
| 1075 | + | require.Same(t, ac, orphaned.ac) | |
| 1076 | + | require.NotNil(t, otherMarker, "fixture: other attachment marker must remain published") | |
| 1077 | + | require.Same(t, otherAC, otherMarker.ac) | |
| 1078 | + | ||
| 1079 | + | waiterArmed := make(chan struct{}) | |
| 1080 | + | d.afterParkingWaitArmed = func() { close(waiterArmed) } | |
| 1081 | + | waiterDone := make(chan bool, 1) | |
| 1082 | + | go func() { | |
| 1083 | + | waiterDone <- d.waitParkingInFlight(helloResumeCapable(ports.IntentResume, "work", token)) | |
| 1084 | + | }() | |
| 1085 | + | awaitTestCompletion(t, waiterArmed, "parking waiter did not arm on the orphaned marker") | |
| 1086 | + | ||
| 1087 | + | d.clientGone(sess, ac, oldTr, true) | |
| 1088 | + | require.Nil(t, sess.client, "explicit detach must clear the live owner") | |
| 1089 | + | ||
| 1090 | + | waited := awaitTestValue(t, waiterDone, "parking waiter stayed blocked after explicit detach") | |
| 1091 | + | require.True(t, waited, "waiter must observe the same-attachment marker before it is cleared") | |
| 1092 | + | ||
| 1093 | + | d.mu.Lock() | |
| 1094 | + | _, stillParking := d.parking[token] | |
| 1095 | + | otherAfter := d.parking[otherToken] | |
| 1096 | + | d.mu.Unlock() | |
| 1097 | + | require.False(t, stillParking, "explicit winner must clear the same attachment's orphaned marker") | |
| 1098 | + | require.NotNil(t, otherAfter, "explicit winner must not clear another attachment's marker") | |
| 1099 | + | require.Same(t, otherAC, otherAfter.ac) | |
| 1100 | + | ||
| 1101 | + | _, _, err = d.route(helloResumeCapable(ports.IntentResume, "work", 0xdecafbad), &closeTrackingTransport{}) | |
| 1102 | + | require.Error(t, err) | |
| 1103 | + | var unknownErr *protoErr | |
| 1104 | + | require.ErrorAs(t, err, &unknownErr) | |
| 1105 | + | require.Equal(t, ports.ErrNoSuchSession, unknownErr.code) | |
| 1106 | + | require.Contains(t, unknownErr.Error(), "resume token is no longer valid") | |
| 1107 | + | ||
| 1108 | + | _, _, err = d.route(helloResumeCapable(ports.IntentResume, "work", token), &closeTrackingTransport{}) | |
| 1109 | + | require.Error(t, err) | |
| 1110 | + | var retiredErr *protoErr | |
| 1111 | + | require.ErrorAs(t, err, &retiredErr) | |
| 1112 | + | require.Equal(t, ports.ErrNoSuchSession, retiredErr.code) | |
| 1113 | + | require.Contains(t, retiredErr.Error(), "resume token is no longer valid") | |
| 1114 | + | ||
| 1115 | + | // A later parked lifecycle for another attachment must survive a clear | |
| 1116 | + | // scoped to the retired attachment identity. | |
| 1117 | + | require.True(t, d.parkAttachment(otherSess, otherAC)) | |
| 1118 | + | d.mu.Lock() | |
| 1119 | + | parkedOther := d.parked[otherToken] | |
| 1120 | + | _, otherStillParking := d.parking[otherToken] | |
| 1121 | + | d.mu.Unlock() | |
| 1122 | + | require.NotNil(t, parkedOther, "subsequent park must publish the other attachment") | |
| 1123 | + | require.False(t, otherStillParking, "park consumes the other attachment's in-flight marker") | |
| 1124 | + | ||
| 1125 | + | d.clearParkingInFlight(otherToken, ac) | |
| 1126 | + | d.mu.Lock() | |
| 1127 | + | parkedAfterMismatchedClear := d.parked[otherToken] | |
| 1128 | + | d.mu.Unlock() | |
| 1129 | + | require.Same(t, parkedOther, parkedAfterMismatchedClear, "mismatched clear must not drop a subsequent parked lifecycle") | |
| 1130 | + | } | |
| 1131 | + | ||
| 1132 | + | // TestLiveResumeRejectsLateMarkerAfterTerminalCleanupWins covers the race where | |
| 1133 | + | // live resume has validated the same-client credential, then explicit detach or | |
| 1134 | + | // session removal wins before markParkingInFlight. Late marker publication must | |
| 1135 | + | // be rejected so IntentResume returns promptly ErrNoSuchSession with no parking | |
| 1136 | + | // marker left to strand waiters. Direct parkAttachment after detach still works. | |
| 1137 | + | func TestLiveResumeRejectsLateMarkerAfterTerminalCleanupWins(t *testing.T) { | |
| 1138 | + | cases := []struct { | |
| 1139 | + | name string | |
| 1140 | + | win func(t *testing.T, d *Daemon, sess *session, ac *attachedClient, oldTr *closeTrackingTransport, token uint64) | |
| 1141 | + | }{ | |
| 1142 | + | { | |
| 1143 | + | name: "explicit_detach", | |
| 1144 | + | win: func(t *testing.T, d *Daemon, sess *session, ac *attachedClient, oldTr *closeTrackingTransport, token uint64) { | |
| 1145 | + | t.Helper() | |
| 1146 | + | d.clientGone(sess, ac, oldTr, true) | |
| 1147 | + | require.Nil(t, sess.client, "explicit detach must clear the live owner before late mark") | |
| 1148 | + | }, | |
| 1149 | + | }, | |
| 1150 | + | { | |
| 1151 | + | name: "session_removal", | |
| 1152 | + | win: func(t *testing.T, d *Daemon, sess *session, ac *attachedClient, oldTr *closeTrackingTransport, token uint64) { | |
| 1153 | + | t.Helper() | |
| 1154 | + | require.NoError(t, d.killSession(sess, ports.ReasonSessionKilled, true)) | |
| 1155 | + | d.mu.Lock() | |
| 1156 | + | registered := d.sessions[sess.id] == sess | |
| 1157 | + | d.mu.Unlock() | |
| 1158 | + | require.False(t, registered, "killSession must unregister before late mark") | |
| 1159 | + | }, | |
| 1160 | + | }, | |
| 1161 | + | } | |
| 1162 | + | for _, tc := range cases { | |
| 1163 | + | t.Run(tc.name, func(t *testing.T) { | |
| 1164 | + | pty, release := newBlockingPTY(t) | |
| 1165 | + | defer release() | |
| 1166 | + | d := newTestDaemon(t, newFactory(t, pty), stubClock{}) | |
| 1167 | + | ||
| 1168 | + | oldTr := &closeTrackingTransport{} | |
| 1169 | + | sess, ac, err := d.route(helloResumeCapable(ports.IntentNew, "work", 0), oldTr) | |
| 1170 | + | require.NoError(t, err) | |
| 1171 | + | token := ac.resumeToken | |
| 1172 | + | require.NotZero(t, token) | |
| 1173 | + | require.Same(t, ac, sess.client) | |
| 1174 | + | ||
| 1175 | + | validated := make(chan struct{}) | |
| 1176 | + | releaseMark := make(chan struct{}) | |
| 1177 | + | var releaseOnce sync.Once | |
| 1178 | + | t.Cleanup(func() { releaseOnce.Do(func() { close(releaseMark) }) }) | |
| 1179 | + | d.beforeMarkParkingInFlight = func() { | |
| 1180 | + | close(validated) | |
| 1181 | + | <-releaseMark | |
| 1182 | + | } | |
| 1183 | + | ||
| 1184 | + | type routeResult struct { | |
| 1185 | + | err error | |
| 1186 | + | } | |
| 1187 | + | result := make(chan routeResult, 1) | |
| 1188 | + | go func() { | |
| 1189 | + | _, _, routeErr := d.route(helloResumeCapable(ports.IntentResume, "work", token), &closeTrackingTransport{}) | |
| 1190 | + | result <- routeResult{err: routeErr} | |
| 1191 | + | }() | |
| 1192 | + | awaitTestCompletion(t, validated, "live resume did not pause after credential validation before mark") | |
| 1193 | + | ||
| 1194 | + | tc.win(t, d, sess, ac, oldTr, token) | |
| 1195 | + | ||
| 1196 | + | d.mu.Lock() | |
| 1197 | + | _, parkingBeforeRelease := d.parking[token] | |
| 1198 | + | d.mu.Unlock() | |
| 1199 | + | require.False(t, parkingBeforeRelease, "terminal cleanup must not leave a parking marker before late mark resumes") | |
| 1200 | + | ||
| 1201 | + | releaseOnce.Do(func() { close(releaseMark) }) | |
| 1202 | + | got := awaitTestValue(t, result, "live IntentResume stayed blocked after terminal cleanup won before mark") | |
| 1203 | + | require.Error(t, got.err) | |
| 1204 | + | var pe *protoErr | |
| 1205 | + | require.ErrorAs(t, got.err, &pe) | |
| 1206 | + | require.Equal(t, ports.ErrNoSuchSession, pe.code) | |
| 1207 | + | require.Contains(t, pe.Error(), "resume token is no longer valid") | |
| 1208 | + | ||
| 1209 | + | d.mu.Lock() | |
| 1210 | + | _, stillParking := d.parking[token] | |
| 1211 | + | _, stillParked := d.parked[token] | |
| 1212 | + | d.mu.Unlock() | |
| 1213 | + | require.False(t, stillParking, "late markParkingInFlight must not recreate a parking marker after terminal cleanup") | |
| 1214 | + | require.False(t, stillParked, "terminal cleanup must not leave a parked credential") | |
| 1215 | + | }) | |
| 1216 | + | } | |
| 1217 | + | ||
| 1218 | + | t.Run("direct_park_after_detach_without_marker", func(t *testing.T) { | |
| 1219 | + | pty, release := newBlockingPTY(t) | |
| 1220 | + | defer release() | |
| 1221 | + | d := newTestDaemon(t, newFactory(t, pty), stubClock{}) | |
| 1222 | + | ||
| 1223 | + | oldTr := &closeTrackingTransport{} | |
| 1224 | + | sess, ac, err := d.route(helloResumeCapable(ports.IntentNew, "work", 0), oldTr) | |
| 1225 | + | require.NoError(t, err) | |
| 1226 | + | token := ac.resumeToken | |
| 1227 | + | require.NotZero(t, token) | |
| 1228 | + | ||
| 1229 | + | require.True(t, d.detachIfCurrentTransport(sess, ac, ac.transportSnapshot())) | |
| 1230 | + | require.Nil(t, sess.client) | |
| 1231 | + | require.Zero(t, d.markParkingInFlight(sess, ac), "detached active attachment must not publish a late marker") | |
| 1232 | + | ||
| 1233 | + | d.mu.Lock() | |
| 1234 | + | _, parkingBeforePark := d.parking[token] | |
| 1235 | + | d.mu.Unlock() | |
| 1236 | + | require.False(t, parkingBeforePark) | |
| 1237 | + | ||
| 1238 | + | require.True(t, d.parkAttachment(sess, ac), "direct parkAttachment after detach must still park without recreating an in-flight marker") | |
| 1239 | + | d.mu.Lock() | |
| 1240 | + | parked := d.parked[token] | |
| 1241 | + | _, stillParking := d.parking[token] | |
| 1242 | + | d.mu.Unlock() | |
| 1243 | + | require.NotNil(t, parked) | |
| 1244 | + | require.Same(t, ac, parked.ac) | |
| 1245 | + | require.False(t, stillParking, "park must not leave an in-flight marker") | |
| 1246 | + | }) | |
| 1247 | + | } | |
| 1248 | + | ||
| 1249 | + | // TestMarkParkingInFlightRequiresExactLiveOwnership covers lock-safe rejection | |
| 1250 | + | // when the attachment is no longer the exact active or snatched owner, while | |
| 1251 | + | // still advertising for both live roles. | |
| 1252 | + | func TestMarkParkingInFlightRequiresExactLiveOwnership(t *testing.T) { | |
| 1253 | + | pty, release := newBlockingPTY(t) | |
| 1254 | + | defer release() | |
| 1255 | + | d := newTestDaemon(t, newFactory(t, pty), stubClock{}) | |
| 1256 | + | ||
| 1257 | + | oldTr := &closeTrackingTransport{} | |
| 1258 | + | sess, waiting, err := d.route(helloResumeCapable(ports.IntentNew, "work", 0), oldTr) | |
| 1259 | + | require.NoError(t, err) | |
| 1260 | + | token := waiting.resumeToken | |
| 1261 | + | require.NotZero(t, token) | |
| 1262 | + | ||
| 1263 | + | require.Equal(t, token, d.markParkingInFlight(sess, waiting), "active owner must publish") | |
| 1264 | + | d.clearParkingInFlight(token, waiting) | |
| 1265 | + | ||
| 1266 | + | _, active, err := d.route(helloResumeCapable(ports.IntentAttach, "work", 0), &closeTrackingTransport{}) | |
| 1267 | + | require.NoError(t, err) | |
| 1268 | + | d.attachmentCleanupWg.Wait() | |
| 1269 | + | require.Equal(t, attachmentSnatched, sess.attachmentRole(waiting)) | |
| 1270 | + | require.Same(t, active, sess.client) | |
| 1271 | + | ||
| 1272 | + | require.Equal(t, token, d.markParkingInFlight(sess, waiting), "snatched owner must publish") | |
| 1273 | + | d.clearParkingInFlight(token, waiting) | |
| 1274 | + | ||
| 1275 | + | roleToken := sess.attachmentToken(waiting, oldTr) | |
| 1276 | + | require.True(t, d.unrouteSnatchedAttachment(roleToken, false)) | |
| 1277 | + | require.Equal(t, attachmentDetached, sess.attachmentRole(waiting)) | |
| 1278 | + | require.Zero(t, d.markParkingInFlight(sess, waiting), "detached snatched attachment must not publish") | |
| 1279 | + | ||
| 1280 | + | d.mu.Lock() | |
| 1281 | + | _, stillParking := d.parking[token] | |
| 1282 | + | d.mu.Unlock() | |
| 1283 | + | require.False(t, stillParking) | |
| 1284 | + | ||
| 1285 | + | require.Equal(t, active.resumeToken, d.markParkingInFlight(sess, active), "active owner must still publish after snatched unroute") | |
| 1286 | + | d.clearParkingInFlight(active.resumeToken, active) | |
| 1287 | + | } | |
| 1288 | + | ||
| 478 | 1289 | // TestStaleClientGoneAfterTransportCheckDoesNotDetachReboundAttachment covers | |
| 479 | 1290 | // the TOCTOU where clientGone passes the old-transport precheck, then a live | |
| 480 | 1291 | // resume rebinds the same attachment before detach runs. | |
| @@ -947,6 +1758,76 @@ func TestKilledSessionPurgesParkedResumeToken(t *testing.T) { | |||
| 947 | 1758 | require.False(t, ok, "killed session cannot be resumed") | |
| 948 | 1759 | } | |
| 949 | 1760 | ||
| 1761 | + | // TestKilledSessionUnblocksParkingWaiterFailsClosed covers session removal while | |
| 1762 | + | // a same-token IntentResume is waiting on an in-flight parking marker. killSession | |
| 1763 | + | // must purge only that session's markers so the waiter unblocks and fails closed, | |
| 1764 | + | // without touching another session's parking marker. | |
| 1765 | + | func TestKilledSessionUnblocksParkingWaiterFailsClosed(t *testing.T) { | |
| 1766 | + | workPTY, releaseWork := newBlockingPTY(t) | |
| 1767 | + | defer releaseWork() | |
| 1768 | + | otherPTY, releaseOther := newBlockingPTY(t) | |
| 1769 | + | defer releaseOther() | |
| 1770 | + | // Separate PTYs are required: a shared mock PTY makes killSession(work) | |
| 1771 | + | // close other's pane and cascade-remove the other session under -race, | |
| 1772 | + | // falsely failing the other-marker / fail-closed assertions. | |
| 1773 | + | d := newTestDaemon(t, newFactorySeq(t, workPTY, otherPTY), stubClock{}) | |
| 1774 | + | ||
| 1775 | + | oldTr := &closeTrackingTransport{} | |
| 1776 | + | sess, ac, err := d.route(helloResumeCapable(ports.IntentNew, "work", 0), oldTr) | |
| 1777 | + | require.NoError(t, err) | |
| 1778 | + | token := ac.resumeToken | |
| 1779 | + | require.NotZero(t, token) | |
| 1780 | + | ||
| 1781 | + | otherTr := &closeTrackingTransport{} | |
| 1782 | + | otherSess, otherAC, err := d.route(helloResumeCapable(ports.IntentNew, "other", 0), otherTr) | |
| 1783 | + | require.NoError(t, err) | |
| 1784 | + | otherToken := d.markParkingInFlight(otherSess, otherAC) | |
| 1785 | + | require.NotZero(t, otherToken) | |
| 1786 | + | require.NotEqual(t, token, otherToken) | |
| 1787 | + | ||
| 1788 | + | require.Equal(t, token, d.markParkingInFlight(sess, ac)) | |
| 1789 | + | d.mu.Lock() | |
| 1790 | + | parkingMarker := d.parking[token] | |
| 1791 | + | otherMarker := d.parking[otherToken] | |
| 1792 | + | d.mu.Unlock() | |
| 1793 | + | require.NotNil(t, parkingMarker, "fixture: parking marker must be published before kill") | |
| 1794 | + | require.Same(t, ac, parkingMarker.ac) | |
| 1795 | + | require.NotNil(t, otherMarker, "fixture: other session marker must remain published") | |
| 1796 | + | require.Same(t, otherAC, otherMarker.ac) | |
| 1797 | + | ||
| 1798 | + | waiterArmed := make(chan struct{}) | |
| 1799 | + | d.afterParkingWaitArmed = func() { close(waiterArmed) } | |
| 1800 | + | waiterDone := make(chan bool, 1) | |
| 1801 | + | go func() { | |
| 1802 | + | waiterDone <- d.waitParkingInFlight(helloResumeCapable(ports.IntentResume, "work", token)) | |
| 1803 | + | }() | |
| 1804 | + | awaitTestCompletion(t, waiterArmed, "parking waiter did not arm before session kill") | |
| 1805 | + | ||
| 1806 | + | require.NoError(t, d.killSession(sess, ports.ReasonSessionKilled, true)) | |
| 1807 | + | ||
| 1808 | + | waited := awaitTestValue(t, waiterDone, "parking waiter stayed blocked after session kill") | |
| 1809 | + | require.True(t, waited, "waiter must observe the session marker before it is purged") | |
| 1810 | + | ||
| 1811 | + | d.mu.Lock() | |
| 1812 | + | _, stillParking := d.parking[token] | |
| 1813 | + | otherAfter := d.parking[otherToken] | |
| 1814 | + | otherRegistered := d.sessions[otherSess.id] == otherSess | |
| 1815 | + | closing := d.closing | |
| 1816 | + | d.mu.Unlock() | |
| 1817 | + | require.False(t, stillParking, "killSession must purge the killed session's parking marker") | |
| 1818 | + | require.NotNil(t, otherAfter, "killSession must not purge another session's parking marker") | |
| 1819 | + | require.Same(t, otherAC, otherAfter.ac) | |
| 1820 | + | require.True(t, otherRegistered, "other session must survive work kill when PTYs are isolated") | |
| 1821 | + | require.False(t, closing, "daemon must not shut down while the other session remains") | |
| 1822 | + | ||
| 1823 | + | _, _, err = d.route(helloResumeCapable(ports.IntentResume, "work", token), &closeTrackingTransport{}) | |
| 1824 | + | require.Error(t, err) | |
| 1825 | + | var killedErr *protoErr | |
| 1826 | + | require.ErrorAs(t, err, &killedErr) | |
| 1827 | + | require.Equal(t, ports.ErrNoSuchSession, killedErr.code) | |
| 1828 | + | require.Contains(t, killedErr.Error(), "resume token is no longer valid") | |
| 1829 | + | } | |
| 1830 | + | ||
| 950 | 1831 | func TestParkedPredecessorResumesSnatchedWithoutStealingActiveAttachment(t *testing.T) { | |
| 951 | 1832 | pty, release := newBlockingPTY(t) | |
| 952 | 1833 | defer release() | |
| @@ -5,6 +5,7 @@ import ( | |||||
| 5 | "errors" | 5 | "errors" | ||
| 6 | "strings" | 6 | "strings" | ||
| 7 | "sync" | 7 | "sync" | ||
| 8 | + | "sync/atomic" | |||
| 8 | "testing" | 9 | "testing" | ||
| 9 | "time" | 10 | "time" | ||
| 10 | 11 | ||||
| @@ -475,6 +476,816 @@ func TestResumeActiveTokenBeforeParkRecoversSameAttachment(t *testing.T) { | |||||
| 475 | require.False(t, newTr.Closed()) | 476 | require.False(t, newTr.Closed()) | ||
| 476 | } | 477 | } | ||
| 477 | 478 | ||||
| 479 | + | // TestResumeDuringTeardownBeforeParkRecoversSameAttachment covers the opposite | |||
| 480 | + | // interleaving from live-before-park recovery: old-link teardown wins | |||
| 481 | + | // detachIfCurrentTransport, then pauses before parkAttachment publishes the | |||
| 482 | + | // resume token. The parking-in-flight marker must already be visible in that | |||
| 483 | + | // gap (published before detach). Same-client IntentResume must wait it out and | |||
| 484 | + | // recover; unknown tokens stay fail-closed. | |||
| 485 | + | func TestResumeDuringTeardownBeforeParkRecoversSameAttachment(t *testing.T) { | |||
| 486 | + | pty, release := newBlockingPTY(t) | |||
| 487 | + | defer release() | |||
| 488 | + | d := newTestDaemon(t, newFactory(t, pty), stubClock{}) | |||
| 489 | + | ||||
| 490 | + | oldTr := &closeTrackingTransport{} | |||
| 491 | + | sess, ac, err := d.route(helloResumeCapable(ports.IntentNew, "work", 0), oldTr) | |||
| 492 | + | require.NoError(t, err) | |||
| 493 | + | token := ac.resumeToken | |||
| 494 | + | require.NotZero(t, token) | |||
| 495 | + | require.Same(t, ac, sess.client) | |||
| 496 | + | ||||
| 497 | + | reachedGap := make(chan struct{}) | |||
| 498 | + | releaseGap := make(chan struct{}) | |||
| 499 | + | var releaseOnce sync.Once | |||
| 500 | + | t.Cleanup(func() { releaseOnce.Do(func() { close(releaseGap) }) }) | |||
| 501 | + | d.afterClientGoneDetach = func() { | |||
| 502 | + | close(reachedGap) | |||
| 503 | + | <-releaseGap | |||
| 504 | + | } | |||
| 505 | + | ||||
| 506 | + | goneDone := make(chan struct{}) | |||
| 507 | + | go func() { | |||
| 508 | + | d.clientGone(sess, ac, oldTr, false) | |||
| 509 | + | close(goneDone) | |||
| 510 | + | }() | |||
| 511 | + | awaitTestCompletion(t, reachedGap, "teardown did not pause after detach before park") | |||
| 512 | + | ||||
| 513 | + | require.Nil(t, sess.client, "fixture: detach must have cleared the live owner") | |||
| 514 | + | d.mu.Lock() | |||
| 515 | + | parkingInGap := d.parking[token] | |||
| 516 | + | _, parkedInGap := d.parked[token] | |||
| 517 | + | d.mu.Unlock() | |||
| 518 | + | require.NotNil(t, parkingInGap, "fixture: parking marker must precede detach publication") | |||
| 519 | + | require.Same(t, ac, parkingInGap.ac) | |||
| 520 | + | require.Same(t, sess, parkingInGap.sess) | |||
| 521 | + | require.False(t, parkedInGap, "fixture: park must not have published yet") | |||
| 522 | + | ||||
| 523 | + | _, _, err = d.route(helloResumeCapable(ports.IntentResume, "work", 0xdecafbad), &closeTrackingTransport{}) | |||
| 524 | + | require.Error(t, err) | |||
| 525 | + | var unknownErr *protoErr | |||
| 526 | + | require.ErrorAs(t, err, &unknownErr) | |||
| 527 | + | require.Equal(t, ports.ErrNoSuchSession, unknownErr.code) | |||
| 528 | + | require.Contains(t, unknownErr.Error(), "resume token is no longer valid") | |||
| 529 | + | ||||
| 530 | + | wrongClient := helloResumeCapable(ports.IntentResume, "work", token) | |||
| 531 | + | wrongClient.ClientID = [16]byte{9, 9, 9, 9} | |||
| 532 | + | _, _, err = d.route(wrongClient, &closeTrackingTransport{}) | |||
| 533 | + | require.Error(t, err) | |||
| 534 | + | var mismatchErr *protoErr | |||
| 535 | + | require.ErrorAs(t, err, &mismatchErr) | |||
| 536 | + | require.Equal(t, ports.ErrNoSuchSession, mismatchErr.code) | |||
| 537 | + | require.Contains(t, mismatchErr.Error(), "resume token is no longer valid") | |||
| 538 | + | ||||
| 539 | + | newTr := &closeTrackingTransport{} | |||
| 540 | + | type routeResult struct { | |||
| 541 | + | sess *session | |||
| 542 | + | ac *attachedClient | |||
| 543 | + | err error | |||
| 544 | + | } | |||
| 545 | + | waiterArmed := make(chan struct{}) | |||
| 546 | + | d.afterParkingWaitArmed = func() { close(waiterArmed) } | |||
| 547 | + | result := make(chan routeResult, 1) | |||
| 548 | + | go func() { | |||
| 549 | + | routedSess, routedAC, routeErr := d.route(helloResumeCapable(ports.IntentResume, "work", token), newTr) | |||
| 550 | + | result <- routeResult{sess: routedSess, ac: routedAC, err: routeErr} | |||
| 551 | + | }() | |||
| 552 | + | awaitTestCompletion(t, waiterArmed, "resume waiter did not arm on parking marker before park") | |||
| 553 | + | ||||
| 554 | + | releaseOnce.Do(func() { close(releaseGap) }) | |||
| 555 | + | awaitTestCompletion(t, goneDone, "teardown did not finish after park release") | |||
| 556 | + | got := awaitTestValue(t, result, "IntentResume did not finish after teardown-before-park gap") | |||
| 557 | + | ||||
| 558 | + | require.NoError(t, got.err, "same-client token must recover across detach-before-park") | |||
| 559 | + | require.Same(t, sess, got.sess) | |||
| 560 | + | require.Same(t, ac, got.ac) | |||
| 561 | + | require.Same(t, ac, sess.client) | |||
| 562 | + | require.Same(t, newTr, ac.transport()) | |||
| 563 | + | require.NotEqual(t, token, ac.resumeToken, "successful resume rotates the credential") | |||
| 564 | + | require.True(t, oldTr.Closed(), "teardown/resume must retire the old transport") | |||
| 565 | + | require.False(t, newTr.Closed(), "rebound transport must survive") | |||
| 566 | + | d.mu.Lock() | |||
| 567 | + | _, oldTokenParked := d.parked[token] | |||
| 568 | + | _, stillParking := d.parking[token] | |||
| 569 | + | d.mu.Unlock() | |||
| 570 | + | require.False(t, oldTokenParked, "consumed credential must not remain resumeable") | |||
| 571 | + | require.False(t, stillParking, "parking marker must be consumed after park/resume") | |||
| 572 | + | ||||
| 573 | + | d.clientGone(sess, ac, oldTr, false) | |||
| 574 | + | require.Same(t, ac, sess.client, "stale old-link cleanup must not detach the rebound attachment") | |||
| 575 | + | require.Same(t, newTr, ac.transport()) | |||
| 576 | + | require.False(t, newTr.Closed()) | |||
| 577 | + | } | |||
| 578 | + | ||||
| 579 | + | // TestConcurrentLiveResumesWaitParkingMarkerBeforePark covers two same-token | |||
| 580 | + | // IntentResume handshakes where the winner of detachIfCurrentTransport pauses | |||
| 581 | + | // before parkAttachment. The parking marker must already be published so the | |||
| 582 | + | // loser waits instead of treating the credential as unknown across that gap. | |||
| 583 | + | func TestConcurrentLiveResumesWaitParkingMarkerBeforePark(t *testing.T) { | |||
| 584 | + | pty, release := newBlockingPTY(t) | |||
| 585 | + | defer release() | |||
| 586 | + | d := newTestDaemon(t, newFactory(t, pty), stubClock{}) | |||
| 587 | + | ||||
| 588 | + | oldTr := &closeTrackingTransport{} | |||
| 589 | + | sess, ac, err := d.route(helloResumeCapable(ports.IntentNew, "work", 0), oldTr) | |||
| 590 | + | require.NoError(t, err) | |||
| 591 | + | token := ac.resumeToken | |||
| 592 | + | require.NotZero(t, token) | |||
| 593 | + | require.Same(t, ac, sess.client) | |||
| 594 | + | ||||
| 595 | + | reachedGap := make(chan struct{}) | |||
| 596 | + | releaseGap := make(chan struct{}) | |||
| 597 | + | var releaseOnce sync.Once | |||
| 598 | + | t.Cleanup(func() { releaseOnce.Do(func() { close(releaseGap) }) }) | |||
| 599 | + | d.afterResumeLiveDetach = func() { | |||
| 600 | + | close(reachedGap) | |||
| 601 | + | <-releaseGap | |||
| 602 | + | } | |||
| 603 | + | ||||
| 604 | + | type routeResult struct { | |||
| 605 | + | sess *session | |||
| 606 | + | ac *attachedClient | |||
| 607 | + | err error | |||
| 608 | + | } | |||
| 609 | + | firstTr := &closeTrackingTransport{} | |||
| 610 | + | firstResult := make(chan routeResult, 1) | |||
| 611 | + | go func() { | |||
| 612 | + | routedSess, routedAC, routeErr := d.route(helloResumeCapable(ports.IntentResume, "work", token), firstTr) | |||
| 613 | + | firstResult <- routeResult{sess: routedSess, ac: routedAC, err: routeErr} | |||
| 614 | + | }() | |||
| 615 | + | awaitTestCompletion(t, reachedGap, "first live resume did not pause after detach before park") | |||
| 616 | + | ||||
| 617 | + | require.Nil(t, sess.client, "fixture: winning resume must have cleared the live owner") | |||
| 618 | + | d.mu.Lock() | |||
| 619 | + | parkingInGap := d.parking[token] | |||
| 620 | + | _, parkedInGap := d.parked[token] | |||
| 621 | + | d.mu.Unlock() | |||
| 622 | + | require.NotNil(t, parkingInGap, "fixture: parking marker must precede detach for concurrent resumes") | |||
| 623 | + | require.Same(t, ac, parkingInGap.ac) | |||
| 624 | + | require.False(t, parkedInGap, "fixture: park must not have published yet") | |||
| 625 | + | ||||
| 626 | + | _, _, err = d.route(helloResumeCapable(ports.IntentResume, "work", 0xdecafbad), &closeTrackingTransport{}) | |||
| 627 | + | require.Error(t, err) | |||
| 628 | + | var unknownErr *protoErr | |||
| 629 | + | require.ErrorAs(t, err, &unknownErr) | |||
| 630 | + | require.Equal(t, ports.ErrNoSuchSession, unknownErr.code) | |||
| 631 | + | ||||
| 632 | + | secondTr := &closeTrackingTransport{} | |||
| 633 | + | secondWaiting := make(chan struct{}) | |||
| 634 | + | d.afterParkingWaitArmed = func() { | |||
| 635 | + | close(secondWaiting) | |||
| 636 | + | } | |||
| 637 | + | secondResult := make(chan routeResult, 1) | |||
| 638 | + | go func() { | |||
| 639 | + | routedSess, routedAC, routeErr := d.route(helloResumeCapable(ports.IntentResume, "work", token), secondTr) | |||
| 640 | + | secondResult <- routeResult{sess: routedSess, ac: routedAC, err: routeErr} | |||
| 641 | + | }() | |||
| 642 | + | awaitTestCompletion(t, secondWaiting, "second live resume did not arm parking wait") | |||
| 643 | + | ||||
| 644 | + | releaseOnce.Do(func() { close(releaseGap) }) | |||
| 645 | + | first := awaitTestValue(t, firstResult, "first concurrent IntentResume did not finish") | |||
| 646 | + | second := awaitTestValue(t, secondResult, "second concurrent IntentResume did not finish") | |||
| 647 | + | ||||
| 648 | + | var winner, loser routeResult | |||
| 649 | + | var winnerTr, loserTr *closeTrackingTransport | |||
| 650 | + | switch { | |||
| 651 | + | case first.err == nil && second.err != nil: | |||
| 652 | + | winner, loser = first, second | |||
| 653 | + | winnerTr, loserTr = firstTr, secondTr | |||
| 654 | + | case second.err == nil && first.err != nil: | |||
| 655 | + | winner, loser = second, first | |||
| 656 | + | winnerTr, loserTr = secondTr, firstTr | |||
| 657 | + | default: | |||
| 658 | + | t.Fatalf("expected exactly one concurrent resume to succeed; first=%v second=%v", first.err, second.err) | |||
| 659 | + | } | |||
| 660 | + | ||||
| 661 | + | require.Same(t, sess, winner.sess) | |||
| 662 | + | require.Same(t, ac, winner.ac) | |||
| 663 | + | require.Same(t, ac, sess.client) | |||
| 664 | + | require.Same(t, winnerTr, ac.transport()) | |||
| 665 | + | require.NotEqual(t, token, ac.resumeToken, "successful resume rotates the credential") | |||
| 666 | + | require.True(t, oldTr.Closed(), "winning resume must retire the old transport") | |||
| 667 | + | require.False(t, winnerTr.Closed(), "winning rebound transport must survive") | |||
| 668 | + | ||||
| 669 | + | var loserErr *protoErr | |||
| 670 | + | require.ErrorAs(t, loser.err, &loserErr) | |||
| 671 | + | require.Equal(t, ports.ErrNoSuchSession, loserErr.code) | |||
| 672 | + | require.Contains(t, loserErr.Error(), "resume token is no longer valid") | |||
| 673 | + | require.True(t, loserTr.Closed() || len(loserTr.Sends()) == 0, "losing resume must not complete Welcome") | |||
| 674 | + | ||||
| 675 | + | d.mu.Lock() | |||
| 676 | + | _, oldTokenParked := d.parked[token] | |||
| 677 | + | _, stillParking := d.parking[token] | |||
| 678 | + | d.mu.Unlock() | |||
| 679 | + | require.False(t, oldTokenParked, "consumed credential must not remain resumeable") | |||
| 680 | + | require.False(t, stillParking, "parking marker must be consumed after park/resume") | |||
| 681 | + | ||||
| 682 | + | d.clientGone(sess, ac, oldTr, false) | |||
| 683 | + | require.Same(t, ac, sess.client, "stale old-link cleanup must not detach the rebound attachment") | |||
| 684 | + | require.Same(t, winnerTr, ac.transport()) | |||
| 685 | + | require.False(t, winnerTr.Closed()) | |||
| 686 | + | } | |||
| 687 | + | ||||
| 688 | + | func setupResumableSnatchedAttachment(t *testing.T, d *Daemon) (*session, *attachedClient, *attachedClient, *closeTrackingTransport, uint64) { | |||
| 689 | + | t.Helper() | |||
| 690 | + | oldTr := &closeTrackingTransport{} | |||
| 691 | + | sess, waiting, err := d.route(helloResumeCapable(ports.IntentNew, "work", 0), oldTr) | |||
| 692 | + | require.NoError(t, err) | |||
| 693 | + | token := waiting.resumeToken | |||
| 694 | + | require.NotZero(t, token) | |||
| 695 | + | ||||
| 696 | + | _, active, err := d.route(helloResumeCapable(ports.IntentAttach, "work", 0), &closeTrackingTransport{}) | |||
| 697 | + | require.NoError(t, err) | |||
| 698 | + | d.attachmentCleanupWg.Wait() | |||
| 699 | + | require.Equal(t, attachmentSnatched, sess.attachmentRole(waiting)) | |||
| 700 | + | require.Same(t, active, sess.client) | |||
| 701 | + | return sess, waiting, active, oldTr, token | |||
| 702 | + | } | |||
| 703 | + | ||||
| 704 | + | // TestSnatchedResumeDuringTeardownBeforeParkRecoversSameAttachment covers the | |||
| 705 | + | // snatched unroute→park gap: parkSnatchedAttachment publishes the parking | |||
| 706 | + | // marker before ownership removal, then pauses before parkAttachmentAs. | |||
| 707 | + | // Same-client IntentResume must wait it out and recover as snatched without | |||
| 708 | + | // replacing the active owner; unknown/mismatched tokens stay fail-closed. | |||
| 709 | + | func TestSnatchedResumeDuringTeardownBeforeParkRecoversSameAttachment(t *testing.T) { | |||
| 710 | + | pty, release := newBlockingPTY(t) | |||
| 711 | + | defer release() | |||
| 712 | + | d := newTestDaemon(t, newFactory(t, pty), stubClock{}) | |||
| 713 | + | ||||
| 714 | + | sess, waiting, active, oldTr, token := setupResumableSnatchedAttachment(t, d) | |||
| 715 | + | roleToken := sess.attachmentToken(waiting, oldTr) | |||
| 716 | + | ||||
| 717 | + | reachedGap := make(chan struct{}) | |||
| 718 | + | releaseGap := make(chan struct{}) | |||
| 719 | + | var releaseOnce sync.Once | |||
| 720 | + | t.Cleanup(func() { releaseOnce.Do(func() { close(releaseGap) }) }) | |||
| 721 | + | d.afterSnatchedUnrouteBeforePark = func() { | |||
| 722 | + | close(reachedGap) | |||
| 723 | + | <-releaseGap | |||
| 724 | + | } | |||
| 725 | + | ||||
| 726 | + | parkDone := make(chan bool, 1) | |||
| 727 | + | go func() { | |||
| 728 | + | parkDone <- d.parkSnatchedAttachment(roleToken) | |||
| 729 | + | }() | |||
| 730 | + | awaitTestCompletion(t, reachedGap, "snatched park did not pause after unroute before park") | |||
| 731 | + | ||||
| 732 | + | require.Equal(t, attachmentDetached, sess.attachmentRole(waiting), "fixture: unroute must have cleared snatched ownership") | |||
| 733 | + | d.mu.Lock() | |||
| 734 | + | parkingInGap := d.parking[token] | |||
| 735 | + | _, parkedInGap := d.parked[token] | |||
| 736 | + | d.mu.Unlock() | |||
| 737 | + | require.NotNil(t, parkingInGap, "fixture: parking marker must precede snatched ownership removal") | |||
| 738 | + | require.Same(t, waiting, parkingInGap.ac) | |||
| 739 | + | require.Same(t, sess, parkingInGap.sess) | |||
| 740 | + | require.False(t, parkedInGap, "fixture: park must not have published yet") | |||
| 741 | + | require.Same(t, active, sess.client, "fixture: active owner must remain undisturbed") | |||
| 742 | + | ||||
| 743 | + | _, _, err := d.route(helloResumeCapable(ports.IntentResume, "work", 0xdecafbad), &closeTrackingTransport{}) | |||
| 744 | + | require.Error(t, err) | |||
| 745 | + | var unknownErr *protoErr | |||
| 746 | + | require.ErrorAs(t, err, &unknownErr) | |||
| 747 | + | require.Equal(t, ports.ErrNoSuchSession, unknownErr.code) | |||
| 748 | + | require.Contains(t, unknownErr.Error(), "resume token is no longer valid") | |||
| 749 | + | ||||
| 750 | + | wrongClient := helloResumeCapable(ports.IntentResume, "work", token) | |||
| 751 | + | wrongClient.ClientID = [16]byte{9, 9, 9, 9} | |||
| 752 | + | _, _, err = d.route(wrongClient, &closeTrackingTransport{}) | |||
| 753 | + | require.Error(t, err) | |||
| 754 | + | var mismatchErr *protoErr | |||
| 755 | + | require.ErrorAs(t, err, &mismatchErr) | |||
| 756 | + | require.Equal(t, ports.ErrNoSuchSession, mismatchErr.code) | |||
| 757 | + | require.Contains(t, mismatchErr.Error(), "resume token is no longer valid") | |||
| 758 | + | ||||
| 759 | + | newTr := &closeTrackingTransport{} | |||
| 760 | + | type routeResult struct { | |||
| 761 | + | sess *session | |||
| 762 | + | ac *attachedClient | |||
| 763 | + | err error | |||
| 764 | + | } | |||
| 765 | + | waiterArmed := make(chan struct{}) | |||
| 766 | + | d.afterParkingWaitArmed = func() { close(waiterArmed) } | |||
| 767 | + | result := make(chan routeResult, 1) | |||
| 768 | + | go func() { | |||
| 769 | + | routedSess, routedAC, routeErr := d.route(helloResumeCapable(ports.IntentResume, "work", token), newTr) | |||
| 770 | + | result <- routeResult{sess: routedSess, ac: routedAC, err: routeErr} | |||
| 771 | + | }() | |||
| 772 | + | awaitTestCompletion(t, waiterArmed, "resume waiter did not arm on snatched parking marker before park") | |||
| 773 | + | ||||
| 774 | + | releaseOnce.Do(func() { close(releaseGap) }) | |||
| 775 | + | require.True(t, awaitTestValue(t, parkDone, "snatched park did not finish after release"), "parkSnatchedAttachment must succeed") | |||
| 776 | + | got := awaitTestValue(t, result, "IntentResume did not finish after snatched teardown-before-park gap") | |||
| 777 | + | ||||
| 778 | + | require.NoError(t, got.err, "same-client snatched token must recover across unroute-before-park") | |||
| 779 | + | require.Same(t, sess, got.sess) | |||
| 780 | + | require.Same(t, waiting, got.ac) | |||
| 781 | + | require.Same(t, active, sess.client, "snatched resume must not replace the active owner") | |||
| 782 | + | require.Equal(t, attachmentSnatched, sess.attachmentRole(waiting)) | |||
| 783 | + | require.Same(t, newTr, waiting.transport()) | |||
| 784 | + | require.NotEqual(t, token, waiting.resumeToken, "successful resume rotates the credential") | |||
| 785 | + | require.True(t, oldTr.Closed(), "teardown/resume must retire the old transport") | |||
| 786 | + | require.False(t, newTr.Closed(), "rebound transport must survive") | |||
| 787 | + | d.mu.Lock() | |||
| 788 | + | _, oldTokenParked := d.parked[token] | |||
| 789 | + | _, stillParking := d.parking[token] | |||
| 790 | + | d.mu.Unlock() | |||
| 791 | + | require.False(t, oldTokenParked, "consumed credential must not remain resumeable") | |||
| 792 | + | require.False(t, stillParking, "parking marker must be consumed after park/resume") | |||
| 793 | + | } | |||
| 794 | + | ||||
| 795 | + | // TestConcurrentSnatchedResumesWaitParkingMarkerBeforePark covers two same-token | |||
| 796 | + | // IntentResume handshakes while parkSnatchedAttachment pauses after unroute and | |||
| 797 | + | // before park. The parking marker must already be published so losers wait | |||
| 798 | + | // instead of treating the credential as unknown across that gap. | |||
| 799 | + | func TestConcurrentSnatchedResumesWaitParkingMarkerBeforePark(t *testing.T) { | |||
| 800 | + | pty, release := newBlockingPTY(t) | |||
| 801 | + | defer release() | |||
| 802 | + | d := newTestDaemon(t, newFactory(t, pty), stubClock{}) | |||
| 803 | + | ||||
| 804 | + | sess, waiting, active, oldTr, token := setupResumableSnatchedAttachment(t, d) | |||
| 805 | + | roleToken := sess.attachmentToken(waiting, oldTr) | |||
| 806 | + | ||||
| 807 | + | reachedGap := make(chan struct{}) | |||
| 808 | + | releaseGap := make(chan struct{}) | |||
| 809 | + | var releaseOnce sync.Once | |||
| 810 | + | t.Cleanup(func() { releaseOnce.Do(func() { close(releaseGap) }) }) | |||
| 811 | + | d.afterSnatchedUnrouteBeforePark = func() { | |||
| 812 | + | close(reachedGap) | |||
| 813 | + | <-releaseGap | |||
| 814 | + | } | |||
| 815 | + | ||||
| 816 | + | parkDone := make(chan bool, 1) | |||
| 817 | + | go func() { | |||
| 818 | + | parkDone <- d.parkSnatchedAttachment(roleToken) | |||
| 819 | + | }() | |||
| 820 | + | awaitTestCompletion(t, reachedGap, "snatched park did not pause after unroute before park") | |||
| 821 | + | ||||
| 822 | + | require.Equal(t, attachmentDetached, sess.attachmentRole(waiting), "fixture: unroute must have cleared snatched ownership") | |||
| 823 | + | d.mu.Lock() | |||
| 824 | + | parkingInGap := d.parking[token] | |||
| 825 | + | _, parkedInGap := d.parked[token] | |||
| 826 | + | d.mu.Unlock() | |||
| 827 | + | require.NotNil(t, parkingInGap, "fixture: parking marker must precede unroute for concurrent resumes") | |||
| 828 | + | require.Same(t, waiting, parkingInGap.ac) | |||
| 829 | + | require.False(t, parkedInGap, "fixture: park must not have published yet") | |||
| 830 | + | ||||
| 831 | + | _, _, err := d.route(helloResumeCapable(ports.IntentResume, "work", 0xdecafbad), &closeTrackingTransport{}) | |||
| 832 | + | require.Error(t, err) | |||
| 833 | + | var unknownErr *protoErr | |||
| 834 | + | require.ErrorAs(t, err, &unknownErr) | |||
| 835 | + | require.Equal(t, ports.ErrNoSuchSession, unknownErr.code) | |||
| 836 | + | ||||
| 837 | + | type routeResult struct { | |||
| 838 | + | sess *session | |||
| 839 | + | ac *attachedClient | |||
| 840 | + | err error | |||
| 841 | + | } | |||
| 842 | + | firstTr := &closeTrackingTransport{} | |||
| 843 | + | secondTr := &closeTrackingTransport{} | |||
| 844 | + | bothWaitersArmed := make(chan struct{}) | |||
| 845 | + | var armedCount atomic.Int32 | |||
| 846 | + | d.afterParkingWaitArmed = func() { | |||
| 847 | + | if armedCount.Add(1) == 2 { | |||
| 848 | + | close(bothWaitersArmed) | |||
| 849 | + | } | |||
| 850 | + | } | |||
| 851 | + | firstResult := make(chan routeResult, 1) | |||
| 852 | + | secondResult := make(chan routeResult, 1) | |||
| 853 | + | go func() { | |||
| 854 | + | routedSess, routedAC, routeErr := d.route(helloResumeCapable(ports.IntentResume, "work", token), firstTr) | |||
| 855 | + | firstResult <- routeResult{sess: routedSess, ac: routedAC, err: routeErr} | |||
| 856 | + | }() | |||
| 857 | + | go func() { | |||
| 858 | + | routedSess, routedAC, routeErr := d.route(helloResumeCapable(ports.IntentResume, "work", token), secondTr) | |||
| 859 | + | secondResult <- routeResult{sess: routedSess, ac: routedAC, err: routeErr} | |||
| 860 | + | }() | |||
| 861 | + | awaitTestCompletion(t, bothWaitersArmed, "both snatched resume waiters did not arm on parking marker before park") | |||
| 862 | + | ||||
| 863 | + | releaseOnce.Do(func() { close(releaseGap) }) | |||
| 864 | + | require.True(t, awaitTestValue(t, parkDone, "snatched park did not finish after release")) | |||
| 865 | + | first := awaitTestValue(t, firstResult, "first concurrent snatched IntentResume did not finish") | |||
| 866 | + | second := awaitTestValue(t, secondResult, "second concurrent snatched IntentResume did not finish") | |||
| 867 | + | ||||
| 868 | + | var winner, loser routeResult | |||
| 869 | + | var winnerTr, loserTr *closeTrackingTransport | |||
| 870 | + | switch { | |||
| 871 | + | case first.err == nil && second.err != nil: | |||
| 872 | + | winner, loser = first, second | |||
| 873 | + | winnerTr, loserTr = firstTr, secondTr | |||
| 874 | + | case second.err == nil && first.err != nil: | |||
| 875 | + | winner, loser = second, first | |||
| 876 | + | winnerTr, loserTr = secondTr, firstTr | |||
| 877 | + | default: | |||
| 878 | + | t.Fatalf("expected exactly one concurrent snatched resume to succeed; first=%v second=%v", first.err, second.err) | |||
| 879 | + | } | |||
| 880 | + | ||||
| 881 | + | require.Same(t, sess, winner.sess) | |||
| 882 | + | require.Same(t, waiting, winner.ac) | |||
| 883 | + | require.Same(t, active, sess.client, "snatched resume must not replace the active owner") | |||
| 884 | + | require.Equal(t, attachmentSnatched, sess.attachmentRole(waiting)) | |||
| 885 | + | require.Same(t, winnerTr, waiting.transport()) | |||
| 886 | + | require.NotEqual(t, token, waiting.resumeToken, "successful resume rotates the credential") | |||
| 887 | + | require.True(t, oldTr.Closed(), "winning resume must retire the old transport") | |||
| 888 | + | require.False(t, winnerTr.Closed(), "winning rebound transport must survive") | |||
| 889 | + | ||||
| 890 | + | var loserErr *protoErr | |||
| 891 | + | require.ErrorAs(t, loser.err, &loserErr) | |||
| 892 | + | require.Equal(t, ports.ErrNoSuchSession, loserErr.code) | |||
| 893 | + | require.Contains(t, loserErr.Error(), "resume token is no longer valid") | |||
| 894 | + | require.True(t, loserTr.Closed() || len(loserTr.Sends()) == 0, "losing resume must not complete Welcome") | |||
| 895 | + | ||||
| 896 | + | d.mu.Lock() | |||
| 897 | + | _, oldTokenParked := d.parked[token] | |||
| 898 | + | _, stillParking := d.parking[token] | |||
| 899 | + | d.mu.Unlock() | |||
| 900 | + | require.False(t, oldTokenParked, "consumed credential must not remain resumeable") | |||
| 901 | + | require.False(t, stillParking, "parking marker must be consumed after park/resume") | |||
| 902 | + | } | |||
| 903 | + | ||||
| 904 | + | // TestStaleSnatchedParkClearsOnlyAbandonedParkingMarker covers a stale | |||
| 905 | + | // incarnation that publishes a parking marker then loses exact unroute. Only | |||
| 906 | + | // that attachment's abandoned marker is cleared; another attachment's marker | |||
| 907 | + | // and the fresh snatched incarnation remain intact. | |||
| 908 | + | func TestStaleSnatchedParkClearsOnlyAbandonedParkingMarker(t *testing.T) { | |||
| 909 | + | workPTY, releaseWork := newBlockingPTY(t) | |||
| 910 | + | defer releaseWork() | |||
| 911 | + | otherPTY, releaseOther := newBlockingPTY(t) | |||
| 912 | + | defer releaseOther() | |||
| 913 | + | // Separate PTYs so work teardown cannot cascade-close the other session. | |||
| 914 | + | d := newTestDaemon(t, newFactorySeq(t, workPTY, otherPTY), stubClock{}) | |||
| 915 | + | ||||
| 916 | + | sess, waiting, active, oldTr, token := setupResumableSnatchedAttachment(t, d) | |||
| 917 | + | stale := sess.attachmentToken(waiting, oldTr) | |||
| 918 | + | ||||
| 919 | + | otherTr := &closeTrackingTransport{} | |||
| 920 | + | otherSess, otherAC, err := d.route(helloResumeCapable(ports.IntentNew, "other", 0), otherTr) | |||
| 921 | + | require.NoError(t, err) | |||
| 922 | + | otherToken := d.markParkingInFlight(otherSess, otherAC) | |||
| 923 | + | require.NotZero(t, otherToken) | |||
| 924 | + | require.NotEqual(t, token, otherToken) | |||
| 925 | + | ||||
| 926 | + | fresh := &closeTrackingTransport{} | |||
| 927 | + | waiting.replaceTransport(fresh) | |||
| 928 | + | ||||
| 929 | + | require.False(t, d.parkSnatchedAttachment(stale)) | |||
| 930 | + | d.mu.Lock() | |||
| 931 | + | _, parked := d.parked[token] | |||
| 932 | + | _, stillParking := d.parking[token] | |||
| 933 | + | otherMarker := d.parking[otherToken] | |||
| 934 | + | d.mu.Unlock() | |||
| 935 | + | require.False(t, parked) | |||
| 936 | + | require.False(t, stillParking, "stale unroute must clear only the abandoned same-attachment marker") | |||
| 937 | + | require.NotNil(t, otherMarker, "stale unroute must not clear another attachment's marker") | |||
| 938 | + | require.Same(t, otherAC, otherMarker.ac) | |||
| 939 | + | require.Equal(t, attachmentSnatched, sess.attachmentRole(waiting)) | |||
| 940 | + | require.Same(t, fresh, waiting.transport()) | |||
| 941 | + | require.False(t, fresh.Closed()) | |||
| 942 | + | require.Same(t, active, sess.client) | |||
| 943 | + | require.Equal(t, token, waiting.resumeToken) | |||
| 944 | + | } | |||
| 945 | + | ||||
| 946 | + | // TestTerminalSnatchedRemovalClearsOrphanedParkingMarker covers the race where a | |||
| 947 | + | // park path publishes a same-attachment parking-in-flight marker, then terminal | |||
| 948 | + | // snatched removal wins before park completes. Only that attachment's marker is | |||
| 949 | + | // retired and waiters unblock; other markers stay intact and the stale token | |||
| 950 | + | // fails closed. A subsequent legitimate snatched park remains resumable. | |||
| 951 | + | func TestTerminalSnatchedRemovalClearsOrphanedParkingMarker(t *testing.T) { | |||
| 952 | + | workPTY, releaseWork := newBlockingPTY(t) | |||
| 953 | + | defer releaseWork() | |||
| 954 | + | otherPTY, releaseOther := newBlockingPTY(t) | |||
| 955 | + | defer releaseOther() | |||
| 956 | + | legitPTY, releaseLegit := newBlockingPTY(t) | |||
| 957 | + | defer releaseLegit() | |||
| 958 | + | // Separate PTYs so session teardown cannot cascade across marker fixtures. | |||
| 959 | + | d := newTestDaemon(t, newFactorySeq(t, workPTY, otherPTY, legitPTY), stubClock{}) | |||
| 960 | + | ||||
| 961 | + | sess, waiting, active, oldTr, token := setupResumableSnatchedAttachment(t, d) | |||
| 962 | + | roleToken := sess.attachmentToken(waiting, oldTr) | |||
| 963 | + | ||||
| 964 | + | otherTr := &closeTrackingTransport{} | |||
| 965 | + | otherSess, otherAC, err := d.route(helloResumeCapable(ports.IntentNew, "other", 0), otherTr) | |||
| 966 | + | require.NoError(t, err) | |||
| 967 | + | otherToken := d.markParkingInFlight(otherSess, otherAC) | |||
| 968 | + | require.NotZero(t, otherToken) | |||
| 969 | + | require.NotEqual(t, token, otherToken) | |||
| 970 | + | ||||
| 971 | + | // Simulate the losing park path publishing before terminal unroute wins. | |||
| 972 | + | require.Equal(t, token, d.markParkingInFlight(sess, waiting)) | |||
| 973 | + | d.mu.Lock() | |||
| 974 | + | orphaned := d.parking[token] | |||
| 975 | + | otherMarker := d.parking[otherToken] | |||
| 976 | + | d.mu.Unlock() | |||
| 977 | + | require.NotNil(t, orphaned, "fixture: park path must publish the parking marker") | |||
| 978 | + | require.Same(t, waiting, orphaned.ac) | |||
| 979 | + | require.NotNil(t, otherMarker, "fixture: other attachment marker must remain published") | |||
| 980 | + | require.Same(t, otherAC, otherMarker.ac) | |||
| 981 | + | ||||
| 982 | + | waiterArmed := make(chan struct{}) | |||
| 983 | + | d.afterParkingWaitArmed = func() { close(waiterArmed) } | |||
| 984 | + | waiterDone := make(chan bool, 1) | |||
| 985 | + | go func() { | |||
| 986 | + | waiterDone <- d.waitParkingInFlight(helloResumeCapable(ports.IntentResume, "work", token)) | |||
| 987 | + | }() | |||
| 988 | + | awaitTestCompletion(t, waiterArmed, "parking waiter did not arm on the orphaned snatched marker") | |||
| 989 | + | ||||
| 990 | + | require.True(t, d.removeSnatchedAttachment(roleToken), "terminal snatched removal must win exact unroute") | |||
| 991 | + | require.Equal(t, attachmentDetached, sess.attachmentRole(waiting)) | |||
| 992 | + | require.Same(t, active, sess.client, "terminal snatched removal must not disturb the active owner") | |||
| 993 | + | ||||
| 994 | + | waited := awaitTestValue(t, waiterDone, "parking waiter stayed blocked after terminal snatched removal") | |||
| 995 | + | require.True(t, waited, "waiter must observe the same-attachment marker before it is cleared") | |||
| 996 | + | ||||
| 997 | + | d.mu.Lock() | |||
| 998 | + | _, stillParking := d.parking[token] | |||
| 999 | + | otherAfter := d.parking[otherToken] | |||
| 1000 | + | _, parked := d.parked[token] | |||
| 1001 | + | d.mu.Unlock() | |||
| 1002 | + | require.False(t, stillParking, "terminal winner must clear the same attachment's orphaned marker") | |||
| 1003 | + | require.False(t, parked, "terminal removal must not publish parked ownership") | |||
| 1004 | + | require.NotNil(t, otherAfter, "terminal winner must not clear another attachment's marker") | |||
| 1005 | + | require.Same(t, otherAC, otherAfter.ac) | |||
| 1006 | + | ||||
| 1007 | + | _, _, err = d.route(helloResumeCapable(ports.IntentResume, "work", 0xdecafbad), &closeTrackingTransport{}) | |||
| 1008 | + | require.Error(t, err) | |||
| 1009 | + | var unknownErr *protoErr | |||
| 1010 | + | require.ErrorAs(t, err, &unknownErr) | |||
| 1011 | + | require.Equal(t, ports.ErrNoSuchSession, unknownErr.code) | |||
| 1012 | + | require.Contains(t, unknownErr.Error(), "resume token is no longer valid") | |||
| 1013 | + | ||||
| 1014 | + | _, _, err = d.route(helloResumeCapable(ports.IntentResume, "work", token), &closeTrackingTransport{}) | |||
| 1015 | + | require.Error(t, err) | |||
| 1016 | + | var retiredErr *protoErr | |||
| 1017 | + | require.ErrorAs(t, err, &retiredErr) | |||
| 1018 | + | require.Equal(t, ports.ErrNoSuchSession, retiredErr.code) | |||
| 1019 | + | require.Contains(t, retiredErr.Error(), "resume token is no longer valid") | |||
| 1020 | + | ||||
| 1021 | + | // A later legitimate snatched park on a fresh attachment must still resume. | |||
| 1022 | + | legitTr := &closeTrackingTransport{} | |||
| 1023 | + | legitSess, legitWaiting, err := d.route(helloResumeCapable(ports.IntentNew, "legit", 0), legitTr) | |||
| 1024 | + | require.NoError(t, err) | |||
| 1025 | + | legitToken := legitWaiting.resumeToken | |||
| 1026 | + | require.NotZero(t, legitToken) | |||
| 1027 | + | _, _, err = d.route(helloResumeCapable(ports.IntentAttach, "legit", 0), &closeTrackingTransport{}) | |||
| 1028 | + | require.NoError(t, err) | |||
| 1029 | + | d.attachmentCleanupWg.Wait() | |||
| 1030 | + | require.Equal(t, attachmentSnatched, legitSess.attachmentRole(legitWaiting)) | |||
| 1031 | + | require.True(t, d.parkSnatchedAttachment(legitSess.attachmentToken(legitWaiting, legitTr))) | |||
| 1032 | + | ||||
| 1033 | + | resumeTr := &closeTrackingTransport{} | |||
| 1034 | + | routedSess, routedAC, err := d.route(helloResumeCapable(ports.IntentResume, "legit", legitToken), resumeTr) | |||
| 1035 | + | require.NoError(t, err) | |||
| 1036 | + | require.Same(t, legitSess, routedSess) | |||
| 1037 | + | require.Same(t, legitWaiting, routedAC) | |||
| 1038 | + | require.Equal(t, attachmentSnatched, legitSess.attachmentRole(legitWaiting)) | |||
| 1039 | + | require.Same(t, resumeTr, legitWaiting.transport()) | |||
| 1040 | + | } | |||
| 1041 | + | ||||
| 1042 | + | // TestExplicitDetachClearsOrphanedSameAttachmentParkingMarker covers the race | |||
| 1043 | + | // where non-explicit teardown publishes a parking-in-flight marker, then | |||
| 1044 | + | // explicit detach wins the seat. The winner must clear only that attachment's | |||
| 1045 | + | // orphaned marker and unblock waiters; other attachments' markers and a later | |||
| 1046 | + | // parked lifecycle must stay intact. Unknown tokens stay fail-closed. | |||
| 1047 | + | func TestExplicitDetachClearsOrphanedSameAttachmentParkingMarker(t *testing.T) { | |||
| 1048 | + | workPTY, releaseWork := newBlockingPTY(t) | |||
| 1049 | + | defer releaseWork() | |||
| 1050 | + | otherPTY, releaseOther := newBlockingPTY(t) | |||
| 1051 | + | defer releaseOther() | |||
| 1052 | + | // Separate PTYs keep the other session alive while work detaches. | |||
| 1053 | + | d := newTestDaemon(t, newFactorySeq(t, workPTY, otherPTY), stubClock{}) | |||
| 1054 | + | ||||
| 1055 | + | oldTr := &closeTrackingTransport{} | |||
| 1056 | + | sess, ac, err := d.route(helloResumeCapable(ports.IntentNew, "work", 0), oldTr) | |||
| 1057 | + | require.NoError(t, err) | |||
| 1058 | + | token := ac.resumeToken | |||
| 1059 | + | require.NotZero(t, token) | |||
| 1060 | + | ||||
| 1061 | + | otherTr := &closeTrackingTransport{} | |||
| 1062 | + | otherSess, otherAC, err := d.route(helloResumeCapable(ports.IntentNew, "other", 0), otherTr) | |||
| 1063 | + | require.NoError(t, err) | |||
| 1064 | + | otherToken := d.markParkingInFlight(otherSess, otherAC) | |||
| 1065 | + | require.NotZero(t, otherToken) | |||
| 1066 | + | require.NotEqual(t, token, otherToken) | |||
| 1067 | + | ||||
| 1068 | + | // Pre-mark as a same-attachment non-explicit teardown would before detach. | |||
| 1069 | + | require.Equal(t, token, d.markParkingInFlight(sess, ac)) | |||
| 1070 | + | d.mu.Lock() | |||
| 1071 | + | orphaned := d.parking[token] | |||
| 1072 | + | otherMarker := d.parking[otherToken] | |||
| 1073 | + | d.mu.Unlock() | |||
| 1074 | + | require.NotNil(t, orphaned, "fixture: non-explicit teardown must publish the parking marker") | |||
| 1075 | + | require.Same(t, ac, orphaned.ac) | |||
| 1076 | + | require.NotNil(t, otherMarker, "fixture: other attachment marker must remain published") | |||
| 1077 | + | require.Same(t, otherAC, otherMarker.ac) | |||
| 1078 | + | ||||
| 1079 | + | waiterArmed := make(chan struct{}) | |||
| 1080 | + | d.afterParkingWaitArmed = func() { close(waiterArmed) } | |||
| 1081 | + | waiterDone := make(chan bool, 1) | |||
| 1082 | + | go func() { | |||
| 1083 | + | waiterDone <- d.waitParkingInFlight(helloResumeCapable(ports.IntentResume, "work", token)) | |||
| 1084 | + | }() | |||
| 1085 | + | awaitTestCompletion(t, waiterArmed, "parking waiter did not arm on the orphaned marker") | |||
| 1086 | + | ||||
| 1087 | + | d.clientGone(sess, ac, oldTr, true) | |||
| 1088 | + | require.Nil(t, sess.client, "explicit detach must clear the live owner") | |||
| 1089 | + | ||||
| 1090 | + | waited := awaitTestValue(t, waiterDone, "parking waiter stayed blocked after explicit detach") | |||
| 1091 | + | require.True(t, waited, "waiter must observe the same-attachment marker before it is cleared") | |||
| 1092 | + | ||||
| 1093 | + | d.mu.Lock() | |||
| 1094 | + | _, stillParking := d.parking[token] | |||
| 1095 | + | otherAfter := d.parking[otherToken] | |||
| 1096 | + | d.mu.Unlock() | |||
| 1097 | + | require.False(t, stillParking, "explicit winner must clear the same attachment's orphaned marker") | |||
| 1098 | + | require.NotNil(t, otherAfter, "explicit winner must not clear another attachment's marker") | |||
| 1099 | + | require.Same(t, otherAC, otherAfter.ac) | |||
| 1100 | + | ||||
| 1101 | + | _, _, err = d.route(helloResumeCapable(ports.IntentResume, "work", 0xdecafbad), &closeTrackingTransport{}) | |||
| 1102 | + | require.Error(t, err) | |||
| 1103 | + | var unknownErr *protoErr | |||
| 1104 | + | require.ErrorAs(t, err, &unknownErr) | |||
| 1105 | + | require.Equal(t, ports.ErrNoSuchSession, unknownErr.code) | |||
| 1106 | + | require.Contains(t, unknownErr.Error(), "resume token is no longer valid") | |||
| 1107 | + | ||||
| 1108 | + | _, _, err = d.route(helloResumeCapable(ports.IntentResume, "work", token), &closeTrackingTransport{}) | |||
| 1109 | + | require.Error(t, err) | |||
| 1110 | + | var retiredErr *protoErr | |||
| 1111 | + | require.ErrorAs(t, err, &retiredErr) | |||
| 1112 | + | require.Equal(t, ports.ErrNoSuchSession, retiredErr.code) | |||
| 1113 | + | require.Contains(t, retiredErr.Error(), "resume token is no longer valid") | |||
| 1114 | + | ||||
| 1115 | + | // A later parked lifecycle for another attachment must survive a clear | |||
| 1116 | + | // scoped to the retired attachment identity. | |||
| 1117 | + | require.True(t, d.parkAttachment(otherSess, otherAC)) | |||
| 1118 | + | d.mu.Lock() | |||
| 1119 | + | parkedOther := d.parked[otherToken] | |||
| 1120 | + | _, otherStillParking := d.parking[otherToken] | |||
| 1121 | + | d.mu.Unlock() | |||
| 1122 | + | require.NotNil(t, parkedOther, "subsequent park must publish the other attachment") | |||
| 1123 | + | require.False(t, otherStillParking, "park consumes the other attachment's in-flight marker") | |||
| 1124 | + | ||||
| 1125 | + | d.clearParkingInFlight(otherToken, ac) | |||
| 1126 | + | d.mu.Lock() | |||
| 1127 | + | parkedAfterMismatchedClear := d.parked[otherToken] | |||
| 1128 | + | d.mu.Unlock() | |||
| 1129 | + | require.Same(t, parkedOther, parkedAfterMismatchedClear, "mismatched clear must not drop a subsequent parked lifecycle") | |||
| 1130 | + | } | |||
| 1131 | + | ||||
| 1132 | + | // TestLiveResumeRejectsLateMarkerAfterTerminalCleanupWins covers the race where | |||
| 1133 | + | // live resume has validated the same-client credential, then explicit detach or | |||
| 1134 | + | // session removal wins before markParkingInFlight. Late marker publication must | |||
| 1135 | + | // be rejected so IntentResume returns promptly ErrNoSuchSession with no parking | |||
| 1136 | + | // marker left to strand waiters. Direct parkAttachment after detach still works. | |||
| 1137 | + | func TestLiveResumeRejectsLateMarkerAfterTerminalCleanupWins(t *testing.T) { | |||
| 1138 | + | cases := []struct { | |||
| 1139 | + | name string | |||
| 1140 | + | win func(t *testing.T, d *Daemon, sess *session, ac *attachedClient, oldTr *closeTrackingTransport, token uint64) | |||
| 1141 | + | }{ | |||
| 1142 | + | { | |||
| 1143 | + | name: "explicit_detach", | |||
| 1144 | + | win: func(t *testing.T, d *Daemon, sess *session, ac *attachedClient, oldTr *closeTrackingTransport, token uint64) { | |||
| 1145 | + | t.Helper() | |||
| 1146 | + | d.clientGone(sess, ac, oldTr, true) | |||
| 1147 | + | require.Nil(t, sess.client, "explicit detach must clear the live owner before late mark") | |||
| 1148 | + | }, | |||
| 1149 | + | }, | |||
| 1150 | + | { | |||
| 1151 | + | name: "session_removal", | |||
| 1152 | + | win: func(t *testing.T, d *Daemon, sess *session, ac *attachedClient, oldTr *closeTrackingTransport, token uint64) { | |||
| 1153 | + | t.Helper() | |||
| 1154 | + | require.NoError(t, d.killSession(sess, ports.ReasonSessionKilled, true)) | |||
| 1155 | + | d.mu.Lock() | |||
| 1156 | + | registered := d.sessions[sess.id] == sess | |||
| 1157 | + | d.mu.Unlock() | |||
| 1158 | + | require.False(t, registered, "killSession must unregister before late mark") | |||
| 1159 | + | }, | |||
| 1160 | + | }, | |||
| 1161 | + | } | |||
| 1162 | + | for _, tc := range cases { | |||
| 1163 | + | t.Run(tc.name, func(t *testing.T) { | |||
| 1164 | + | pty, release := newBlockingPTY(t) | |||
| 1165 | + | defer release() | |||
| 1166 | + | d := newTestDaemon(t, newFactory(t, pty), stubClock{}) | |||
| 1167 | + | ||||
| 1168 | + | oldTr := &closeTrackingTransport{} | |||
| 1169 | + | sess, ac, err := d.route(helloResumeCapable(ports.IntentNew, "work", 0), oldTr) | |||
| 1170 | + | require.NoError(t, err) | |||
| 1171 | + | token := ac.resumeToken | |||
| 1172 | + | require.NotZero(t, token) | |||
| 1173 | + | require.Same(t, ac, sess.client) | |||
| 1174 | + | ||||
| 1175 | + | validated := make(chan struct{}) | |||
| 1176 | + | releaseMark := make(chan struct{}) | |||
| 1177 | + | var releaseOnce sync.Once | |||
| 1178 | + | t.Cleanup(func() { releaseOnce.Do(func() { close(releaseMark) }) }) | |||
| 1179 | + | d.beforeMarkParkingInFlight = func() { | |||
| 1180 | + | close(validated) | |||
| 1181 | + | <-releaseMark | |||
| 1182 | + | } | |||
| 1183 | + | ||||
| 1184 | + | type routeResult struct { | |||
| 1185 | + | err error | |||
| 1186 | + | } | |||
| 1187 | + | result := make(chan routeResult, 1) | |||
| 1188 | + | go func() { | |||
| 1189 | + | _, _, routeErr := d.route(helloResumeCapable(ports.IntentResume, "work", token), &closeTrackingTransport{}) | |||
| 1190 | + | result <- routeResult{err: routeErr} | |||
| 1191 | + | }() | |||
| 1192 | + | awaitTestCompletion(t, validated, "live resume did not pause after credential validation before mark") | |||
| 1193 | + | ||||
| 1194 | + | tc.win(t, d, sess, ac, oldTr, token) | |||
| 1195 | + | ||||
| 1196 | + | d.mu.Lock() | |||
| 1197 | + | _, parkingBeforeRelease := d.parking[token] | |||
| 1198 | + | d.mu.Unlock() | |||
| 1199 | + | require.False(t, parkingBeforeRelease, "terminal cleanup must not leave a parking marker before late mark resumes") | |||
| 1200 | + | ||||
| 1201 | + | releaseOnce.Do(func() { close(releaseMark) }) | |||
| 1202 | + | got := awaitTestValue(t, result, "live IntentResume stayed blocked after terminal cleanup won before mark") | |||
| 1203 | + | require.Error(t, got.err) | |||
| 1204 | + | var pe *protoErr | |||
| 1205 | + | require.ErrorAs(t, got.err, &pe) | |||
| 1206 | + | require.Equal(t, ports.ErrNoSuchSession, pe.code) | |||
| 1207 | + | require.Contains(t, pe.Error(), "resume token is no longer valid") | |||
| 1208 | + | ||||
| 1209 | + | d.mu.Lock() | |||
| 1210 | + | _, stillParking := d.parking[token] | |||
| 1211 | + | _, stillParked := d.parked[token] | |||
| 1212 | + | d.mu.Unlock() | |||
| 1213 | + | require.False(t, stillParking, "late markParkingInFlight must not recreate a parking marker after terminal cleanup") | |||
| 1214 | + | require.False(t, stillParked, "terminal cleanup must not leave a parked credential") | |||
| 1215 | + | }) | |||
| 1216 | + | } | |||
| 1217 | + | ||||
| 1218 | + | t.Run("direct_park_after_detach_without_marker", func(t *testing.T) { | |||
| 1219 | + | pty, release := newBlockingPTY(t) | |||
| 1220 | + | defer release() | |||
| 1221 | + | d := newTestDaemon(t, newFactory(t, pty), stubClock{}) | |||
| 1222 | + | ||||
| 1223 | + | oldTr := &closeTrackingTransport{} | |||
| 1224 | + | sess, ac, err := d.route(helloResumeCapable(ports.IntentNew, "work", 0), oldTr) | |||
| 1225 | + | require.NoError(t, err) | |||
| 1226 | + | token := ac.resumeToken | |||
| 1227 | + | require.NotZero(t, token) | |||
| 1228 | + | ||||
| 1229 | + | require.True(t, d.detachIfCurrentTransport(sess, ac, ac.transportSnapshot())) | |||
| 1230 | + | require.Nil(t, sess.client) | |||
| 1231 | + | require.Zero(t, d.markParkingInFlight(sess, ac), "detached active attachment must not publish a late marker") | |||
| 1232 | + | ||||
| 1233 | + | d.mu.Lock() | |||
| 1234 | + | _, parkingBeforePark := d.parking[token] | |||
| 1235 | + | d.mu.Unlock() | |||
| 1236 | + | require.False(t, parkingBeforePark) | |||
| 1237 | + | ||||
| 1238 | + | require.True(t, d.parkAttachment(sess, ac), "direct parkAttachment after detach must still park without recreating an in-flight marker") | |||
| 1239 | + | d.mu.Lock() | |||
| 1240 | + | parked := d.parked[token] | |||
| 1241 | + | _, stillParking := d.parking[token] | |||
| 1242 | + | d.mu.Unlock() | |||
| 1243 | + | require.NotNil(t, parked) | |||
| 1244 | + | require.Same(t, ac, parked.ac) | |||
| 1245 | + | require.False(t, stillParking, "park must not leave an in-flight marker") | |||
| 1246 | + | }) | |||
| 1247 | + | } | |||
| 1248 | + | ||||
| 1249 | + | // TestMarkParkingInFlightRequiresExactLiveOwnership covers lock-safe rejection | |||
| 1250 | + | // when the attachment is no longer the exact active or snatched owner, while | |||
| 1251 | + | // still advertising for both live roles. | |||
| 1252 | + | func TestMarkParkingInFlightRequiresExactLiveOwnership(t *testing.T) { | |||
| 1253 | + | pty, release := newBlockingPTY(t) | |||
| 1254 | + | defer release() | |||
| 1255 | + | d := newTestDaemon(t, newFactory(t, pty), stubClock{}) | |||
| 1256 | + | ||||
| 1257 | + | oldTr := &closeTrackingTransport{} | |||
| 1258 | + | sess, waiting, err := d.route(helloResumeCapable(ports.IntentNew, "work", 0), oldTr) | |||
| 1259 | + | require.NoError(t, err) | |||
| 1260 | + | token := waiting.resumeToken | |||
| 1261 | + | require.NotZero(t, token) | |||
| 1262 | + | ||||
| 1263 | + | require.Equal(t, token, d.markParkingInFlight(sess, waiting), "active owner must publish") | |||
| 1264 | + | d.clearParkingInFlight(token, waiting) | |||
| 1265 | + | ||||
| 1266 | + | _, active, err := d.route(helloResumeCapable(ports.IntentAttach, "work", 0), &closeTrackingTransport{}) | |||
| 1267 | + | require.NoError(t, err) | |||
| 1268 | + | d.attachmentCleanupWg.Wait() | |||
| 1269 | + | require.Equal(t, attachmentSnatched, sess.attachmentRole(waiting)) | |||
| 1270 | + | require.Same(t, active, sess.client) | |||
| 1271 | + | ||||
| 1272 | + | require.Equal(t, token, d.markParkingInFlight(sess, waiting), "snatched owner must publish") | |||
| 1273 | + | d.clearParkingInFlight(token, waiting) | |||
| 1274 | + | ||||
| 1275 | + | roleToken := sess.attachmentToken(waiting, oldTr) | |||
| 1276 | + | require.True(t, d.unrouteSnatchedAttachment(roleToken, false)) | |||
| 1277 | + | require.Equal(t, attachmentDetached, sess.attachmentRole(waiting)) | |||
| 1278 | + | require.Zero(t, d.markParkingInFlight(sess, waiting), "detached snatched attachment must not publish") | |||
| 1279 | + | ||||
| 1280 | + | d.mu.Lock() | |||
| 1281 | + | _, stillParking := d.parking[token] | |||
| 1282 | + | d.mu.Unlock() | |||
| 1283 | + | require.False(t, stillParking) | |||
| 1284 | + | ||||
| 1285 | + | require.Equal(t, active.resumeToken, d.markParkingInFlight(sess, active), "active owner must still publish after snatched unroute") | |||
| 1286 | + | d.clearParkingInFlight(active.resumeToken, active) | |||
| 1287 | + | } | |||
| 1288 | + | ||||
| 478 | // TestStaleClientGoneAfterTransportCheckDoesNotDetachReboundAttachment covers | 1289 | // TestStaleClientGoneAfterTransportCheckDoesNotDetachReboundAttachment covers | ||
| 479 | // the TOCTOU where clientGone passes the old-transport precheck, then a live | 1290 | // the TOCTOU where clientGone passes the old-transport precheck, then a live | ||
| 480 | // resume rebinds the same attachment before detach runs. | 1291 | // resume rebinds the same attachment before detach runs. | ||
| @@ -947,6 +1758,76 @@ func TestKilledSessionPurgesParkedResumeToken(t *testing.T) { | |||||
| 947 | require.False(t, ok, "killed session cannot be resumed") | 1758 | require.False(t, ok, "killed session cannot be resumed") | ||
| 948 | } | 1759 | } | ||
| 949 | 1760 | ||||
| 1761 | + | // TestKilledSessionUnblocksParkingWaiterFailsClosed covers session removal while | |||
| 1762 | + | // a same-token IntentResume is waiting on an in-flight parking marker. killSession | |||
| 1763 | + | // must purge only that session's markers so the waiter unblocks and fails closed, | |||
| 1764 | + | // without touching another session's parking marker. | |||
| 1765 | + | func TestKilledSessionUnblocksParkingWaiterFailsClosed(t *testing.T) { | |||
| 1766 | + | workPTY, releaseWork := newBlockingPTY(t) | |||
| 1767 | + | defer releaseWork() | |||
| 1768 | + | otherPTY, releaseOther := newBlockingPTY(t) | |||
| 1769 | + | defer releaseOther() | |||
| 1770 | + | // Separate PTYs are required: a shared mock PTY makes killSession(work) | |||
| 1771 | + | // close other's pane and cascade-remove the other session under -race, | |||
| 1772 | + | // falsely failing the other-marker / fail-closed assertions. | |||
| 1773 | + | d := newTestDaemon(t, newFactorySeq(t, workPTY, otherPTY), stubClock{}) | |||
| 1774 | + | ||||
| 1775 | + | oldTr := &closeTrackingTransport{} | |||
| 1776 | + | sess, ac, err := d.route(helloResumeCapable(ports.IntentNew, "work", 0), oldTr) | |||
| 1777 | + | require.NoError(t, err) | |||
| 1778 | + | token := ac.resumeToken | |||
| 1779 | + | require.NotZero(t, token) | |||
| 1780 | + | ||||
| 1781 | + | otherTr := &closeTrackingTransport{} | |||
| 1782 | + | otherSess, otherAC, err := d.route(helloResumeCapable(ports.IntentNew, "other", 0), otherTr) | |||
| 1783 | + | require.NoError(t, err) | |||
| 1784 | + | otherToken := d.markParkingInFlight(otherSess, otherAC) | |||
| 1785 | + | require.NotZero(t, otherToken) | |||
| 1786 | + | require.NotEqual(t, token, otherToken) | |||
| 1787 | + | ||||
| 1788 | + | require.Equal(t, token, d.markParkingInFlight(sess, ac)) | |||
| 1789 | + | d.mu.Lock() | |||
| 1790 | + | parkingMarker := d.parking[token] | |||
| 1791 | + | otherMarker := d.parking[otherToken] | |||
| 1792 | + | d.mu.Unlock() | |||
| 1793 | + | require.NotNil(t, parkingMarker, "fixture: parking marker must be published before kill") | |||
| 1794 | + | require.Same(t, ac, parkingMarker.ac) | |||
| 1795 | + | require.NotNil(t, otherMarker, "fixture: other session marker must remain published") | |||
| 1796 | + | require.Same(t, otherAC, otherMarker.ac) | |||
| 1797 | + | ||||
| 1798 | + | waiterArmed := make(chan struct{}) | |||
| 1799 | + | d.afterParkingWaitArmed = func() { close(waiterArmed) } | |||
| 1800 | + | waiterDone := make(chan bool, 1) | |||
| 1801 | + | go func() { | |||
| 1802 | + | waiterDone <- d.waitParkingInFlight(helloResumeCapable(ports.IntentResume, "work", token)) | |||
| 1803 | + | }() | |||
| 1804 | + | awaitTestCompletion(t, waiterArmed, "parking waiter did not arm before session kill") | |||
| 1805 | + | ||||
| 1806 | + | require.NoError(t, d.killSession(sess, ports.ReasonSessionKilled, true)) | |||
| 1807 | + | ||||
| 1808 | + | waited := awaitTestValue(t, waiterDone, "parking waiter stayed blocked after session kill") | |||
| 1809 | + | require.True(t, waited, "waiter must observe the session marker before it is purged") | |||
| 1810 | + | ||||
| 1811 | + | d.mu.Lock() | |||
| 1812 | + | _, stillParking := d.parking[token] | |||
| 1813 | + | otherAfter := d.parking[otherToken] | |||
| 1814 | + | otherRegistered := d.sessions[otherSess.id] == otherSess | |||
| 1815 | + | closing := d.closing | |||
| 1816 | + | d.mu.Unlock() | |||
| 1817 | + | require.False(t, stillParking, "killSession must purge the killed session's parking marker") | |||
| 1818 | + | require.NotNil(t, otherAfter, "killSession must not purge another session's parking marker") | |||
| 1819 | + | require.Same(t, otherAC, otherAfter.ac) | |||
| 1820 | + | require.True(t, otherRegistered, "other session must survive work kill when PTYs are isolated") | |||
| 1821 | + | require.False(t, closing, "daemon must not shut down while the other session remains") | |||
| 1822 | + | ||||
| 1823 | + | _, _, err = d.route(helloResumeCapable(ports.IntentResume, "work", token), &closeTrackingTransport{}) | |||
| 1824 | + | require.Error(t, err) | |||
| 1825 | + | var killedErr *protoErr | |||
| 1826 | + | require.ErrorAs(t, err, &killedErr) | |||
| 1827 | + | require.Equal(t, ports.ErrNoSuchSession, killedErr.code) | |||
| 1828 | + | require.Contains(t, killedErr.Error(), "resume token is no longer valid") | |||
| 1829 | + | } | |||
| 1830 | + | ||||
| 950 | func TestParkedPredecessorResumesSnatchedWithoutStealingActiveAttachment(t *testing.T) { | 1831 | func TestParkedPredecessorResumesSnatchedWithoutStealingActiveAttachment(t *testing.T) { | ||
| 951 | pty, release := newBlockingPTY(t) | 1832 | pty, release := newBlockingPTY(t) | ||
| 952 | defer release() | 1833 | defer release() | ||
M internal/usecase/daemon/session.go +1 -0
| @@ -1481,6 +1481,7 @@ func (d *Daemon) killSessionWithSnapshotDeadline(sess *session, reason uint8, pu | |||
| 1481 | 1481 | // while the complete immutable gate set remains frozen. | |
| 1482 | 1482 | delete(d.sessions, sess.id) | |
| 1483 | 1483 | d.clearBarScriptsForSession(sess.id) | |
| 1484 | + | d.purgeParkingForSessionLocked(sess) | |
| 1484 | 1485 | parkedRetirements := d.purgeParkedForSessionLocked(sess) | |
| 1485 | 1486 | detachedActive := sess.client | |
| 1486 | 1487 | attachments := make([]detachedAttachmentSnapshot, 0, len(participants.roleGates)) | |
| @@ -1481,6 +1481,7 @@ func (d *Daemon) killSessionWithSnapshotDeadline(sess *session, reason uint8, pu | |||||
| 1481 | // while the complete immutable gate set remains frozen. | 1481 | // while the complete immutable gate set remains frozen. | ||
| 1482 | delete(d.sessions, sess.id) | 1482 | delete(d.sessions, sess.id) | ||
| 1483 | d.clearBarScriptsForSession(sess.id) | 1483 | d.clearBarScriptsForSession(sess.id) | ||
| 1484 | + | d.purgeParkingForSessionLocked(sess) | |||
| 1484 | parkedRetirements := d.purgeParkedForSessionLocked(sess) | 1485 | parkedRetirements := d.purgeParkedForSessionLocked(sess) | ||
| 1485 | detachedActive := sess.client | 1486 | detachedActive := sess.client | ||
| 1486 | attachments := make([]detachedAttachmentSnapshot, 0, len(participants.roleGates)) | 1487 | attachments := make([]detachedAttachmentSnapshot, 0, len(participants.roleGates)) | ||
M internal/usecase/daemon/snatched_cleanup.go +43 -5
| @@ -37,21 +37,43 @@ func (d *Daemon) unrouteSnatchedAttachment(token attachmentRoleToken, terminal b | |||
| 37 | 37 | } | |
| 38 | 38 | token.ac.setSession(nil) | |
| 39 | 39 | token.ac.clearCaptureFrames() | |
| 40 | + | if terminal { | |
| 41 | + | // A losing park path may have published a same-attachment marker before | |
| 42 | + | // this terminal unroute won. Retire only that attachment's marker so | |
| 43 | + | // waiters fail closed instead of hanging; park paths (terminal=false) | |
| 44 | + | // keep the marker until parkAttachmentAs or abandonment cleanup. | |
| 45 | + | d.clearParkingInFlight(token.ac.resumeToken, token.ac) | |
| 46 | + | } | |
| 40 | 47 | return true | |
| 41 | 48 | } | |
| 42 | 49 | ||
| 43 | 50 | // parkSnatchedAttachment removes only the exact lost snatched incarnation and | |
| 44 | 51 | // retains its resume identity for a role-preserving reconnect. | |
| 45 | 52 | func (d *Daemon) parkSnatchedAttachment(token attachmentRoleToken) bool { | |
| 46 | - | if token.ac == nil || !token.ac.resumeCapable || !d.unrouteSnatchedAttachment(token, false) { | |
| 53 | + | if token.ac == nil || !token.ac.resumeCapable { | |
| 47 | 54 | return false | |
| 48 | 55 | } | |
| 56 | + | // Advertise parking before unroute so IntentResume never observes both the | |
| 57 | + | // snatched seat and parking/parked registries empty for a still-valid token. | |
| 58 | + | parkingToken := d.markParkingInFlight(token.sess, token.ac) | |
| 59 | + | if !d.unrouteSnatchedAttachment(token, false) { | |
| 60 | + | d.clearParkingInFlightIfAbandoned(token.sess, token.ac, parkingToken) | |
| 61 | + | return false | |
| 62 | + | } | |
| 63 | + | if d.afterSnatchedUnrouteBeforePark != nil { | |
| 64 | + | d.afterSnatchedUnrouteBeforePark() | |
| 65 | + | } | |
| 49 | 66 | if d.parkAttachmentAs(token.sess, token.ac, attachmentSnatched) { | |
| 50 | - | _ = token.ac.closeCapturedTransport(token.ac.revokeTransport(token.transport.transport)) | |
| 67 | + | captured := token.transport.transport | |
| 68 | + | _ = token.ac.revokeTransport(captured) | |
| 69 | + | _ = token.ac.closeCapturedTransport(captured) | |
| 51 | 70 | return true | |
| 52 | 71 | } | |
| 72 | + | d.clearParkingInFlight(parkingToken, token.ac) | |
| 53 | 73 | token.ac.clearPreviousSession() | |
| 54 | - | _ = token.ac.closeCapturedTransport(token.ac.revokeTransport(token.transport.transport)) | |
| 74 | + | captured := token.transport.transport | |
| 75 | + | _ = token.ac.revokeTransport(captured) | |
| 76 | + | _ = token.ac.closeCapturedTransport(captured) | |
| 55 | 77 | return false | |
| 56 | 78 | } | |
| 57 | 79 | ||
| @@ -85,6 +107,13 @@ func (d *Daemon) cleanupInterruptedSnatchedAttachment(token attachmentRoleToken) | |||
| 85 | 107 | return false | |
| 86 | 108 | } | |
| 87 | 109 | ||
| 110 | + | // Advertise parking before ownership removal so IntentResume can wait out | |
| 111 | + | // the unroute→park gap for a still-valid snatched credential. | |
| 112 | + | var parkingToken uint64 | |
| 113 | + | if token.ac.resumeCapable { | |
| 114 | + | parkingToken = d.markParkingInFlight(token.sess, token.ac) | |
| 115 | + | } | |
| 116 | + | ||
| 88 | 117 | // The frozen gate prevents a promotion from publishing between overlay | |
| 89 | 118 | // cleanup and terminal registry publication. | |
| 90 | 119 | d.clearForSnatch(token) | |
| @@ -102,17 +131,26 @@ func (d *Daemon) cleanupInterruptedSnatchedAttachment(token attachmentRoleToken) | |||
| 102 | 131 | token.sess.mu.Unlock() | |
| 103 | 132 | d.notices.routingMu.Unlock() | |
| 104 | 133 | if !current { | |
| 134 | + | d.clearParkingInFlightIfAbandoned(token.sess, token.ac, parkingToken) | |
| 105 | 135 | return false | |
| 106 | 136 | } | |
| 107 | 137 | ||
| 108 | 138 | token.ac.setSession(nil) | |
| 139 | + | if d.afterSnatchedUnrouteBeforePark != nil { | |
| 140 | + | d.afterSnatchedUnrouteBeforePark() | |
| 141 | + | } | |
| 109 | 142 | if token.ac.resumeCapable && d.parkAttachmentAs(token.sess, token.ac, attachmentSnatched) { | |
| 110 | - | _ = token.ac.closeCapturedTransport(token.ac.revokeTransport(token.transport.transport)) | |
| 143 | + | captured := token.transport.transport | |
| 144 | + | _ = token.ac.revokeTransport(captured) | |
| 145 | + | _ = token.ac.closeCapturedTransport(captured) | |
| 111 | 146 | token.ac.clearSnatchedInput() | |
| 112 | 147 | d.unregisterPreview(token.ac) | |
| 113 | 148 | return true | |
| 114 | 149 | } | |
| 115 | - | _ = token.ac.closeCapturedTransport(token.ac.revokeTransport(token.transport.transport)) | |
| 150 | + | d.clearParkingInFlight(parkingToken, token.ac) | |
| 151 | + | captured := token.transport.transport | |
| 152 | + | _ = token.ac.revokeTransport(captured) | |
| 153 | + | _ = token.ac.closeCapturedTransport(captured) | |
| 116 | 154 | token.ac.clearSnatchedInput() | |
| 117 | 155 | d.unregisterPreview(token.ac) | |
| 118 | 156 | token.ac.clearPreviousSession() | |
| @@ -37,21 +37,43 @@ func (d *Daemon) unrouteSnatchedAttachment(token attachmentRoleToken, terminal b | |||||
| 37 | } | 37 | } | ||
| 38 | token.ac.setSession(nil) | 38 | token.ac.setSession(nil) | ||
| 39 | token.ac.clearCaptureFrames() | 39 | token.ac.clearCaptureFrames() | ||
| 40 | + | if terminal { | |||
| 41 | + | // A losing park path may have published a same-attachment marker before | |||
| 42 | + | // this terminal unroute won. Retire only that attachment's marker so | |||
| 43 | + | // waiters fail closed instead of hanging; park paths (terminal=false) | |||
| 44 | + | // keep the marker until parkAttachmentAs or abandonment cleanup. | |||
| 45 | + | d.clearParkingInFlight(token.ac.resumeToken, token.ac) | |||
| 46 | + | } | |||
| 40 | return true | 47 | return true | ||
| 41 | } | 48 | } | ||
| 42 | 49 | ||||
| 43 | // parkSnatchedAttachment removes only the exact lost snatched incarnation and | 50 | // parkSnatchedAttachment removes only the exact lost snatched incarnation and | ||
| 44 | // retains its resume identity for a role-preserving reconnect. | 51 | // retains its resume identity for a role-preserving reconnect. | ||
| 45 | func (d *Daemon) parkSnatchedAttachment(token attachmentRoleToken) bool { | 52 | func (d *Daemon) parkSnatchedAttachment(token attachmentRoleToken) bool { | ||
| 46 | - | if token.ac == nil || !token.ac.resumeCapable || !d.unrouteSnatchedAttachment(token, false) { | 53 | + | if token.ac == nil || !token.ac.resumeCapable { |
| 47 | return false | 54 | return false | ||
| 48 | } | 55 | } | ||
| 56 | + | // Advertise parking before unroute so IntentResume never observes both the | |||
| 57 | + | // snatched seat and parking/parked registries empty for a still-valid token. | |||
| 58 | + | parkingToken := d.markParkingInFlight(token.sess, token.ac) | |||
| 59 | + | if !d.unrouteSnatchedAttachment(token, false) { | |||
| 60 | + | d.clearParkingInFlightIfAbandoned(token.sess, token.ac, parkingToken) | |||
| 61 | + | return false | |||
| 62 | + | } | |||
| 63 | + | if d.afterSnatchedUnrouteBeforePark != nil { | |||
| 64 | + | d.afterSnatchedUnrouteBeforePark() | |||
| 65 | + | } | |||
| 49 | if d.parkAttachmentAs(token.sess, token.ac, attachmentSnatched) { | 66 | if d.parkAttachmentAs(token.sess, token.ac, attachmentSnatched) { | ||
| 50 | - | _ = token.ac.closeCapturedTransport(token.ac.revokeTransport(token.transport.transport)) | 67 | + | captured := token.transport.transport |
| 68 | + | _ = token.ac.revokeTransport(captured) | |||
| 69 | + | _ = token.ac.closeCapturedTransport(captured) | |||
| 51 | return true | 70 | return true | ||
| 52 | } | 71 | } | ||
| 72 | + | d.clearParkingInFlight(parkingToken, token.ac) | |||
| 53 | token.ac.clearPreviousSession() | 73 | token.ac.clearPreviousSession() | ||
| 54 | - | _ = token.ac.closeCapturedTransport(token.ac.revokeTransport(token.transport.transport)) | 74 | + | captured := token.transport.transport |
| 75 | + | _ = token.ac.revokeTransport(captured) | |||
| 76 | + | _ = token.ac.closeCapturedTransport(captured) | |||
| 55 | return false | 77 | return false | ||
| 56 | } | 78 | } | ||
| 57 | 79 | ||||
| @@ -85,6 +107,13 @@ func (d *Daemon) cleanupInterruptedSnatchedAttachment(token attachmentRoleToken) | |||||
| 85 | return false | 107 | return false | ||
| 86 | } | 108 | } | ||
| 87 | 109 | ||||
| 110 | + | // Advertise parking before ownership removal so IntentResume can wait out | |||
| 111 | + | // the unroute→park gap for a still-valid snatched credential. | |||
| 112 | + | var parkingToken uint64 | |||
| 113 | + | if token.ac.resumeCapable { | |||
| 114 | + | parkingToken = d.markParkingInFlight(token.sess, token.ac) | |||
| 115 | + | } | |||
| 116 | + | ||||
| 88 | // The frozen gate prevents a promotion from publishing between overlay | 117 | // The frozen gate prevents a promotion from publishing between overlay | ||
| 89 | // cleanup and terminal registry publication. | 118 | // cleanup and terminal registry publication. | ||
| 90 | d.clearForSnatch(token) | 119 | d.clearForSnatch(token) | ||
| @@ -102,17 +131,26 @@ func (d *Daemon) cleanupInterruptedSnatchedAttachment(token attachmentRoleToken) | |||||
| 102 | token.sess.mu.Unlock() | 131 | token.sess.mu.Unlock() | ||
| 103 | d.notices.routingMu.Unlock() | 132 | d.notices.routingMu.Unlock() | ||
| 104 | if !current { | 133 | if !current { | ||
| 134 | + | d.clearParkingInFlightIfAbandoned(token.sess, token.ac, parkingToken) | |||
| 105 | return false | 135 | return false | ||
| 106 | } | 136 | } | ||
| 107 | 137 | ||||
| 108 | token.ac.setSession(nil) | 138 | token.ac.setSession(nil) | ||
| 139 | + | if d.afterSnatchedUnrouteBeforePark != nil { | |||
| 140 | + | d.afterSnatchedUnrouteBeforePark() | |||
| 141 | + | } | |||
| 109 | if token.ac.resumeCapable && d.parkAttachmentAs(token.sess, token.ac, attachmentSnatched) { | 142 | if token.ac.resumeCapable && d.parkAttachmentAs(token.sess, token.ac, attachmentSnatched) { | ||
| 110 | - | _ = token.ac.closeCapturedTransport(token.ac.revokeTransport(token.transport.transport)) | 143 | + | captured := token.transport.transport |
| 144 | + | _ = token.ac.revokeTransport(captured) | |||
| 145 | + | _ = token.ac.closeCapturedTransport(captured) | |||
| 111 | token.ac.clearSnatchedInput() | 146 | token.ac.clearSnatchedInput() | ||
| 112 | d.unregisterPreview(token.ac) | 147 | d.unregisterPreview(token.ac) | ||
| 113 | return true | 148 | return true | ||
| 114 | } | 149 | } | ||
| 115 | - | _ = token.ac.closeCapturedTransport(token.ac.revokeTransport(token.transport.transport)) | 150 | + | d.clearParkingInFlight(parkingToken, token.ac) |
| 151 | + | captured := token.transport.transport | |||
| 152 | + | _ = token.ac.revokeTransport(captured) | |||
| 153 | + | _ = token.ac.closeCapturedTransport(captured) | |||
| 116 | token.ac.clearSnatchedInput() | 154 | token.ac.clearSnatchedInput() | ||
| 117 | d.unregisterPreview(token.ac) | 155 | d.unregisterPreview(token.ac) | ||
| 118 | token.ac.clearPreviousSession() | 156 | token.ac.clearPreviousSession() | ||