refactor(daemon): fence pane effects by owner generation
- commit
- 64cc0d8d1dfa0c37acd2e3e656e05b55aad64469
- author
- brice · · 2026-07-28 03:38 UTC
- parent
- bbbadc6
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email
11 files · +910 -122
M internal/usecase/daemon/client.go +0 -39
| @@ -357,45 +357,6 @@ func (d *Daemon) boundedSendOutputErrTransport(ac *attachedClient, b []byte) (po | |||
| 357 | 357 | }) | |
| 358 | 358 | } | |
| 359 | 359 | ||
| 360 | - | // boundedSendOutputErrTransportForRole sends a queued side effect only through | |
| 361 | - | // the exact active attachment capability captured when the work was queued. | |
| 362 | - | // The fresh ticket keeps role publication behind an admitted send; only the | |
| 363 | - | // external Send interval is marked interruptible for replacement liveness. | |
| 364 | - | func (d *Daemon) boundedSendOutputErrTransportForRole(token attachmentRoleToken, ticket *roleEffectTicket, b []byte) (ports.Transport, error) { | |
| 365 | - | if token.ac == nil || ticket == nil || ticket.ended.Load() || token.transport.transport == nil { | |
| 366 | - | return token.transport.transport, errAttachmentTransition | |
| 367 | - | } | |
| 368 | - | frame := token.ac.output.sideEffect(b, token.ac.echoAck.Load()) | |
| 369 | - | expected := token.transport | |
| 370 | - | send := func(owned bool) error { | |
| 371 | - | ac := token.ac | |
| 372 | - | ac.sendMu.Lock() | |
| 373 | - | defer ac.sendMu.Unlock() | |
| 374 | - | if ticket.ended.Load() || !ac.transportSnapshotCurrent(expected) { | |
| 375 | - | return errAttachmentTransition | |
| 376 | - | } | |
| 377 | - | interruptible := ticket.beginTransportSend(expected) | |
| 378 | - | if !interruptible { | |
| 379 | - | return errAttachmentTransition | |
| 380 | - | } | |
| 381 | - | var err error | |
| 382 | - | if owned { | |
| 383 | - | err = expected.transport.(ports.OwnedSynchronousTransport).SendSynchronous(frame) | |
| 384 | - | } else { | |
| 385 | - | err = expected.transport.Send(frame) | |
| 386 | - | } | |
| 387 | - | if err != nil { | |
| 388 | - | ticket.reportTransportFailure(expected) | |
| 389 | - | } | |
| 390 | - | ticket.endTransportSend() | |
| 391 | - | return err | |
| 392 | - | } | |
| 393 | - | if _, owned := expected.transport.(ports.OwnedSynchronousTransport); owned { | |
| 394 | - | return expected.transport, send(true) | |
| 395 | - | } | |
| 396 | - | return d.boundedSendWith(expected.transport, func() error { return send(false) }) | |
| 397 | - | } | |
| 398 | - | ||
| 399 | 360 | var errSendTimedOut = errors.New("send timed out") | |
| 400 | 361 | ||
| 401 | 362 | func (d *Daemon) boundedSendWith(tr ports.Transport, send func() error) (ports.Transport, error) { | |
| @@ -357,45 +357,6 @@ func (d *Daemon) boundedSendOutputErrTransport(ac *attachedClient, b []byte) (po | |||||
| 357 | }) | 357 | }) | ||
| 358 | } | 358 | } | ||
| 359 | 359 | ||||
| 360 | - | // boundedSendOutputErrTransportForRole sends a queued side effect only through | |||
| 361 | - | // the exact active attachment capability captured when the work was queued. | |||
| 362 | - | // The fresh ticket keeps role publication behind an admitted send; only the | |||
| 363 | - | // external Send interval is marked interruptible for replacement liveness. | |||
| 364 | - | func (d *Daemon) boundedSendOutputErrTransportForRole(token attachmentRoleToken, ticket *roleEffectTicket, b []byte) (ports.Transport, error) { | |||
| 365 | - | if token.ac == nil || ticket == nil || ticket.ended.Load() || token.transport.transport == nil { | |||
| 366 | - | return token.transport.transport, errAttachmentTransition | |||
| 367 | - | } | |||
| 368 | - | frame := token.ac.output.sideEffect(b, token.ac.echoAck.Load()) | |||
| 369 | - | expected := token.transport | |||
| 370 | - | send := func(owned bool) error { | |||
| 371 | - | ac := token.ac | |||
| 372 | - | ac.sendMu.Lock() | |||
| 373 | - | defer ac.sendMu.Unlock() | |||
| 374 | - | if ticket.ended.Load() || !ac.transportSnapshotCurrent(expected) { | |||
| 375 | - | return errAttachmentTransition | |||
| 376 | - | } | |||
| 377 | - | interruptible := ticket.beginTransportSend(expected) | |||
| 378 | - | if !interruptible { | |||
| 379 | - | return errAttachmentTransition | |||
| 380 | - | } | |||
| 381 | - | var err error | |||
| 382 | - | if owned { | |||
| 383 | - | err = expected.transport.(ports.OwnedSynchronousTransport).SendSynchronous(frame) | |||
| 384 | - | } else { | |||
| 385 | - | err = expected.transport.Send(frame) | |||
| 386 | - | } | |||
| 387 | - | if err != nil { | |||
| 388 | - | ticket.reportTransportFailure(expected) | |||
| 389 | - | } | |||
| 390 | - | ticket.endTransportSend() | |||
| 391 | - | return err | |||
| 392 | - | } | |||
| 393 | - | if _, owned := expected.transport.(ports.OwnedSynchronousTransport); owned { | |||
| 394 | - | return expected.transport, send(true) | |||
| 395 | - | } | |||
| 396 | - | return d.boundedSendWith(expected.transport, func() error { return send(false) }) | |||
| 397 | - | } | |||
| 398 | - | ||||
| 399 | var errSendTimedOut = errors.New("send timed out") | 360 | var errSendTimedOut = errors.New("send timed out") | ||
| 400 | 361 | ||||
| 401 | func (d *Daemon) boundedSendWith(tr ports.Transport, send func() error) (ports.Transport, error) { | 362 | func (d *Daemon) boundedSendWith(tr ports.Transport, send func() error) (ports.Transport, error) { | ||
M internal/usecase/daemon/clipboard.go +71 -5
| @@ -186,6 +186,7 @@ func clipboardExt(mime string) string { | |||
| 186 | 186 | } | |
| 187 | 187 | ||
| 188 | 188 | type clipboardForward struct { | |
| 189 | + | owner paneEffectLease | |
| 189 | 190 | token attachmentRoleToken | |
| 190 | 191 | seq []byte | |
| 191 | 192 | } | |
| @@ -199,11 +200,12 @@ type clipboardForward struct { | |||
| 199 | 200 | // | |
| 200 | 201 | // Requests are queued on the session and drained by one worker so clipboard | |
| 201 | 202 | // writes keep arrival order without making the PTY reader wait on client I/O. | |
| 202 | - | func (d *Daemon) forwardClipboardAsync(sess *session, b64 string) { | |
| 203 | + | func (d *Daemon) forwardClipboardAsync(owner paneEffectLease, b64 string) { | |
| 203 | 204 | seq := scopy.OSC52FromBase64(b64) | |
| 204 | - | if seq == nil { | |
| 205 | + | if seq == nil || !owner.Current() { | |
| 205 | 206 | return | |
| 206 | 207 | } | |
| 208 | + | sess := owner.owner.session | |
| 207 | 209 | ||
| 208 | 210 | sess.mu.Lock() | |
| 209 | 211 | if sess.ctx != nil { | |
| @@ -239,7 +241,7 @@ func (d *Daemon) forwardClipboardAsync(sess *session, b64 string) { | |||
| 239 | 241 | sess.mu.Unlock() | |
| 240 | 242 | return | |
| 241 | 243 | } | |
| 242 | - | sess.clipboardQueue = append(sess.clipboardQueue, clipboardForward{token: token, seq: seq}) | |
| 244 | + | sess.clipboardQueue = append(sess.clipboardQueue, clipboardForward{owner: owner, token: token, seq: seq}) | |
| 243 | 245 | if !sess.clipboardWorkerRunning { | |
| 244 | 246 | sess.clipboardWorkerRunning = true | |
| 245 | 247 | go d.clipboardWorker(sess) | |
| @@ -247,22 +249,86 @@ func (d *Daemon) forwardClipboardAsync(sess *session, b64 string) { | |||
| 247 | 249 | sess.mu.Unlock() | |
| 248 | 250 | } | |
| 249 | 251 | ||
| 252 | + | func (d *Daemon) boundedSendClipboardForward(item clipboardForward, ticket *roleEffectTicket) (ports.Transport, error) { | |
| 253 | + | token := item.token | |
| 254 | + | if token.ac == nil || ticket == nil || ticket.ended.Load() || token.transport.transport == nil { | |
| 255 | + | return token.transport.transport, errAttachmentTransition | |
| 256 | + | } | |
| 257 | + | expected := token.transport | |
| 258 | + | send := func(owned bool) error { | |
| 259 | + | ac := token.ac | |
| 260 | + | ac.sendMu.Lock() | |
| 261 | + | defer ac.sendMu.Unlock() | |
| 262 | + | if ticket.ended.Load() || !ac.transportSnapshotCurrent(expected) { | |
| 263 | + | return errAttachmentTransition | |
| 264 | + | } | |
| 265 | + | if !beginClipboardOwnerSend(item.owner, ticket, expected) { | |
| 266 | + | return errAttachmentTransition | |
| 267 | + | } | |
| 268 | + | frame := ac.output.sideEffect(item.seq, ac.echoAck.Load()) | |
| 269 | + | var err error | |
| 270 | + | if owned { | |
| 271 | + | err = expected.transport.(ports.OwnedSynchronousTransport).SendSynchronous(frame) | |
| 272 | + | } else { | |
| 273 | + | err = expected.transport.Send(frame) | |
| 274 | + | } | |
| 275 | + | if err != nil { | |
| 276 | + | ticket.reportTransportFailure(expected) | |
| 277 | + | } | |
| 278 | + | ticket.endTransportSend() | |
| 279 | + | return err | |
| 280 | + | } | |
| 281 | + | if _, owned := expected.transport.(ports.OwnedSynchronousTransport); owned { | |
| 282 | + | return expected.transport, send(true) | |
| 283 | + | } | |
| 284 | + | return d.boundedSendWith(expected.transport, func() error { return send(false) }) | |
| 285 | + | } | |
| 286 | + | ||
| 287 | + | // beginClipboardOwnerSend validates the source pane generation after sendMu | |
| 288 | + | // admission and marks the transport interval while ownership publication is | |
| 289 | + | // excluded. Publication therefore orders either before this check (the send is | |
| 290 | + | // dropped) or after transport admission (the send belongs to the old owner). | |
| 291 | + | func beginClipboardOwnerSend(lease paneEffectLease, ticket *roleEffectTicket, expected transportSnapshot) bool { | |
| 292 | + | if lease.pane == nil || lease.owner == nil || lease.owner.session == nil || lease.owner.tab == nil { | |
| 293 | + | return false | |
| 294 | + | } | |
| 295 | + | owner := lease.owner | |
| 296 | + | if owner.floatingSlotGeneration != 0 { | |
| 297 | + | owner.tab.mu.Lock() | |
| 298 | + | defer owner.tab.mu.Unlock() | |
| 299 | + | } | |
| 300 | + | lease.pane.mu.Lock() | |
| 301 | + | defer lease.pane.mu.Unlock() | |
| 302 | + | if lease.pane.owner.Load() != owner { | |
| 303 | + | return false | |
| 304 | + | } | |
| 305 | + | if owner.floatingSlotGeneration != 0 && | |
| 306 | + | (owner.tab.floating.pane != lease.pane || owner.tab.floating.generation != owner.floatingSlotGeneration || | |
| 307 | + | (owner.tab.floating.state != floatingHidden && owner.tab.floating.state != floatingVisible)) { | |
| 308 | + | return false | |
| 309 | + | } | |
| 310 | + | return ticket.beginTransportSend(expected) | |
| 311 | + | } | |
| 312 | + | ||
| 250 | 313 | func (d *Daemon) clipboardWorker(sess *session) { | |
| 251 | 314 | for { | |
| 252 | 315 | item, ok := nextClipboardForward(sess) | |
| 253 | 316 | if !ok { | |
| 254 | 317 | return | |
| 255 | 318 | } | |
| 319 | + | if !item.owner.Current() { | |
| 320 | + | continue | |
| 321 | + | } | |
| 256 | 322 | ticket, admitted := item.token.ac.beginRoleEffect(item.token) | |
| 257 | 323 | if !admitted { | |
| 258 | 324 | continue | |
| 259 | 325 | } | |
| 260 | - | failed, err := d.boundedSendOutputErrTransportForRole(item.token, ticket, item.seq) | |
| 326 | + | failed, err := d.boundedSendClipboardForward(item, ticket) | |
| 261 | 327 | if errors.Is(err, errSendTimedOut) { | |
| 262 | 328 | _ = item.token.ac.closeCapturedTransport(failed) | |
| 263 | 329 | } | |
| 264 | 330 | ticket.End() | |
| 265 | - | if err != nil { | |
| 331 | + | if err != nil && item.owner.Current() { | |
| 266 | 332 | d.detachOnRoleSendError(item.token, failed) | |
| 267 | 333 | } | |
| 268 | 334 | } | |
| @@ -186,6 +186,7 @@ func clipboardExt(mime string) string { | |||||
| 186 | } | 186 | } | ||
| 187 | 187 | ||||
| 188 | type clipboardForward struct { | 188 | type clipboardForward struct { | ||
| 189 | + | owner paneEffectLease | |||
| 189 | token attachmentRoleToken | 190 | token attachmentRoleToken | ||
| 190 | seq []byte | 191 | seq []byte | ||
| 191 | } | 192 | } | ||
| @@ -199,11 +200,12 @@ type clipboardForward struct { | |||||
| 199 | // | 200 | // | ||
| 200 | // Requests are queued on the session and drained by one worker so clipboard | 201 | // Requests are queued on the session and drained by one worker so clipboard | ||
| 201 | // writes keep arrival order without making the PTY reader wait on client I/O. | 202 | // writes keep arrival order without making the PTY reader wait on client I/O. | ||
| 202 | - | func (d *Daemon) forwardClipboardAsync(sess *session, b64 string) { | 203 | + | func (d *Daemon) forwardClipboardAsync(owner paneEffectLease, b64 string) { |
| 203 | seq := scopy.OSC52FromBase64(b64) | 204 | seq := scopy.OSC52FromBase64(b64) | ||
| 204 | - | if seq == nil { | 205 | + | if seq == nil || !owner.Current() { |
| 205 | return | 206 | return | ||
| 206 | } | 207 | } | ||
| 208 | + | sess := owner.owner.session | |||
| 207 | 209 | ||||
| 208 | sess.mu.Lock() | 210 | sess.mu.Lock() | ||
| 209 | if sess.ctx != nil { | 211 | if sess.ctx != nil { | ||
| @@ -239,7 +241,7 @@ func (d *Daemon) forwardClipboardAsync(sess *session, b64 string) { | |||||
| 239 | sess.mu.Unlock() | 241 | sess.mu.Unlock() | ||
| 240 | return | 242 | return | ||
| 241 | } | 243 | } | ||
| 242 | - | sess.clipboardQueue = append(sess.clipboardQueue, clipboardForward{token: token, seq: seq}) | 244 | + | sess.clipboardQueue = append(sess.clipboardQueue, clipboardForward{owner: owner, token: token, seq: seq}) |
| 243 | if !sess.clipboardWorkerRunning { | 245 | if !sess.clipboardWorkerRunning { | ||
| 244 | sess.clipboardWorkerRunning = true | 246 | sess.clipboardWorkerRunning = true | ||
| 245 | go d.clipboardWorker(sess) | 247 | go d.clipboardWorker(sess) | ||
| @@ -247,22 +249,86 @@ func (d *Daemon) forwardClipboardAsync(sess *session, b64 string) { | |||||
| 247 | sess.mu.Unlock() | 249 | sess.mu.Unlock() | ||
| 248 | } | 250 | } | ||
| 249 | 251 | ||||
| 252 | + | func (d *Daemon) boundedSendClipboardForward(item clipboardForward, ticket *roleEffectTicket) (ports.Transport, error) { | |||
| 253 | + | token := item.token | |||
| 254 | + | if token.ac == nil || ticket == nil || ticket.ended.Load() || token.transport.transport == nil { | |||
| 255 | + | return token.transport.transport, errAttachmentTransition | |||
| 256 | + | } | |||
| 257 | + | expected := token.transport | |||
| 258 | + | send := func(owned bool) error { | |||
| 259 | + | ac := token.ac | |||
| 260 | + | ac.sendMu.Lock() | |||
| 261 | + | defer ac.sendMu.Unlock() | |||
| 262 | + | if ticket.ended.Load() || !ac.transportSnapshotCurrent(expected) { | |||
| 263 | + | return errAttachmentTransition | |||
| 264 | + | } | |||
| 265 | + | if !beginClipboardOwnerSend(item.owner, ticket, expected) { | |||
| 266 | + | return errAttachmentTransition | |||
| 267 | + | } | |||
| 268 | + | frame := ac.output.sideEffect(item.seq, ac.echoAck.Load()) | |||
| 269 | + | var err error | |||
| 270 | + | if owned { | |||
| 271 | + | err = expected.transport.(ports.OwnedSynchronousTransport).SendSynchronous(frame) | |||
| 272 | + | } else { | |||
| 273 | + | err = expected.transport.Send(frame) | |||
| 274 | + | } | |||
| 275 | + | if err != nil { | |||
| 276 | + | ticket.reportTransportFailure(expected) | |||
| 277 | + | } | |||
| 278 | + | ticket.endTransportSend() | |||
| 279 | + | return err | |||
| 280 | + | } | |||
| 281 | + | if _, owned := expected.transport.(ports.OwnedSynchronousTransport); owned { | |||
| 282 | + | return expected.transport, send(true) | |||
| 283 | + | } | |||
| 284 | + | return d.boundedSendWith(expected.transport, func() error { return send(false) }) | |||
| 285 | + | } | |||
| 286 | + | ||||
| 287 | + | // beginClipboardOwnerSend validates the source pane generation after sendMu | |||
| 288 | + | // admission and marks the transport interval while ownership publication is | |||
| 289 | + | // excluded. Publication therefore orders either before this check (the send is | |||
| 290 | + | // dropped) or after transport admission (the send belongs to the old owner). | |||
| 291 | + | func beginClipboardOwnerSend(lease paneEffectLease, ticket *roleEffectTicket, expected transportSnapshot) bool { | |||
| 292 | + | if lease.pane == nil || lease.owner == nil || lease.owner.session == nil || lease.owner.tab == nil { | |||
| 293 | + | return false | |||
| 294 | + | } | |||
| 295 | + | owner := lease.owner | |||
| 296 | + | if owner.floatingSlotGeneration != 0 { | |||
| 297 | + | owner.tab.mu.Lock() | |||
| 298 | + | defer owner.tab.mu.Unlock() | |||
| 299 | + | } | |||
| 300 | + | lease.pane.mu.Lock() | |||
| 301 | + | defer lease.pane.mu.Unlock() | |||
| 302 | + | if lease.pane.owner.Load() != owner { | |||
| 303 | + | return false | |||
| 304 | + | } | |||
| 305 | + | if owner.floatingSlotGeneration != 0 && | |||
| 306 | + | (owner.tab.floating.pane != lease.pane || owner.tab.floating.generation != owner.floatingSlotGeneration || | |||
| 307 | + | (owner.tab.floating.state != floatingHidden && owner.tab.floating.state != floatingVisible)) { | |||
| 308 | + | return false | |||
| 309 | + | } | |||
| 310 | + | return ticket.beginTransportSend(expected) | |||
| 311 | + | } | |||
| 312 | + | ||||
| 250 | func (d *Daemon) clipboardWorker(sess *session) { | 313 | func (d *Daemon) clipboardWorker(sess *session) { | ||
| 251 | for { | 314 | for { | ||
| 252 | item, ok := nextClipboardForward(sess) | 315 | item, ok := nextClipboardForward(sess) | ||
| 253 | if !ok { | 316 | if !ok { | ||
| 254 | return | 317 | return | ||
| 255 | } | 318 | } | ||
| 319 | + | if !item.owner.Current() { | |||
| 320 | + | continue | |||
| 321 | + | } | |||
| 256 | ticket, admitted := item.token.ac.beginRoleEffect(item.token) | 322 | ticket, admitted := item.token.ac.beginRoleEffect(item.token) | ||
| 257 | if !admitted { | 323 | if !admitted { | ||
| 258 | continue | 324 | continue | ||
| 259 | } | 325 | } | ||
| 260 | - | failed, err := d.boundedSendOutputErrTransportForRole(item.token, ticket, item.seq) | 326 | + | failed, err := d.boundedSendClipboardForward(item, ticket) |
| 261 | if errors.Is(err, errSendTimedOut) { | 327 | if errors.Is(err, errSendTimedOut) { | ||
| 262 | _ = item.token.ac.closeCapturedTransport(failed) | 328 | _ = item.token.ac.closeCapturedTransport(failed) | ||
| 263 | } | 329 | } | ||
| 264 | ticket.End() | 330 | ticket.End() | ||
| 265 | - | if err != nil { | 331 | + | if err != nil && item.owner.Current() { |
| 266 | d.detachOnRoleSendError(item.token, failed) | 332 | d.detachOnRoleSendError(item.token, failed) | ||
| 267 | } | 333 | } | ||
| 268 | } | 334 | } | ||
M internal/usecase/daemon/clipboard_test.go +145 -11
| @@ -209,6 +209,14 @@ func publishActiveClipboardCapability(d *Daemon, sess *session, ac *attachedClie | |||
| 209 | 209 | ac.publishRoleCapability(token) | |
| 210 | 210 | } | |
| 211 | 211 | ||
| 212 | + | func clipboardOwnerLease(sess *session) paneEffectLease { | |
| 213 | + | tb := sess.activeTab() | |
| 214 | + | tb.mu.Lock() | |
| 215 | + | p := tb.terminalTargetLocked() | |
| 216 | + | tb.mu.Unlock() | |
| 217 | + | return p.effectLease() | |
| 218 | + | } | |
| 219 | + | ||
| 212 | 220 | func TestPTYReaderForwardsOSC52ClipboardToAttachedClient(t *testing.T) { | |
| 213 | 221 | d := newTestDaemon(t, nil, stubClock{}) | |
| 214 | 222 | p := chunkReadPTY(t, []byte("\x1b]52;c;aGVsbG8=\x07")) | |
| @@ -220,6 +228,7 @@ func TestPTYReaderForwardsOSC52ClipboardToAttachedClient(t *testing.T) { | |||
| 220 | 228 | ac := &attachedClient{tr: tr, output: newOutputStateStream()} | |
| 221 | 229 | ac.initOverlays() | |
| 222 | 230 | sess := &session{id: "clip", name: "clip", tabs: []*tab{win}, ctx: sctx, cancel: cancel, client: ac} | |
| 231 | + | publishTiledPaneOwners(sess, win) | |
| 223 | 232 | ac.setSession(sess) | |
| 224 | 233 | d.sessions[sess.id] = sess | |
| 225 | 234 | publishActiveClipboardCapability(d, sess, ac, tr) | |
| @@ -253,6 +262,7 @@ func TestPTYReaderDropsOversizedClipboardPayload(t *testing.T) { | |||
| 253 | 262 | ac := &attachedClient{tr: tr, output: newOutputStateStream()} | |
| 254 | 263 | ac.initOverlays() | |
| 255 | 264 | sess := &session{id: "clip-big", name: "clip-big", tabs: []*tab{win}, ctx: sctx, cancel: cancel, client: ac} | |
| 265 | + | publishTiledPaneOwners(sess, win) | |
| 256 | 266 | ac.setSession(sess) | |
| 257 | 267 | d.sessions[sess.id] = sess | |
| 258 | 268 | ||
| @@ -277,6 +287,7 @@ func TestPTYReaderDropsInvalidBase64Clipboard(t *testing.T) { | |||
| 277 | 287 | ac := &attachedClient{tr: tr, output: newOutputStateStream()} | |
| 278 | 288 | ac.initOverlays() | |
| 279 | 289 | sess := &session{id: "clip-bad", name: "clip-bad", tabs: []*tab{win}, ctx: sctx, cancel: cancel, client: ac} | |
| 290 | + | publishTiledPaneOwners(sess, win) | |
| 280 | 291 | ac.setSession(sess) | |
| 281 | 292 | d.sessions[sess.id] = sess | |
| 282 | 293 | ||
| @@ -298,6 +309,7 @@ func TestPTYReaderClipboardNoAttachedClientDoesNotPanic(t *testing.T) { | |||
| 298 | 309 | defer cancel() | |
| 299 | 310 | win := newTestTabWithContext(p, sctx, cancel) | |
| 300 | 311 | sess := &session{id: "clip-noclient", name: "clip-noclient", tabs: []*tab{win}, ctx: sctx, cancel: cancel} | |
| 312 | + | publishTiledPaneOwners(sess, win) | |
| 301 | 313 | d.sessions[sess.id] = sess | |
| 302 | 314 | ||
| 303 | 315 | d.sessWg.Add(1) | |
| @@ -327,6 +339,133 @@ func (t *staleClipboardErrorTransport) sendCount() int { | |||
| 327 | 339 | return t.sends | |
| 328 | 340 | } | |
| 329 | 341 | ||
| 342 | + | func TestQueuedClipboardAfterPaneMoveDoesNotSendToFormerOwner(t *testing.T) { | |
| 343 | + | d, sess, ac, _ := newManualSessionWithPTYs(t, &transactionalResizePTY{}) | |
| 344 | + | oldTransport := &staleClipboardErrorTransport{} | |
| 345 | + | ac.replaceTransport(oldTransport) | |
| 346 | + | publishActiveClipboardCapability(d, sess, ac, oldTransport) | |
| 347 | + | ||
| 348 | + | sess.mu.Lock() | |
| 349 | + | sess.clipboardWorkerRunning = true | |
| 350 | + | sess.mu.Unlock() | |
| 351 | + | d.forwardClipboardAsync(clipboardOwnerLease(sess), base64.StdEncoding.EncodeToString([]byte("queued"))) | |
| 352 | + | ||
| 353 | + | sourceTab := sess.activeTab() | |
| 354 | + | p := sourceTab.focusedPane() | |
| 355 | + | destination := &session{id: "destination", name: "destination"} | |
| 356 | + | publishPaneOwner(p, destination, &tab{}, 0) | |
| 357 | + | publishPaneOwner(p, sess, sourceTab, 0) | |
| 358 | + | ||
| 359 | + | d.clipboardWorker(sess) | |
| 360 | + | ||
| 361 | + | require.Zero(t, oldTransport.sendCount(), "clipboard queued by the former pane owner reached its client") | |
| 362 | + | require.Same(t, ac, sess.client, "stale clipboard send handling detached the former owner's client") | |
| 363 | + | } | |
| 364 | + | ||
| 365 | + | type movingClipboardErrorTransport struct { | |
| 366 | + | started chan struct{} | |
| 367 | + | release chan struct{} | |
| 368 | + | closed chan struct{} | |
| 369 | + | closeOnce sync.Once | |
| 370 | + | } | |
| 371 | + | ||
| 372 | + | func newMovingClipboardErrorTransport() *movingClipboardErrorTransport { | |
| 373 | + | return &movingClipboardErrorTransport{ | |
| 374 | + | started: make(chan struct{}), | |
| 375 | + | release: make(chan struct{}), | |
| 376 | + | closed: make(chan struct{}), | |
| 377 | + | } | |
| 378 | + | } | |
| 379 | + | ||
| 380 | + | func (t *movingClipboardErrorTransport) Send(ports.Frame) error { | |
| 381 | + | close(t.started) | |
| 382 | + | <-t.release | |
| 383 | + | return errors.New("clipboard send failed after pane move") | |
| 384 | + | } | |
| 385 | + | ||
| 386 | + | func (*movingClipboardErrorTransport) Recv() (ports.Frame, error) { return ports.Frame{}, io.EOF } | |
| 387 | + | ||
| 388 | + | func (t *movingClipboardErrorTransport) Close() error { | |
| 389 | + | t.closeOnce.Do(func() { close(t.closed) }) | |
| 390 | + | return nil | |
| 391 | + | } | |
| 392 | + | ||
| 393 | + | func TestQueuedClipboardRevalidatesOwnerAfterWaitingForClientSendLock(t *testing.T) { | |
| 394 | + | d, sess, ac, _ := newManualSessionWithPTYs(t, &transactionalResizePTY{}) | |
| 395 | + | oldTransport := &staleClipboardErrorTransport{} | |
| 396 | + | ac.replaceTransport(oldTransport) | |
| 397 | + | publishActiveClipboardCapability(d, sess, ac, oldTransport) | |
| 398 | + | ||
| 399 | + | sess.mu.Lock() | |
| 400 | + | sess.clipboardWorkerRunning = true | |
| 401 | + | sess.mu.Unlock() | |
| 402 | + | d.forwardClipboardAsync(clipboardOwnerLease(sess), base64.StdEncoding.EncodeToString([]byte("queued"))) | |
| 403 | + | ||
| 404 | + | ac.sendMu.Lock() | |
| 405 | + | sendMuLocked := true | |
| 406 | + | defer func() { | |
| 407 | + | if sendMuLocked { | |
| 408 | + | ac.sendMu.Unlock() | |
| 409 | + | } | |
| 410 | + | }() | |
| 411 | + | workerDone := make(chan struct{}) | |
| 412 | + | go func() { | |
| 413 | + | d.clipboardWorker(sess) | |
| 414 | + | close(workerDone) | |
| 415 | + | }() | |
| 416 | + | require.Eventually(t, func() bool { | |
| 417 | + | ac.roleEffects.mu.Lock() | |
| 418 | + | defer ac.roleEffects.mu.Unlock() | |
| 419 | + | return ac.roleEffects.inFlight == 1 | |
| 420 | + | }, time.Second, time.Millisecond, "clipboard send was not admitted before waiting on sendMu") | |
| 421 | + | ||
| 422 | + | sourceTab := sess.activeTab() | |
| 423 | + | p := sourceTab.focusedPane() | |
| 424 | + | destination := &session{id: "destination", name: "destination"} | |
| 425 | + | publishPaneOwner(p, destination, &tab{}, 0) | |
| 426 | + | publishPaneOwner(p, sess, sourceTab, 0) | |
| 427 | + | ac.sendMu.Unlock() | |
| 428 | + | sendMuLocked = false | |
| 429 | + | awaitTestCompletion(t, workerDone, "clipboard worker did not finish") | |
| 430 | + | ||
| 431 | + | require.Zero(t, oldTransport.sendCount(), "clipboard send was not revalidated immediately before transport I/O") | |
| 432 | + | require.Same(t, ac, sess.client) | |
| 433 | + | } | |
| 434 | + | ||
| 435 | + | func TestClipboardSendErrorAfterPaneMoveDoesNotDetachFormerOwner(t *testing.T) { | |
| 436 | + | d, sess, ac, _ := newManualSessionWithPTYs(t, &transactionalResizePTY{}) | |
| 437 | + | oldTransport := newMovingClipboardErrorTransport() | |
| 438 | + | ac.replaceTransport(oldTransport) | |
| 439 | + | publishActiveClipboardCapability(d, sess, ac, oldTransport) | |
| 440 | + | ||
| 441 | + | sess.mu.Lock() | |
| 442 | + | sess.clipboardWorkerRunning = true | |
| 443 | + | sess.mu.Unlock() | |
| 444 | + | d.forwardClipboardAsync(clipboardOwnerLease(sess), base64.StdEncoding.EncodeToString([]byte("queued"))) | |
| 445 | + | ||
| 446 | + | workerDone := make(chan struct{}) | |
| 447 | + | go func() { | |
| 448 | + | d.clipboardWorker(sess) | |
| 449 | + | close(workerDone) | |
| 450 | + | }() | |
| 451 | + | awaitTestCompletion(t, oldTransport.started, "clipboard send did not start") | |
| 452 | + | ||
| 453 | + | sourceTab := sess.activeTab() | |
| 454 | + | p := sourceTab.focusedPane() | |
| 455 | + | destination := &session{id: "destination", name: "destination"} | |
| 456 | + | publishPaneOwner(p, destination, &tab{}, 0) | |
| 457 | + | publishPaneOwner(p, sess, sourceTab, 0) | |
| 458 | + | close(oldTransport.release) | |
| 459 | + | awaitTestCompletion(t, workerDone, "clipboard worker did not finish") | |
| 460 | + | ||
| 461 | + | require.Same(t, ac, sess.client, "a send error from the pane's retired owner detached its client") | |
| 462 | + | select { | |
| 463 | + | case <-oldTransport.closed: | |
| 464 | + | t.Fatal("a send error from the pane's retired owner closed its client transport") | |
| 465 | + | default: | |
| 466 | + | } | |
| 467 | + | } | |
| 468 | + | ||
| 330 | 469 | func TestQueuedClipboardBeforeSnatchDropsExactStaleCapability(t *testing.T) { | |
| 331 | 470 | d, sess, old, _ := newManualSessionWithPTYs(t, &transactionalResizePTY{}) | |
| 332 | 471 | oldTransport := &staleClipboardErrorTransport{} | |
| @@ -339,7 +478,7 @@ func TestQueuedClipboardBeforeSnatchDropsExactStaleCapability(t *testing.T) { | |||
| 339 | 478 | sess.mu.Lock() | |
| 340 | 479 | sess.clipboardWorkerRunning = true | |
| 341 | 480 | sess.mu.Unlock() | |
| 342 | - | d.forwardClipboardAsync(sess, base64.StdEncoding.EncodeToString([]byte("queued"))) | |
| 481 | + | d.forwardClipboardAsync(clipboardOwnerLease(sess), base64.StdEncoding.EncodeToString([]byte("queued"))) | |
| 343 | 482 | ||
| 344 | 483 | newTransport := &closeTrackingTransport{} | |
| 345 | 484 | next := &attachedClient{tr: newTransport, output: newOutputStateStream(), size: old.size} | |
| @@ -358,23 +497,18 @@ func TestQueuedClipboardBeforeSnatchDropsExactStaleCapability(t *testing.T) { | |||
| 358 | 497 | } | |
| 359 | 498 | ||
| 360 | 499 | func TestForwardClipboardAsyncSerializesClipboardWrites(t *testing.T) { | |
| 361 | - | d := newTestDaemon(t, nil, stubClock{}) | |
| 500 | + | d, sess, ac, _ := newManualSessionWithPTYs(t, &transactionalResizePTY{}) | |
| 362 | 501 | tr := newBlockingClipboardTransport() | |
| 363 | - | ac := &attachedClient{tr: tr, output: newOutputStateStream()} | |
| 364 | - | ac.initOverlays() | |
| 365 | - | sctx, cancel := context.WithCancel(context.Background()) | |
| 366 | - | defer cancel() | |
| 367 | - | sess := &session{id: "clip-order", name: "clip-order", ctx: sctx, cancel: cancel, client: ac} | |
| 368 | - | ac.setSession(sess) | |
| 369 | - | d.sessions[sess.id] = sess | |
| 502 | + | ac.replaceTransport(tr) | |
| 370 | 503 | publishActiveClipboardCapability(d, sess, ac, tr) | |
| 504 | + | owner := clipboardOwnerLease(sess) | |
| 371 | 505 | ||
| 372 | 506 | first := base64.StdEncoding.EncodeToString([]byte("first")) | |
| 373 | 507 | second := base64.StdEncoding.EncodeToString([]byte("second")) | |
| 374 | - | d.forwardClipboardAsync(sess, first) | |
| 508 | + | d.forwardClipboardAsync(owner, first) | |
| 375 | 509 | require.Equal(t, "first", <-tr.started) | |
| 376 | 510 | ||
| 377 | - | d.forwardClipboardAsync(sess, second) | |
| 511 | + | d.forwardClipboardAsync(owner, second) | |
| 378 | 512 | select { | |
| 379 | 513 | case got := <-tr.started: | |
| 380 | 514 | t.Fatalf("second clipboard send started before first completed: %q", got) | |
| @@ -209,6 +209,14 @@ func publishActiveClipboardCapability(d *Daemon, sess *session, ac *attachedClie | |||||
| 209 | ac.publishRoleCapability(token) | 209 | ac.publishRoleCapability(token) | ||
| 210 | } | 210 | } | ||
| 211 | 211 | ||||
| 212 | + | func clipboardOwnerLease(sess *session) paneEffectLease { | |||
| 213 | + | tb := sess.activeTab() | |||
| 214 | + | tb.mu.Lock() | |||
| 215 | + | p := tb.terminalTargetLocked() | |||
| 216 | + | tb.mu.Unlock() | |||
| 217 | + | return p.effectLease() | |||
| 218 | + | } | |||
| 219 | + | ||||
| 212 | func TestPTYReaderForwardsOSC52ClipboardToAttachedClient(t *testing.T) { | 220 | func TestPTYReaderForwardsOSC52ClipboardToAttachedClient(t *testing.T) { | ||
| 213 | d := newTestDaemon(t, nil, stubClock{}) | 221 | d := newTestDaemon(t, nil, stubClock{}) | ||
| 214 | p := chunkReadPTY(t, []byte("\x1b]52;c;aGVsbG8=\x07")) | 222 | p := chunkReadPTY(t, []byte("\x1b]52;c;aGVsbG8=\x07")) | ||
| @@ -220,6 +228,7 @@ func TestPTYReaderForwardsOSC52ClipboardToAttachedClient(t *testing.T) { | |||||
| 220 | ac := &attachedClient{tr: tr, output: newOutputStateStream()} | 228 | ac := &attachedClient{tr: tr, output: newOutputStateStream()} | ||
| 221 | ac.initOverlays() | 229 | ac.initOverlays() | ||
| 222 | sess := &session{id: "clip", name: "clip", tabs: []*tab{win}, ctx: sctx, cancel: cancel, client: ac} | 230 | sess := &session{id: "clip", name: "clip", tabs: []*tab{win}, ctx: sctx, cancel: cancel, client: ac} | ||
| 231 | + | publishTiledPaneOwners(sess, win) | |||
| 223 | ac.setSession(sess) | 232 | ac.setSession(sess) | ||
| 224 | d.sessions[sess.id] = sess | 233 | d.sessions[sess.id] = sess | ||
| 225 | publishActiveClipboardCapability(d, sess, ac, tr) | 234 | publishActiveClipboardCapability(d, sess, ac, tr) | ||
| @@ -253,6 +262,7 @@ func TestPTYReaderDropsOversizedClipboardPayload(t *testing.T) { | |||||
| 253 | ac := &attachedClient{tr: tr, output: newOutputStateStream()} | 262 | ac := &attachedClient{tr: tr, output: newOutputStateStream()} | ||
| 254 | ac.initOverlays() | 263 | ac.initOverlays() | ||
| 255 | sess := &session{id: "clip-big", name: "clip-big", tabs: []*tab{win}, ctx: sctx, cancel: cancel, client: ac} | 264 | sess := &session{id: "clip-big", name: "clip-big", tabs: []*tab{win}, ctx: sctx, cancel: cancel, client: ac} | ||
| 265 | + | publishTiledPaneOwners(sess, win) | |||
| 256 | ac.setSession(sess) | 266 | ac.setSession(sess) | ||
| 257 | d.sessions[sess.id] = sess | 267 | d.sessions[sess.id] = sess | ||
| 258 | 268 | ||||
| @@ -277,6 +287,7 @@ func TestPTYReaderDropsInvalidBase64Clipboard(t *testing.T) { | |||||
| 277 | ac := &attachedClient{tr: tr, output: newOutputStateStream()} | 287 | ac := &attachedClient{tr: tr, output: newOutputStateStream()} | ||
| 278 | ac.initOverlays() | 288 | ac.initOverlays() | ||
| 279 | sess := &session{id: "clip-bad", name: "clip-bad", tabs: []*tab{win}, ctx: sctx, cancel: cancel, client: ac} | 289 | sess := &session{id: "clip-bad", name: "clip-bad", tabs: []*tab{win}, ctx: sctx, cancel: cancel, client: ac} | ||
| 290 | + | publishTiledPaneOwners(sess, win) | |||
| 280 | ac.setSession(sess) | 291 | ac.setSession(sess) | ||
| 281 | d.sessions[sess.id] = sess | 292 | d.sessions[sess.id] = sess | ||
| 282 | 293 | ||||
| @@ -298,6 +309,7 @@ func TestPTYReaderClipboardNoAttachedClientDoesNotPanic(t *testing.T) { | |||||
| 298 | defer cancel() | 309 | defer cancel() | ||
| 299 | win := newTestTabWithContext(p, sctx, cancel) | 310 | win := newTestTabWithContext(p, sctx, cancel) | ||
| 300 | sess := &session{id: "clip-noclient", name: "clip-noclient", tabs: []*tab{win}, ctx: sctx, cancel: cancel} | 311 | sess := &session{id: "clip-noclient", name: "clip-noclient", tabs: []*tab{win}, ctx: sctx, cancel: cancel} | ||
| 312 | + | publishTiledPaneOwners(sess, win) | |||
| 301 | d.sessions[sess.id] = sess | 313 | d.sessions[sess.id] = sess | ||
| 302 | 314 | ||||
| 303 | d.sessWg.Add(1) | 315 | d.sessWg.Add(1) | ||
| @@ -327,6 +339,133 @@ func (t *staleClipboardErrorTransport) sendCount() int { | |||||
| 327 | return t.sends | 339 | return t.sends | ||
| 328 | } | 340 | } | ||
| 329 | 341 | ||||
| 342 | + | func TestQueuedClipboardAfterPaneMoveDoesNotSendToFormerOwner(t *testing.T) { | |||
| 343 | + | d, sess, ac, _ := newManualSessionWithPTYs(t, &transactionalResizePTY{}) | |||
| 344 | + | oldTransport := &staleClipboardErrorTransport{} | |||
| 345 | + | ac.replaceTransport(oldTransport) | |||
| 346 | + | publishActiveClipboardCapability(d, sess, ac, oldTransport) | |||
| 347 | + | ||||
| 348 | + | sess.mu.Lock() | |||
| 349 | + | sess.clipboardWorkerRunning = true | |||
| 350 | + | sess.mu.Unlock() | |||
| 351 | + | d.forwardClipboardAsync(clipboardOwnerLease(sess), base64.StdEncoding.EncodeToString([]byte("queued"))) | |||
| 352 | + | ||||
| 353 | + | sourceTab := sess.activeTab() | |||
| 354 | + | p := sourceTab.focusedPane() | |||
| 355 | + | destination := &session{id: "destination", name: "destination"} | |||
| 356 | + | publishPaneOwner(p, destination, &tab{}, 0) | |||
| 357 | + | publishPaneOwner(p, sess, sourceTab, 0) | |||
| 358 | + | ||||
| 359 | + | d.clipboardWorker(sess) | |||
| 360 | + | ||||
| 361 | + | require.Zero(t, oldTransport.sendCount(), "clipboard queued by the former pane owner reached its client") | |||
| 362 | + | require.Same(t, ac, sess.client, "stale clipboard send handling detached the former owner's client") | |||
| 363 | + | } | |||
| 364 | + | ||||
| 365 | + | type movingClipboardErrorTransport struct { | |||
| 366 | + | started chan struct{} | |||
| 367 | + | release chan struct{} | |||
| 368 | + | closed chan struct{} | |||
| 369 | + | closeOnce sync.Once | |||
| 370 | + | } | |||
| 371 | + | ||||
| 372 | + | func newMovingClipboardErrorTransport() *movingClipboardErrorTransport { | |||
| 373 | + | return &movingClipboardErrorTransport{ | |||
| 374 | + | started: make(chan struct{}), | |||
| 375 | + | release: make(chan struct{}), | |||
| 376 | + | closed: make(chan struct{}), | |||
| 377 | + | } | |||
| 378 | + | } | |||
| 379 | + | ||||
| 380 | + | func (t *movingClipboardErrorTransport) Send(ports.Frame) error { | |||
| 381 | + | close(t.started) | |||
| 382 | + | <-t.release | |||
| 383 | + | return errors.New("clipboard send failed after pane move") | |||
| 384 | + | } | |||
| 385 | + | ||||
| 386 | + | func (*movingClipboardErrorTransport) Recv() (ports.Frame, error) { return ports.Frame{}, io.EOF } | |||
| 387 | + | ||||
| 388 | + | func (t *movingClipboardErrorTransport) Close() error { | |||
| 389 | + | t.closeOnce.Do(func() { close(t.closed) }) | |||
| 390 | + | return nil | |||
| 391 | + | } | |||
| 392 | + | ||||
| 393 | + | func TestQueuedClipboardRevalidatesOwnerAfterWaitingForClientSendLock(t *testing.T) { | |||
| 394 | + | d, sess, ac, _ := newManualSessionWithPTYs(t, &transactionalResizePTY{}) | |||
| 395 | + | oldTransport := &staleClipboardErrorTransport{} | |||
| 396 | + | ac.replaceTransport(oldTransport) | |||
| 397 | + | publishActiveClipboardCapability(d, sess, ac, oldTransport) | |||
| 398 | + | ||||
| 399 | + | sess.mu.Lock() | |||
| 400 | + | sess.clipboardWorkerRunning = true | |||
| 401 | + | sess.mu.Unlock() | |||
| 402 | + | d.forwardClipboardAsync(clipboardOwnerLease(sess), base64.StdEncoding.EncodeToString([]byte("queued"))) | |||
| 403 | + | ||||
| 404 | + | ac.sendMu.Lock() | |||
| 405 | + | sendMuLocked := true | |||
| 406 | + | defer func() { | |||
| 407 | + | if sendMuLocked { | |||
| 408 | + | ac.sendMu.Unlock() | |||
| 409 | + | } | |||
| 410 | + | }() | |||
| 411 | + | workerDone := make(chan struct{}) | |||
| 412 | + | go func() { | |||
| 413 | + | d.clipboardWorker(sess) | |||
| 414 | + | close(workerDone) | |||
| 415 | + | }() | |||
| 416 | + | require.Eventually(t, func() bool { | |||
| 417 | + | ac.roleEffects.mu.Lock() | |||
| 418 | + | defer ac.roleEffects.mu.Unlock() | |||
| 419 | + | return ac.roleEffects.inFlight == 1 | |||
| 420 | + | }, time.Second, time.Millisecond, "clipboard send was not admitted before waiting on sendMu") | |||
| 421 | + | ||||
| 422 | + | sourceTab := sess.activeTab() | |||
| 423 | + | p := sourceTab.focusedPane() | |||
| 424 | + | destination := &session{id: "destination", name: "destination"} | |||
| 425 | + | publishPaneOwner(p, destination, &tab{}, 0) | |||
| 426 | + | publishPaneOwner(p, sess, sourceTab, 0) | |||
| 427 | + | ac.sendMu.Unlock() | |||
| 428 | + | sendMuLocked = false | |||
| 429 | + | awaitTestCompletion(t, workerDone, "clipboard worker did not finish") | |||
| 430 | + | ||||
| 431 | + | require.Zero(t, oldTransport.sendCount(), "clipboard send was not revalidated immediately before transport I/O") | |||
| 432 | + | require.Same(t, ac, sess.client) | |||
| 433 | + | } | |||
| 434 | + | ||||
| 435 | + | func TestClipboardSendErrorAfterPaneMoveDoesNotDetachFormerOwner(t *testing.T) { | |||
| 436 | + | d, sess, ac, _ := newManualSessionWithPTYs(t, &transactionalResizePTY{}) | |||
| 437 | + | oldTransport := newMovingClipboardErrorTransport() | |||
| 438 | + | ac.replaceTransport(oldTransport) | |||
| 439 | + | publishActiveClipboardCapability(d, sess, ac, oldTransport) | |||
| 440 | + | ||||
| 441 | + | sess.mu.Lock() | |||
| 442 | + | sess.clipboardWorkerRunning = true | |||
| 443 | + | sess.mu.Unlock() | |||
| 444 | + | d.forwardClipboardAsync(clipboardOwnerLease(sess), base64.StdEncoding.EncodeToString([]byte("queued"))) | |||
| 445 | + | ||||
| 446 | + | workerDone := make(chan struct{}) | |||
| 447 | + | go func() { | |||
| 448 | + | d.clipboardWorker(sess) | |||
| 449 | + | close(workerDone) | |||
| 450 | + | }() | |||
| 451 | + | awaitTestCompletion(t, oldTransport.started, "clipboard send did not start") | |||
| 452 | + | ||||
| 453 | + | sourceTab := sess.activeTab() | |||
| 454 | + | p := sourceTab.focusedPane() | |||
| 455 | + | destination := &session{id: "destination", name: "destination"} | |||
| 456 | + | publishPaneOwner(p, destination, &tab{}, 0) | |||
| 457 | + | publishPaneOwner(p, sess, sourceTab, 0) | |||
| 458 | + | close(oldTransport.release) | |||
| 459 | + | awaitTestCompletion(t, workerDone, "clipboard worker did not finish") | |||
| 460 | + | ||||
| 461 | + | require.Same(t, ac, sess.client, "a send error from the pane's retired owner detached its client") | |||
| 462 | + | select { | |||
| 463 | + | case <-oldTransport.closed: | |||
| 464 | + | t.Fatal("a send error from the pane's retired owner closed its client transport") | |||
| 465 | + | default: | |||
| 466 | + | } | |||
| 467 | + | } | |||
| 468 | + | ||||
| 330 | func TestQueuedClipboardBeforeSnatchDropsExactStaleCapability(t *testing.T) { | 469 | func TestQueuedClipboardBeforeSnatchDropsExactStaleCapability(t *testing.T) { | ||
| 331 | d, sess, old, _ := newManualSessionWithPTYs(t, &transactionalResizePTY{}) | 470 | d, sess, old, _ := newManualSessionWithPTYs(t, &transactionalResizePTY{}) | ||
| 332 | oldTransport := &staleClipboardErrorTransport{} | 471 | oldTransport := &staleClipboardErrorTransport{} | ||
| @@ -339,7 +478,7 @@ func TestQueuedClipboardBeforeSnatchDropsExactStaleCapability(t *testing.T) { | |||||
| 339 | sess.mu.Lock() | 478 | sess.mu.Lock() | ||
| 340 | sess.clipboardWorkerRunning = true | 479 | sess.clipboardWorkerRunning = true | ||
| 341 | sess.mu.Unlock() | 480 | sess.mu.Unlock() | ||
| 342 | - | d.forwardClipboardAsync(sess, base64.StdEncoding.EncodeToString([]byte("queued"))) | 481 | + | d.forwardClipboardAsync(clipboardOwnerLease(sess), base64.StdEncoding.EncodeToString([]byte("queued"))) |
| 343 | 482 | ||||
| 344 | newTransport := &closeTrackingTransport{} | 483 | newTransport := &closeTrackingTransport{} | ||
| 345 | next := &attachedClient{tr: newTransport, output: newOutputStateStream(), size: old.size} | 484 | next := &attachedClient{tr: newTransport, output: newOutputStateStream(), size: old.size} | ||
| @@ -358,23 +497,18 @@ func TestQueuedClipboardBeforeSnatchDropsExactStaleCapability(t *testing.T) { | |||||
| 358 | } | 497 | } | ||
| 359 | 498 | ||||
| 360 | func TestForwardClipboardAsyncSerializesClipboardWrites(t *testing.T) { | 499 | func TestForwardClipboardAsyncSerializesClipboardWrites(t *testing.T) { | ||
| 361 | - | d := newTestDaemon(t, nil, stubClock{}) | 500 | + | d, sess, ac, _ := newManualSessionWithPTYs(t, &transactionalResizePTY{}) |
| 362 | tr := newBlockingClipboardTransport() | 501 | tr := newBlockingClipboardTransport() | ||
| 363 | - | ac := &attachedClient{tr: tr, output: newOutputStateStream()} | 502 | + | ac.replaceTransport(tr) |
| 364 | - | ac.initOverlays() | |||
| 365 | - | sctx, cancel := context.WithCancel(context.Background()) | |||
| 366 | - | defer cancel() | |||
| 367 | - | sess := &session{id: "clip-order", name: "clip-order", ctx: sctx, cancel: cancel, client: ac} | |||
| 368 | - | ac.setSession(sess) | |||
| 369 | - | d.sessions[sess.id] = sess | |||
| 370 | publishActiveClipboardCapability(d, sess, ac, tr) | 503 | publishActiveClipboardCapability(d, sess, ac, tr) | ||
| 504 | + | owner := clipboardOwnerLease(sess) | |||
| 371 | 505 | ||||
| 372 | first := base64.StdEncoding.EncodeToString([]byte("first")) | 506 | first := base64.StdEncoding.EncodeToString([]byte("first")) | ||
| 373 | second := base64.StdEncoding.EncodeToString([]byte("second")) | 507 | second := base64.StdEncoding.EncodeToString([]byte("second")) | ||
| 374 | - | d.forwardClipboardAsync(sess, first) | 508 | + | d.forwardClipboardAsync(owner, first) |
| 375 | require.Equal(t, "first", <-tr.started) | 509 | require.Equal(t, "first", <-tr.started) | ||
| 376 | 510 | ||||
| 377 | - | d.forwardClipboardAsync(sess, second) | 511 | + | d.forwardClipboardAsync(owner, second) |
| 378 | select { | 512 | select { | ||
| 379 | case got := <-tr.started: | 513 | case got := <-tr.started: | ||
| 380 | t.Fatalf("second clipboard send started before first completed: %q", got) | 514 | t.Fatalf("second clipboard send started before first completed: %q", got) | ||
M internal/usecase/daemon/pane_actions.go +101 -1
| @@ -3,6 +3,7 @@ package daemon | |||
| 3 | 3 | import ( | |
| 4 | 4 | "errors" | |
| 5 | 5 | "fmt" | |
| 6 | + | "slices" | |
| 6 | 7 | ||
| 7 | 8 | "github.com/bnema/vev/internal/domain" | |
| 8 | 9 | "github.com/bnema/vev/internal/usecase/layout" | |
| @@ -203,11 +204,110 @@ func (d *Daemon) closeFocusedPane(sess *session, ac *attachedClient) error { | |||
| 203 | 204 | return d.closePane(sess, tb, id, ac, true) | |
| 204 | 205 | } | |
| 205 | 206 | ||
| 207 | + | // reapPane retains the explicit-owner entry point for focused close tests. PTY | |
| 208 | + | // readers use reapPaneOwner so an owner publication cannot redirect exit to a | |
| 209 | + | // replacement that reused the same tab-local pane ID. | |
| 206 | 210 | func (d *Daemon) reapPane(sess *session, tb *tab, p *pane) { | |
| 207 | 211 | if p == nil { | |
| 208 | 212 | return | |
| 209 | 213 | } | |
| 210 | - | _ = d.closePane(sess, tb, p.id, nil, true) | |
| 214 | + | lease := p.effectLease() | |
| 215 | + | if lease.owner == nil { | |
| 216 | + | _ = d.closePane(sess, tb, p.id, nil, true) | |
| 217 | + | return | |
| 218 | + | } | |
| 219 | + | if lease.owner.session != sess || lease.owner.tab != tb { | |
| 220 | + | return | |
| 221 | + | } | |
| 222 | + | d.reapPaneOwner(p) | |
| 223 | + | } | |
| 224 | + | ||
| 225 | + | // reapPaneOwner resolves the owner afresh after EOF. A stale attempt retries | |
| 226 | + | // the immutable owner pointer; a successful attempt revokes owner publication | |
| 227 | + | // under pane.mu before releasing membership locks, so a competing close or | |
| 228 | + | // move can never reap the pane from a second owner. | |
| 229 | + | func (d *Daemon) reapPaneOwner(p *pane) { | |
| 230 | + | for p != nil { | |
| 231 | + | lease := p.effectLease() | |
| 232 | + | if lease.owner == nil { | |
| 233 | + | return | |
| 234 | + | } | |
| 235 | + | if d.reapTiledPaneLease(lease) { | |
| 236 | + | return | |
| 237 | + | } | |
| 238 | + | } | |
| 239 | + | } | |
| 240 | + | ||
| 241 | + | func (d *Daemon) reapTiledPaneLease(lease paneEffectLease) bool { | |
| 242 | + | p, owner := lease.pane, lease.owner | |
| 243 | + | if p == nil || owner == nil || owner.session == nil || owner.tab == nil || owner.floatingSlotGeneration != 0 { | |
| 244 | + | return true | |
| 245 | + | } | |
| 246 | + | sess, tb := owner.session, owner.tab | |
| 247 | + | sess.mu.Lock() | |
| 248 | + | ownedTab := slices.Contains(sess.tabs, tb) | |
| 249 | + | if p.owner.Load() != owner { | |
| 250 | + | sess.mu.Unlock() | |
| 251 | + | return false | |
| 252 | + | } | |
| 253 | + | if !ownedTab { | |
| 254 | + | sess.mu.Unlock() | |
| 255 | + | return true | |
| 256 | + | } | |
| 257 | + | ac := sess.client | |
| 258 | + | sessionName := sess.name | |
| 259 | + | tb.mu.Lock() | |
| 260 | + | if p.owner.Load() != owner { | |
| 261 | + | tb.mu.Unlock() | |
| 262 | + | sess.mu.Unlock() | |
| 263 | + | return false | |
| 264 | + | } | |
| 265 | + | if tb.panes[p.id] != p || tb.tree == nil || !layout.ContainsLeaf(tb.tree.Root, p.id) { | |
| 266 | + | tb.mu.Unlock() | |
| 267 | + | sess.mu.Unlock() | |
| 268 | + | return true | |
| 269 | + | } | |
| 270 | + | p.mu.Lock() | |
| 271 | + | if p.owner.Load() != owner { | |
| 272 | + | p.mu.Unlock() | |
| 273 | + | tb.mu.Unlock() | |
| 274 | + | sess.mu.Unlock() | |
| 275 | + | return false | |
| 276 | + | } | |
| 277 | + | finalPane := len(layout.LeafIDs(tb.tree.Root)) <= 1 | |
| 278 | + | if !finalPane { | |
| 279 | + | if err := tb.tree.Close(p.id); err != nil { | |
| 280 | + | p.mu.Unlock() | |
| 281 | + | tb.mu.Unlock() | |
| 282 | + | sess.mu.Unlock() | |
| 283 | + | return true | |
| 284 | + | } | |
| 285 | + | delete(tb.panes, p.id) | |
| 286 | + | tb.bumpLayoutGenerationLocked() | |
| 287 | + | } | |
| 288 | + | p.clearOwnerLocked() | |
| 289 | + | p.mu.Unlock() | |
| 290 | + | tb.mu.Unlock() | |
| 291 | + | sess.mu.Unlock() | |
| 292 | + | ||
| 293 | + | if finalPane { | |
| 294 | + | _ = d.closeTab(sess, tb, true) | |
| 295 | + | return true | |
| 296 | + | } | |
| 297 | + | d.applyTabLayout(sess, tb) | |
| 298 | + | if ac != nil { | |
| 299 | + | ac.overlays.clearCopyModeForPane(p) | |
| 300 | + | ac.pruneCaptureFrames(p) | |
| 301 | + | } | |
| 302 | + | if rc := sess.renderCoordinator(); rc != nil { | |
| 303 | + | rc.noteSyncPaneRemoved(p) | |
| 304 | + | } | |
| 305 | + | closePaneProcess(p) | |
| 306 | + | d.log.Info("pane closed", "session", sessionName, "pane", p.id) | |
| 307 | + | if ac != nil { | |
| 308 | + | d.invalidateRender(sess, ac, true, "pane_actions.go") | |
| 309 | + | } | |
| 310 | + | return true | |
| 211 | 311 | } | |
| 212 | 312 | ||
| 213 | 313 | func (d *Daemon) closePane(sess *session, tb *tab, id layout.PaneID, ac *attachedClient, repaint bool) error { | |
| @@ -3,6 +3,7 @@ package daemon | |||||
| 3 | import ( | 3 | import ( | ||
| 4 | "errors" | 4 | "errors" | ||
| 5 | "fmt" | 5 | "fmt" | ||
| 6 | + | "slices" | |||
| 6 | 7 | ||||
| 7 | "github.com/bnema/vev/internal/domain" | 8 | "github.com/bnema/vev/internal/domain" | ||
| 8 | "github.com/bnema/vev/internal/usecase/layout" | 9 | "github.com/bnema/vev/internal/usecase/layout" | ||
| @@ -203,11 +204,110 @@ func (d *Daemon) closeFocusedPane(sess *session, ac *attachedClient) error { | |||||
| 203 | return d.closePane(sess, tb, id, ac, true) | 204 | return d.closePane(sess, tb, id, ac, true) | ||
| 204 | } | 205 | } | ||
| 205 | 206 | ||||
| 207 | + | // reapPane retains the explicit-owner entry point for focused close tests. PTY | |||
| 208 | + | // readers use reapPaneOwner so an owner publication cannot redirect exit to a | |||
| 209 | + | // replacement that reused the same tab-local pane ID. | |||
| 206 | func (d *Daemon) reapPane(sess *session, tb *tab, p *pane) { | 210 | func (d *Daemon) reapPane(sess *session, tb *tab, p *pane) { | ||
| 207 | if p == nil { | 211 | if p == nil { | ||
| 208 | return | 212 | return | ||
| 209 | } | 213 | } | ||
| 210 | - | _ = d.closePane(sess, tb, p.id, nil, true) | 214 | + | lease := p.effectLease() |
| 215 | + | if lease.owner == nil { | |||
| 216 | + | _ = d.closePane(sess, tb, p.id, nil, true) | |||
| 217 | + | return | |||
| 218 | + | } | |||
| 219 | + | if lease.owner.session != sess || lease.owner.tab != tb { | |||
| 220 | + | return | |||
| 221 | + | } | |||
| 222 | + | d.reapPaneOwner(p) | |||
| 223 | + | } | |||
| 224 | + | ||||
| 225 | + | // reapPaneOwner resolves the owner afresh after EOF. A stale attempt retries | |||
| 226 | + | // the immutable owner pointer; a successful attempt revokes owner publication | |||
| 227 | + | // under pane.mu before releasing membership locks, so a competing close or | |||
| 228 | + | // move can never reap the pane from a second owner. | |||
| 229 | + | func (d *Daemon) reapPaneOwner(p *pane) { | |||
| 230 | + | for p != nil { | |||
| 231 | + | lease := p.effectLease() | |||
| 232 | + | if lease.owner == nil { | |||
| 233 | + | return | |||
| 234 | + | } | |||
| 235 | + | if d.reapTiledPaneLease(lease) { | |||
| 236 | + | return | |||
| 237 | + | } | |||
| 238 | + | } | |||
| 239 | + | } | |||
| 240 | + | ||||
| 241 | + | func (d *Daemon) reapTiledPaneLease(lease paneEffectLease) bool { | |||
| 242 | + | p, owner := lease.pane, lease.owner | |||
| 243 | + | if p == nil || owner == nil || owner.session == nil || owner.tab == nil || owner.floatingSlotGeneration != 0 { | |||
| 244 | + | return true | |||
| 245 | + | } | |||
| 246 | + | sess, tb := owner.session, owner.tab | |||
| 247 | + | sess.mu.Lock() | |||
| 248 | + | ownedTab := slices.Contains(sess.tabs, tb) | |||
| 249 | + | if p.owner.Load() != owner { | |||
| 250 | + | sess.mu.Unlock() | |||
| 251 | + | return false | |||
| 252 | + | } | |||
| 253 | + | if !ownedTab { | |||
| 254 | + | sess.mu.Unlock() | |||
| 255 | + | return true | |||
| 256 | + | } | |||
| 257 | + | ac := sess.client | |||
| 258 | + | sessionName := sess.name | |||
| 259 | + | tb.mu.Lock() | |||
| 260 | + | if p.owner.Load() != owner { | |||
| 261 | + | tb.mu.Unlock() | |||
| 262 | + | sess.mu.Unlock() | |||
| 263 | + | return false | |||
| 264 | + | } | |||
| 265 | + | if tb.panes[p.id] != p || tb.tree == nil || !layout.ContainsLeaf(tb.tree.Root, p.id) { | |||
| 266 | + | tb.mu.Unlock() | |||
| 267 | + | sess.mu.Unlock() | |||
| 268 | + | return true | |||
| 269 | + | } | |||
| 270 | + | p.mu.Lock() | |||
| 271 | + | if p.owner.Load() != owner { | |||
| 272 | + | p.mu.Unlock() | |||
| 273 | + | tb.mu.Unlock() | |||
| 274 | + | sess.mu.Unlock() | |||
| 275 | + | return false | |||
| 276 | + | } | |||
| 277 | + | finalPane := len(layout.LeafIDs(tb.tree.Root)) <= 1 | |||
| 278 | + | if !finalPane { | |||
| 279 | + | if err := tb.tree.Close(p.id); err != nil { | |||
| 280 | + | p.mu.Unlock() | |||
| 281 | + | tb.mu.Unlock() | |||
| 282 | + | sess.mu.Unlock() | |||
| 283 | + | return true | |||
| 284 | + | } | |||
| 285 | + | delete(tb.panes, p.id) | |||
| 286 | + | tb.bumpLayoutGenerationLocked() | |||
| 287 | + | } | |||
| 288 | + | p.clearOwnerLocked() | |||
| 289 | + | p.mu.Unlock() | |||
| 290 | + | tb.mu.Unlock() | |||
| 291 | + | sess.mu.Unlock() | |||
| 292 | + | ||||
| 293 | + | if finalPane { | |||
| 294 | + | _ = d.closeTab(sess, tb, true) | |||
| 295 | + | return true | |||
| 296 | + | } | |||
| 297 | + | d.applyTabLayout(sess, tb) | |||
| 298 | + | if ac != nil { | |||
| 299 | + | ac.overlays.clearCopyModeForPane(p) | |||
| 300 | + | ac.pruneCaptureFrames(p) | |||
| 301 | + | } | |||
| 302 | + | if rc := sess.renderCoordinator(); rc != nil { | |||
| 303 | + | rc.noteSyncPaneRemoved(p) | |||
| 304 | + | } | |||
| 305 | + | closePaneProcess(p) | |||
| 306 | + | d.log.Info("pane closed", "session", sessionName, "pane", p.id) | |||
| 307 | + | if ac != nil { | |||
| 308 | + | d.invalidateRender(sess, ac, true, "pane_actions.go") | |||
| 309 | + | } | |||
| 310 | + | return true | |||
| 211 | } | 311 | } | ||
| 212 | 312 | ||||
| 213 | func (d *Daemon) closePane(sess *session, tb *tab, id layout.PaneID, ac *attachedClient, repaint bool) error { | 313 | func (d *Daemon) closePane(sess *session, tb *tab, id layout.PaneID, ac *attachedClient, repaint bool) error { | ||
M internal/usecase/daemon/pane_owner.go +26 -0
| @@ -42,6 +42,32 @@ func (p *pane) effectLeaseLocked() paneEffectLease { | |||
| 42 | 42 | return paneEffectLease{pane: p, owner: p.owner.Load()} | |
| 43 | 43 | } | |
| 44 | 44 | ||
| 45 | + | // effectLease captures an owner generation outside an existing pane critical | |
| 46 | + | // section. Callers use effectLeaseLocked when parsing already owns pane.mu. | |
| 47 | + | func (p *pane) effectLease() paneEffectLease { | |
| 48 | + | if p == nil { | |
| 49 | + | return paneEffectLease{} | |
| 50 | + | } | |
| 51 | + | p.mu.Lock() | |
| 52 | + | lease := p.effectLeaseLocked() | |
| 53 | + | p.mu.Unlock() | |
| 54 | + | return lease | |
| 55 | + | } | |
| 56 | + | ||
| 57 | + | // paneEffectSessionName snapshots log metadata only while the lease remains | |
| 58 | + | // current. Holding session.mu across the final pointer check linearizes the | |
| 59 | + | // snapshot with owner publication, whose move commit also owns the session. | |
| 60 | + | func paneEffectSessionName(lease paneEffectLease) (string, bool) { | |
| 61 | + | if lease.pane == nil || lease.owner == nil || lease.owner.session == nil || !lease.Current() { | |
| 62 | + | return "", false | |
| 63 | + | } | |
| 64 | + | lease.owner.session.mu.Lock() | |
| 65 | + | current := lease.pane.owner.Load() == lease.owner | |
| 66 | + | name := lease.owner.session.name | |
| 67 | + | lease.owner.session.mu.Unlock() | |
| 68 | + | return name, current | |
| 69 | + | } | |
| 70 | + | ||
| 45 | 71 | // publishOwnerLocked publishes a new immutable owner generation. The caller | |
| 46 | 72 | // must hold pane.mu, and must also hold tab.mu when publishing a floating owner. | |
| 47 | 73 | func (p *pane) publishOwnerLocked(sess *session, tb *tab, floatingSlotGeneration uint64) (paneEffectLease, paneEffectLease) { | |
| @@ -42,6 +42,32 @@ func (p *pane) effectLeaseLocked() paneEffectLease { | |||||
| 42 | return paneEffectLease{pane: p, owner: p.owner.Load()} | 42 | return paneEffectLease{pane: p, owner: p.owner.Load()} | ||
| 43 | } | 43 | } | ||
| 44 | 44 | ||||
| 45 | + | // effectLease captures an owner generation outside an existing pane critical | |||
| 46 | + | // section. Callers use effectLeaseLocked when parsing already owns pane.mu. | |||
| 47 | + | func (p *pane) effectLease() paneEffectLease { | |||
| 48 | + | if p == nil { | |||
| 49 | + | return paneEffectLease{} | |||
| 50 | + | } | |||
| 51 | + | p.mu.Lock() | |||
| 52 | + | lease := p.effectLeaseLocked() | |||
| 53 | + | p.mu.Unlock() | |||
| 54 | + | return lease | |||
| 55 | + | } | |||
| 56 | + | ||||
| 57 | + | // paneEffectSessionName snapshots log metadata only while the lease remains | |||
| 58 | + | // current. Holding session.mu across the final pointer check linearizes the | |||
| 59 | + | // snapshot with owner publication, whose move commit also owns the session. | |||
| 60 | + | func paneEffectSessionName(lease paneEffectLease) (string, bool) { | |||
| 61 | + | if lease.pane == nil || lease.owner == nil || lease.owner.session == nil || !lease.Current() { | |||
| 62 | + | return "", false | |||
| 63 | + | } | |||
| 64 | + | lease.owner.session.mu.Lock() | |||
| 65 | + | current := lease.pane.owner.Load() == lease.owner | |||
| 66 | + | name := lease.owner.session.name | |||
| 67 | + | lease.owner.session.mu.Unlock() | |||
| 68 | + | return name, current | |||
| 69 | + | } | |||
| 70 | + | ||||
| 45 | // publishOwnerLocked publishes a new immutable owner generation. The caller | 71 | // publishOwnerLocked publishes a new immutable owner generation. The caller | ||
| 46 | // must hold pane.mu, and must also hold tab.mu when publishing a floating owner. | 72 | // must hold pane.mu, and must also hold tab.mu when publishing a floating owner. | ||
| 47 | func (p *pane) publishOwnerLocked(sess *session, tb *tab, floatingSlotGeneration uint64) (paneEffectLease, paneEffectLease) { | 73 | func (p *pane) publishOwnerLocked(sess *session, tb *tab, floatingSlotGeneration uint64) (paneEffectLease, paneEffectLease) { | ||
A internal/usecase/daemon/pane_owner_reader_test.go +187 -0
| @@ -0,0 +1,187 @@ | |||
| 1 | + | package daemon | |
| 2 | + | ||
| 3 | + | import ( | |
| 4 | + | "io" | |
| 5 | + | "sync/atomic" | |
| 6 | + | "testing" | |
| 7 | + | ||
| 8 | + | "github.com/bnema/vev/internal/domain" | |
| 9 | + | "github.com/bnema/vev/internal/ports" | |
| 10 | + | "github.com/bnema/vev/internal/usecase/layout" | |
| 11 | + | "github.com/stretchr/testify/require" | |
| 12 | + | ) | |
| 13 | + | ||
| 14 | + | type ownerRoutingPTY struct { | |
| 15 | + | steps chan channelPTYStep | |
| 16 | + | processed chan struct{} | |
| 17 | + | activeReaders atomic.Int32 | |
| 18 | + | maxReaders atomic.Int32 | |
| 19 | + | reads atomic.Int32 | |
| 20 | + | closes atomic.Int32 | |
| 21 | + | } | |
| 22 | + | ||
| 23 | + | func newOwnerRoutingPTY() *ownerRoutingPTY { | |
| 24 | + | return &ownerRoutingPTY{ | |
| 25 | + | steps: make(chan channelPTYStep), | |
| 26 | + | processed: make(chan struct{}, 4), | |
| 27 | + | } | |
| 28 | + | } | |
| 29 | + | ||
| 30 | + | func (p *ownerRoutingPTY) Read(buf []byte) (int, error) { | |
| 31 | + | active := p.activeReaders.Add(1) | |
| 32 | + | defer p.activeReaders.Add(-1) | |
| 33 | + | for { | |
| 34 | + | maximum := p.maxReaders.Load() | |
| 35 | + | if active <= maximum || p.maxReaders.CompareAndSwap(maximum, active) { | |
| 36 | + | break | |
| 37 | + | } | |
| 38 | + | } | |
| 39 | + | if p.reads.Add(1) > 1 { | |
| 40 | + | p.processed <- struct{}{} | |
| 41 | + | } | |
| 42 | + | step := <-p.steps | |
| 43 | + | return copy(buf, step.data), step.err | |
| 44 | + | } | |
| 45 | + | ||
| 46 | + | func (*ownerRoutingPTY) Write(b []byte) (int, error) { return len(b), nil } | |
| 47 | + | func (p *ownerRoutingPTY) Close() error { p.closes.Add(1); return nil } | |
| 48 | + | func (*ownerRoutingPTY) Resize(domain.Size) error { return nil } | |
| 49 | + | func (*ownerRoutingPTY) Pid() int { return 0 } | |
| 50 | + | func (*ownerRoutingPTY) ForegroundPgid() (int, error) { | |
| 51 | + | return 0, nil | |
| 52 | + | } | |
| 53 | + | ||
| 54 | + | func installTiledPaneOwnerForTest(sess *session, tb *tab, p *pane) { | |
| 55 | + | tb.mu.Lock() | |
| 56 | + | old := tb.panes[layout.PaneID("pane-1")] | |
| 57 | + | delete(tb.panes, layout.PaneID("pane-1")) | |
| 58 | + | if old != nil { | |
| 59 | + | old.clearOwnerForTest() | |
| 60 | + | } | |
| 61 | + | p.id = layout.PaneID("pane-1") | |
| 62 | + | tb.panes[p.id] = p | |
| 63 | + | tb.tree = &layout.Tree{Root: layout.NewLeaf(p.id), Focus: p.id} | |
| 64 | + | publishPaneOwner(p, sess, tb, 0) | |
| 65 | + | tb.mu.Unlock() | |
| 66 | + | } | |
| 67 | + | ||
| 68 | + | func (p *pane) clearOwnerForTest() { | |
| 69 | + | p.mu.Lock() | |
| 70 | + | p.clearOwnerLocked() | |
| 71 | + | p.mu.Unlock() | |
| 72 | + | } | |
| 73 | + | ||
| 74 | + | func TestPTYReaderRoutesEffectsToOwnerPublishedForEachRead(t *testing.T) { | |
| 75 | + | pty := newOwnerRoutingPTY() | |
| 76 | + | d, source, _, _ := newManualSessionWithPTYs(t, pty) | |
| 77 | + | pane := source.activeTab().focusedPane() | |
| 78 | + | pane.onExit = func() {} | |
| 79 | + | source.snapEligible.Store(true) | |
| 80 | + | ||
| 81 | + | destinationTab := newTab(nil, domain.Size{Cols: 80, Rows: 23}) | |
| 82 | + | destination := &session{ | |
| 83 | + | id: domain.SessionID("destination"), | |
| 84 | + | name: "destination", | |
| 85 | + | tabs: []*tab{destinationTab}, | |
| 86 | + | client: &attachedClient{}, | |
| 87 | + | snapEligible: atomic.Bool{}, | |
| 88 | + | } | |
| 89 | + | destination.snapEligible.Store(true) | |
| 90 | + | destination.client.initOverlays() | |
| 91 | + | destination.client.setSession(destination) | |
| 92 | + | d.sessions[destination.id] = destination | |
| 93 | + | ||
| 94 | + | sourceInvalidations := make(chan renderInvalidation, 2) | |
| 95 | + | destinationInvalidations := make(chan renderInvalidation, 2) | |
| 96 | + | sourceCoordinator := d.attachCoordinator(source, nil, source.client, true) | |
| 97 | + | sourceCoordinator.opts.onInvalidate = func(invalidation renderInvalidation) { sourceInvalidations <- invalidation } | |
| 98 | + | destinationCoordinator := d.attachCoordinator(destination, nil, destination.client, true) | |
| 99 | + | destinationCoordinator.opts.onInvalidate = func(invalidation renderInvalidation) { destinationInvalidations <- invalidation } | |
| 100 | + | ||
| 101 | + | d.startPaneGoroutines(source, source.activeTab(), pane) | |
| 102 | + | pty.steps <- channelPTYStep{data: []byte("source\a")} | |
| 103 | + | <-pty.processed | |
| 104 | + | require.Len(t, sourceInvalidations, 1) | |
| 105 | + | require.Empty(t, destinationInvalidations) | |
| 106 | + | require.Equal(t, uint64(1), source.snapshotGeneration) | |
| 107 | + | require.Zero(t, destination.snapshotGeneration) | |
| 108 | + | require.True(t, source.activeTab().attention) | |
| 109 | + | ||
| 110 | + | installTiledPaneOwnerForTest(destination, destinationTab, pane) | |
| 111 | + | pty.steps <- channelPTYStep{data: []byte("destination\a")} | |
| 112 | + | <-pty.processed | |
| 113 | + | ||
| 114 | + | require.Len(t, sourceInvalidations, 1, "output after owner publication must not invalidate the source") | |
| 115 | + | require.Len(t, destinationInvalidations, 1, "output after owner publication must invalidate the destination") | |
| 116 | + | require.Equal(t, uint64(1), source.snapshotGeneration, "source snapshot must not be dirtied after transfer") | |
| 117 | + | require.Equal(t, uint64(1), destination.snapshotGeneration) | |
| 118 | + | require.True(t, destinationTab.attention) | |
| 119 | + | require.Equal(t, int32(1), pty.maxReaders.Load(), "owner publication must not start another PTY reader") | |
| 120 | + | ||
| 121 | + | pty.steps <- channelPTYStep{err: io.EOF} | |
| 122 | + | d.sessWg.Wait() | |
| 123 | + | } | |
| 124 | + | ||
| 125 | + | func TestPTYReaderExitReapsExactlyCurrentOwnerAfterPublication(t *testing.T) { | |
| 126 | + | pty := newOwnerRoutingPTY() | |
| 127 | + | d, source, _, _ := newManualSessionWithPTYs(t, pty) | |
| 128 | + | sourceTab := source.activeTab() | |
| 129 | + | moved := sourceTab.focusedPane() | |
| 130 | + | ||
| 131 | + | destinationTab := newTab(newQuietPTY(), domain.Size{Cols: 80, Rows: 23}) | |
| 132 | + | other := newPane(layout.PaneID("pane-2"), newQuietPTY(), domain.Size{Cols: 40, Rows: 23}) | |
| 133 | + | destinationTab.mu.Lock() | |
| 134 | + | require.NoError(t, destinationTab.tree.Split(layout.PaneID("pane-1"), layout.Right, true, other.id, domain.Rect{Width: 80, Height: 23})) | |
| 135 | + | destinationTab.panes[other.id] = other | |
| 136 | + | destinationTab.mu.Unlock() | |
| 137 | + | destination := &session{id: domain.SessionID("destination"), name: "destination", tabs: []*tab{destinationTab}} | |
| 138 | + | d.sessions[destination.id] = destination | |
| 139 | + | ||
| 140 | + | // Model a completed owner commit while the one reader remains blocked in | |
| 141 | + | // Read. The source immediately reuses the old local ID, which makes stale | |
| 142 | + | // ID-only exit routing observably close the wrong pane. | |
| 143 | + | replacement := newPane(layout.PaneID("pane-1"), newQuietPTY(), domain.Size{Cols: 80, Rows: 23}) | |
| 144 | + | sourceTab.mu.Lock() | |
| 145 | + | sourceTab.panes[replacement.id] = replacement | |
| 146 | + | publishPaneOwner(replacement, source, sourceTab, 0) | |
| 147 | + | sourceTab.mu.Unlock() | |
| 148 | + | ||
| 149 | + | destinationTab.mu.Lock() | |
| 150 | + | displaced := destinationTab.panes[layout.PaneID("pane-1")] | |
| 151 | + | displaced.clearOwnerForTest() | |
| 152 | + | destinationTab.panes[moved.id] = moved | |
| 153 | + | publishPaneOwner(moved, destination, destinationTab, 0) | |
| 154 | + | destinationTab.mu.Unlock() | |
| 155 | + | ||
| 156 | + | d.sessWg.Add(1) | |
| 157 | + | go d.ptyReader(source, sourceTab, moved) | |
| 158 | + | pty.steps <- channelPTYStep{err: io.EOF} | |
| 159 | + | d.sessWg.Wait() | |
| 160 | + | ||
| 161 | + | d.mu.Lock() | |
| 162 | + | require.Same(t, source, d.sessions[source.id], "stale exit must not close the source replacement") | |
| 163 | + | require.Same(t, destination, d.sessions[destination.id]) | |
| 164 | + | d.mu.Unlock() | |
| 165 | + | sourceTab.mu.Lock() | |
| 166 | + | require.Same(t, replacement, sourceTab.panes[replacement.id]) | |
| 167 | + | sourceTab.mu.Unlock() | |
| 168 | + | destinationTab.mu.Lock() | |
| 169 | + | require.NotContains(t, destinationTab.panes, moved.id, "the exited pane must be removed from its current owner") | |
| 170 | + | require.Same(t, other, destinationTab.panes[other.id]) | |
| 171 | + | destinationTab.mu.Unlock() | |
| 172 | + | require.Nil(t, moved.ownerSnapshot()) | |
| 173 | + | require.True(t, replacement.effectLease().Current()) | |
| 174 | + | require.Equal(t, int32(1), pty.maxReaders.Load()) | |
| 175 | + | ||
| 176 | + | // A stale second reap is a no-op and cannot close either surviving owner. | |
| 177 | + | d.reapPaneOwner(moved) | |
| 178 | + | sourceTab.mu.Lock() | |
| 179 | + | require.Same(t, replacement, sourceTab.panes[replacement.id]) | |
| 180 | + | sourceTab.mu.Unlock() | |
| 181 | + | destinationTab.mu.Lock() | |
| 182 | + | require.Same(t, other, destinationTab.panes[other.id]) | |
| 183 | + | destinationTab.mu.Unlock() | |
| 184 | + | require.Equal(t, int32(1), pty.closes.Load(), "exit must release the process exactly once") | |
| 185 | + | } | |
| 186 | + | ||
| 187 | + | var _ ports.PTY = (*ownerRoutingPTY)(nil) | |
| @@ -0,0 +1,187 @@ | |||||
| 1 | + | package daemon | |||
| 2 | + | ||||
| 3 | + | import ( | |||
| 4 | + | "io" | |||
| 5 | + | "sync/atomic" | |||
| 6 | + | "testing" | |||
| 7 | + | ||||
| 8 | + | "github.com/bnema/vev/internal/domain" | |||
| 9 | + | "github.com/bnema/vev/internal/ports" | |||
| 10 | + | "github.com/bnema/vev/internal/usecase/layout" | |||
| 11 | + | "github.com/stretchr/testify/require" | |||
| 12 | + | ) | |||
| 13 | + | ||||
| 14 | + | type ownerRoutingPTY struct { | |||
| 15 | + | steps chan channelPTYStep | |||
| 16 | + | processed chan struct{} | |||
| 17 | + | activeReaders atomic.Int32 | |||
| 18 | + | maxReaders atomic.Int32 | |||
| 19 | + | reads atomic.Int32 | |||
| 20 | + | closes atomic.Int32 | |||
| 21 | + | } | |||
| 22 | + | ||||
| 23 | + | func newOwnerRoutingPTY() *ownerRoutingPTY { | |||
| 24 | + | return &ownerRoutingPTY{ | |||
| 25 | + | steps: make(chan channelPTYStep), | |||
| 26 | + | processed: make(chan struct{}, 4), | |||
| 27 | + | } | |||
| 28 | + | } | |||
| 29 | + | ||||
| 30 | + | func (p *ownerRoutingPTY) Read(buf []byte) (int, error) { | |||
| 31 | + | active := p.activeReaders.Add(1) | |||
| 32 | + | defer p.activeReaders.Add(-1) | |||
| 33 | + | for { | |||
| 34 | + | maximum := p.maxReaders.Load() | |||
| 35 | + | if active <= maximum || p.maxReaders.CompareAndSwap(maximum, active) { | |||
| 36 | + | break | |||
| 37 | + | } | |||
| 38 | + | } | |||
| 39 | + | if p.reads.Add(1) > 1 { | |||
| 40 | + | p.processed <- struct{}{} | |||
| 41 | + | } | |||
| 42 | + | step := <-p.steps | |||
| 43 | + | return copy(buf, step.data), step.err | |||
| 44 | + | } | |||
| 45 | + | ||||
| 46 | + | func (*ownerRoutingPTY) Write(b []byte) (int, error) { return len(b), nil } | |||
| 47 | + | func (p *ownerRoutingPTY) Close() error { p.closes.Add(1); return nil } | |||
| 48 | + | func (*ownerRoutingPTY) Resize(domain.Size) error { return nil } | |||
| 49 | + | func (*ownerRoutingPTY) Pid() int { return 0 } | |||
| 50 | + | func (*ownerRoutingPTY) ForegroundPgid() (int, error) { | |||
| 51 | + | return 0, nil | |||
| 52 | + | } | |||
| 53 | + | ||||
| 54 | + | func installTiledPaneOwnerForTest(sess *session, tb *tab, p *pane) { | |||
| 55 | + | tb.mu.Lock() | |||
| 56 | + | old := tb.panes[layout.PaneID("pane-1")] | |||
| 57 | + | delete(tb.panes, layout.PaneID("pane-1")) | |||
| 58 | + | if old != nil { | |||
| 59 | + | old.clearOwnerForTest() | |||
| 60 | + | } | |||
| 61 | + | p.id = layout.PaneID("pane-1") | |||
| 62 | + | tb.panes[p.id] = p | |||
| 63 | + | tb.tree = &layout.Tree{Root: layout.NewLeaf(p.id), Focus: p.id} | |||
| 64 | + | publishPaneOwner(p, sess, tb, 0) | |||
| 65 | + | tb.mu.Unlock() | |||
| 66 | + | } | |||
| 67 | + | ||||
| 68 | + | func (p *pane) clearOwnerForTest() { | |||
| 69 | + | p.mu.Lock() | |||
| 70 | + | p.clearOwnerLocked() | |||
| 71 | + | p.mu.Unlock() | |||
| 72 | + | } | |||
| 73 | + | ||||
| 74 | + | func TestPTYReaderRoutesEffectsToOwnerPublishedForEachRead(t *testing.T) { | |||
| 75 | + | pty := newOwnerRoutingPTY() | |||
| 76 | + | d, source, _, _ := newManualSessionWithPTYs(t, pty) | |||
| 77 | + | pane := source.activeTab().focusedPane() | |||
| 78 | + | pane.onExit = func() {} | |||
| 79 | + | source.snapEligible.Store(true) | |||
| 80 | + | ||||
| 81 | + | destinationTab := newTab(nil, domain.Size{Cols: 80, Rows: 23}) | |||
| 82 | + | destination := &session{ | |||
| 83 | + | id: domain.SessionID("destination"), | |||
| 84 | + | name: "destination", | |||
| 85 | + | tabs: []*tab{destinationTab}, | |||
| 86 | + | client: &attachedClient{}, | |||
| 87 | + | snapEligible: atomic.Bool{}, | |||
| 88 | + | } | |||
| 89 | + | destination.snapEligible.Store(true) | |||
| 90 | + | destination.client.initOverlays() | |||
| 91 | + | destination.client.setSession(destination) | |||
| 92 | + | d.sessions[destination.id] = destination | |||
| 93 | + | ||||
| 94 | + | sourceInvalidations := make(chan renderInvalidation, 2) | |||
| 95 | + | destinationInvalidations := make(chan renderInvalidation, 2) | |||
| 96 | + | sourceCoordinator := d.attachCoordinator(source, nil, source.client, true) | |||
| 97 | + | sourceCoordinator.opts.onInvalidate = func(invalidation renderInvalidation) { sourceInvalidations <- invalidation } | |||
| 98 | + | destinationCoordinator := d.attachCoordinator(destination, nil, destination.client, true) | |||
| 99 | + | destinationCoordinator.opts.onInvalidate = func(invalidation renderInvalidation) { destinationInvalidations <- invalidation } | |||
| 100 | + | ||||
| 101 | + | d.startPaneGoroutines(source, source.activeTab(), pane) | |||
| 102 | + | pty.steps <- channelPTYStep{data: []byte("source\a")} | |||
| 103 | + | <-pty.processed | |||
| 104 | + | require.Len(t, sourceInvalidations, 1) | |||
| 105 | + | require.Empty(t, destinationInvalidations) | |||
| 106 | + | require.Equal(t, uint64(1), source.snapshotGeneration) | |||
| 107 | + | require.Zero(t, destination.snapshotGeneration) | |||
| 108 | + | require.True(t, source.activeTab().attention) | |||
| 109 | + | ||||
| 110 | + | installTiledPaneOwnerForTest(destination, destinationTab, pane) | |||
| 111 | + | pty.steps <- channelPTYStep{data: []byte("destination\a")} | |||
| 112 | + | <-pty.processed | |||
| 113 | + | ||||
| 114 | + | require.Len(t, sourceInvalidations, 1, "output after owner publication must not invalidate the source") | |||
| 115 | + | require.Len(t, destinationInvalidations, 1, "output after owner publication must invalidate the destination") | |||
| 116 | + | require.Equal(t, uint64(1), source.snapshotGeneration, "source snapshot must not be dirtied after transfer") | |||
| 117 | + | require.Equal(t, uint64(1), destination.snapshotGeneration) | |||
| 118 | + | require.True(t, destinationTab.attention) | |||
| 119 | + | require.Equal(t, int32(1), pty.maxReaders.Load(), "owner publication must not start another PTY reader") | |||
| 120 | + | ||||
| 121 | + | pty.steps <- channelPTYStep{err: io.EOF} | |||
| 122 | + | d.sessWg.Wait() | |||
| 123 | + | } | |||
| 124 | + | ||||
| 125 | + | func TestPTYReaderExitReapsExactlyCurrentOwnerAfterPublication(t *testing.T) { | |||
| 126 | + | pty := newOwnerRoutingPTY() | |||
| 127 | + | d, source, _, _ := newManualSessionWithPTYs(t, pty) | |||
| 128 | + | sourceTab := source.activeTab() | |||
| 129 | + | moved := sourceTab.focusedPane() | |||
| 130 | + | ||||
| 131 | + | destinationTab := newTab(newQuietPTY(), domain.Size{Cols: 80, Rows: 23}) | |||
| 132 | + | other := newPane(layout.PaneID("pane-2"), newQuietPTY(), domain.Size{Cols: 40, Rows: 23}) | |||
| 133 | + | destinationTab.mu.Lock() | |||
| 134 | + | require.NoError(t, destinationTab.tree.Split(layout.PaneID("pane-1"), layout.Right, true, other.id, domain.Rect{Width: 80, Height: 23})) | |||
| 135 | + | destinationTab.panes[other.id] = other | |||
| 136 | + | destinationTab.mu.Unlock() | |||
| 137 | + | destination := &session{id: domain.SessionID("destination"), name: "destination", tabs: []*tab{destinationTab}} | |||
| 138 | + | d.sessions[destination.id] = destination | |||
| 139 | + | ||||
| 140 | + | // Model a completed owner commit while the one reader remains blocked in | |||
| 141 | + | // Read. The source immediately reuses the old local ID, which makes stale | |||
| 142 | + | // ID-only exit routing observably close the wrong pane. | |||
| 143 | + | replacement := newPane(layout.PaneID("pane-1"), newQuietPTY(), domain.Size{Cols: 80, Rows: 23}) | |||
| 144 | + | sourceTab.mu.Lock() | |||
| 145 | + | sourceTab.panes[replacement.id] = replacement | |||
| 146 | + | publishPaneOwner(replacement, source, sourceTab, 0) | |||
| 147 | + | sourceTab.mu.Unlock() | |||
| 148 | + | ||||
| 149 | + | destinationTab.mu.Lock() | |||
| 150 | + | displaced := destinationTab.panes[layout.PaneID("pane-1")] | |||
| 151 | + | displaced.clearOwnerForTest() | |||
| 152 | + | destinationTab.panes[moved.id] = moved | |||
| 153 | + | publishPaneOwner(moved, destination, destinationTab, 0) | |||
| 154 | + | destinationTab.mu.Unlock() | |||
| 155 | + | ||||
| 156 | + | d.sessWg.Add(1) | |||
| 157 | + | go d.ptyReader(source, sourceTab, moved) | |||
| 158 | + | pty.steps <- channelPTYStep{err: io.EOF} | |||
| 159 | + | d.sessWg.Wait() | |||
| 160 | + | ||||
| 161 | + | d.mu.Lock() | |||
| 162 | + | require.Same(t, source, d.sessions[source.id], "stale exit must not close the source replacement") | |||
| 163 | + | require.Same(t, destination, d.sessions[destination.id]) | |||
| 164 | + | d.mu.Unlock() | |||
| 165 | + | sourceTab.mu.Lock() | |||
| 166 | + | require.Same(t, replacement, sourceTab.panes[replacement.id]) | |||
| 167 | + | sourceTab.mu.Unlock() | |||
| 168 | + | destinationTab.mu.Lock() | |||
| 169 | + | require.NotContains(t, destinationTab.panes, moved.id, "the exited pane must be removed from its current owner") | |||
| 170 | + | require.Same(t, other, destinationTab.panes[other.id]) | |||
| 171 | + | destinationTab.mu.Unlock() | |||
| 172 | + | require.Nil(t, moved.ownerSnapshot()) | |||
| 173 | + | require.True(t, replacement.effectLease().Current()) | |||
| 174 | + | require.Equal(t, int32(1), pty.maxReaders.Load()) | |||
| 175 | + | ||||
| 176 | + | // A stale second reap is a no-op and cannot close either surviving owner. | |||
| 177 | + | d.reapPaneOwner(moved) | |||
| 178 | + | sourceTab.mu.Lock() | |||
| 179 | + | require.Same(t, replacement, sourceTab.panes[replacement.id]) | |||
| 180 | + | sourceTab.mu.Unlock() | |||
| 181 | + | destinationTab.mu.Lock() | |||
| 182 | + | require.Same(t, other, destinationTab.panes[other.id]) | |||
| 183 | + | destinationTab.mu.Unlock() | |||
| 184 | + | require.Equal(t, int32(1), pty.closes.Load(), "exit must release the process exactly once") | |||
| 185 | + | } | |||
| 186 | + | ||||
| 187 | + | var _ ports.PTY = (*ownerRoutingPTY)(nil) | |||
A internal/usecase/daemon/pane_sync_owner.go +47 -0
| @@ -0,0 +1,47 @@ | |||
| 1 | + | package daemon | |
| 2 | + | ||
| 3 | + | // migratePaneSyncOwnerLocked transfers synchronized-output coordination after | |
| 4 | + | // an ownership commit. The caller must hold p.mu and pass the exact leases | |
| 5 | + | // returned by publishOwnerLocked. Holding the pane parsing fence keeps PTY | |
| 6 | + | // writes from observing a coordinator between owners. | |
| 7 | + | // | |
| 8 | + | // The returned cleanup must be finished after releasing pane and coordinator | |
| 9 | + | // locks. It stops detached timers but never joins their workers. | |
| 10 | + | func (d *Daemon) migratePaneSyncOwnerLocked(p *pane, oldOwner, newOwner paneEffectLease) syncTimerCleanup { | |
| 11 | + | var cleanup syncTimerCleanup | |
| 12 | + | if p == nil || oldOwner.pane != p || newOwner.pane != p || newOwner.owner == nil || p.owner.Load() != newOwner.owner { | |
| 13 | + | return cleanup | |
| 14 | + | } | |
| 15 | + | ||
| 16 | + | oldGeneration := p.syncGen | |
| 17 | + | if oldOwner.owner != nil && oldOwner.owner.session != nil { | |
| 18 | + | if coordinator := oldOwner.owner.session.renderCoordinator(); coordinator != nil { | |
| 19 | + | cleanup.append(coordinator.detachSyncBatchGeneration(p, oldGeneration)) | |
| 20 | + | } | |
| 21 | + | } | |
| 22 | + | p.syncGen = 0 | |
| 23 | + | if p.screen == nil || !p.screen.SyncUpdateActive() || newOwner.owner.session == nil || newOwner.owner.tab == nil { | |
| 24 | + | return cleanup | |
| 25 | + | } | |
| 26 | + | ||
| 27 | + | generation := newOwner.owner.session.syncGen.Add(1) | |
| 28 | + | p.syncGen = generation | |
| 29 | + | coordinator := newOwner.owner.session.renderCoordinator() | |
| 30 | + | if coordinator == nil { | |
| 31 | + | return cleanup | |
| 32 | + | } | |
| 33 | + | ||
| 34 | + | lease := newOwner | |
| 35 | + | renderable := func() bool { | |
| 36 | + | return lease.Current() && d.paneRenderable(lease.owner.session, lease.owner.tab, p) | |
| 37 | + | } | |
| 38 | + | forceEnd := func() { | |
| 39 | + | p.mu.Lock() | |
| 40 | + | if p.owner.Load() == lease.owner && p.syncGen == generation && p.screen.SyncUpdateActive() { | |
| 41 | + | p.screen.ForceSyncEnd() | |
| 42 | + | } | |
| 43 | + | p.mu.Unlock() | |
| 44 | + | } | |
| 45 | + | cleanup.append(coordinator.beginSyncBatchWithRenderability(p, generation, renderable, forceEnd)) | |
| 46 | + | return cleanup | |
| 47 | + | } | |
| @@ -0,0 +1,47 @@ | |||||
| 1 | + | package daemon | |||
| 2 | + | ||||
| 3 | + | // migratePaneSyncOwnerLocked transfers synchronized-output coordination after | |||
| 4 | + | // an ownership commit. The caller must hold p.mu and pass the exact leases | |||
| 5 | + | // returned by publishOwnerLocked. Holding the pane parsing fence keeps PTY | |||
| 6 | + | // writes from observing a coordinator between owners. | |||
| 7 | + | // | |||
| 8 | + | // The returned cleanup must be finished after releasing pane and coordinator | |||
| 9 | + | // locks. It stops detached timers but never joins their workers. | |||
| 10 | + | func (d *Daemon) migratePaneSyncOwnerLocked(p *pane, oldOwner, newOwner paneEffectLease) syncTimerCleanup { | |||
| 11 | + | var cleanup syncTimerCleanup | |||
| 12 | + | if p == nil || oldOwner.pane != p || newOwner.pane != p || newOwner.owner == nil || p.owner.Load() != newOwner.owner { | |||
| 13 | + | return cleanup | |||
| 14 | + | } | |||
| 15 | + | ||||
| 16 | + | oldGeneration := p.syncGen | |||
| 17 | + | if oldOwner.owner != nil && oldOwner.owner.session != nil { | |||
| 18 | + | if coordinator := oldOwner.owner.session.renderCoordinator(); coordinator != nil { | |||
| 19 | + | cleanup.append(coordinator.detachSyncBatchGeneration(p, oldGeneration)) | |||
| 20 | + | } | |||
| 21 | + | } | |||
| 22 | + | p.syncGen = 0 | |||
| 23 | + | if p.screen == nil || !p.screen.SyncUpdateActive() || newOwner.owner.session == nil || newOwner.owner.tab == nil { | |||
| 24 | + | return cleanup | |||
| 25 | + | } | |||
| 26 | + | ||||
| 27 | + | generation := newOwner.owner.session.syncGen.Add(1) | |||
| 28 | + | p.syncGen = generation | |||
| 29 | + | coordinator := newOwner.owner.session.renderCoordinator() | |||
| 30 | + | if coordinator == nil { | |||
| 31 | + | return cleanup | |||
| 32 | + | } | |||
| 33 | + | ||||
| 34 | + | lease := newOwner | |||
| 35 | + | renderable := func() bool { | |||
| 36 | + | return lease.Current() && d.paneRenderable(lease.owner.session, lease.owner.tab, p) | |||
| 37 | + | } | |||
| 38 | + | forceEnd := func() { | |||
| 39 | + | p.mu.Lock() | |||
| 40 | + | if p.owner.Load() == lease.owner && p.syncGen == generation && p.screen.SyncUpdateActive() { | |||
| 41 | + | p.screen.ForceSyncEnd() | |||
| 42 | + | } | |||
| 43 | + | p.mu.Unlock() | |||
| 44 | + | } | |||
| 45 | + | cleanup.append(coordinator.beginSyncBatchWithRenderability(p, generation, renderable, forceEnd)) | |||
| 46 | + | return cleanup | |||
| 47 | + | } | |||
A internal/usecase/daemon/pane_sync_owner_test.go +175 -0
| @@ -0,0 +1,175 @@ | |||
| 1 | + | package daemon | |
| 2 | + | ||
| 3 | + | import ( | |
| 4 | + | "sync" | |
| 5 | + | "testing" | |
| 6 | + | "time" | |
| 7 | + | ||
| 8 | + | "github.com/bnema/vev/internal/domain" | |
| 9 | + | "github.com/bnema/vev/internal/usecase/layout" | |
| 10 | + | "github.com/bnema/vev/pkg/renderer" | |
| 11 | + | "github.com/stretchr/testify/require" | |
| 12 | + | ) | |
| 13 | + | ||
| 14 | + | type paneSyncOwnerFixture struct { | |
| 15 | + | d *Daemon | |
| 16 | + | pane *pane | |
| 17 | + | source, destination *session | |
| 18 | + | sourceTab, targetTab *tab | |
| 19 | + | sourceCoordinator *renderCoordinator | |
| 20 | + | targetCoordinator *renderCoordinator | |
| 21 | + | sourceClock *coordinatorMockClock | |
| 22 | + | targetClock *coordinatorMockClock | |
| 23 | + | } | |
| 24 | + | ||
| 25 | + | func newPaneSyncOwnerFixture(t *testing.T) paneSyncOwnerFixture { | |
| 26 | + | t.Helper() | |
| 27 | + | size := domain.Size{Cols: 80, Rows: 23} | |
| 28 | + | sourceTab := newTab(newQuietPTY(), size) | |
| 29 | + | targetTab := newTab(newQuietPTY(), size) | |
| 30 | + | source := &session{id: "sync-source", tabs: []*tab{sourceTab}, active: 0, client: &attachedClient{}} | |
| 31 | + | destination := &session{id: "sync-destination", tabs: []*tab{targetTab}, active: 0, client: &attachedClient{}} | |
| 32 | + | d := &Daemon{sessions: map[domain.SessionID]*session{source.id: source, destination.id: destination}} | |
| 33 | + | p := sourceTab.focusedPane() | |
| 34 | + | publishPaneOwner(p, source, sourceTab, 0) | |
| 35 | + | ||
| 36 | + | // Model the membership commit that accompanies owner publication. Keeping | |
| 37 | + | // the source membership is harmless here; callbacks must route by owner. | |
| 38 | + | targetTab.mu.Lock() | |
| 39 | + | targetTab.panes[p.id] = p | |
| 40 | + | targetTab.tree.Root = layout.NewLeaf(p.id) | |
| 41 | + | targetTab.tree.Focus = p.id | |
| 42 | + | targetTab.mu.Unlock() | |
| 43 | + | ||
| 44 | + | sourceClock := newCoordinatorMockClock(t, 8) | |
| 45 | + | targetClock := newCoordinatorMockClock(t, 8) | |
| 46 | + | sourceCoordinator := newRenderCoordinator(renderCoordinatorOptions{clock: sourceClock.clock}) | |
| 47 | + | targetCoordinator := newRenderCoordinator(renderCoordinatorOptions{clock: targetClock.clock}) | |
| 48 | + | source.installRenderCoordinator(sourceCoordinator) | |
| 49 | + | destination.installRenderCoordinator(targetCoordinator) | |
| 50 | + | return paneSyncOwnerFixture{ | |
| 51 | + | d: d, pane: p, source: source, destination: destination, | |
| 52 | + | sourceTab: sourceTab, targetTab: targetTab, | |
| 53 | + | sourceCoordinator: sourceCoordinator, targetCoordinator: targetCoordinator, | |
| 54 | + | sourceClock: sourceClock, targetClock: targetClock, | |
| 55 | + | } | |
| 56 | + | } | |
| 57 | + | ||
| 58 | + | func (f paneSyncOwnerFixture) armSourceBatch(t *testing.T, active bool, force func()) (*coordinatorMockTimer, paneEffectLease) { | |
| 59 | + | t.Helper() | |
| 60 | + | f.pane.mu.Lock() | |
| 61 | + | if active { | |
| 62 | + | f.pane.screen.Write([]byte(renderer.SyncStartCSI)) | |
| 63 | + | } | |
| 64 | + | generation := f.source.syncGen.Add(1) | |
| 65 | + | f.pane.syncGen = generation | |
| 66 | + | lease := f.pane.effectLeaseLocked() | |
| 67 | + | f.pane.mu.Unlock() | |
| 68 | + | f.sourceCoordinator.noteSyncBeginWithRenderability(f.pane, generation, lease.Current, force) | |
| 69 | + | return awaitCoordinatorScheduledTimer(t, f.sourceClock), lease | |
| 70 | + | } | |
| 71 | + | ||
| 72 | + | func TestMigratePaneSynchronizedOutputOwner(t *testing.T) { | |
| 73 | + | t.Run("active batch moves to destination and only destination consumes sync end", func(t *testing.T) { | |
| 74 | + | fixture := newPaneSyncOwnerFixture(t) | |
| 75 | + | forceEntered := make(chan struct{}) | |
| 76 | + | var enterOnce sync.Once | |
| 77 | + | var oldLease paneEffectLease | |
| 78 | + | oldTimer, oldLease := fixture.armSourceBatch(t, true, func() { | |
| 79 | + | enterOnce.Do(func() { close(forceEntered) }) | |
| 80 | + | fixture.pane.mu.Lock() | |
| 81 | + | if fixture.pane.owner.Load() == oldLease.owner && fixture.pane.screen.SyncUpdateActive() { | |
| 82 | + | fixture.pane.screen.ForceSyncEnd() | |
| 83 | + | } | |
| 84 | + | fixture.pane.mu.Unlock() | |
| 85 | + | }) | |
| 86 | + | fixture.sourceCoordinator.mu.Lock() | |
| 87 | + | oldWorkerDone := fixture.sourceCoordinator.syncBatches[fixture.pane].lane.token.done | |
| 88 | + | fixture.sourceCoordinator.mu.Unlock() | |
| 89 | + | ||
| 90 | + | // Select the old watchdog while the pane parsing fence is held. The | |
| 91 | + | // migration must detach it without waiting for its blocked callback. | |
| 92 | + | fixture.pane.mu.Lock() | |
| 93 | + | oldTimer.ch <- time.Time{} | |
| 94 | + | <-forceEntered | |
| 95 | + | oldOwner, newOwner := fixture.pane.publishOwnerLocked(fixture.destination, fixture.targetTab, 0) | |
| 96 | + | cleanup := fixture.d.migratePaneSyncOwnerLocked(fixture.pane, oldOwner, newOwner) | |
| 97 | + | oldTimer.mock.AssertNotCalled(t, "Stop") | |
| 98 | + | fixture.sourceCoordinator.mu.Lock() | |
| 99 | + | require.NotContains(t, fixture.sourceCoordinator.syncBatches, fixture.pane) | |
| 100 | + | fixture.sourceCoordinator.mu.Unlock() | |
| 101 | + | fixture.targetCoordinator.mu.Lock() | |
| 102 | + | targetBatch := fixture.targetCoordinator.syncBatches[fixture.pane] | |
| 103 | + | fixture.targetCoordinator.mu.Unlock() | |
| 104 | + | require.NotNil(t, targetBatch) | |
| 105 | + | require.Equal(t, fixture.destination.syncGen.Load(), targetBatch.generation) | |
| 106 | + | fixture.pane.mu.Unlock() | |
| 107 | + | cleanup.finish() | |
| 108 | + | oldTimer.mock.AssertNumberOfCalls(t, "Stop", 1) | |
| 109 | + | <-oldWorkerDone | |
| 110 | + | ||
| 111 | + | fixture.pane.mu.Lock() | |
| 112 | + | require.True(t, fixture.pane.screen.SyncUpdateActive(), "selected old watchdog must become a stale no-op") | |
| 113 | + | fixture.pane.mu.Unlock() | |
| 114 | + | ||
| 115 | + | fixture.d.processPanePTYData(fixture.pane, []byte(renderer.SyncEndCSI), false) | |
| 116 | + | fixture.sourceCoordinator.mu.Lock() | |
| 117 | + | require.NotContains(t, fixture.sourceCoordinator.syncBatches, fixture.pane) | |
| 118 | + | fixture.sourceCoordinator.mu.Unlock() | |
| 119 | + | fixture.targetCoordinator.mu.Lock() | |
| 120 | + | require.NotContains(t, fixture.targetCoordinator.syncBatches, fixture.pane) | |
| 121 | + | fixture.targetCoordinator.mu.Unlock() | |
| 122 | + | fixture.pane.mu.Lock() | |
| 123 | + | require.False(t, fixture.pane.screen.SyncUpdateActive()) | |
| 124 | + | fixture.pane.mu.Unlock() | |
| 125 | + | }) | |
| 126 | + | ||
| 127 | + | t.Run("destination callbacks reject a newer owner generation", func(t *testing.T) { | |
| 128 | + | fixture := newPaneSyncOwnerFixture(t) | |
| 129 | + | _, _ = fixture.armSourceBatch(t, true, nil) | |
| 130 | + | fixture.pane.mu.Lock() | |
| 131 | + | oldOwner, newOwner := fixture.pane.publishOwnerLocked(fixture.destination, fixture.targetTab, 0) | |
| 132 | + | cleanup := fixture.d.migratePaneSyncOwnerLocked(fixture.pane, oldOwner, newOwner) | |
| 133 | + | fixture.pane.mu.Unlock() | |
| 134 | + | cleanup.finish() | |
| 135 | + | _ = awaitCoordinatorScheduledTimer(t, fixture.targetClock) | |
| 136 | + | ||
| 137 | + | fixture.targetCoordinator.mu.Lock() | |
| 138 | + | targetBatch := fixture.targetCoordinator.syncBatches[fixture.pane] | |
| 139 | + | fixture.targetCoordinator.mu.Unlock() | |
| 140 | + | require.NotNil(t, targetBatch) | |
| 141 | + | require.True(t, targetBatch.renderable()) | |
| 142 | + | ||
| 143 | + | fixture.pane.mu.Lock() | |
| 144 | + | fixture.pane.publishOwnerLocked(fixture.source, fixture.sourceTab, 0) | |
| 145 | + | fixture.pane.mu.Unlock() | |
| 146 | + | require.False(t, targetBatch.renderable(), "renderability must validate the destination owner generation") | |
| 147 | + | targetBatch.force() | |
| 148 | + | fixture.pane.mu.Lock() | |
| 149 | + | require.True(t, fixture.pane.screen.SyncUpdateActive(), "force-end must validate the destination owner generation") | |
| 150 | + | fixture.pane.mu.Unlock() | |
| 151 | + | fixture.targetCoordinator.noteSyncPaneRemoved(fixture.pane) | |
| 152 | + | }) | |
| 153 | + | ||
| 154 | + | t.Run("inactive screen removes source batch without creating destination state", func(t *testing.T) { | |
| 155 | + | fixture := newPaneSyncOwnerFixture(t) | |
| 156 | + | oldTimer, _ := fixture.armSourceBatch(t, false, nil) | |
| 157 | + | ||
| 158 | + | fixture.pane.mu.Lock() | |
| 159 | + | oldOwner, newOwner := fixture.pane.publishOwnerLocked(fixture.destination, fixture.targetTab, 0) | |
| 160 | + | cleanup := fixture.d.migratePaneSyncOwnerLocked(fixture.pane, oldOwner, newOwner) | |
| 161 | + | require.Zero(t, fixture.pane.syncGen) | |
| 162 | + | fixture.pane.mu.Unlock() | |
| 163 | + | cleanup.finish() | |
| 164 | + | ||
| 165 | + | fixture.sourceCoordinator.mu.Lock() | |
| 166 | + | require.NotContains(t, fixture.sourceCoordinator.syncBatches, fixture.pane) | |
| 167 | + | fixture.sourceCoordinator.mu.Unlock() | |
| 168 | + | fixture.targetCoordinator.mu.Lock() | |
| 169 | + | require.NotContains(t, fixture.targetCoordinator.syncBatches, fixture.pane) | |
| 170 | + | fixture.targetCoordinator.mu.Unlock() | |
| 171 | + | require.Zero(t, fixture.destination.syncGen.Load()) | |
| 172 | + | require.Empty(t, fixture.targetClock.timers) | |
| 173 | + | oldTimer.mock.AssertNumberOfCalls(t, "Stop", 1) | |
| 174 | + | }) | |
| 175 | + | } | |
| @@ -0,0 +1,175 @@ | |||||
| 1 | + | package daemon | |||
| 2 | + | ||||
| 3 | + | import ( | |||
| 4 | + | "sync" | |||
| 5 | + | "testing" | |||
| 6 | + | "time" | |||
| 7 | + | ||||
| 8 | + | "github.com/bnema/vev/internal/domain" | |||
| 9 | + | "github.com/bnema/vev/internal/usecase/layout" | |||
| 10 | + | "github.com/bnema/vev/pkg/renderer" | |||
| 11 | + | "github.com/stretchr/testify/require" | |||
| 12 | + | ) | |||
| 13 | + | ||||
| 14 | + | type paneSyncOwnerFixture struct { | |||
| 15 | + | d *Daemon | |||
| 16 | + | pane *pane | |||
| 17 | + | source, destination *session | |||
| 18 | + | sourceTab, targetTab *tab | |||
| 19 | + | sourceCoordinator *renderCoordinator | |||
| 20 | + | targetCoordinator *renderCoordinator | |||
| 21 | + | sourceClock *coordinatorMockClock | |||
| 22 | + | targetClock *coordinatorMockClock | |||
| 23 | + | } | |||
| 24 | + | ||||
| 25 | + | func newPaneSyncOwnerFixture(t *testing.T) paneSyncOwnerFixture { | |||
| 26 | + | t.Helper() | |||
| 27 | + | size := domain.Size{Cols: 80, Rows: 23} | |||
| 28 | + | sourceTab := newTab(newQuietPTY(), size) | |||
| 29 | + | targetTab := newTab(newQuietPTY(), size) | |||
| 30 | + | source := &session{id: "sync-source", tabs: []*tab{sourceTab}, active: 0, client: &attachedClient{}} | |||
| 31 | + | destination := &session{id: "sync-destination", tabs: []*tab{targetTab}, active: 0, client: &attachedClient{}} | |||
| 32 | + | d := &Daemon{sessions: map[domain.SessionID]*session{source.id: source, destination.id: destination}} | |||
| 33 | + | p := sourceTab.focusedPane() | |||
| 34 | + | publishPaneOwner(p, source, sourceTab, 0) | |||
| 35 | + | ||||
| 36 | + | // Model the membership commit that accompanies owner publication. Keeping | |||
| 37 | + | // the source membership is harmless here; callbacks must route by owner. | |||
| 38 | + | targetTab.mu.Lock() | |||
| 39 | + | targetTab.panes[p.id] = p | |||
| 40 | + | targetTab.tree.Root = layout.NewLeaf(p.id) | |||
| 41 | + | targetTab.tree.Focus = p.id | |||
| 42 | + | targetTab.mu.Unlock() | |||
| 43 | + | ||||
| 44 | + | sourceClock := newCoordinatorMockClock(t, 8) | |||
| 45 | + | targetClock := newCoordinatorMockClock(t, 8) | |||
| 46 | + | sourceCoordinator := newRenderCoordinator(renderCoordinatorOptions{clock: sourceClock.clock}) | |||
| 47 | + | targetCoordinator := newRenderCoordinator(renderCoordinatorOptions{clock: targetClock.clock}) | |||
| 48 | + | source.installRenderCoordinator(sourceCoordinator) | |||
| 49 | + | destination.installRenderCoordinator(targetCoordinator) | |||
| 50 | + | return paneSyncOwnerFixture{ | |||
| 51 | + | d: d, pane: p, source: source, destination: destination, | |||
| 52 | + | sourceTab: sourceTab, targetTab: targetTab, | |||
| 53 | + | sourceCoordinator: sourceCoordinator, targetCoordinator: targetCoordinator, | |||
| 54 | + | sourceClock: sourceClock, targetClock: targetClock, | |||
| 55 | + | } | |||
| 56 | + | } | |||
| 57 | + | ||||
| 58 | + | func (f paneSyncOwnerFixture) armSourceBatch(t *testing.T, active bool, force func()) (*coordinatorMockTimer, paneEffectLease) { | |||
| 59 | + | t.Helper() | |||
| 60 | + | f.pane.mu.Lock() | |||
| 61 | + | if active { | |||
| 62 | + | f.pane.screen.Write([]byte(renderer.SyncStartCSI)) | |||
| 63 | + | } | |||
| 64 | + | generation := f.source.syncGen.Add(1) | |||
| 65 | + | f.pane.syncGen = generation | |||
| 66 | + | lease := f.pane.effectLeaseLocked() | |||
| 67 | + | f.pane.mu.Unlock() | |||
| 68 | + | f.sourceCoordinator.noteSyncBeginWithRenderability(f.pane, generation, lease.Current, force) | |||
| 69 | + | return awaitCoordinatorScheduledTimer(t, f.sourceClock), lease | |||
| 70 | + | } | |||
| 71 | + | ||||
| 72 | + | func TestMigratePaneSynchronizedOutputOwner(t *testing.T) { | |||
| 73 | + | t.Run("active batch moves to destination and only destination consumes sync end", func(t *testing.T) { | |||
| 74 | + | fixture := newPaneSyncOwnerFixture(t) | |||
| 75 | + | forceEntered := make(chan struct{}) | |||
| 76 | + | var enterOnce sync.Once | |||
| 77 | + | var oldLease paneEffectLease | |||
| 78 | + | oldTimer, oldLease := fixture.armSourceBatch(t, true, func() { | |||
| 79 | + | enterOnce.Do(func() { close(forceEntered) }) | |||
| 80 | + | fixture.pane.mu.Lock() | |||
| 81 | + | if fixture.pane.owner.Load() == oldLease.owner && fixture.pane.screen.SyncUpdateActive() { | |||
| 82 | + | fixture.pane.screen.ForceSyncEnd() | |||
| 83 | + | } | |||
| 84 | + | fixture.pane.mu.Unlock() | |||
| 85 | + | }) | |||
| 86 | + | fixture.sourceCoordinator.mu.Lock() | |||
| 87 | + | oldWorkerDone := fixture.sourceCoordinator.syncBatches[fixture.pane].lane.token.done | |||
| 88 | + | fixture.sourceCoordinator.mu.Unlock() | |||
| 89 | + | ||||
| 90 | + | // Select the old watchdog while the pane parsing fence is held. The | |||
| 91 | + | // migration must detach it without waiting for its blocked callback. | |||
| 92 | + | fixture.pane.mu.Lock() | |||
| 93 | + | oldTimer.ch <- time.Time{} | |||
| 94 | + | <-forceEntered | |||
| 95 | + | oldOwner, newOwner := fixture.pane.publishOwnerLocked(fixture.destination, fixture.targetTab, 0) | |||
| 96 | + | cleanup := fixture.d.migratePaneSyncOwnerLocked(fixture.pane, oldOwner, newOwner) | |||
| 97 | + | oldTimer.mock.AssertNotCalled(t, "Stop") | |||
| 98 | + | fixture.sourceCoordinator.mu.Lock() | |||
| 99 | + | require.NotContains(t, fixture.sourceCoordinator.syncBatches, fixture.pane) | |||
| 100 | + | fixture.sourceCoordinator.mu.Unlock() | |||
| 101 | + | fixture.targetCoordinator.mu.Lock() | |||
| 102 | + | targetBatch := fixture.targetCoordinator.syncBatches[fixture.pane] | |||
| 103 | + | fixture.targetCoordinator.mu.Unlock() | |||
| 104 | + | require.NotNil(t, targetBatch) | |||
| 105 | + | require.Equal(t, fixture.destination.syncGen.Load(), targetBatch.generation) | |||
| 106 | + | fixture.pane.mu.Unlock() | |||
| 107 | + | cleanup.finish() | |||
| 108 | + | oldTimer.mock.AssertNumberOfCalls(t, "Stop", 1) | |||
| 109 | + | <-oldWorkerDone | |||
| 110 | + | ||||
| 111 | + | fixture.pane.mu.Lock() | |||
| 112 | + | require.True(t, fixture.pane.screen.SyncUpdateActive(), "selected old watchdog must become a stale no-op") | |||
| 113 | + | fixture.pane.mu.Unlock() | |||
| 114 | + | ||||
| 115 | + | fixture.d.processPanePTYData(fixture.pane, []byte(renderer.SyncEndCSI), false) | |||
| 116 | + | fixture.sourceCoordinator.mu.Lock() | |||
| 117 | + | require.NotContains(t, fixture.sourceCoordinator.syncBatches, fixture.pane) | |||
| 118 | + | fixture.sourceCoordinator.mu.Unlock() | |||
| 119 | + | fixture.targetCoordinator.mu.Lock() | |||
| 120 | + | require.NotContains(t, fixture.targetCoordinator.syncBatches, fixture.pane) | |||
| 121 | + | fixture.targetCoordinator.mu.Unlock() | |||
| 122 | + | fixture.pane.mu.Lock() | |||
| 123 | + | require.False(t, fixture.pane.screen.SyncUpdateActive()) | |||
| 124 | + | fixture.pane.mu.Unlock() | |||
| 125 | + | }) | |||
| 126 | + | ||||
| 127 | + | t.Run("destination callbacks reject a newer owner generation", func(t *testing.T) { | |||
| 128 | + | fixture := newPaneSyncOwnerFixture(t) | |||
| 129 | + | _, _ = fixture.armSourceBatch(t, true, nil) | |||
| 130 | + | fixture.pane.mu.Lock() | |||
| 131 | + | oldOwner, newOwner := fixture.pane.publishOwnerLocked(fixture.destination, fixture.targetTab, 0) | |||
| 132 | + | cleanup := fixture.d.migratePaneSyncOwnerLocked(fixture.pane, oldOwner, newOwner) | |||
| 133 | + | fixture.pane.mu.Unlock() | |||
| 134 | + | cleanup.finish() | |||
| 135 | + | _ = awaitCoordinatorScheduledTimer(t, fixture.targetClock) | |||
| 136 | + | ||||
| 137 | + | fixture.targetCoordinator.mu.Lock() | |||
| 138 | + | targetBatch := fixture.targetCoordinator.syncBatches[fixture.pane] | |||
| 139 | + | fixture.targetCoordinator.mu.Unlock() | |||
| 140 | + | require.NotNil(t, targetBatch) | |||
| 141 | + | require.True(t, targetBatch.renderable()) | |||
| 142 | + | ||||
| 143 | + | fixture.pane.mu.Lock() | |||
| 144 | + | fixture.pane.publishOwnerLocked(fixture.source, fixture.sourceTab, 0) | |||
| 145 | + | fixture.pane.mu.Unlock() | |||
| 146 | + | require.False(t, targetBatch.renderable(), "renderability must validate the destination owner generation") | |||
| 147 | + | targetBatch.force() | |||
| 148 | + | fixture.pane.mu.Lock() | |||
| 149 | + | require.True(t, fixture.pane.screen.SyncUpdateActive(), "force-end must validate the destination owner generation") | |||
| 150 | + | fixture.pane.mu.Unlock() | |||
| 151 | + | fixture.targetCoordinator.noteSyncPaneRemoved(fixture.pane) | |||
| 152 | + | }) | |||
| 153 | + | ||||
| 154 | + | t.Run("inactive screen removes source batch without creating destination state", func(t *testing.T) { | |||
| 155 | + | fixture := newPaneSyncOwnerFixture(t) | |||
| 156 | + | oldTimer, _ := fixture.armSourceBatch(t, false, nil) | |||
| 157 | + | ||||
| 158 | + | fixture.pane.mu.Lock() | |||
| 159 | + | oldOwner, newOwner := fixture.pane.publishOwnerLocked(fixture.destination, fixture.targetTab, 0) | |||
| 160 | + | cleanup := fixture.d.migratePaneSyncOwnerLocked(fixture.pane, oldOwner, newOwner) | |||
| 161 | + | require.Zero(t, fixture.pane.syncGen) | |||
| 162 | + | fixture.pane.mu.Unlock() | |||
| 163 | + | cleanup.finish() | |||
| 164 | + | ||||
| 165 | + | fixture.sourceCoordinator.mu.Lock() | |||
| 166 | + | require.NotContains(t, fixture.sourceCoordinator.syncBatches, fixture.pane) | |||
| 167 | + | fixture.sourceCoordinator.mu.Unlock() | |||
| 168 | + | fixture.targetCoordinator.mu.Lock() | |||
| 169 | + | require.NotContains(t, fixture.targetCoordinator.syncBatches, fixture.pane) | |||
| 170 | + | fixture.targetCoordinator.mu.Unlock() | |||
| 171 | + | require.Zero(t, fixture.destination.syncGen.Load()) | |||
| 172 | + | require.Empty(t, fixture.targetClock.timers) | |||
| 173 | + | oldTimer.mock.AssertNumberOfCalls(t, "Stop", 1) | |||
| 174 | + | }) | |||
| 175 | + | } | |||
M internal/usecase/daemon/render.go +108 -58
| @@ -82,7 +82,16 @@ func (d *Daemon) tabIsPickerPreview(tb *tab) bool { | |||
| 82 | 82 | return false | |
| 83 | 83 | } | |
| 84 | 84 | ||
| 85 | + | // ptyReader is retained as a compatibility entry point for focused tests. The | |
| 86 | + | // production goroutine calls readPanePTY, whose closure owns only the pane. | |
| 85 | 87 | func (d *Daemon) ptyReader(sess *session, tb *tab, p *pane) { | |
| 88 | + | if p != nil && p.ownerSnapshot() == nil && sess != nil && tb != nil { | |
| 89 | + | publishPaneOwner(p, sess, tb, 0) | |
| 90 | + | } | |
| 91 | + | d.readPanePTY(p) | |
| 92 | + | } | |
| 93 | + | ||
| 94 | + | func (d *Daemon) readPanePTY(p *pane) { | |
| 86 | 95 | defer d.sessWg.Done() | |
| 87 | 96 | if p == nil { | |
| 88 | 97 | return | |
| @@ -98,103 +107,144 @@ func (d *Daemon) ptyReader(sess *session, tb *tab, p *pane) { | |||
| 98 | 107 | for { | |
| 99 | 108 | n, err := p.pty.Read(buf) | |
| 100 | 109 | if n > 0 { | |
| 101 | - | d.processPTYData(sess, tb, p, buf[:n], true) | |
| 110 | + | d.processPanePTYData(p, buf[:n], true) | |
| 102 | 111 | } | |
| 103 | 112 | if err != nil { | |
| 104 | - | sess.mu.Lock() | |
| 105 | - | sessionName := sess.name | |
| 106 | - | sess.mu.Unlock() | |
| 107 | - | d.log.Info("pane pty closed", "err", err, "session", sessionName) | |
| 113 | + | lease := p.effectLease() | |
| 114 | + | if sessionName, current := paneEffectSessionName(lease); current { | |
| 115 | + | d.log.Info("pane pty closed", "err", err, "session", sessionName) | |
| 116 | + | } else { | |
| 117 | + | d.log.Info("pane pty closed", "err", err) | |
| 118 | + | } | |
| 108 | 119 | if p.onExit != nil { | |
| 109 | 120 | p.onExit() | |
| 110 | 121 | } else { | |
| 111 | - | d.reapPane(sess, tb, p) | |
| 122 | + | d.reapPaneOwner(p) | |
| 112 | 123 | } | |
| 113 | 124 | return | |
| 114 | 125 | } | |
| 115 | 126 | } | |
| 116 | 127 | } | |
| 117 | 128 | ||
| 118 | - | // processPTYData is the sole VT parsing path. While a resize apply owns the | |
| 129 | + | type panePTYEffects struct { | |
| 130 | + | lease paneEffectLease | |
| 131 | + | renderCoordinator *renderCoordinator | |
| 132 | + | wasSyncing bool | |
| 133 | + | isSyncing bool | |
| 134 | + | completeSyncRead bool | |
| 135 | + | syncEnded bool | |
| 136 | + | attention bool | |
| 137 | + | responses []byte | |
| 138 | + | clipboards []string | |
| 139 | + | } | |
| 140 | + | ||
| 141 | + | // processPTYData retains the old call shape for resize replay and focused tests, | |
| 142 | + | // but routing is always derived from the pane owner captured under pane.mu. | |
| 143 | + | func (d *Daemon) processPTYData(_ *session, _ *tab, p *pane, data []byte, bufferDuringApply bool) { | |
| 144 | + | d.processPanePTYData(p, data, bufferDuringApply) | |
| 145 | + | } | |
| 146 | + | ||
| 147 | + | // processPanePTYData is the sole VT parsing path. While a resize apply owns the | |
| 119 | 148 | // pane, reader calls append exact bytes and returns immediately; it never waits | |
| 120 | 149 | // for PTY.Resize or loses a short read. | |
| 121 | - | func (d *Daemon) processPTYData(sess *session, tb *tab, p *pane, data []byte, bufferDuringApply bool) { | |
| 122 | - | rc := sess.renderCoordinator() | |
| 150 | + | func (d *Daemon) processPanePTYData(p *pane, data []byte, bufferDuringApply bool) { | |
| 151 | + | if p == nil { | |
| 152 | + | return | |
| 153 | + | } | |
| 123 | 154 | p.mu.Lock() | |
| 124 | 155 | if bufferDuringApply && p.resizeApplying { | |
| 125 | 156 | p.resizePending = append(p.resizePending, data...) | |
| 126 | 157 | p.mu.Unlock() | |
| 127 | 158 | return | |
| 128 | 159 | } | |
| 129 | - | wasSyncing := p.screen.SyncUpdateActive() | |
| 160 | + | effects := panePTYEffects{lease: p.effectLeaseLocked()} | |
| 161 | + | owner := effects.lease.owner | |
| 162 | + | if owner != nil { | |
| 163 | + | effects.renderCoordinator = owner.session.renderCoordinator() | |
| 164 | + | } | |
| 165 | + | effects.wasSyncing = p.screen.SyncUpdateActive() | |
| 130 | 166 | p.screen.Write(data) | |
| 131 | 167 | p.refreshTerminalTitleLocked() | |
| 132 | - | isSyncing := p.screen.SyncUpdateActive() | |
| 133 | - | completeSyncRead := !wasSyncing && !isSyncing && completedSynchronizedUpdate(data) | |
| 134 | - | var syncGen uint64 | |
| 135 | - | syncEnded := false | |
| 136 | - | if wasSyncing != isSyncing { | |
| 137 | - | if isSyncing { | |
| 138 | - | syncGen = sess.syncGen.Add(1) | |
| 168 | + | effects.isSyncing = p.screen.SyncUpdateActive() | |
| 169 | + | effects.completeSyncRead = !effects.wasSyncing && !effects.isSyncing && completedSynchronizedUpdate(data) | |
| 170 | + | if effects.wasSyncing != effects.isSyncing && owner != nil { | |
| 171 | + | if effects.isSyncing { | |
| 172 | + | syncGen := owner.session.syncGen.Add(1) | |
| 139 | 173 | p.syncGen = syncGen | |
| 140 | - | if rc != nil { | |
| 141 | - | rc.noteSyncBeginWithRenderability(p, syncGen, func() bool { return d.paneRenderable(sess, tb, p) }, func() { | |
| 174 | + | if effects.renderCoordinator != nil { | |
| 175 | + | lease := effects.lease | |
| 176 | + | effects.renderCoordinator.noteSyncBeginWithRenderability(p, syncGen, func() bool { | |
| 177 | + | return lease.Current() && d.paneRenderable(owner.session, owner.tab, p) | |
| 178 | + | }, func() { | |
| 142 | 179 | p.mu.Lock() | |
| 143 | - | if p.syncGen == syncGen && p.screen.SyncUpdateActive() { | |
| 180 | + | if p.owner.Load() == lease.owner && p.syncGen == syncGen && p.screen.SyncUpdateActive() { | |
| 144 | 181 | p.screen.ForceSyncEnd() | |
| 145 | 182 | } | |
| 146 | 183 | p.mu.Unlock() | |
| 147 | 184 | }) | |
| 148 | 185 | } | |
| 149 | 186 | } else { | |
| 150 | - | syncGen = p.syncGen | |
| 151 | - | syncEnded = rc != nil && rc.removeSyncEnd(p, syncGen, true) | |
| 152 | - | } | |
| 153 | - | } | |
| 154 | - | p.mu.Unlock() | |
| 155 | - | renderable := d.paneRenderable(sess, tb, p) | |
| 156 | - | markSnapshotDirty(sess) | |
| 157 | - | d.flushPTYEffects(sess, tb, p) | |
| 158 | - | if rc != nil { | |
| 159 | - | if wasSyncing != isSyncing { | |
| 160 | - | if isSyncing { | |
| 161 | - | if renderable { | |
| 162 | - | rc.invalidate(renderInvalidation{class: invalidateOutput, producer: "render.go"}) | |
| 163 | - | } | |
| 164 | - | } else if syncEnded && renderable { | |
| 165 | - | rc.invalidate(renderInvalidation{class: invalidateUrgent, producer: "render.go"}) | |
| 166 | - | } | |
| 167 | - | } | |
| 168 | - | if completeSyncRead && renderable { | |
| 169 | - | rc.invalidate(renderInvalidation{class: invalidateUrgent, producer: "render.go"}) | |
| 170 | - | } else if renderable && !isSyncing && wasSyncing == isSyncing { | |
| 171 | - | rc.invalidate(renderInvalidation{class: invalidateOutput, producer: "render.go"}) | |
| 187 | + | effects.syncEnded = effects.renderCoordinator != nil && effects.renderCoordinator.removeSyncEnd(p, p.syncGen, true) | |
| 172 | 188 | } | |
| 173 | 189 | } | |
| 174 | - | } | |
| 175 | - | ||
| 176 | - | // flushPTYEffects preserves the normal response/title/attention/clipboard | |
| 177 | - | // behavior for both direct reads and resize-buffer replay. | |
| 178 | - | func (d *Daemon) flushPTYEffects(sess *session, tb *tab, p *pane) { | |
| 179 | - | p.mu.Lock() | |
| 180 | - | attention := p.ptyAttention | |
| 190 | + | effects.attention = p.ptyAttention | |
| 181 | 191 | p.ptyAttention = false | |
| 182 | - | responses := append([]byte(nil), p.ptyResponses...) | |
| 192 | + | effects.responses = append([]byte(nil), p.ptyResponses...) | |
| 183 | 193 | p.ptyResponses = p.ptyResponses[:0] | |
| 184 | - | clipboards := append([]string(nil), p.ptyClipboards...) | |
| 194 | + | effects.clipboards = append([]string(nil), p.ptyClipboards...) | |
| 185 | 195 | p.ptyClipboards = p.ptyClipboards[:0] | |
| 186 | 196 | p.mu.Unlock() | |
| 187 | - | if attention { | |
| 188 | - | d.noteAttention(sess, tb) | |
| 197 | + | d.applyPanePTYEffects(p, effects) | |
| 198 | + | } | |
| 199 | + | ||
| 200 | + | // applyPanePTYEffects emits only effects still bound to the owner generation | |
| 201 | + | // that parsed them. Each owner-sensitive boundary revalidates independently so | |
| 202 | + | // ownership publication can retire the remaining effects without rerouting | |
| 203 | + | // bytes that belonged to the old owner. | |
| 204 | + | func (d *Daemon) applyPanePTYEffects(p *pane, effects panePTYEffects) { | |
| 205 | + | owner := effects.lease.owner | |
| 206 | + | if owner == nil { | |
| 207 | + | return | |
| 208 | + | } | |
| 209 | + | if effects.lease.Current() { | |
| 210 | + | markSnapshotDirty(owner.session) | |
| 211 | + | } | |
| 212 | + | if effects.attention && effects.lease.Current() { | |
| 213 | + | d.noteAttention(owner.session, owner.tab) | |
| 214 | + | } | |
| 215 | + | for _, b64 := range effects.clipboards { | |
| 216 | + | if effects.lease.Current() { | |
| 217 | + | d.forwardClipboardAsync(effects.lease, b64) | |
| 218 | + | } | |
| 189 | 219 | } | |
| 190 | - | for _, b64 := range clipboards { | |
| 191 | - | d.forwardClipboardAsync(sess, b64) | |
| 220 | + | if len(effects.responses) > 0 && effects.lease.Current() { | |
| 221 | + | if _, err := p.pty.Write(effects.responses); err != nil { | |
| 222 | + | if sessionName, current := paneEffectSessionName(effects.lease); current { | |
| 223 | + | d.log.Warn("pty response write failed", "err", err, "session", sessionName) | |
| 224 | + | } | |
| 225 | + | } | |
| 226 | + | } | |
| 227 | + | if effects.renderCoordinator == nil || !effects.lease.Current() { | |
| 228 | + | return | |
| 192 | 229 | } | |
| 193 | - | if len(responses) > 0 { | |
| 194 | - | if _, err := p.pty.Write(responses); err != nil { | |
| 195 | - | d.log.Warn("pty response write failed", "err", err, "session", sess.name) | |
| 230 | + | renderable := d.paneRenderable(owner.session, owner.tab, p) | |
| 231 | + | if !effects.lease.Current() { | |
| 232 | + | return | |
| 233 | + | } | |
| 234 | + | if effects.wasSyncing != effects.isSyncing { | |
| 235 | + | if effects.isSyncing { | |
| 236 | + | if renderable { | |
| 237 | + | effects.renderCoordinator.invalidate(renderInvalidation{class: invalidateOutput, producer: "render.go"}) | |
| 238 | + | } | |
| 239 | + | } else if effects.syncEnded && renderable { | |
| 240 | + | effects.renderCoordinator.invalidate(renderInvalidation{class: invalidateUrgent, producer: "render.go"}) | |
| 196 | 241 | } | |
| 197 | 242 | } | |
| 243 | + | if effects.completeSyncRead && renderable { | |
| 244 | + | effects.renderCoordinator.invalidate(renderInvalidation{class: invalidateUrgent, producer: "render.go"}) | |
| 245 | + | } else if renderable && !effects.isSyncing && effects.wasSyncing == effects.isSyncing { | |
| 246 | + | effects.renderCoordinator.invalidate(renderInvalidation{class: invalidateOutput, producer: "render.go"}) | |
| 247 | + | } | |
| 198 | 248 | } | |
| 199 | 249 | ||
| 200 | 250 | // paint draws the composed client frame (active tab plus status bar) and | |
| @@ -82,7 +82,16 @@ func (d *Daemon) tabIsPickerPreview(tb *tab) bool { | |||||
| 82 | return false | 82 | return false | ||
| 83 | } | 83 | } | ||
| 84 | 84 | ||||
| 85 | + | // ptyReader is retained as a compatibility entry point for focused tests. The | |||
| 86 | + | // production goroutine calls readPanePTY, whose closure owns only the pane. | |||
| 85 | func (d *Daemon) ptyReader(sess *session, tb *tab, p *pane) { | 87 | func (d *Daemon) ptyReader(sess *session, tb *tab, p *pane) { | ||
| 88 | + | if p != nil && p.ownerSnapshot() == nil && sess != nil && tb != nil { | |||
| 89 | + | publishPaneOwner(p, sess, tb, 0) | |||
| 90 | + | } | |||
| 91 | + | d.readPanePTY(p) | |||
| 92 | + | } | |||
| 93 | + | ||||
| 94 | + | func (d *Daemon) readPanePTY(p *pane) { | |||
| 86 | defer d.sessWg.Done() | 95 | defer d.sessWg.Done() | ||
| 87 | if p == nil { | 96 | if p == nil { | ||
| 88 | return | 97 | return | ||
| @@ -98,103 +107,144 @@ func (d *Daemon) ptyReader(sess *session, tb *tab, p *pane) { | |||||
| 98 | for { | 107 | for { | ||
| 99 | n, err := p.pty.Read(buf) | 108 | n, err := p.pty.Read(buf) | ||
| 100 | if n > 0 { | 109 | if n > 0 { | ||
| 101 | - | d.processPTYData(sess, tb, p, buf[:n], true) | 110 | + | d.processPanePTYData(p, buf[:n], true) |
| 102 | } | 111 | } | ||
| 103 | if err != nil { | 112 | if err != nil { | ||
| 104 | - | sess.mu.Lock() | 113 | + | lease := p.effectLease() |
| 105 | - | sessionName := sess.name | 114 | + | if sessionName, current := paneEffectSessionName(lease); current { |
| 106 | - | sess.mu.Unlock() | 115 | + | d.log.Info("pane pty closed", "err", err, "session", sessionName) |
| 107 | - | d.log.Info("pane pty closed", "err", err, "session", sessionName) | 116 | + | } else { |
| 117 | + | d.log.Info("pane pty closed", "err", err) | |||
| 118 | + | } | |||
| 108 | if p.onExit != nil { | 119 | if p.onExit != nil { | ||
| 109 | p.onExit() | 120 | p.onExit() | ||
| 110 | } else { | 121 | } else { | ||
| 111 | - | d.reapPane(sess, tb, p) | 122 | + | d.reapPaneOwner(p) |
| 112 | } | 123 | } | ||
| 113 | return | 124 | return | ||
| 114 | } | 125 | } | ||
| 115 | } | 126 | } | ||
| 116 | } | 127 | } | ||
| 117 | 128 | ||||
| 118 | - | // processPTYData is the sole VT parsing path. While a resize apply owns the | 129 | + | type panePTYEffects struct { |
| 130 | + | lease paneEffectLease | |||
| 131 | + | renderCoordinator *renderCoordinator | |||
| 132 | + | wasSyncing bool | |||
| 133 | + | isSyncing bool | |||
| 134 | + | completeSyncRead bool | |||
| 135 | + | syncEnded bool | |||
| 136 | + | attention bool | |||
| 137 | + | responses []byte | |||
| 138 | + | clipboards []string | |||
| 139 | + | } | |||
| 140 | + | ||||
| 141 | + | // processPTYData retains the old call shape for resize replay and focused tests, | |||
| 142 | + | // but routing is always derived from the pane owner captured under pane.mu. | |||
| 143 | + | func (d *Daemon) processPTYData(_ *session, _ *tab, p *pane, data []byte, bufferDuringApply bool) { | |||
| 144 | + | d.processPanePTYData(p, data, bufferDuringApply) | |||
| 145 | + | } | |||
| 146 | + | ||||
| 147 | + | // processPanePTYData is the sole VT parsing path. While a resize apply owns the | |||
| 119 | // pane, reader calls append exact bytes and returns immediately; it never waits | 148 | // pane, reader calls append exact bytes and returns immediately; it never waits | ||
| 120 | // for PTY.Resize or loses a short read. | 149 | // for PTY.Resize or loses a short read. | ||
| 121 | - | func (d *Daemon) processPTYData(sess *session, tb *tab, p *pane, data []byte, bufferDuringApply bool) { | 150 | + | func (d *Daemon) processPanePTYData(p *pane, data []byte, bufferDuringApply bool) { |
| 122 | - | rc := sess.renderCoordinator() | 151 | + | if p == nil { |
| 152 | + | return | |||
| 153 | + | } | |||
| 123 | p.mu.Lock() | 154 | p.mu.Lock() | ||
| 124 | if bufferDuringApply && p.resizeApplying { | 155 | if bufferDuringApply && p.resizeApplying { | ||
| 125 | p.resizePending = append(p.resizePending, data...) | 156 | p.resizePending = append(p.resizePending, data...) | ||
| 126 | p.mu.Unlock() | 157 | p.mu.Unlock() | ||
| 127 | return | 158 | return | ||
| 128 | } | 159 | } | ||
| 129 | - | wasSyncing := p.screen.SyncUpdateActive() | 160 | + | effects := panePTYEffects{lease: p.effectLeaseLocked()} |
| 161 | + | owner := effects.lease.owner | |||
| 162 | + | if owner != nil { | |||
| 163 | + | effects.renderCoordinator = owner.session.renderCoordinator() | |||
| 164 | + | } | |||
| 165 | + | effects.wasSyncing = p.screen.SyncUpdateActive() | |||
| 130 | p.screen.Write(data) | 166 | p.screen.Write(data) | ||
| 131 | p.refreshTerminalTitleLocked() | 167 | p.refreshTerminalTitleLocked() | ||
| 132 | - | isSyncing := p.screen.SyncUpdateActive() | 168 | + | effects.isSyncing = p.screen.SyncUpdateActive() |
| 133 | - | completeSyncRead := !wasSyncing && !isSyncing && completedSynchronizedUpdate(data) | 169 | + | effects.completeSyncRead = !effects.wasSyncing && !effects.isSyncing && completedSynchronizedUpdate(data) |
| 134 | - | var syncGen uint64 | 170 | + | if effects.wasSyncing != effects.isSyncing && owner != nil { |
| 135 | - | syncEnded := false | 171 | + | if effects.isSyncing { |
| 136 | - | if wasSyncing != isSyncing { | 172 | + | syncGen := owner.session.syncGen.Add(1) |
| 137 | - | if isSyncing { | |||
| 138 | - | syncGen = sess.syncGen.Add(1) | |||
| 139 | p.syncGen = syncGen | 173 | p.syncGen = syncGen | ||
| 140 | - | if rc != nil { | 174 | + | if effects.renderCoordinator != nil { |
| 141 | - | rc.noteSyncBeginWithRenderability(p, syncGen, func() bool { return d.paneRenderable(sess, tb, p) }, func() { | 175 | + | lease := effects.lease |
| 176 | + | effects.renderCoordinator.noteSyncBeginWithRenderability(p, syncGen, func() bool { | |||
| 177 | + | return lease.Current() && d.paneRenderable(owner.session, owner.tab, p) | |||
| 178 | + | }, func() { | |||
| 142 | p.mu.Lock() | 179 | p.mu.Lock() | ||
| 143 | - | if p.syncGen == syncGen && p.screen.SyncUpdateActive() { | 180 | + | if p.owner.Load() == lease.owner && p.syncGen == syncGen && p.screen.SyncUpdateActive() { |
| 144 | p.screen.ForceSyncEnd() | 181 | p.screen.ForceSyncEnd() | ||
| 145 | } | 182 | } | ||
| 146 | p.mu.Unlock() | 183 | p.mu.Unlock() | ||
| 147 | }) | 184 | }) | ||
| 148 | } | 185 | } | ||
| 149 | } else { | 186 | } else { | ||
| 150 | - | syncGen = p.syncGen | 187 | + | effects.syncEnded = effects.renderCoordinator != nil && effects.renderCoordinator.removeSyncEnd(p, p.syncGen, true) |
| 151 | - | syncEnded = rc != nil && rc.removeSyncEnd(p, syncGen, true) | |||
| 152 | - | } | |||
| 153 | - | } | |||
| 154 | - | p.mu.Unlock() | |||
| 155 | - | renderable := d.paneRenderable(sess, tb, p) | |||
| 156 | - | markSnapshotDirty(sess) | |||
| 157 | - | d.flushPTYEffects(sess, tb, p) | |||
| 158 | - | if rc != nil { | |||
| 159 | - | if wasSyncing != isSyncing { | |||
| 160 | - | if isSyncing { | |||
| 161 | - | if renderable { | |||
| 162 | - | rc.invalidate(renderInvalidation{class: invalidateOutput, producer: "render.go"}) | |||
| 163 | - | } | |||
| 164 | - | } else if syncEnded && renderable { | |||
| 165 | - | rc.invalidate(renderInvalidation{class: invalidateUrgent, producer: "render.go"}) | |||
| 166 | - | } | |||
| 167 | - | } | |||
| 168 | - | if completeSyncRead && renderable { | |||
| 169 | - | rc.invalidate(renderInvalidation{class: invalidateUrgent, producer: "render.go"}) | |||
| 170 | - | } else if renderable && !isSyncing && wasSyncing == isSyncing { | |||
| 171 | - | rc.invalidate(renderInvalidation{class: invalidateOutput, producer: "render.go"}) | |||
| 172 | } | 188 | } | ||
| 173 | } | 189 | } | ||
| 174 | - | } | 190 | + | effects.attention = p.ptyAttention |
| 175 | - | ||||
| 176 | - | // flushPTYEffects preserves the normal response/title/attention/clipboard | |||
| 177 | - | // behavior for both direct reads and resize-buffer replay. | |||
| 178 | - | func (d *Daemon) flushPTYEffects(sess *session, tb *tab, p *pane) { | |||
| 179 | - | p.mu.Lock() | |||
| 180 | - | attention := p.ptyAttention | |||
| 181 | p.ptyAttention = false | 191 | p.ptyAttention = false | ||
| 182 | - | responses := append([]byte(nil), p.ptyResponses...) | 192 | + | effects.responses = append([]byte(nil), p.ptyResponses...) |
| 183 | p.ptyResponses = p.ptyResponses[:0] | 193 | p.ptyResponses = p.ptyResponses[:0] | ||
| 184 | - | clipboards := append([]string(nil), p.ptyClipboards...) | 194 | + | effects.clipboards = append([]string(nil), p.ptyClipboards...) |
| 185 | p.ptyClipboards = p.ptyClipboards[:0] | 195 | p.ptyClipboards = p.ptyClipboards[:0] | ||
| 186 | p.mu.Unlock() | 196 | p.mu.Unlock() | ||
| 187 | - | if attention { | 197 | + | d.applyPanePTYEffects(p, effects) |
| 188 | - | d.noteAttention(sess, tb) | 198 | + | } |
| 199 | + | ||||
| 200 | + | // applyPanePTYEffects emits only effects still bound to the owner generation | |||
| 201 | + | // that parsed them. Each owner-sensitive boundary revalidates independently so | |||
| 202 | + | // ownership publication can retire the remaining effects without rerouting | |||
| 203 | + | // bytes that belonged to the old owner. | |||
| 204 | + | func (d *Daemon) applyPanePTYEffects(p *pane, effects panePTYEffects) { | |||
| 205 | + | owner := effects.lease.owner | |||
| 206 | + | if owner == nil { | |||
| 207 | + | return | |||
| 208 | + | } | |||
| 209 | + | if effects.lease.Current() { | |||
| 210 | + | markSnapshotDirty(owner.session) | |||
| 211 | + | } | |||
| 212 | + | if effects.attention && effects.lease.Current() { | |||
| 213 | + | d.noteAttention(owner.session, owner.tab) | |||
| 214 | + | } | |||
| 215 | + | for _, b64 := range effects.clipboards { | |||
| 216 | + | if effects.lease.Current() { | |||
| 217 | + | d.forwardClipboardAsync(effects.lease, b64) | |||
| 218 | + | } | |||
| 189 | } | 219 | } | ||
| 190 | - | for _, b64 := range clipboards { | 220 | + | if len(effects.responses) > 0 && effects.lease.Current() { |
| 191 | - | d.forwardClipboardAsync(sess, b64) | 221 | + | if _, err := p.pty.Write(effects.responses); err != nil { |
| 222 | + | if sessionName, current := paneEffectSessionName(effects.lease); current { | |||
| 223 | + | d.log.Warn("pty response write failed", "err", err, "session", sessionName) | |||
| 224 | + | } | |||
| 225 | + | } | |||
| 226 | + | } | |||
| 227 | + | if effects.renderCoordinator == nil || !effects.lease.Current() { | |||
| 228 | + | return | |||
| 192 | } | 229 | } | ||
| 193 | - | if len(responses) > 0 { | 230 | + | renderable := d.paneRenderable(owner.session, owner.tab, p) |
| 194 | - | if _, err := p.pty.Write(responses); err != nil { | 231 | + | if !effects.lease.Current() { |
| 195 | - | d.log.Warn("pty response write failed", "err", err, "session", sess.name) | 232 | + | return |
| 233 | + | } | |||
| 234 | + | if effects.wasSyncing != effects.isSyncing { | |||
| 235 | + | if effects.isSyncing { | |||
| 236 | + | if renderable { | |||
| 237 | + | effects.renderCoordinator.invalidate(renderInvalidation{class: invalidateOutput, producer: "render.go"}) | |||
| 238 | + | } | |||
| 239 | + | } else if effects.syncEnded && renderable { | |||
| 240 | + | effects.renderCoordinator.invalidate(renderInvalidation{class: invalidateUrgent, producer: "render.go"}) | |||
| 196 | } | 241 | } | ||
| 197 | } | 242 | } | ||
| 243 | + | if effects.completeSyncRead && renderable { | |||
| 244 | + | effects.renderCoordinator.invalidate(renderInvalidation{class: invalidateUrgent, producer: "render.go"}) | |||
| 245 | + | } else if renderable && !effects.isSyncing && effects.wasSyncing == effects.isSyncing { | |||
| 246 | + | effects.renderCoordinator.invalidate(renderInvalidation{class: invalidateOutput, producer: "render.go"}) | |||
| 247 | + | } | |||
| 198 | } | 248 | } | ||
| 199 | 249 | ||||
| 200 | // paint draws the composed client frame (active tab plus status bar) and | 250 | // paint draws the composed client frame (active tab plus status bar) and | ||
M internal/usecase/daemon/render_coordinator.go +49 -7
| @@ -287,20 +287,47 @@ func (c *renderCoordinator) noteSyncBegin(p *pane, gen uint64, force ...func()) | |||
| 287 | 287 | c.noteSyncBeginWithRenderability(p, gen, renderable, force...) | |
| 288 | 288 | } | |
| 289 | 289 | ||
| 290 | + | // syncTimerCleanup carries detached timer work across an enclosing pane or | |
| 291 | + | // coordinator critical section. finish never waits for a worker. | |
| 292 | + | type syncTimerCleanup struct{ tokens []*timerToken } | |
| 293 | + | ||
| 294 | + | func (cleanup *syncTimerCleanup) add(token *timerToken) { | |
| 295 | + | if token != nil { | |
| 296 | + | cleanup.tokens = append(cleanup.tokens, token) | |
| 297 | + | } | |
| 298 | + | } | |
| 299 | + | ||
| 300 | + | func (cleanup *syncTimerCleanup) append(other syncTimerCleanup) { | |
| 301 | + | cleanup.tokens = append(cleanup.tokens, other.tokens...) | |
| 302 | + | } | |
| 303 | + | ||
| 304 | + | func (cleanup syncTimerCleanup) finish() { | |
| 305 | + | for _, token := range cleanup.tokens { | |
| 306 | + | stopDetachedTimer(token) | |
| 307 | + | } | |
| 308 | + | } | |
| 309 | + | ||
| 290 | 310 | // noteSyncBeginWithRenderability records lifecycle unconditionally while the | |
| 291 | 311 | // supplied predicate decides dynamically whether this batch gates composition. | |
| 292 | 312 | func (c *renderCoordinator) noteSyncBeginWithRenderability(p *pane, gen uint64, renderable func() bool, force ...func()) { | |
| 313 | + | c.beginSyncBatchWithRenderability(p, gen, renderable, force...).finish() | |
| 314 | + | } | |
| 315 | + | ||
| 316 | + | // beginSyncBatchWithRenderability publishes a batch and returns every timer | |
| 317 | + | // detached while doing so. Callers that already own pane.mu defer finish until | |
| 318 | + | // after releasing the parsing fence. | |
| 319 | + | func (c *renderCoordinator) beginSyncBatchWithRenderability(p *pane, gen uint64, renderable func() bool, force ...func()) syncTimerCleanup { | |
| 320 | + | var cleanup syncTimerCleanup | |
| 293 | 321 | c.mu.Lock() | |
| 294 | 322 | if c.torndown { | |
| 295 | 323 | c.mu.Unlock() | |
| 296 | - | return | |
| 324 | + | return cleanup | |
| 297 | 325 | } | |
| 298 | 326 | if c.syncBatches == nil { | |
| 299 | 327 | c.syncBatches = make(map[*pane]*syncBatch) | |
| 300 | 328 | } | |
| 301 | - | var old *timerToken | |
| 302 | 329 | if previous := c.syncBatches[p]; previous != nil { | |
| 303 | - | old = detachSyncBatchLocked(previous) | |
| 330 | + | cleanup.add(detachSyncBatchLocked(previous)) | |
| 304 | 331 | } | |
| 305 | 332 | batch := &syncBatch{generation: gen, renderable: renderable} | |
| 306 | 333 | if len(force) != 0 { | |
| @@ -310,9 +337,8 @@ func (c *renderCoordinator) noteSyncBeginWithRenderability(p *pane, gen uint64, | |||
| 310 | 337 | c.syncRegistryVersion++ | |
| 311 | 338 | clock := c.opts.clock | |
| 312 | 339 | c.mu.Unlock() | |
| 313 | - | stopDetachedTimer(old) | |
| 314 | 340 | if clock == nil { | |
| 315 | - | return | |
| 341 | + | return cleanup | |
| 316 | 342 | } | |
| 317 | 343 | ||
| 318 | 344 | // Both timer operations are external. A reentrant callback may replace or | |
| @@ -332,9 +358,25 @@ func (c *renderCoordinator) noteSyncBeginWithRenderability(p *pane, gen uint64, | |||
| 332 | 358 | } | |
| 333 | 359 | c.mu.Unlock() | |
| 334 | 360 | if !valid || timerC == nil { | |
| 335 | - | stopDetachedTimer(&timerToken{timer: timer}) | |
| 336 | - | return | |
| 361 | + | cleanup.add(&timerToken{timer: timer}) | |
| 362 | + | } | |
| 363 | + | return cleanup | |
| 364 | + | } | |
| 365 | + | ||
| 366 | + | // detachSyncBatchGeneration removes only the exact pane generation. It returns | |
| 367 | + | // timer cleanup to the caller so no Timer method runs under an enclosing pane | |
| 368 | + | // or coordinator lock. | |
| 369 | + | func (c *renderCoordinator) detachSyncBatchGeneration(p *pane, gen uint64) syncTimerCleanup { | |
| 370 | + | var cleanup syncTimerCleanup | |
| 371 | + | c.mu.Lock() | |
| 372 | + | batch := c.syncBatches[p] | |
| 373 | + | if batch != nil && batch.generation == gen { | |
| 374 | + | delete(c.syncBatches, p) | |
| 375 | + | c.syncRegistryVersion++ | |
| 376 | + | cleanup.add(detachSyncBatchLocked(batch)) | |
| 337 | 377 | } | |
| 378 | + | c.mu.Unlock() | |
| 379 | + | return cleanup | |
| 338 | 380 | } | |
| 339 | 381 | ||
| 340 | 382 | func (c *renderCoordinator) runSyncWatchdog(p *pane, batch *syncBatch, token *timerToken) { | |
| @@ -287,20 +287,47 @@ func (c *renderCoordinator) noteSyncBegin(p *pane, gen uint64, force ...func()) | |||||
| 287 | c.noteSyncBeginWithRenderability(p, gen, renderable, force...) | 287 | c.noteSyncBeginWithRenderability(p, gen, renderable, force...) | ||
| 288 | } | 288 | } | ||
| 289 | 289 | ||||
| 290 | + | // syncTimerCleanup carries detached timer work across an enclosing pane or | |||
| 291 | + | // coordinator critical section. finish never waits for a worker. | |||
| 292 | + | type syncTimerCleanup struct{ tokens []*timerToken } | |||
| 293 | + | ||||
| 294 | + | func (cleanup *syncTimerCleanup) add(token *timerToken) { | |||
| 295 | + | if token != nil { | |||
| 296 | + | cleanup.tokens = append(cleanup.tokens, token) | |||
| 297 | + | } | |||
| 298 | + | } | |||
| 299 | + | ||||
| 300 | + | func (cleanup *syncTimerCleanup) append(other syncTimerCleanup) { | |||
| 301 | + | cleanup.tokens = append(cleanup.tokens, other.tokens...) | |||
| 302 | + | } | |||
| 303 | + | ||||
| 304 | + | func (cleanup syncTimerCleanup) finish() { | |||
| 305 | + | for _, token := range cleanup.tokens { | |||
| 306 | + | stopDetachedTimer(token) | |||
| 307 | + | } | |||
| 308 | + | } | |||
| 309 | + | ||||
| 290 | // noteSyncBeginWithRenderability records lifecycle unconditionally while the | 310 | // noteSyncBeginWithRenderability records lifecycle unconditionally while the | ||
| 291 | // supplied predicate decides dynamically whether this batch gates composition. | 311 | // supplied predicate decides dynamically whether this batch gates composition. | ||
| 292 | func (c *renderCoordinator) noteSyncBeginWithRenderability(p *pane, gen uint64, renderable func() bool, force ...func()) { | 312 | func (c *renderCoordinator) noteSyncBeginWithRenderability(p *pane, gen uint64, renderable func() bool, force ...func()) { | ||
| 313 | + | c.beginSyncBatchWithRenderability(p, gen, renderable, force...).finish() | |||
| 314 | + | } | |||
| 315 | + | ||||
| 316 | + | // beginSyncBatchWithRenderability publishes a batch and returns every timer | |||
| 317 | + | // detached while doing so. Callers that already own pane.mu defer finish until | |||
| 318 | + | // after releasing the parsing fence. | |||
| 319 | + | func (c *renderCoordinator) beginSyncBatchWithRenderability(p *pane, gen uint64, renderable func() bool, force ...func()) syncTimerCleanup { | |||
| 320 | + | var cleanup syncTimerCleanup | |||
| 293 | c.mu.Lock() | 321 | c.mu.Lock() | ||
| 294 | if c.torndown { | 322 | if c.torndown { | ||
| 295 | c.mu.Unlock() | 323 | c.mu.Unlock() | ||
| 296 | - | return | 324 | + | return cleanup |
| 297 | } | 325 | } | ||
| 298 | if c.syncBatches == nil { | 326 | if c.syncBatches == nil { | ||
| 299 | c.syncBatches = make(map[*pane]*syncBatch) | 327 | c.syncBatches = make(map[*pane]*syncBatch) | ||
| 300 | } | 328 | } | ||
| 301 | - | var old *timerToken | |||
| 302 | if previous := c.syncBatches[p]; previous != nil { | 329 | if previous := c.syncBatches[p]; previous != nil { | ||
| 303 | - | old = detachSyncBatchLocked(previous) | 330 | + | cleanup.add(detachSyncBatchLocked(previous)) |
| 304 | } | 331 | } | ||
| 305 | batch := &syncBatch{generation: gen, renderable: renderable} | 332 | batch := &syncBatch{generation: gen, renderable: renderable} | ||
| 306 | if len(force) != 0 { | 333 | if len(force) != 0 { | ||
| @@ -310,9 +337,8 @@ func (c *renderCoordinator) noteSyncBeginWithRenderability(p *pane, gen uint64, | |||||
| 310 | c.syncRegistryVersion++ | 337 | c.syncRegistryVersion++ | ||
| 311 | clock := c.opts.clock | 338 | clock := c.opts.clock | ||
| 312 | c.mu.Unlock() | 339 | c.mu.Unlock() | ||
| 313 | - | stopDetachedTimer(old) | |||
| 314 | if clock == nil { | 340 | if clock == nil { | ||
| 315 | - | return | 341 | + | return cleanup |
| 316 | } | 342 | } | ||
| 317 | 343 | ||||
| 318 | // Both timer operations are external. A reentrant callback may replace or | 344 | // Both timer operations are external. A reentrant callback may replace or | ||
| @@ -332,9 +358,25 @@ func (c *renderCoordinator) noteSyncBeginWithRenderability(p *pane, gen uint64, | |||||
| 332 | } | 358 | } | ||
| 333 | c.mu.Unlock() | 359 | c.mu.Unlock() | ||
| 334 | if !valid || timerC == nil { | 360 | if !valid || timerC == nil { | ||
| 335 | - | stopDetachedTimer(&timerToken{timer: timer}) | 361 | + | cleanup.add(&timerToken{timer: timer}) |
| 336 | - | return | 362 | + | } |
| 363 | + | return cleanup | |||
| 364 | + | } | |||
| 365 | + | ||||
| 366 | + | // detachSyncBatchGeneration removes only the exact pane generation. It returns | |||
| 367 | + | // timer cleanup to the caller so no Timer method runs under an enclosing pane | |||
| 368 | + | // or coordinator lock. | |||
| 369 | + | func (c *renderCoordinator) detachSyncBatchGeneration(p *pane, gen uint64) syncTimerCleanup { | |||
| 370 | + | var cleanup syncTimerCleanup | |||
| 371 | + | c.mu.Lock() | |||
| 372 | + | batch := c.syncBatches[p] | |||
| 373 | + | if batch != nil && batch.generation == gen { | |||
| 374 | + | delete(c.syncBatches, p) | |||
| 375 | + | c.syncRegistryVersion++ | |||
| 376 | + | cleanup.add(detachSyncBatchLocked(batch)) | |||
| 337 | } | 377 | } | ||
| 378 | + | c.mu.Unlock() | |||
| 379 | + | return cleanup | |||
| 338 | } | 380 | } | ||
| 339 | 381 | ||||
| 340 | func (c *renderCoordinator) runSyncWatchdog(p *pane, batch *syncBatch, token *timerToken) { | 382 | func (c *renderCoordinator) runSyncWatchdog(p *pane, batch *syncBatch, token *timerToken) { | ||
M internal/usecase/daemon/session.go +1 -1
| @@ -653,7 +653,7 @@ func (d *Daemon) startPaneGoroutines(sess *session, tb *tab, p *pane) { | |||
| 653 | 653 | d.log.Info("pane created", "session", name, "pane", p.id) | |
| 654 | 654 | // Scheduler ownership was removed; this launch creates exactly one reader. | |
| 655 | 655 | d.sessWg.Add(1) | |
| 656 | - | go d.ptyReader(sess, tb, p) | |
| 656 | + | go d.readPanePTY(p) | |
| 657 | 657 | } | |
| 658 | 658 | ||
| 659 | 659 | // attachClient makes ac the session's current client, displacing any prior one | |
| @@ -653,7 +653,7 @@ func (d *Daemon) startPaneGoroutines(sess *session, tb *tab, p *pane) { | |||||
| 653 | d.log.Info("pane created", "session", name, "pane", p.id) | 653 | d.log.Info("pane created", "session", name, "pane", p.id) | ||
| 654 | // Scheduler ownership was removed; this launch creates exactly one reader. | 654 | // Scheduler ownership was removed; this launch creates exactly one reader. | ||
| 655 | d.sessWg.Add(1) | 655 | d.sessWg.Add(1) | ||
| 656 | - | go d.ptyReader(sess, tb, p) | 656 | + | go d.readPanePTY(p) |
| 657 | } | 657 | } | ||
| 658 | 658 | ||||
| 659 | // attachClient makes ac the session's current client, displacing any prior one | 659 | // attachClient makes ac the session's current client, displacing any prior one | ||