fix(daemon): complete coordinator handoff and watchdog ordering
- commit
- 6732890f81c6119de74f14562e79deda7eed066d
- author
- brice · · 2026-07-11 19:37 UTC
- parent
- 39df6d2
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email
4 files · +35 -21
M internal/usecase/daemon/client.go +13 -0
| @@ -362,6 +362,19 @@ func (d *Daemon) attachClient(sess *session, tr ports.Transport, sz domain.Size, | |||
| 362 | 362 | return ac, old | |
| 363 | 363 | } | |
| 364 | 364 | ||
| 365 | + | // handoffCoordinator completes a cross-session ownership transfer before the | |
| 366 | + | // destination's first paint. It invalidates source callbacks, rebases the | |
| 367 | + | // attachment's output dependency chain, then installs the destination owner. | |
| 368 | + | func (d *Daemon) handoffCoordinator(from, target *session, old, current *attachedClient) { | |
| 369 | + | if rc := from.renderCoordinator(); rc != nil { | |
| 370 | + | rc.noteDetach(current) | |
| 371 | + | } | |
| 372 | + | current.sendMu.Lock() | |
| 373 | + | current.output.rebase() | |
| 374 | + | current.sendMu.Unlock() | |
| 375 | + | d.attachCoordinator(target, old, current) | |
| 376 | + | } | |
| 377 | + | ||
| 365 | 378 | // attachCoordinator is the sole direct attachment handoff. It creates at | |
| 366 | 379 | // most one coordinator for sess and changes its identity before any caller | |
| 367 | 380 | // can publish resize or render state for the new client. | |
| @@ -362,6 +362,19 @@ func (d *Daemon) attachClient(sess *session, tr ports.Transport, sz domain.Size, | |||||
| 362 | return ac, old | 362 | return ac, old | ||
| 363 | } | 363 | } | ||
| 364 | 364 | ||||
| 365 | + | // handoffCoordinator completes a cross-session ownership transfer before the | |||
| 366 | + | // destination's first paint. It invalidates source callbacks, rebases the | |||
| 367 | + | // attachment's output dependency chain, then installs the destination owner. | |||
| 368 | + | func (d *Daemon) handoffCoordinator(from, target *session, old, current *attachedClient) { | |||
| 369 | + | if rc := from.renderCoordinator(); rc != nil { | |||
| 370 | + | rc.noteDetach(current) | |||
| 371 | + | } | |||
| 372 | + | current.sendMu.Lock() | |||
| 373 | + | current.output.rebase() | |||
| 374 | + | current.sendMu.Unlock() | |||
| 375 | + | d.attachCoordinator(target, old, current) | |||
| 376 | + | } | |||
| 377 | + | ||||
| 365 | // attachCoordinator is the sole direct attachment handoff. It creates at | 378 | // attachCoordinator is the sole direct attachment handoff. It creates at | ||
| 366 | // most one coordinator for sess and changes its identity before any caller | 379 | // most one coordinator for sess and changes its identity before any caller | ||
| 367 | // can publish resize or render state for the new client. | 380 | // can publish resize or render state for the new client. | ||
M internal/usecase/daemon/picker.go +2 -18
| @@ -406,17 +406,7 @@ func (d *Daemon) stealClientForTarget(from *session, ac *attachedClient, targetS | |||
| 406 | 406 | ac.setSession(targetSess) | |
| 407 | 407 | ac.recordPreviousSession(from) | |
| 408 | 408 | d.mu.Unlock() | |
| 409 | - | // Both session coordinators must observe the ownership transfer before | |
| 410 | - | // stale readers/timers can publish into either session. | |
| 411 | - | if rc := from.renderCoordinator(); rc != nil { | |
| 412 | - | rc.noteDetach(ac) | |
| 413 | - | } | |
| 414 | - | // Output states from the prior session cannot be acknowledged against the | |
| 415 | - | // destination's full first frame. | |
| 416 | - | ac.sendMu.Lock() | |
| 417 | - | ac.output.rebase() | |
| 418 | - | ac.sendMu.Unlock() | |
| 419 | - | d.attachCoordinator(targetSess, old, ac) | |
| 409 | + | d.handoffCoordinator(from, targetSess, old, ac) | |
| 420 | 410 | return old | |
| 421 | 411 | } | |
| 422 | 412 | ||
| @@ -455,13 +445,7 @@ func (d *Daemon) resumeStoppedAndSwitch(from *session, ac *attachedClient, targe | |||
| 455 | 445 | ac.setSession(targetSess) | |
| 456 | 446 | ac.recordPreviousSession(from) | |
| 457 | 447 | d.mu.Unlock() | |
| 458 | - | if rc := from.renderCoordinator(); rc != nil { | |
| 459 | - | rc.noteDetach(ac) | |
| 460 | - | } | |
| 461 | - | ac.sendMu.Lock() | |
| 462 | - | ac.output.rebase() | |
| 463 | - | ac.sendMu.Unlock() | |
| 464 | - | d.attachCoordinator(targetSess, nil, ac) | |
| 448 | + | d.handoffCoordinator(from, targetSess, nil, ac) | |
| 465 | 449 | d.firstPaint(targetSess, ac, ac.size) | |
| 466 | 450 | return true | |
| 467 | 451 | } | |
| @@ -406,17 +406,7 @@ func (d *Daemon) stealClientForTarget(from *session, ac *attachedClient, targetS | |||||
| 406 | ac.setSession(targetSess) | 406 | ac.setSession(targetSess) | ||
| 407 | ac.recordPreviousSession(from) | 407 | ac.recordPreviousSession(from) | ||
| 408 | d.mu.Unlock() | 408 | d.mu.Unlock() | ||
| 409 | - | // Both session coordinators must observe the ownership transfer before | 409 | + | d.handoffCoordinator(from, targetSess, old, ac) |
| 410 | - | // stale readers/timers can publish into either session. | |||
| 411 | - | if rc := from.renderCoordinator(); rc != nil { | |||
| 412 | - | rc.noteDetach(ac) | |||
| 413 | - | } | |||
| 414 | - | // Output states from the prior session cannot be acknowledged against the | |||
| 415 | - | // destination's full first frame. | |||
| 416 | - | ac.sendMu.Lock() | |||
| 417 | - | ac.output.rebase() | |||
| 418 | - | ac.sendMu.Unlock() | |||
| 419 | - | d.attachCoordinator(targetSess, old, ac) | |||
| 420 | return old | 410 | return old | ||
| 421 | } | 411 | } | ||
| 422 | 412 | ||||
| @@ -455,13 +445,7 @@ func (d *Daemon) resumeStoppedAndSwitch(from *session, ac *attachedClient, targe | |||||
| 455 | ac.setSession(targetSess) | 445 | ac.setSession(targetSess) | ||
| 456 | ac.recordPreviousSession(from) | 446 | ac.recordPreviousSession(from) | ||
| 457 | d.mu.Unlock() | 447 | d.mu.Unlock() | ||
| 458 | - | if rc := from.renderCoordinator(); rc != nil { | 448 | + | d.handoffCoordinator(from, targetSess, nil, ac) |
| 459 | - | rc.noteDetach(ac) | |||
| 460 | - | } | |||
| 461 | - | ac.sendMu.Lock() | |||
| 462 | - | ac.output.rebase() | |||
| 463 | - | ac.sendMu.Unlock() | |||
| 464 | - | d.attachCoordinator(targetSess, nil, ac) | |||
| 465 | d.firstPaint(targetSess, ac, ac.size) | 449 | d.firstPaint(targetSess, ac, ac.size) | ||
| 466 | return true | 450 | return true | ||
| 467 | } | 451 | } | ||
M internal/usecase/daemon/render_coordinator.go +17 -3
| @@ -362,9 +362,26 @@ func (c *renderCoordinator) noteSyncBeginWithRenderability(p *pane, gen uint64, | |||
| 362 | 362 | case <-cancel: | |
| 363 | 363 | return | |
| 364 | 364 | } | |
| 365 | + | // Keep the batch registered while force runs. A concurrent render must | |
| 366 | + | // continue to observe the synchronized-output gate until the VT has | |
| 367 | + | // authoritatively closed the batch. | |
| 365 | 368 | c.mu.Lock() | |
| 366 | 369 | current := c.syncBatches[p] | |
| 367 | 370 | valid := !c.torndown && current == batch && current.generation == gen | |
| 371 | + | c.mu.Unlock() | |
| 372 | + | if !valid { | |
| 373 | + | return | |
| 374 | + | } | |
| 375 | + | if batch.force != nil { | |
| 376 | + | batch.force() | |
| 377 | + | } | |
| 378 | + | ||
| 379 | + | // force may synchronously end this batch, replace it with a newer | |
| 380 | + | // generation, or tear down the coordinator. Remove only the exact | |
| 381 | + | // snapshot that expired, then publish one urgent completion wake. | |
| 382 | + | c.mu.Lock() | |
| 383 | + | current = c.syncBatches[p] | |
| 384 | + | valid = !c.torndown && current == batch && current.generation == gen | |
| 368 | 385 | if valid { | |
| 369 | 386 | delete(c.syncBatches, p) | |
| 370 | 387 | c.syncRegistryVersion++ | |
| @@ -375,9 +392,6 @@ func (c *renderCoordinator) noteSyncBeginWithRenderability(p *pane, gen uint64, | |||
| 375 | 392 | } | |
| 376 | 393 | c.mu.Unlock() | |
| 377 | 394 | if valid { | |
| 378 | - | if batch.force != nil { | |
| 379 | - | batch.force() | |
| 380 | - | } | |
| 381 | 395 | // Reevaluate the aggregate gate only after forcing this pane. | |
| 382 | 396 | c.fireCurrent(true) | |
| 383 | 397 | } | |
| @@ -362,9 +362,26 @@ func (c *renderCoordinator) noteSyncBeginWithRenderability(p *pane, gen uint64, | |||||
| 362 | case <-cancel: | 362 | case <-cancel: | ||
| 363 | return | 363 | return | ||
| 364 | } | 364 | } | ||
| 365 | + | // Keep the batch registered while force runs. A concurrent render must | |||
| 366 | + | // continue to observe the synchronized-output gate until the VT has | |||
| 367 | + | // authoritatively closed the batch. | |||
| 365 | c.mu.Lock() | 368 | c.mu.Lock() | ||
| 366 | current := c.syncBatches[p] | 369 | current := c.syncBatches[p] | ||
| 367 | valid := !c.torndown && current == batch && current.generation == gen | 370 | valid := !c.torndown && current == batch && current.generation == gen | ||
| 371 | + | c.mu.Unlock() | |||
| 372 | + | if !valid { | |||
| 373 | + | return | |||
| 374 | + | } | |||
| 375 | + | if batch.force != nil { | |||
| 376 | + | batch.force() | |||
| 377 | + | } | |||
| 378 | + | ||||
| 379 | + | // force may synchronously end this batch, replace it with a newer | |||
| 380 | + | // generation, or tear down the coordinator. Remove only the exact | |||
| 381 | + | // snapshot that expired, then publish one urgent completion wake. | |||
| 382 | + | c.mu.Lock() | |||
| 383 | + | current = c.syncBatches[p] | |||
| 384 | + | valid = !c.torndown && current == batch && current.generation == gen | |||
| 368 | if valid { | 385 | if valid { | ||
| 369 | delete(c.syncBatches, p) | 386 | delete(c.syncBatches, p) | ||
| 370 | c.syncRegistryVersion++ | 387 | c.syncRegistryVersion++ | ||
| @@ -375,9 +392,6 @@ func (c *renderCoordinator) noteSyncBeginWithRenderability(p *pane, gen uint64, | |||||
| 375 | } | 392 | } | ||
| 376 | c.mu.Unlock() | 393 | c.mu.Unlock() | ||
| 377 | if valid { | 394 | if valid { | ||
| 378 | - | if batch.force != nil { | |||
| 379 | - | batch.force() | |||
| 380 | - | } | |||
| 381 | // Reevaluate the aggregate gate only after forcing this pane. | 395 | // Reevaluate the aggregate gate only after forcing this pane. | ||
| 382 | c.fireCurrent(true) | 396 | c.fireCurrent(true) | ||
| 383 | } | 397 | } | ||
M internal/usecase/daemon/session.go +3 -0
| @@ -247,6 +247,9 @@ func (d *Daemon) createSessionAndSwitch(from *session, ac *attachedClient, name | |||
| 247 | 247 | d.log.Info("client attached", "session", newSess.name, "resume", ac.resumeCapable) | |
| 248 | 248 | d.mu.Unlock() | |
| 249 | 249 | ||
| 250 | + | // Match every other cross-session path: source callbacks are stale and | |
| 251 | + | // destination ownership is installed before its first full frame. | |
| 252 | + | d.handoffCoordinator(from, newSess, nil, ac) | |
| 250 | 253 | d.firstPaint(newSess, ac, sz) | |
| 251 | 254 | return nil | |
| 252 | 255 | } | |
| @@ -247,6 +247,9 @@ func (d *Daemon) createSessionAndSwitch(from *session, ac *attachedClient, name | |||||
| 247 | d.log.Info("client attached", "session", newSess.name, "resume", ac.resumeCapable) | 247 | d.log.Info("client attached", "session", newSess.name, "resume", ac.resumeCapable) | ||
| 248 | d.mu.Unlock() | 248 | d.mu.Unlock() | ||
| 249 | 249 | ||||
| 250 | + | // Match every other cross-session path: source callbacks are stale and | |||
| 251 | + | // destination ownership is installed before its first full frame. | |||
| 252 | + | d.handoffCoordinator(from, newSess, nil, ac) | |||
| 250 | d.firstPaint(newSess, ac, sz) | 253 | d.firstPaint(newSess, ac, sz) | ||
| 251 | return nil | 254 | return nil | ||
| 252 | } | 255 | } | ||