refactor(daemon): make pane lifetime transferable
- commit
- bbbadc63522d51d57f60c9f9ae174e30d76dabf9
- author
- brice · · 2026-07-28 03:05 UTC
- parent
- ffcb75d
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email
13 files · +1435 -97
M internal/usecase/daemon/daemon.go +35 -18
| @@ -85,6 +85,19 @@ type Daemon struct { | |||
| 85 | 85 | // under the same mutex — so a Hello racing shutdown can never insert a new | |
| 86 | 86 | // session that nobody would tear down. | |
| 87 | 87 | closing bool | |
| 88 | + | ||
| 89 | + | // moveLifecycleMu is the daemon-level admission gate for transferable | |
| 90 | + | // ownership changes. It is acquired before any session teardownMu, and is | |
| 91 | + | // never held while a move, teardown, or external operation runs. | |
| 92 | + | moveLifecycleMu sync.Mutex | |
| 93 | + | moveLifecycleClosing bool | |
| 94 | + | moveLifecycleActive uint | |
| 95 | + | moveLifecycleChanged chan struct{} | |
| 96 | + | // paneProcessCtx roots transferable pane processes outside any one session. | |
| 97 | + | // Shutdown cancels it only after the global move gate drains. | |
| 98 | + | paneProcessCtx context.Context | |
| 99 | + | paneProcessCancel context.CancelFunc | |
| 100 | + | ||
| 88 | 101 | // notifies holds one completion channel per in-flight async Detached | |
| 89 | 102 | // notification (guarded by mu, pruned on insert). Channels rather than a | |
| 90 | 103 | // WaitGroup: notifications are spawned from arbitrary goroutines while | |
| @@ -488,25 +501,28 @@ func New(ptys ports.PTYFactory, clock ports.Clock, log *slog.Logger, opts ...Opt | |||
| 488 | 501 | if shell == "" { | |
| 489 | 502 | shell = "/bin/sh" | |
| 490 | 503 | } | |
| 504 | + | paneProcessCtx, paneProcessCancel := context.WithCancel(context.Background()) | |
| 491 | 505 | d := &Daemon{ | |
| 492 | - | sessions: make(map[domain.SessionID]*session), | |
| 493 | - | stopped: make(map[string]stoppedSession), | |
| 494 | - | creating: make(map[string]struct{}), | |
| 495 | - | parked: make(map[uint64]*parkedAttachment), | |
| 496 | - | ptys: ptys, | |
| 497 | - | clock: clock, | |
| 498 | - | log: log, | |
| 499 | - | baseEnv: os.Environ(), | |
| 500 | - | shell: shell, | |
| 501 | - | dirOrHome: dirOrHome, | |
| 502 | - | done: make(chan struct{}), | |
| 503 | - | restoreDone: make(chan struct{}), | |
| 504 | - | animWake: make(chan struct{}, 1), | |
| 505 | - | snapshotJobs: make(chan *snapshotCapture, snapshotQueueCapacity), | |
| 506 | - | snapshotAdmitted: make(map[*snapshotCapture]struct{}), | |
| 507 | - | snapshotWake: make(chan struct{}, 1), | |
| 508 | - | notices: newNoticeCenter(), | |
| 509 | - | resumeParkGrace: defaultResumeParkGrace, | |
| 506 | + | sessions: make(map[domain.SessionID]*session), | |
| 507 | + | stopped: make(map[string]stoppedSession), | |
| 508 | + | creating: make(map[string]struct{}), | |
| 509 | + | parked: make(map[uint64]*parkedAttachment), | |
| 510 | + | paneProcessCtx: paneProcessCtx, | |
| 511 | + | paneProcessCancel: paneProcessCancel, | |
| 512 | + | ptys: ptys, | |
| 513 | + | clock: clock, | |
| 514 | + | log: log, | |
| 515 | + | baseEnv: os.Environ(), | |
| 516 | + | shell: shell, | |
| 517 | + | dirOrHome: dirOrHome, | |
| 518 | + | done: make(chan struct{}), | |
| 519 | + | restoreDone: make(chan struct{}), | |
| 520 | + | animWake: make(chan struct{}, 1), | |
| 521 | + | snapshotJobs: make(chan *snapshotCapture, snapshotQueueCapacity), | |
| 522 | + | snapshotAdmitted: make(map[*snapshotCapture]struct{}), | |
| 523 | + | snapshotWake: make(chan struct{}, 1), | |
| 524 | + | notices: newNoticeCenter(), | |
| 525 | + | resumeParkGrace: defaultResumeParkGrace, | |
| 510 | 526 | barScripts: &barScriptState{ | |
| 511 | 527 | cfg: barConfigFromDomain(domain.Defaults().Bar), | |
| 512 | 528 | outputs: make(map[domain.SessionID]barScriptOutputs), | |
| @@ -680,6 +696,7 @@ func (d *Daemon) shutdownAll(reason uint8) (checkpointIncomplete bool) { | |||
| 680 | 696 | } | |
| 681 | 697 | ||
| 682 | 698 | func (d *Daemon) shutdownAllWithSnapshotDeadline(reason uint8, deadline *snapshotShutdownDeadline) (checkpointIncomplete bool) { | |
| 699 | + | d.closeMoveLifecycles() | |
| 683 | 700 | d.mu.Lock() | |
| 684 | 701 | d.closing = true | |
| 685 | 702 | parkedRetirements := d.purgeAllParkedLocked() | |
| @@ -85,6 +85,19 @@ type Daemon struct { | |||||
| 85 | // under the same mutex — so a Hello racing shutdown can never insert a new | 85 | // under the same mutex — so a Hello racing shutdown can never insert a new | ||
| 86 | // session that nobody would tear down. | 86 | // session that nobody would tear down. | ||
| 87 | closing bool | 87 | closing bool | ||
| 88 | + | ||||
| 89 | + | // moveLifecycleMu is the daemon-level admission gate for transferable | |||
| 90 | + | // ownership changes. It is acquired before any session teardownMu, and is | |||
| 91 | + | // never held while a move, teardown, or external operation runs. | |||
| 92 | + | moveLifecycleMu sync.Mutex | |||
| 93 | + | moveLifecycleClosing bool | |||
| 94 | + | moveLifecycleActive uint | |||
| 95 | + | moveLifecycleChanged chan struct{} | |||
| 96 | + | // paneProcessCtx roots transferable pane processes outside any one session. | |||
| 97 | + | // Shutdown cancels it only after the global move gate drains. | |||
| 98 | + | paneProcessCtx context.Context | |||
| 99 | + | paneProcessCancel context.CancelFunc | |||
| 100 | + | ||||
| 88 | // notifies holds one completion channel per in-flight async Detached | 101 | // notifies holds one completion channel per in-flight async Detached | ||
| 89 | // notification (guarded by mu, pruned on insert). Channels rather than a | 102 | // notification (guarded by mu, pruned on insert). Channels rather than a | ||
| 90 | // WaitGroup: notifications are spawned from arbitrary goroutines while | 103 | // WaitGroup: notifications are spawned from arbitrary goroutines while | ||
| @@ -488,25 +501,28 @@ func New(ptys ports.PTYFactory, clock ports.Clock, log *slog.Logger, opts ...Opt | |||||
| 488 | if shell == "" { | 501 | if shell == "" { | ||
| 489 | shell = "/bin/sh" | 502 | shell = "/bin/sh" | ||
| 490 | } | 503 | } | ||
| 504 | + | paneProcessCtx, paneProcessCancel := context.WithCancel(context.Background()) | |||
| 491 | d := &Daemon{ | 505 | d := &Daemon{ | ||
| 492 | - | sessions: make(map[domain.SessionID]*session), | 506 | + | sessions: make(map[domain.SessionID]*session), |
| 493 | - | stopped: make(map[string]stoppedSession), | 507 | + | stopped: make(map[string]stoppedSession), |
| 494 | - | creating: make(map[string]struct{}), | 508 | + | creating: make(map[string]struct{}), |
| 495 | - | parked: make(map[uint64]*parkedAttachment), | 509 | + | parked: make(map[uint64]*parkedAttachment), |
| 496 | - | ptys: ptys, | 510 | + | paneProcessCtx: paneProcessCtx, |
| 497 | - | clock: clock, | 511 | + | paneProcessCancel: paneProcessCancel, |
| 498 | - | log: log, | 512 | + | ptys: ptys, |
| 499 | - | baseEnv: os.Environ(), | 513 | + | clock: clock, |
| 500 | - | shell: shell, | 514 | + | log: log, |
| 501 | - | dirOrHome: dirOrHome, | 515 | + | baseEnv: os.Environ(), |
| 502 | - | done: make(chan struct{}), | 516 | + | shell: shell, |
| 503 | - | restoreDone: make(chan struct{}), | 517 | + | dirOrHome: dirOrHome, |
| 504 | - | animWake: make(chan struct{}, 1), | 518 | + | done: make(chan struct{}), |
| 505 | - | snapshotJobs: make(chan *snapshotCapture, snapshotQueueCapacity), | 519 | + | restoreDone: make(chan struct{}), |
| 506 | - | snapshotAdmitted: make(map[*snapshotCapture]struct{}), | 520 | + | animWake: make(chan struct{}, 1), |
| 507 | - | snapshotWake: make(chan struct{}, 1), | 521 | + | snapshotJobs: make(chan *snapshotCapture, snapshotQueueCapacity), |
| 508 | - | notices: newNoticeCenter(), | 522 | + | snapshotAdmitted: make(map[*snapshotCapture]struct{}), |
| 509 | - | resumeParkGrace: defaultResumeParkGrace, | 523 | + | snapshotWake: make(chan struct{}, 1), |
| 524 | + | notices: newNoticeCenter(), | |||
| 525 | + | resumeParkGrace: defaultResumeParkGrace, | |||
| 510 | barScripts: &barScriptState{ | 526 | barScripts: &barScriptState{ | ||
| 511 | cfg: barConfigFromDomain(domain.Defaults().Bar), | 527 | cfg: barConfigFromDomain(domain.Defaults().Bar), | ||
| 512 | outputs: make(map[domain.SessionID]barScriptOutputs), | 528 | outputs: make(map[domain.SessionID]barScriptOutputs), | ||
| @@ -680,6 +696,7 @@ func (d *Daemon) shutdownAll(reason uint8) (checkpointIncomplete bool) { | |||||
| 680 | } | 696 | } | ||
| 681 | 697 | ||||
| 682 | func (d *Daemon) shutdownAllWithSnapshotDeadline(reason uint8, deadline *snapshotShutdownDeadline) (checkpointIncomplete bool) { | 698 | func (d *Daemon) shutdownAllWithSnapshotDeadline(reason uint8, deadline *snapshotShutdownDeadline) (checkpointIncomplete bool) { | ||
| 699 | + | d.closeMoveLifecycles() | |||
| 683 | d.mu.Lock() | 700 | d.mu.Lock() | ||
| 684 | d.closing = true | 701 | d.closing = true | ||
| 685 | parkedRetirements := d.purgeAllParkedLocked() | 702 | parkedRetirements := d.purgeAllParkedLocked() | ||
M internal/usecase/daemon/daemon_test.go +3 -0
| @@ -402,6 +402,9 @@ func newManualSessionWithPTYsCleanup(t testing.TB, registerCleanup bool, ptys .. | |||
| 402 | 402 | tabs = append(tabs, tb) | |
| 403 | 403 | } | |
| 404 | 404 | sess := &session{id: "manual", name: "work", ctx: sctx, cancel: cancel, tabs: tabs, client: ac} | |
| 405 | + | for _, tb := range tabs { | |
| 406 | + | publishTiledPaneOwners(sess, tb) | |
| 407 | + | } | |
| 405 | 408 | ac.setSession(sess) | |
| 406 | 409 | ac.keys = keys.NewRouter(d.clock, daemonKeyHandler{d: d, ac: ac}, nil) | |
| 407 | 410 | d.sessions[sess.id] = sess | |
| @@ -402,6 +402,9 @@ func newManualSessionWithPTYsCleanup(t testing.TB, registerCleanup bool, ptys .. | |||||
| 402 | tabs = append(tabs, tb) | 402 | tabs = append(tabs, tb) | ||
| 403 | } | 403 | } | ||
| 404 | sess := &session{id: "manual", name: "work", ctx: sctx, cancel: cancel, tabs: tabs, client: ac} | 404 | sess := &session{id: "manual", name: "work", ctx: sctx, cancel: cancel, tabs: tabs, client: ac} | ||
| 405 | + | for _, tb := range tabs { | |||
| 406 | + | publishTiledPaneOwners(sess, tb) | |||
| 407 | + | } | |||
| 405 | ac.setSession(sess) | 408 | ac.setSession(sess) | ||
| 406 | ac.keys = keys.NewRouter(d.clock, daemonKeyHandler{d: d, ac: ac}, nil) | 409 | ac.keys = keys.NewRouter(d.clock, daemonKeyHandler{d: d, ac: ac}, nil) | ||
| 407 | d.sessions[sess.id] = sess | 410 | d.sessions[sess.id] = sess | ||
M internal/usecase/daemon/floating.go +26 -23
| @@ -387,17 +387,22 @@ func (d *Daemon) openAndInstallFloating(sess *session, tb *tab, spec floatingLau | |||
| 387 | 387 | if err := spec.parentCtx.Err(); err != nil { | |
| 388 | 388 | return | |
| 389 | 389 | } | |
| 390 | - | openCtx, cancelOpen := context.WithCancel(spec.parentCtx) | |
| 391 | - | stopSessionCancel := context.AfterFunc(sess.ctx, cancelOpen) | |
| 390 | + | lifetime := d.newPaneProcessLifetime(spec.parentCtx, sess.ctx) | |
| 392 | 391 | ownedByPane := false | |
| 393 | 392 | defer func() { | |
| 394 | 393 | if !ownedByPane { | |
| 395 | - | stopSessionCancel() | |
| 396 | - | cancelOpen() | |
| 394 | + | lifetime.abort() | |
| 397 | 395 | } | |
| 398 | 396 | }() | |
| 399 | - | pty, err := d.ptys.Open(openCtx, spec.command, spec.args, spec.env, spec.cwd, spec.size) | |
| 397 | + | pty, err := d.ptys.Open(lifetime.ctx, spec.command, spec.args, spec.env, spec.cwd, spec.size) | |
| 400 | 398 | if err != nil { | |
| 399 | + | // Open retains ownership of a nonnil PTY only on success. Some factory | |
| 400 | + | // implementations can return a partially opened PTY with an error, so | |
| 401 | + | // release both resources before publishing the launch failure. | |
| 402 | + | lifetime.abort() | |
| 403 | + | if pty != nil { | |
| 404 | + | _ = pty.Close() | |
| 405 | + | } | |
| 401 | 406 | d.failFloatingLaunch(sess, tb, generation, spec.userOpen, spec.sessionName, err) | |
| 402 | 407 | return | |
| 403 | 408 | } | |
| @@ -405,12 +410,10 @@ func (d *Daemon) openAndInstallFloating(sess *session, tb *tab, spec floatingLau | |||
| 405 | 410 | p.rect = spec.geometry.Inner | |
| 406 | 411 | p.popupGeometry = spec.geometry | |
| 407 | 412 | p.title.displayFallback = spec.fallback | |
| 408 | - | // CommandContext owns the child through openCtx, so retain both this context | |
| 409 | - | // and its cancellation until the installed pane is reaped or torn down. | |
| 410 | - | p.ctx = openCtx | |
| 411 | - | p.cancel = func() { | |
| 412 | - | stopSessionCancel() | |
| 413 | - | cancelOpen() | |
| 413 | + | if !lifetime.publish(p) { | |
| 414 | + | _ = pty.Close() | |
| 415 | + | d.failFloatingLaunch(sess, tb, generation, spec.userOpen, spec.sessionName, lifetime.ctx.Err()) | |
| 416 | + | return | |
| 414 | 417 | } | |
| 415 | 418 | ownedByPane = true | |
| 416 | 419 | // The reader may run as soon as install returns; make its exit policy | |
| @@ -444,8 +447,18 @@ func (d *Daemon) failFloatingLaunch(sess *session, tb *tab, generation uint64, u | |||
| 444 | 447 | // installFloating installs only the current generation. A stale successful | |
| 445 | 448 | // launch is cancelled and closed outside the tab lock. | |
| 446 | 449 | func (d *Daemon) installFloating(sess *session, tb *tab, p *pane, generation uint64) { | |
| 450 | + | if sess == nil || tb == nil || p == nil { | |
| 451 | + | closeFloatingPane(p) | |
| 452 | + | return | |
| 453 | + | } | |
| 447 | 454 | tb.mu.Lock() | |
| 448 | - | installed := tb.installFloatingLocked(p, generation) | |
| 455 | + | installable := tb.floating.state == floatingWarming && tb.floating.generation == generation | |
| 456 | + | if installable { | |
| 457 | + | // Publish under pane.mu before the slot becomes hidden or visible. The | |
| 458 | + | // tab-to-pane lock order also linearizes owner and slot publication. | |
| 459 | + | publishPaneOwner(p, sess, tb, generation) | |
| 460 | + | } | |
| 461 | + | installed := installable && tb.installFloatingLocked(p, generation) | |
| 449 | 462 | visible := installed && tb.floating.state == floatingVisible | |
| 450 | 463 | tb.mu.Unlock() | |
| 451 | 464 | if !installed { | |
| @@ -506,17 +519,7 @@ func (d *Daemon) teardownFloating(tb *tab, ac *attachedClient) { | |||
| 506 | 519 | // closeFloatingPane releases a floating runtime at most once. It is called | |
| 507 | 520 | // only after a generation/identity check has detached the pane from its slot. | |
| 508 | 521 | func closeFloatingPane(p *pane) { | |
| 509 | - | if p == nil { | |
| 510 | - | return | |
| 511 | - | } | |
| 512 | - | p.floatingCloseOnce.Do(func() { | |
| 513 | - | if p.cancel != nil { | |
| 514 | - | p.cancel() | |
| 515 | - | } | |
| 516 | - | if p.pty != nil { | |
| 517 | - | _ = p.pty.Close() | |
| 518 | - | } | |
| 519 | - | }) | |
| 522 | + | closePaneProcess(p) | |
| 520 | 523 | } | |
| 521 | 524 | ||
| 522 | 525 | func floatingCommandFallback(command, shell string) string { | |
| @@ -387,17 +387,22 @@ func (d *Daemon) openAndInstallFloating(sess *session, tb *tab, spec floatingLau | |||||
| 387 | if err := spec.parentCtx.Err(); err != nil { | 387 | if err := spec.parentCtx.Err(); err != nil { | ||
| 388 | return | 388 | return | ||
| 389 | } | 389 | } | ||
| 390 | - | openCtx, cancelOpen := context.WithCancel(spec.parentCtx) | 390 | + | lifetime := d.newPaneProcessLifetime(spec.parentCtx, sess.ctx) |
| 391 | - | stopSessionCancel := context.AfterFunc(sess.ctx, cancelOpen) | |||
| 392 | ownedByPane := false | 391 | ownedByPane := false | ||
| 393 | defer func() { | 392 | defer func() { | ||
| 394 | if !ownedByPane { | 393 | if !ownedByPane { | ||
| 395 | - | stopSessionCancel() | 394 | + | lifetime.abort() |
| 396 | - | cancelOpen() | |||
| 397 | } | 395 | } | ||
| 398 | }() | 396 | }() | ||
| 399 | - | pty, err := d.ptys.Open(openCtx, spec.command, spec.args, spec.env, spec.cwd, spec.size) | 397 | + | pty, err := d.ptys.Open(lifetime.ctx, spec.command, spec.args, spec.env, spec.cwd, spec.size) |
| 400 | if err != nil { | 398 | if err != nil { | ||
| 399 | + | // Open retains ownership of a nonnil PTY only on success. Some factory | |||
| 400 | + | // implementations can return a partially opened PTY with an error, so | |||
| 401 | + | // release both resources before publishing the launch failure. | |||
| 402 | + | lifetime.abort() | |||
| 403 | + | if pty != nil { | |||
| 404 | + | _ = pty.Close() | |||
| 405 | + | } | |||
| 401 | d.failFloatingLaunch(sess, tb, generation, spec.userOpen, spec.sessionName, err) | 406 | d.failFloatingLaunch(sess, tb, generation, spec.userOpen, spec.sessionName, err) | ||
| 402 | return | 407 | return | ||
| 403 | } | 408 | } | ||
| @@ -405,12 +410,10 @@ func (d *Daemon) openAndInstallFloating(sess *session, tb *tab, spec floatingLau | |||||
| 405 | p.rect = spec.geometry.Inner | 410 | p.rect = spec.geometry.Inner | ||
| 406 | p.popupGeometry = spec.geometry | 411 | p.popupGeometry = spec.geometry | ||
| 407 | p.title.displayFallback = spec.fallback | 412 | p.title.displayFallback = spec.fallback | ||
| 408 | - | // CommandContext owns the child through openCtx, so retain both this context | 413 | + | if !lifetime.publish(p) { |
| 409 | - | // and its cancellation until the installed pane is reaped or torn down. | 414 | + | _ = pty.Close() |
| 410 | - | p.ctx = openCtx | 415 | + | d.failFloatingLaunch(sess, tb, generation, spec.userOpen, spec.sessionName, lifetime.ctx.Err()) |
| 411 | - | p.cancel = func() { | 416 | + | return |
| 412 | - | stopSessionCancel() | |||
| 413 | - | cancelOpen() | |||
| 414 | } | 417 | } | ||
| 415 | ownedByPane = true | 418 | ownedByPane = true | ||
| 416 | // The reader may run as soon as install returns; make its exit policy | 419 | // The reader may run as soon as install returns; make its exit policy | ||
| @@ -444,8 +447,18 @@ func (d *Daemon) failFloatingLaunch(sess *session, tb *tab, generation uint64, u | |||||
| 444 | // installFloating installs only the current generation. A stale successful | 447 | // installFloating installs only the current generation. A stale successful | ||
| 445 | // launch is cancelled and closed outside the tab lock. | 448 | // launch is cancelled and closed outside the tab lock. | ||
| 446 | func (d *Daemon) installFloating(sess *session, tb *tab, p *pane, generation uint64) { | 449 | func (d *Daemon) installFloating(sess *session, tb *tab, p *pane, generation uint64) { | ||
| 450 | + | if sess == nil || tb == nil || p == nil { | |||
| 451 | + | closeFloatingPane(p) | |||
| 452 | + | return | |||
| 453 | + | } | |||
| 447 | tb.mu.Lock() | 454 | tb.mu.Lock() | ||
| 448 | - | installed := tb.installFloatingLocked(p, generation) | 455 | + | installable := tb.floating.state == floatingWarming && tb.floating.generation == generation |
| 456 | + | if installable { | |||
| 457 | + | // Publish under pane.mu before the slot becomes hidden or visible. The | |||
| 458 | + | // tab-to-pane lock order also linearizes owner and slot publication. | |||
| 459 | + | publishPaneOwner(p, sess, tb, generation) | |||
| 460 | + | } | |||
| 461 | + | installed := installable && tb.installFloatingLocked(p, generation) | |||
| 449 | visible := installed && tb.floating.state == floatingVisible | 462 | visible := installed && tb.floating.state == floatingVisible | ||
| 450 | tb.mu.Unlock() | 463 | tb.mu.Unlock() | ||
| 451 | if !installed { | 464 | if !installed { | ||
| @@ -506,17 +519,7 @@ func (d *Daemon) teardownFloating(tb *tab, ac *attachedClient) { | |||||
| 506 | // closeFloatingPane releases a floating runtime at most once. It is called | 519 | // closeFloatingPane releases a floating runtime at most once. It is called | ||
| 507 | // only after a generation/identity check has detached the pane from its slot. | 520 | // only after a generation/identity check has detached the pane from its slot. | ||
| 508 | func closeFloatingPane(p *pane) { | 521 | func closeFloatingPane(p *pane) { | ||
| 509 | - | if p == nil { | 522 | + | closePaneProcess(p) |
| 510 | - | return | |||
| 511 | - | } | |||
| 512 | - | p.floatingCloseOnce.Do(func() { | |||
| 513 | - | if p.cancel != nil { | |||
| 514 | - | p.cancel() | |||
| 515 | - | } | |||
| 516 | - | if p.pty != nil { | |||
| 517 | - | _ = p.pty.Close() | |||
| 518 | - | } | |||
| 519 | - | }) | |||
| 520 | } | 523 | } | ||
| 521 | 524 | ||||
| 522 | func floatingCommandFallback(command, shell string) string { | 525 | func floatingCommandFallback(command, shell string) string { | ||
M internal/usecase/daemon/floating_open_test.go +165 -0
| @@ -2,7 +2,9 @@ package daemon | |||
| 2 | 2 | ||
| 3 | 3 | import ( | |
| 4 | 4 | "context" | |
| 5 | + | "io" | |
| 5 | 6 | "sync" | |
| 7 | + | "sync/atomic" | |
| 6 | 8 | "testing" | |
| 7 | 9 | "time" | |
| 8 | 10 | ||
| @@ -154,6 +156,169 @@ func TestFloatingSuccessfulOpenTransfersContextOwnershipToPane(t *testing.T) { | |||
| 154 | 156 | } | |
| 155 | 157 | } | |
| 156 | 158 | ||
| 159 | + | type failedOpenPTY struct { | |
| 160 | + | ctx context.Context | |
| 161 | + | readerStarts atomic.Int32 | |
| 162 | + | closes atomic.Int32 | |
| 163 | + | closed chan struct{} | |
| 164 | + | closeOnce sync.Once | |
| 165 | + | onClose func() | |
| 166 | + | } | |
| 167 | + | ||
| 168 | + | func newFailedOpenPTY() *failedOpenPTY { | |
| 169 | + | return &failedOpenPTY{closed: make(chan struct{})} | |
| 170 | + | } | |
| 171 | + | ||
| 172 | + | func (p *failedOpenPTY) Read([]byte) (int, error) { | |
| 173 | + | p.readerStarts.Add(1) | |
| 174 | + | select { | |
| 175 | + | case <-p.ctx.Done(): | |
| 176 | + | return 0, p.ctx.Err() | |
| 177 | + | case <-p.closed: | |
| 178 | + | return 0, io.EOF | |
| 179 | + | } | |
| 180 | + | } | |
| 181 | + | ||
| 182 | + | func (*failedOpenPTY) Write(b []byte) (int, error) { return len(b), nil } | |
| 183 | + | func (*failedOpenPTY) Resize(domain.Size) error { return nil } | |
| 184 | + | func (*failedOpenPTY) Pid() int { return 1 } | |
| 185 | + | func (*failedOpenPTY) ForegroundPgid() (int, error) { | |
| 186 | + | return 1, nil | |
| 187 | + | } | |
| 188 | + | ||
| 189 | + | func (p *failedOpenPTY) Close() error { | |
| 190 | + | p.closes.Add(1) | |
| 191 | + | if p.onClose != nil { | |
| 192 | + | p.onClose() | |
| 193 | + | } | |
| 194 | + | p.closeOnce.Do(func() { close(p.closed) }) | |
| 195 | + | return nil | |
| 196 | + | } | |
| 197 | + | ||
| 198 | + | func TestFloatingOpenErrorReleasesReturnedPTYBeforePublishingFailure(t *testing.T) { | |
| 199 | + | tests := []struct { | |
| 200 | + | name string | |
| 201 | + | userOpen bool | |
| 202 | + | }{ | |
| 203 | + | {name: "user open", userOpen: true}, | |
| 204 | + | {name: "background prewarm", userOpen: false}, | |
| 205 | + | } | |
| 206 | + | for _, tt := range tests { | |
| 207 | + | t.Run(tt.name, func(t *testing.T) { | |
| 208 | + | cause := io.ErrUnexpectedEOF | |
| 209 | + | pty := newFailedOpenPTY() | |
| 210 | + | factory := portsmocks.NewMockPTYFactory(t) | |
| 211 | + | cancelCallbacks := atomic.Int32{} | |
| 212 | + | cancelled := make(chan struct{}) | |
| 213 | + | factory.EXPECT().Open(mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything).RunAndReturn( | |
| 214 | + | func(ctx context.Context, _ string, _ []string, _ []string, _ string, _ domain.Size) (ports.PTY, error) { | |
| 215 | + | pty.ctx = ctx | |
| 216 | + | context.AfterFunc(ctx, func() { | |
| 217 | + | cancelCallbacks.Add(1) | |
| 218 | + | close(cancelled) | |
| 219 | + | }) | |
| 220 | + | return pty, cause | |
| 221 | + | }).Once() | |
| 222 | + | d := newTestDaemon(t, factory, stubClock{}) | |
| 223 | + | tb := newFloatingTestTab(t) | |
| 224 | + | tr, _ := newCapturingTransport(t) | |
| 225 | + | ac := &attachedClient{tr: tr, output: newOutputStateStream(), size: domain.Size{Cols: 80, Rows: 24}} | |
| 226 | + | ac.initOverlays() | |
| 227 | + | sessCtx, cancelSession := context.WithCancel(t.Context()) | |
| 228 | + | sess := &session{id: "floating-open-error", name: "work", tabs: []*tab{tb}, ctx: sessCtx, cancel: cancelSession, client: ac} | |
| 229 | + | ac.setSession(sess) | |
| 230 | + | ||
| 231 | + | // Close must be safe to run before failure publication and without | |
| 232 | + | // either ownership lock held. | |
| 233 | + | var unpublishedAtClose atomic.Bool | |
| 234 | + | pty.onClose = func() { | |
| 235 | + | tb.mu.Lock() | |
| 236 | + | launchUnpublished := tb.floating.pane == nil && tb.floating.state == floatingWarming | |
| 237 | + | tb.mu.Unlock() | |
| 238 | + | sess.mu.Lock() | |
| 239 | + | sessionName := sess.name | |
| 240 | + | sess.mu.Unlock() | |
| 241 | + | unpublishedAtClose.Store(launchUnpublished && sessionName == "work") | |
| 242 | + | } | |
| 243 | + | ||
| 244 | + | d.startFloating(sess, tb, tt.userOpen) | |
| 245 | + | d.sessWg.Wait() | |
| 246 | + | select { | |
| 247 | + | case <-cancelled: | |
| 248 | + | case <-time.After(time.Second): | |
| 249 | + | t.Fatal("failed floating Open process context was not cancelled") | |
| 250 | + | } | |
| 251 | + | ||
| 252 | + | // Exercise later cancellation/stale cleanup paths; neither may find | |
| 253 | + | // ownership to release a second time. | |
| 254 | + | cancelSession() | |
| 255 | + | d.teardownFloating(tb, ac) | |
| 256 | + | require.Equal(t, int32(1), cancelCallbacks.Load(), "process context must be released exactly once") | |
| 257 | + | require.Equal(t, int32(1), pty.closes.Load(), "returned PTY must be closed exactly once") | |
| 258 | + | require.True(t, unpublishedAtClose.Load(), "PTY must close before any failure publication") | |
| 259 | + | require.Zero(t, pty.readerStarts.Load(), "failed Open must not start a reader") | |
| 260 | + | ||
| 261 | + | tb.mu.Lock() | |
| 262 | + | floating := tb.floating.pane | |
| 263 | + | publishedFailedPTY := false | |
| 264 | + | for _, published := range tb.panes { | |
| 265 | + | publishedFailedPTY = publishedFailedPTY || published.pty == pty | |
| 266 | + | } | |
| 267 | + | tb.mu.Unlock() | |
| 268 | + | require.Nil(t, floating, "failed Open must not publish the floating slot") | |
| 269 | + | require.False(t, publishedFailedPTY, "failed Open must not publish a pane or owner") | |
| 270 | + | if tt.userOpen { | |
| 271 | + | toasts := awaitToastCount(t, ac, 1) | |
| 272 | + | require.Equal(t, domain.NoticeFloatingSpawn, toasts[0].Code) | |
| 273 | + | require.Equal(t, "couldn't open floating pane: command failed to start", toasts[0].Message) | |
| 274 | + | } else { | |
| 275 | + | toasts, _ := visibleToasts(ac) | |
| 276 | + | require.Empty(t, toasts, "background prewarm failure must stay silent") | |
| 277 | + | } | |
| 278 | + | }) | |
| 279 | + | } | |
| 280 | + | } | |
| 281 | + | ||
| 282 | + | func TestInstallFloatingRejectsNilPublicationInputs(t *testing.T) { | |
| 283 | + | tests := []struct { | |
| 284 | + | name string | |
| 285 | + | nilSession bool | |
| 286 | + | nilTab bool | |
| 287 | + | nilPane bool | |
| 288 | + | wantCloses int | |
| 289 | + | }{ | |
| 290 | + | {name: "nil session", nilSession: true, wantCloses: 1}, | |
| 291 | + | {name: "nil tab", nilTab: true, wantCloses: 1}, | |
| 292 | + | {name: "nil pane", nilPane: true}, | |
| 293 | + | {name: "all nil", nilSession: true, nilTab: true, nilPane: true}, | |
| 294 | + | } | |
| 295 | + | for _, tt := range tests { | |
| 296 | + | t.Run(tt.name, func(t *testing.T) { | |
| 297 | + | d := &Daemon{} | |
| 298 | + | sess := &session{} | |
| 299 | + | tb := newFloatingTestTab(t) | |
| 300 | + | tb.mu.Lock() | |
| 301 | + | generation := tb.beginFloatingWarmLocked(true) | |
| 302 | + | tb.mu.Unlock() | |
| 303 | + | pty := newContextAwareFloatingPTY() | |
| 304 | + | p := newPane("floating", pty, domain.Size{Cols: 20, Rows: 8}) | |
| 305 | + | ||
| 306 | + | if tt.nilSession { | |
| 307 | + | sess = nil | |
| 308 | + | } | |
| 309 | + | if tt.nilTab { | |
| 310 | + | tb = nil | |
| 311 | + | } | |
| 312 | + | if tt.nilPane { | |
| 313 | + | p = nil | |
| 314 | + | } | |
| 315 | + | ||
| 316 | + | require.NotPanics(t, func() { d.installFloating(sess, tb, p, generation) }) | |
| 317 | + | require.Equal(t, tt.wantCloses, pty.closeCount) | |
| 318 | + | }) | |
| 319 | + | } | |
| 320 | + | } | |
| 321 | + | ||
| 157 | 322 | func TestFloatingFailedAndStaleOpenCancelContext(t *testing.T) { | |
| 158 | 323 | tests := []struct { | |
| 159 | 324 | name string | |
| @@ -2,7 +2,9 @@ package daemon | |||||
| 2 | 2 | ||||
| 3 | import ( | 3 | import ( | ||
| 4 | "context" | 4 | "context" | ||
| 5 | + | "io" | |||
| 5 | "sync" | 6 | "sync" | ||
| 7 | + | "sync/atomic" | |||
| 6 | "testing" | 8 | "testing" | ||
| 7 | "time" | 9 | "time" | ||
| 8 | 10 | ||||
| @@ -154,6 +156,169 @@ func TestFloatingSuccessfulOpenTransfersContextOwnershipToPane(t *testing.T) { | |||||
| 154 | } | 156 | } | ||
| 155 | } | 157 | } | ||
| 156 | 158 | ||||
| 159 | + | type failedOpenPTY struct { | |||
| 160 | + | ctx context.Context | |||
| 161 | + | readerStarts atomic.Int32 | |||
| 162 | + | closes atomic.Int32 | |||
| 163 | + | closed chan struct{} | |||
| 164 | + | closeOnce sync.Once | |||
| 165 | + | onClose func() | |||
| 166 | + | } | |||
| 167 | + | ||||
| 168 | + | func newFailedOpenPTY() *failedOpenPTY { | |||
| 169 | + | return &failedOpenPTY{closed: make(chan struct{})} | |||
| 170 | + | } | |||
| 171 | + | ||||
| 172 | + | func (p *failedOpenPTY) Read([]byte) (int, error) { | |||
| 173 | + | p.readerStarts.Add(1) | |||
| 174 | + | select { | |||
| 175 | + | case <-p.ctx.Done(): | |||
| 176 | + | return 0, p.ctx.Err() | |||
| 177 | + | case <-p.closed: | |||
| 178 | + | return 0, io.EOF | |||
| 179 | + | } | |||
| 180 | + | } | |||
| 181 | + | ||||
| 182 | + | func (*failedOpenPTY) Write(b []byte) (int, error) { return len(b), nil } | |||
| 183 | + | func (*failedOpenPTY) Resize(domain.Size) error { return nil } | |||
| 184 | + | func (*failedOpenPTY) Pid() int { return 1 } | |||
| 185 | + | func (*failedOpenPTY) ForegroundPgid() (int, error) { | |||
| 186 | + | return 1, nil | |||
| 187 | + | } | |||
| 188 | + | ||||
| 189 | + | func (p *failedOpenPTY) Close() error { | |||
| 190 | + | p.closes.Add(1) | |||
| 191 | + | if p.onClose != nil { | |||
| 192 | + | p.onClose() | |||
| 193 | + | } | |||
| 194 | + | p.closeOnce.Do(func() { close(p.closed) }) | |||
| 195 | + | return nil | |||
| 196 | + | } | |||
| 197 | + | ||||
| 198 | + | func TestFloatingOpenErrorReleasesReturnedPTYBeforePublishingFailure(t *testing.T) { | |||
| 199 | + | tests := []struct { | |||
| 200 | + | name string | |||
| 201 | + | userOpen bool | |||
| 202 | + | }{ | |||
| 203 | + | {name: "user open", userOpen: true}, | |||
| 204 | + | {name: "background prewarm", userOpen: false}, | |||
| 205 | + | } | |||
| 206 | + | for _, tt := range tests { | |||
| 207 | + | t.Run(tt.name, func(t *testing.T) { | |||
| 208 | + | cause := io.ErrUnexpectedEOF | |||
| 209 | + | pty := newFailedOpenPTY() | |||
| 210 | + | factory := portsmocks.NewMockPTYFactory(t) | |||
| 211 | + | cancelCallbacks := atomic.Int32{} | |||
| 212 | + | cancelled := make(chan struct{}) | |||
| 213 | + | factory.EXPECT().Open(mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything).RunAndReturn( | |||
| 214 | + | func(ctx context.Context, _ string, _ []string, _ []string, _ string, _ domain.Size) (ports.PTY, error) { | |||
| 215 | + | pty.ctx = ctx | |||
| 216 | + | context.AfterFunc(ctx, func() { | |||
| 217 | + | cancelCallbacks.Add(1) | |||
| 218 | + | close(cancelled) | |||
| 219 | + | }) | |||
| 220 | + | return pty, cause | |||
| 221 | + | }).Once() | |||
| 222 | + | d := newTestDaemon(t, factory, stubClock{}) | |||
| 223 | + | tb := newFloatingTestTab(t) | |||
| 224 | + | tr, _ := newCapturingTransport(t) | |||
| 225 | + | ac := &attachedClient{tr: tr, output: newOutputStateStream(), size: domain.Size{Cols: 80, Rows: 24}} | |||
| 226 | + | ac.initOverlays() | |||
| 227 | + | sessCtx, cancelSession := context.WithCancel(t.Context()) | |||
| 228 | + | sess := &session{id: "floating-open-error", name: "work", tabs: []*tab{tb}, ctx: sessCtx, cancel: cancelSession, client: ac} | |||
| 229 | + | ac.setSession(sess) | |||
| 230 | + | ||||
| 231 | + | // Close must be safe to run before failure publication and without | |||
| 232 | + | // either ownership lock held. | |||
| 233 | + | var unpublishedAtClose atomic.Bool | |||
| 234 | + | pty.onClose = func() { | |||
| 235 | + | tb.mu.Lock() | |||
| 236 | + | launchUnpublished := tb.floating.pane == nil && tb.floating.state == floatingWarming | |||
| 237 | + | tb.mu.Unlock() | |||
| 238 | + | sess.mu.Lock() | |||
| 239 | + | sessionName := sess.name | |||
| 240 | + | sess.mu.Unlock() | |||
| 241 | + | unpublishedAtClose.Store(launchUnpublished && sessionName == "work") | |||
| 242 | + | } | |||
| 243 | + | ||||
| 244 | + | d.startFloating(sess, tb, tt.userOpen) | |||
| 245 | + | d.sessWg.Wait() | |||
| 246 | + | select { | |||
| 247 | + | case <-cancelled: | |||
| 248 | + | case <-time.After(time.Second): | |||
| 249 | + | t.Fatal("failed floating Open process context was not cancelled") | |||
| 250 | + | } | |||
| 251 | + | ||||
| 252 | + | // Exercise later cancellation/stale cleanup paths; neither may find | |||
| 253 | + | // ownership to release a second time. | |||
| 254 | + | cancelSession() | |||
| 255 | + | d.teardownFloating(tb, ac) | |||
| 256 | + | require.Equal(t, int32(1), cancelCallbacks.Load(), "process context must be released exactly once") | |||
| 257 | + | require.Equal(t, int32(1), pty.closes.Load(), "returned PTY must be closed exactly once") | |||
| 258 | + | require.True(t, unpublishedAtClose.Load(), "PTY must close before any failure publication") | |||
| 259 | + | require.Zero(t, pty.readerStarts.Load(), "failed Open must not start a reader") | |||
| 260 | + | ||||
| 261 | + | tb.mu.Lock() | |||
| 262 | + | floating := tb.floating.pane | |||
| 263 | + | publishedFailedPTY := false | |||
| 264 | + | for _, published := range tb.panes { | |||
| 265 | + | publishedFailedPTY = publishedFailedPTY || published.pty == pty | |||
| 266 | + | } | |||
| 267 | + | tb.mu.Unlock() | |||
| 268 | + | require.Nil(t, floating, "failed Open must not publish the floating slot") | |||
| 269 | + | require.False(t, publishedFailedPTY, "failed Open must not publish a pane or owner") | |||
| 270 | + | if tt.userOpen { | |||
| 271 | + | toasts := awaitToastCount(t, ac, 1) | |||
| 272 | + | require.Equal(t, domain.NoticeFloatingSpawn, toasts[0].Code) | |||
| 273 | + | require.Equal(t, "couldn't open floating pane: command failed to start", toasts[0].Message) | |||
| 274 | + | } else { | |||
| 275 | + | toasts, _ := visibleToasts(ac) | |||
| 276 | + | require.Empty(t, toasts, "background prewarm failure must stay silent") | |||
| 277 | + | } | |||
| 278 | + | }) | |||
| 279 | + | } | |||
| 280 | + | } | |||
| 281 | + | ||||
| 282 | + | func TestInstallFloatingRejectsNilPublicationInputs(t *testing.T) { | |||
| 283 | + | tests := []struct { | |||
| 284 | + | name string | |||
| 285 | + | nilSession bool | |||
| 286 | + | nilTab bool | |||
| 287 | + | nilPane bool | |||
| 288 | + | wantCloses int | |||
| 289 | + | }{ | |||
| 290 | + | {name: "nil session", nilSession: true, wantCloses: 1}, | |||
| 291 | + | {name: "nil tab", nilTab: true, wantCloses: 1}, | |||
| 292 | + | {name: "nil pane", nilPane: true}, | |||
| 293 | + | {name: "all nil", nilSession: true, nilTab: true, nilPane: true}, | |||
| 294 | + | } | |||
| 295 | + | for _, tt := range tests { | |||
| 296 | + | t.Run(tt.name, func(t *testing.T) { | |||
| 297 | + | d := &Daemon{} | |||
| 298 | + | sess := &session{} | |||
| 299 | + | tb := newFloatingTestTab(t) | |||
| 300 | + | tb.mu.Lock() | |||
| 301 | + | generation := tb.beginFloatingWarmLocked(true) | |||
| 302 | + | tb.mu.Unlock() | |||
| 303 | + | pty := newContextAwareFloatingPTY() | |||
| 304 | + | p := newPane("floating", pty, domain.Size{Cols: 20, Rows: 8}) | |||
| 305 | + | ||||
| 306 | + | if tt.nilSession { | |||
| 307 | + | sess = nil | |||
| 308 | + | } | |||
| 309 | + | if tt.nilTab { | |||
| 310 | + | tb = nil | |||
| 311 | + | } | |||
| 312 | + | if tt.nilPane { | |||
| 313 | + | p = nil | |||
| 314 | + | } | |||
| 315 | + | ||||
| 316 | + | require.NotPanics(t, func() { d.installFloating(sess, tb, p, generation) }) | |||
| 317 | + | require.Equal(t, tt.wantCloses, pty.closeCount) | |||
| 318 | + | }) | |||
| 319 | + | } | |||
| 320 | + | } | |||
| 321 | + | ||||
| 157 | func TestFloatingFailedAndStaleOpenCancelContext(t *testing.T) { | 322 | func TestFloatingFailedAndStaleOpenCancelContext(t *testing.T) { | ||
| 158 | tests := []struct { | 323 | tests := []struct { | ||
| 159 | name string | 324 | name string | ||
A internal/usecase/daemon/move_lifecycle.go +149 -0
| @@ -0,0 +1,149 @@ | |||
| 1 | + | package daemon | |
| 2 | + | ||
| 3 | + | import ( | |
| 4 | + | "cmp" | |
| 5 | + | "errors" | |
| 6 | + | "slices" | |
| 7 | + | "sync" | |
| 8 | + | ) | |
| 9 | + | ||
| 10 | + | var errMoveLifecycleUnavailable = errors.New("move lifecycle is no longer available") | |
| 11 | + | ||
| 12 | + | // moveLifecycleReservation pins a unique, canonically ordered set of sessions | |
| 13 | + | // plus the daemon move gate. Release is idempotent so every abort path can use | |
| 14 | + | // defer without coordinating ownership with its caller. | |
| 15 | + | type moveLifecycleReservation struct { | |
| 16 | + | daemon *Daemon | |
| 17 | + | sessions []*session | |
| 18 | + | once sync.Once | |
| 19 | + | } | |
| 20 | + | ||
| 21 | + | // reserveMoveLifecycles admits a move at the daemon gate before touching any | |
| 22 | + | // per-session lifecycle state. All session counts publish atomically while the | |
| 23 | + | // complete ordered teardown lock set is held. | |
| 24 | + | func (d *Daemon) reserveMoveLifecycles(source, destination *session) (*moveLifecycleReservation, error) { | |
| 25 | + | if d == nil || source == nil || destination == nil { | |
| 26 | + | return nil, errMoveLifecycleUnavailable | |
| 27 | + | } | |
| 28 | + | ||
| 29 | + | d.moveLifecycleMu.Lock() | |
| 30 | + | if d.moveLifecycleClosing { | |
| 31 | + | d.moveLifecycleMu.Unlock() | |
| 32 | + | return nil, errMoveLifecycleUnavailable | |
| 33 | + | } | |
| 34 | + | d.mu.Lock() | |
| 35 | + | closing := d.closing | |
| 36 | + | d.mu.Unlock() | |
| 37 | + | if closing { | |
| 38 | + | d.moveLifecycleMu.Unlock() | |
| 39 | + | return nil, errMoveLifecycleUnavailable | |
| 40 | + | } | |
| 41 | + | d.moveLifecycleActive++ | |
| 42 | + | d.signalMoveLifecycleChangedLocked() | |
| 43 | + | d.moveLifecycleMu.Unlock() | |
| 44 | + | ||
| 45 | + | sessions := uniqueMoveSessions(source, destination) | |
| 46 | + | for _, sess := range sessions { | |
| 47 | + | sess.teardownMu.Lock() | |
| 48 | + | } | |
| 49 | + | for _, sess := range sessions { | |
| 50 | + | if sess.teardownActive { | |
| 51 | + | unlockMoveSessions(sessions) | |
| 52 | + | d.moveLifecycleMu.Lock() | |
| 53 | + | d.moveLifecycleActive-- | |
| 54 | + | d.signalMoveLifecycleChangedLocked() | |
| 55 | + | d.moveLifecycleMu.Unlock() | |
| 56 | + | return nil, errMoveLifecycleUnavailable | |
| 57 | + | } | |
| 58 | + | } | |
| 59 | + | for _, sess := range sessions { | |
| 60 | + | sess.moveReservations++ | |
| 61 | + | sess.signalTeardownChangedLocked() | |
| 62 | + | } | |
| 63 | + | unlockMoveSessions(sessions) | |
| 64 | + | ||
| 65 | + | return &moveLifecycleReservation{daemon: d, sessions: sessions}, nil | |
| 66 | + | } | |
| 67 | + | ||
| 68 | + | func uniqueMoveSessions(source, destination *session) []*session { | |
| 69 | + | sessions := []*session{source} | |
| 70 | + | if destination != source { | |
| 71 | + | sessions = append(sessions, destination) | |
| 72 | + | } | |
| 73 | + | slices.SortFunc(sessions, func(left, right *session) int { | |
| 74 | + | return cmp.Compare(left.id, right.id) | |
| 75 | + | }) | |
| 76 | + | return sessions | |
| 77 | + | } | |
| 78 | + | ||
| 79 | + | func unlockMoveSessions(sessions []*session) { | |
| 80 | + | for i := len(sessions) - 1; i >= 0; i-- { | |
| 81 | + | sessions[i].teardownMu.Unlock() | |
| 82 | + | } | |
| 83 | + | } | |
| 84 | + | ||
| 85 | + | func (r *moveLifecycleReservation) Release() { | |
| 86 | + | if r == nil { | |
| 87 | + | return | |
| 88 | + | } | |
| 89 | + | r.once.Do(func() { | |
| 90 | + | r.daemon.moveLifecycleMu.Lock() | |
| 91 | + | for _, sess := range r.sessions { | |
| 92 | + | sess.teardownMu.Lock() | |
| 93 | + | } | |
| 94 | + | for _, sess := range r.sessions { | |
| 95 | + | sess.moveReservations-- | |
| 96 | + | sess.signalTeardownChangedLocked() | |
| 97 | + | } | |
| 98 | + | unlockMoveSessions(r.sessions) | |
| 99 | + | r.daemon.moveLifecycleActive-- | |
| 100 | + | r.daemon.signalMoveLifecycleChangedLocked() | |
| 101 | + | r.daemon.moveLifecycleMu.Unlock() | |
| 102 | + | }) | |
| 103 | + | } | |
| 104 | + | ||
| 105 | + | // closeMoveLifecycles rejects new moves, publishes daemon shutdown to ordinary | |
| 106 | + | // routing, then drains reservations without holding either daemon mutex. Pane | |
| 107 | + | // process lifetime is cancelled only after the final reservation releases. | |
| 108 | + | func (d *Daemon) closeMoveLifecycles() { | |
| 109 | + | if d == nil { | |
| 110 | + | return | |
| 111 | + | } | |
| 112 | + | d.moveLifecycleMu.Lock() | |
| 113 | + | if !d.moveLifecycleClosing { | |
| 114 | + | d.moveLifecycleClosing = true | |
| 115 | + | d.signalMoveLifecycleChangedLocked() | |
| 116 | + | } | |
| 117 | + | d.moveLifecycleMu.Unlock() | |
| 118 | + | ||
| 119 | + | // Publish ordinary shutdown admission separately: no move gate mutex is held | |
| 120 | + | // while taking the architecture registry lock. | |
| 121 | + | d.mu.Lock() | |
| 122 | + | d.closing = true | |
| 123 | + | d.mu.Unlock() | |
| 124 | + | ||
| 125 | + | d.moveLifecycleMu.Lock() | |
| 126 | + | for d.moveLifecycleActive != 0 { | |
| 127 | + | changed := d.moveLifecycleChangeLocked() | |
| 128 | + | d.moveLifecycleMu.Unlock() | |
| 129 | + | <-changed | |
| 130 | + | d.moveLifecycleMu.Lock() | |
| 131 | + | } | |
| 132 | + | cancel := d.paneProcessCancel | |
| 133 | + | d.moveLifecycleMu.Unlock() | |
| 134 | + | if cancel != nil { | |
| 135 | + | cancel() | |
| 136 | + | } | |
| 137 | + | } | |
| 138 | + | ||
| 139 | + | func (d *Daemon) moveLifecycleChangeLocked() chan struct{} { | |
| 140 | + | if d.moveLifecycleChanged == nil { | |
| 141 | + | d.moveLifecycleChanged = make(chan struct{}) | |
| 142 | + | } | |
| 143 | + | return d.moveLifecycleChanged | |
| 144 | + | } | |
| 145 | + | ||
| 146 | + | func (d *Daemon) signalMoveLifecycleChangedLocked() { | |
| 147 | + | close(d.moveLifecycleChangeLocked()) | |
| 148 | + | d.moveLifecycleChanged = make(chan struct{}) | |
| 149 | + | } | |
| @@ -0,0 +1,149 @@ | |||||
| 1 | + | package daemon | |||
| 2 | + | ||||
| 3 | + | import ( | |||
| 4 | + | "cmp" | |||
| 5 | + | "errors" | |||
| 6 | + | "slices" | |||
| 7 | + | "sync" | |||
| 8 | + | ) | |||
| 9 | + | ||||
| 10 | + | var errMoveLifecycleUnavailable = errors.New("move lifecycle is no longer available") | |||
| 11 | + | ||||
| 12 | + | // moveLifecycleReservation pins a unique, canonically ordered set of sessions | |||
| 13 | + | // plus the daemon move gate. Release is idempotent so every abort path can use | |||
| 14 | + | // defer without coordinating ownership with its caller. | |||
| 15 | + | type moveLifecycleReservation struct { | |||
| 16 | + | daemon *Daemon | |||
| 17 | + | sessions []*session | |||
| 18 | + | once sync.Once | |||
| 19 | + | } | |||
| 20 | + | ||||
| 21 | + | // reserveMoveLifecycles admits a move at the daemon gate before touching any | |||
| 22 | + | // per-session lifecycle state. All session counts publish atomically while the | |||
| 23 | + | // complete ordered teardown lock set is held. | |||
| 24 | + | func (d *Daemon) reserveMoveLifecycles(source, destination *session) (*moveLifecycleReservation, error) { | |||
| 25 | + | if d == nil || source == nil || destination == nil { | |||
| 26 | + | return nil, errMoveLifecycleUnavailable | |||
| 27 | + | } | |||
| 28 | + | ||||
| 29 | + | d.moveLifecycleMu.Lock() | |||
| 30 | + | if d.moveLifecycleClosing { | |||
| 31 | + | d.moveLifecycleMu.Unlock() | |||
| 32 | + | return nil, errMoveLifecycleUnavailable | |||
| 33 | + | } | |||
| 34 | + | d.mu.Lock() | |||
| 35 | + | closing := d.closing | |||
| 36 | + | d.mu.Unlock() | |||
| 37 | + | if closing { | |||
| 38 | + | d.moveLifecycleMu.Unlock() | |||
| 39 | + | return nil, errMoveLifecycleUnavailable | |||
| 40 | + | } | |||
| 41 | + | d.moveLifecycleActive++ | |||
| 42 | + | d.signalMoveLifecycleChangedLocked() | |||
| 43 | + | d.moveLifecycleMu.Unlock() | |||
| 44 | + | ||||
| 45 | + | sessions := uniqueMoveSessions(source, destination) | |||
| 46 | + | for _, sess := range sessions { | |||
| 47 | + | sess.teardownMu.Lock() | |||
| 48 | + | } | |||
| 49 | + | for _, sess := range sessions { | |||
| 50 | + | if sess.teardownActive { | |||
| 51 | + | unlockMoveSessions(sessions) | |||
| 52 | + | d.moveLifecycleMu.Lock() | |||
| 53 | + | d.moveLifecycleActive-- | |||
| 54 | + | d.signalMoveLifecycleChangedLocked() | |||
| 55 | + | d.moveLifecycleMu.Unlock() | |||
| 56 | + | return nil, errMoveLifecycleUnavailable | |||
| 57 | + | } | |||
| 58 | + | } | |||
| 59 | + | for _, sess := range sessions { | |||
| 60 | + | sess.moveReservations++ | |||
| 61 | + | sess.signalTeardownChangedLocked() | |||
| 62 | + | } | |||
| 63 | + | unlockMoveSessions(sessions) | |||
| 64 | + | ||||
| 65 | + | return &moveLifecycleReservation{daemon: d, sessions: sessions}, nil | |||
| 66 | + | } | |||
| 67 | + | ||||
| 68 | + | func uniqueMoveSessions(source, destination *session) []*session { | |||
| 69 | + | sessions := []*session{source} | |||
| 70 | + | if destination != source { | |||
| 71 | + | sessions = append(sessions, destination) | |||
| 72 | + | } | |||
| 73 | + | slices.SortFunc(sessions, func(left, right *session) int { | |||
| 74 | + | return cmp.Compare(left.id, right.id) | |||
| 75 | + | }) | |||
| 76 | + | return sessions | |||
| 77 | + | } | |||
| 78 | + | ||||
| 79 | + | func unlockMoveSessions(sessions []*session) { | |||
| 80 | + | for i := len(sessions) - 1; i >= 0; i-- { | |||
| 81 | + | sessions[i].teardownMu.Unlock() | |||
| 82 | + | } | |||
| 83 | + | } | |||
| 84 | + | ||||
| 85 | + | func (r *moveLifecycleReservation) Release() { | |||
| 86 | + | if r == nil { | |||
| 87 | + | return | |||
| 88 | + | } | |||
| 89 | + | r.once.Do(func() { | |||
| 90 | + | r.daemon.moveLifecycleMu.Lock() | |||
| 91 | + | for _, sess := range r.sessions { | |||
| 92 | + | sess.teardownMu.Lock() | |||
| 93 | + | } | |||
| 94 | + | for _, sess := range r.sessions { | |||
| 95 | + | sess.moveReservations-- | |||
| 96 | + | sess.signalTeardownChangedLocked() | |||
| 97 | + | } | |||
| 98 | + | unlockMoveSessions(r.sessions) | |||
| 99 | + | r.daemon.moveLifecycleActive-- | |||
| 100 | + | r.daemon.signalMoveLifecycleChangedLocked() | |||
| 101 | + | r.daemon.moveLifecycleMu.Unlock() | |||
| 102 | + | }) | |||
| 103 | + | } | |||
| 104 | + | ||||
| 105 | + | // closeMoveLifecycles rejects new moves, publishes daemon shutdown to ordinary | |||
| 106 | + | // routing, then drains reservations without holding either daemon mutex. Pane | |||
| 107 | + | // process lifetime is cancelled only after the final reservation releases. | |||
| 108 | + | func (d *Daemon) closeMoveLifecycles() { | |||
| 109 | + | if d == nil { | |||
| 110 | + | return | |||
| 111 | + | } | |||
| 112 | + | d.moveLifecycleMu.Lock() | |||
| 113 | + | if !d.moveLifecycleClosing { | |||
| 114 | + | d.moveLifecycleClosing = true | |||
| 115 | + | d.signalMoveLifecycleChangedLocked() | |||
| 116 | + | } | |||
| 117 | + | d.moveLifecycleMu.Unlock() | |||
| 118 | + | ||||
| 119 | + | // Publish ordinary shutdown admission separately: no move gate mutex is held | |||
| 120 | + | // while taking the architecture registry lock. | |||
| 121 | + | d.mu.Lock() | |||
| 122 | + | d.closing = true | |||
| 123 | + | d.mu.Unlock() | |||
| 124 | + | ||||
| 125 | + | d.moveLifecycleMu.Lock() | |||
| 126 | + | for d.moveLifecycleActive != 0 { | |||
| 127 | + | changed := d.moveLifecycleChangeLocked() | |||
| 128 | + | d.moveLifecycleMu.Unlock() | |||
| 129 | + | <-changed | |||
| 130 | + | d.moveLifecycleMu.Lock() | |||
| 131 | + | } | |||
| 132 | + | cancel := d.paneProcessCancel | |||
| 133 | + | d.moveLifecycleMu.Unlock() | |||
| 134 | + | if cancel != nil { | |||
| 135 | + | cancel() | |||
| 136 | + | } | |||
| 137 | + | } | |||
| 138 | + | ||||
| 139 | + | func (d *Daemon) moveLifecycleChangeLocked() chan struct{} { | |||
| 140 | + | if d.moveLifecycleChanged == nil { | |||
| 141 | + | d.moveLifecycleChanged = make(chan struct{}) | |||
| 142 | + | } | |||
| 143 | + | return d.moveLifecycleChanged | |||
| 144 | + | } | |||
| 145 | + | ||||
| 146 | + | func (d *Daemon) signalMoveLifecycleChangedLocked() { | |||
| 147 | + | close(d.moveLifecycleChangeLocked()) | |||
| 148 | + | d.moveLifecycleChanged = make(chan struct{}) | |||
| 149 | + | } | |||
A internal/usecase/daemon/move_lifecycle_test.go +247 -0
| @@ -0,0 +1,247 @@ | |||
| 1 | + | package daemon | |
| 2 | + | ||
| 3 | + | import ( | |
| 4 | + | "testing" | |
| 5 | + | "time" | |
| 6 | + | ||
| 7 | + | "github.com/stretchr/testify/require" | |
| 8 | + | ||
| 9 | + | "github.com/bnema/vev/internal/ports" | |
| 10 | + | ) | |
| 11 | + | ||
| 12 | + | func awaitMoveTeardownState(t *testing.T, sess *session, reservations, waiters uint) { | |
| 13 | + | t.Helper() | |
| 14 | + | for { | |
| 15 | + | sess.teardownMu.Lock() | |
| 16 | + | gotReservations := sess.moveReservations | |
| 17 | + | gotWaiters := sess.teardownWaiters | |
| 18 | + | changed := sess.teardownChangeLocked() | |
| 19 | + | sess.teardownMu.Unlock() | |
| 20 | + | if gotReservations == reservations && gotWaiters == waiters { | |
| 21 | + | return | |
| 22 | + | } | |
| 23 | + | select { | |
| 24 | + | case <-changed: | |
| 25 | + | case <-time.After(testWaitTimeout): | |
| 26 | + | t.Fatalf("move reservations/waiters = %d/%d, want %d/%d", gotReservations, gotWaiters, reservations, waiters) | |
| 27 | + | } | |
| 28 | + | } | |
| 29 | + | } | |
| 30 | + | ||
| 31 | + | func awaitDaemonMoveActive(t *testing.T, d *Daemon, want uint) { | |
| 32 | + | t.Helper() | |
| 33 | + | for { | |
| 34 | + | d.moveLifecycleMu.Lock() | |
| 35 | + | active := d.moveLifecycleActive | |
| 36 | + | changed := d.moveLifecycleChangeLocked() | |
| 37 | + | d.moveLifecycleMu.Unlock() | |
| 38 | + | if active == want { | |
| 39 | + | return | |
| 40 | + | } | |
| 41 | + | select { | |
| 42 | + | case <-changed: | |
| 43 | + | case <-time.After(testWaitTimeout): | |
| 44 | + | t.Fatalf("active move lifecycles = %d, want %d", active, want) | |
| 45 | + | } | |
| 46 | + | } | |
| 47 | + | } | |
| 48 | + | ||
| 49 | + | func awaitDaemonMoveClosing(t *testing.T, d *Daemon) { | |
| 50 | + | t.Helper() | |
| 51 | + | for { | |
| 52 | + | d.moveLifecycleMu.Lock() | |
| 53 | + | closing := d.moveLifecycleClosing | |
| 54 | + | changed := d.moveLifecycleChangeLocked() | |
| 55 | + | d.moveLifecycleMu.Unlock() | |
| 56 | + | if closing { | |
| 57 | + | return | |
| 58 | + | } | |
| 59 | + | select { | |
| 60 | + | case <-changed: | |
| 61 | + | case <-time.After(testWaitTimeout): | |
| 62 | + | t.Fatal("daemon move lifecycle gate did not begin closing") | |
| 63 | + | } | |
| 64 | + | } | |
| 65 | + | } | |
| 66 | + | ||
| 67 | + | func TestMoveLifecycleReservationMakesTeardownWait(t *testing.T) { | |
| 68 | + | d := newTestDaemon(t, nil, stubClock{}) | |
| 69 | + | sess := &session{id: "source"} | |
| 70 | + | ||
| 71 | + | reservation, err := d.reserveMoveLifecycles(sess, sess) | |
| 72 | + | require.NoError(t, err) | |
| 73 | + | awaitMoveTeardownState(t, sess, 1, 0) | |
| 74 | + | ||
| 75 | + | teardown := make(chan bool, 1) | |
| 76 | + | go func() { teardown <- sess.beginTeardown(nil) }() | |
| 77 | + | awaitMoveTeardownState(t, sess, 1, 1) | |
| 78 | + | select { | |
| 79 | + | case <-teardown: | |
| 80 | + | t.Fatal("teardown claimed ownership while the move lifecycle was reserved") | |
| 81 | + | default: | |
| 82 | + | } | |
| 83 | + | ||
| 84 | + | reservation.Release() | |
| 85 | + | require.True(t, awaitTestValue(t, teardown, "teardown did not wake after move release")) | |
| 86 | + | sess.finishTeardown() | |
| 87 | + | } | |
| 88 | + | ||
| 89 | + | func TestMoveLifecycleReservationsUseStableSessionOrder(t *testing.T) { | |
| 90 | + | d := newTestDaemon(t, nil, stubClock{}) | |
| 91 | + | first := &session{id: "a"} | |
| 92 | + | second := &session{id: "b"} | |
| 93 | + | ||
| 94 | + | // Both opposite-direction reservations must block on a before either takes | |
| 95 | + | // b. This proves order from immutable IDs rather than argument direction. | |
| 96 | + | first.teardownMu.Lock() | |
| 97 | + | results := make(chan *moveLifecycleReservation, 2) | |
| 98 | + | errs := make(chan error, 2) | |
| 99 | + | for _, pair := range [][2]*session{{first, second}, {second, first}} { | |
| 100 | + | go func() { | |
| 101 | + | reservation, err := d.reserveMoveLifecycles(pair[0], pair[1]) | |
| 102 | + | results <- reservation | |
| 103 | + | errs <- err | |
| 104 | + | }() | |
| 105 | + | } | |
| 106 | + | awaitDaemonMoveActive(t, d, 2) | |
| 107 | + | require.True(t, second.teardownMu.TryLock(), "opposite move locked b before the shared lowest-ID session") | |
| 108 | + | second.teardownMu.Unlock() | |
| 109 | + | first.teardownMu.Unlock() | |
| 110 | + | ||
| 111 | + | for range 2 { | |
| 112 | + | require.NoError(t, awaitTestValue(t, errs, "opposite move reservation did not finish")) | |
| 113 | + | reservation := awaitTestValue(t, results, "opposite move reservation token missing") | |
| 114 | + | require.NotNil(t, reservation) | |
| 115 | + | reservation.Release() | |
| 116 | + | } | |
| 117 | + | awaitDaemonMoveActive(t, d, 0) | |
| 118 | + | } | |
| 119 | + | ||
| 120 | + | func TestMoveLifecycleReservationRejectsTeardownWithoutLeakingCounts(t *testing.T) { | |
| 121 | + | for _, teardownSession := range []string{"source", "destination"} { | |
| 122 | + | t.Run(teardownSession, func(t *testing.T) { | |
| 123 | + | d := newTestDaemon(t, nil, stubClock{}) | |
| 124 | + | source := &session{id: "source"} | |
| 125 | + | destination := &session{id: "destination"} | |
| 126 | + | tearingDown := source | |
| 127 | + | if teardownSession == "destination" { | |
| 128 | + | tearingDown = destination | |
| 129 | + | } | |
| 130 | + | require.True(t, tearingDown.beginTeardown(nil)) | |
| 131 | + | ||
| 132 | + | reservation, err := d.reserveMoveLifecycles(source, destination) | |
| 133 | + | require.ErrorIs(t, err, errMoveLifecycleUnavailable) | |
| 134 | + | require.Nil(t, reservation) | |
| 135 | + | awaitMoveTeardownState(t, source, 0, 0) | |
| 136 | + | awaitMoveTeardownState(t, destination, 0, 0) | |
| 137 | + | awaitDaemonMoveActive(t, d, 0) | |
| 138 | + | tearingDown.finishTeardown() | |
| 139 | + | }) | |
| 140 | + | } | |
| 141 | + | } | |
| 142 | + | ||
| 143 | + | func TestMoveLifecycleReservationDeduplicatesAndReleasesExactlyOnce(t *testing.T) { | |
| 144 | + | d := newTestDaemon(t, nil, stubClock{}) | |
| 145 | + | sess := &session{id: "same"} | |
| 146 | + | reservation, err := d.reserveMoveLifecycles(sess, sess) | |
| 147 | + | require.NoError(t, err) | |
| 148 | + | awaitMoveTeardownState(t, sess, 1, 0) | |
| 149 | + | ||
| 150 | + | const releasers = 8 | |
| 151 | + | released := make(chan struct{}, releasers) | |
| 152 | + | for range releasers { | |
| 153 | + | go func() { | |
| 154 | + | reservation.Release() | |
| 155 | + | released <- struct{}{} | |
| 156 | + | }() | |
| 157 | + | } | |
| 158 | + | for range releasers { | |
| 159 | + | awaitTestCompletion(t, released, "concurrent reservation release did not return") | |
| 160 | + | } | |
| 161 | + | awaitMoveTeardownState(t, sess, 0, 0) | |
| 162 | + | awaitDaemonMoveActive(t, d, 0) | |
| 163 | + | } | |
| 164 | + | ||
| 165 | + | func TestMoveLifecycleReservationWaitCanBeCancelledWithoutChangingCounts(t *testing.T) { | |
| 166 | + | d := newTestDaemon(t, nil, stubClock{}) | |
| 167 | + | sess := &session{id: "source"} | |
| 168 | + | reservation, err := d.reserveMoveLifecycles(sess, sess) | |
| 169 | + | require.NoError(t, err) | |
| 170 | + | deadline := &snapshotShutdownDeadline{done: make(chan struct{})} | |
| 171 | + | ||
| 172 | + | teardown := make(chan bool, 1) | |
| 173 | + | go func() { teardown <- sess.beginTeardown(deadline) }() | |
| 174 | + | awaitMoveTeardownState(t, sess, 1, 1) | |
| 175 | + | close(deadline.done) | |
| 176 | + | require.False(t, awaitTestValue(t, teardown, "cancelled teardown waiter did not return")) | |
| 177 | + | awaitMoveTeardownState(t, sess, 1, 0) | |
| 178 | + | awaitDaemonMoveActive(t, d, 1) | |
| 179 | + | ||
| 180 | + | reservation.Release() | |
| 181 | + | awaitMoveTeardownState(t, sess, 0, 0) | |
| 182 | + | awaitDaemonMoveActive(t, d, 0) | |
| 183 | + | } | |
| 184 | + | ||
| 185 | + | func TestMoveLifecycleReservationWakesAllTeardownWaiters(t *testing.T) { | |
| 186 | + | d := newTestDaemon(t, nil, stubClock{}) | |
| 187 | + | sess := &session{id: "source"} | |
| 188 | + | reservation, err := d.reserveMoveLifecycles(sess, sess) | |
| 189 | + | require.NoError(t, err) | |
| 190 | + | ||
| 191 | + | claimed := make(chan struct{}, 2) | |
| 192 | + | finish := make(chan struct{}, 2) | |
| 193 | + | done := make(chan struct{}, 2) | |
| 194 | + | for range 2 { | |
| 195 | + | go func() { | |
| 196 | + | if sess.beginTeardown(nil) { | |
| 197 | + | claimed <- struct{}{} | |
| 198 | + | <-finish | |
| 199 | + | sess.finishTeardown() | |
| 200 | + | } | |
| 201 | + | done <- struct{}{} | |
| 202 | + | }() | |
| 203 | + | } | |
| 204 | + | awaitMoveTeardownState(t, sess, 1, 2) | |
| 205 | + | reservation.Release() | |
| 206 | + | for range 2 { | |
| 207 | + | awaitTestCompletion(t, claimed, "teardown waiter did not claim ownership") | |
| 208 | + | finish <- struct{}{} | |
| 209 | + | } | |
| 210 | + | for range 2 { | |
| 211 | + | awaitTestCompletion(t, done, "teardown waiter did not complete") | |
| 212 | + | } | |
| 213 | + | awaitMoveTeardownState(t, sess, 0, 0) | |
| 214 | + | } | |
| 215 | + | ||
| 216 | + | func TestDaemonShutdownDrainsMoveBeforePaneLifetimeCancellation(t *testing.T) { | |
| 217 | + | d := newTestDaemon(t, nil, stubClock{}) | |
| 218 | + | source := &session{id: "source"} | |
| 219 | + | destination := &session{id: "destination"} | |
| 220 | + | reservation, err := d.reserveMoveLifecycles(source, destination) | |
| 221 | + | require.NoError(t, err) | |
| 222 | + | paneLifetime := d.paneProcessCtx | |
| 223 | + | require.NotNil(t, paneLifetime) | |
| 224 | + | ||
| 225 | + | shutdown := make(chan bool, 1) | |
| 226 | + | go func() { shutdown <- d.shutdownAll(ports.ReasonServerShutdown) }() | |
| 227 | + | awaitDaemonMoveClosing(t, d) | |
| 228 | + | select { | |
| 229 | + | case <-paneLifetime.Done(): | |
| 230 | + | t.Fatal("shutdown cancelled transferable pane lifetime before the move released") | |
| 231 | + | default: | |
| 232 | + | } | |
| 233 | + | select { | |
| 234 | + | case <-shutdown: | |
| 235 | + | t.Fatal("shutdown completed while a move lifecycle remained reserved") | |
| 236 | + | default: | |
| 237 | + | } | |
| 238 | + | ||
| 239 | + | reservation.Release() | |
| 240 | + | awaitTestCompletion(t, paneLifetime.Done(), "shutdown did not cancel pane lifetime after move drain") | |
| 241 | + | require.False(t, awaitTestValue(t, shutdown, "shutdown did not continue after move drain")) | |
| 242 | + | ||
| 243 | + | reservation, err = d.reserveMoveLifecycles(source, destination) | |
| 244 | + | require.ErrorIs(t, err, errMoveLifecycleUnavailable) | |
| 245 | + | require.Nil(t, reservation) | |
| 246 | + | awaitDaemonMoveActive(t, d, 0) | |
| 247 | + | } | |
| @@ -0,0 +1,247 @@ | |||||
| 1 | + | package daemon | |||
| 2 | + | ||||
| 3 | + | import ( | |||
| 4 | + | "testing" | |||
| 5 | + | "time" | |||
| 6 | + | ||||
| 7 | + | "github.com/stretchr/testify/require" | |||
| 8 | + | ||||
| 9 | + | "github.com/bnema/vev/internal/ports" | |||
| 10 | + | ) | |||
| 11 | + | ||||
| 12 | + | func awaitMoveTeardownState(t *testing.T, sess *session, reservations, waiters uint) { | |||
| 13 | + | t.Helper() | |||
| 14 | + | for { | |||
| 15 | + | sess.teardownMu.Lock() | |||
| 16 | + | gotReservations := sess.moveReservations | |||
| 17 | + | gotWaiters := sess.teardownWaiters | |||
| 18 | + | changed := sess.teardownChangeLocked() | |||
| 19 | + | sess.teardownMu.Unlock() | |||
| 20 | + | if gotReservations == reservations && gotWaiters == waiters { | |||
| 21 | + | return | |||
| 22 | + | } | |||
| 23 | + | select { | |||
| 24 | + | case <-changed: | |||
| 25 | + | case <-time.After(testWaitTimeout): | |||
| 26 | + | t.Fatalf("move reservations/waiters = %d/%d, want %d/%d", gotReservations, gotWaiters, reservations, waiters) | |||
| 27 | + | } | |||
| 28 | + | } | |||
| 29 | + | } | |||
| 30 | + | ||||
| 31 | + | func awaitDaemonMoveActive(t *testing.T, d *Daemon, want uint) { | |||
| 32 | + | t.Helper() | |||
| 33 | + | for { | |||
| 34 | + | d.moveLifecycleMu.Lock() | |||
| 35 | + | active := d.moveLifecycleActive | |||
| 36 | + | changed := d.moveLifecycleChangeLocked() | |||
| 37 | + | d.moveLifecycleMu.Unlock() | |||
| 38 | + | if active == want { | |||
| 39 | + | return | |||
| 40 | + | } | |||
| 41 | + | select { | |||
| 42 | + | case <-changed: | |||
| 43 | + | case <-time.After(testWaitTimeout): | |||
| 44 | + | t.Fatalf("active move lifecycles = %d, want %d", active, want) | |||
| 45 | + | } | |||
| 46 | + | } | |||
| 47 | + | } | |||
| 48 | + | ||||
| 49 | + | func awaitDaemonMoveClosing(t *testing.T, d *Daemon) { | |||
| 50 | + | t.Helper() | |||
| 51 | + | for { | |||
| 52 | + | d.moveLifecycleMu.Lock() | |||
| 53 | + | closing := d.moveLifecycleClosing | |||
| 54 | + | changed := d.moveLifecycleChangeLocked() | |||
| 55 | + | d.moveLifecycleMu.Unlock() | |||
| 56 | + | if closing { | |||
| 57 | + | return | |||
| 58 | + | } | |||
| 59 | + | select { | |||
| 60 | + | case <-changed: | |||
| 61 | + | case <-time.After(testWaitTimeout): | |||
| 62 | + | t.Fatal("daemon move lifecycle gate did not begin closing") | |||
| 63 | + | } | |||
| 64 | + | } | |||
| 65 | + | } | |||
| 66 | + | ||||
| 67 | + | func TestMoveLifecycleReservationMakesTeardownWait(t *testing.T) { | |||
| 68 | + | d := newTestDaemon(t, nil, stubClock{}) | |||
| 69 | + | sess := &session{id: "source"} | |||
| 70 | + | ||||
| 71 | + | reservation, err := d.reserveMoveLifecycles(sess, sess) | |||
| 72 | + | require.NoError(t, err) | |||
| 73 | + | awaitMoveTeardownState(t, sess, 1, 0) | |||
| 74 | + | ||||
| 75 | + | teardown := make(chan bool, 1) | |||
| 76 | + | go func() { teardown <- sess.beginTeardown(nil) }() | |||
| 77 | + | awaitMoveTeardownState(t, sess, 1, 1) | |||
| 78 | + | select { | |||
| 79 | + | case <-teardown: | |||
| 80 | + | t.Fatal("teardown claimed ownership while the move lifecycle was reserved") | |||
| 81 | + | default: | |||
| 82 | + | } | |||
| 83 | + | ||||
| 84 | + | reservation.Release() | |||
| 85 | + | require.True(t, awaitTestValue(t, teardown, "teardown did not wake after move release")) | |||
| 86 | + | sess.finishTeardown() | |||
| 87 | + | } | |||
| 88 | + | ||||
| 89 | + | func TestMoveLifecycleReservationsUseStableSessionOrder(t *testing.T) { | |||
| 90 | + | d := newTestDaemon(t, nil, stubClock{}) | |||
| 91 | + | first := &session{id: "a"} | |||
| 92 | + | second := &session{id: "b"} | |||
| 93 | + | ||||
| 94 | + | // Both opposite-direction reservations must block on a before either takes | |||
| 95 | + | // b. This proves order from immutable IDs rather than argument direction. | |||
| 96 | + | first.teardownMu.Lock() | |||
| 97 | + | results := make(chan *moveLifecycleReservation, 2) | |||
| 98 | + | errs := make(chan error, 2) | |||
| 99 | + | for _, pair := range [][2]*session{{first, second}, {second, first}} { | |||
| 100 | + | go func() { | |||
| 101 | + | reservation, err := d.reserveMoveLifecycles(pair[0], pair[1]) | |||
| 102 | + | results <- reservation | |||
| 103 | + | errs <- err | |||
| 104 | + | }() | |||
| 105 | + | } | |||
| 106 | + | awaitDaemonMoveActive(t, d, 2) | |||
| 107 | + | require.True(t, second.teardownMu.TryLock(), "opposite move locked b before the shared lowest-ID session") | |||
| 108 | + | second.teardownMu.Unlock() | |||
| 109 | + | first.teardownMu.Unlock() | |||
| 110 | + | ||||
| 111 | + | for range 2 { | |||
| 112 | + | require.NoError(t, awaitTestValue(t, errs, "opposite move reservation did not finish")) | |||
| 113 | + | reservation := awaitTestValue(t, results, "opposite move reservation token missing") | |||
| 114 | + | require.NotNil(t, reservation) | |||
| 115 | + | reservation.Release() | |||
| 116 | + | } | |||
| 117 | + | awaitDaemonMoveActive(t, d, 0) | |||
| 118 | + | } | |||
| 119 | + | ||||
| 120 | + | func TestMoveLifecycleReservationRejectsTeardownWithoutLeakingCounts(t *testing.T) { | |||
| 121 | + | for _, teardownSession := range []string{"source", "destination"} { | |||
| 122 | + | t.Run(teardownSession, func(t *testing.T) { | |||
| 123 | + | d := newTestDaemon(t, nil, stubClock{}) | |||
| 124 | + | source := &session{id: "source"} | |||
| 125 | + | destination := &session{id: "destination"} | |||
| 126 | + | tearingDown := source | |||
| 127 | + | if teardownSession == "destination" { | |||
| 128 | + | tearingDown = destination | |||
| 129 | + | } | |||
| 130 | + | require.True(t, tearingDown.beginTeardown(nil)) | |||
| 131 | + | ||||
| 132 | + | reservation, err := d.reserveMoveLifecycles(source, destination) | |||
| 133 | + | require.ErrorIs(t, err, errMoveLifecycleUnavailable) | |||
| 134 | + | require.Nil(t, reservation) | |||
| 135 | + | awaitMoveTeardownState(t, source, 0, 0) | |||
| 136 | + | awaitMoveTeardownState(t, destination, 0, 0) | |||
| 137 | + | awaitDaemonMoveActive(t, d, 0) | |||
| 138 | + | tearingDown.finishTeardown() | |||
| 139 | + | }) | |||
| 140 | + | } | |||
| 141 | + | } | |||
| 142 | + | ||||
| 143 | + | func TestMoveLifecycleReservationDeduplicatesAndReleasesExactlyOnce(t *testing.T) { | |||
| 144 | + | d := newTestDaemon(t, nil, stubClock{}) | |||
| 145 | + | sess := &session{id: "same"} | |||
| 146 | + | reservation, err := d.reserveMoveLifecycles(sess, sess) | |||
| 147 | + | require.NoError(t, err) | |||
| 148 | + | awaitMoveTeardownState(t, sess, 1, 0) | |||
| 149 | + | ||||
| 150 | + | const releasers = 8 | |||
| 151 | + | released := make(chan struct{}, releasers) | |||
| 152 | + | for range releasers { | |||
| 153 | + | go func() { | |||
| 154 | + | reservation.Release() | |||
| 155 | + | released <- struct{}{} | |||
| 156 | + | }() | |||
| 157 | + | } | |||
| 158 | + | for range releasers { | |||
| 159 | + | awaitTestCompletion(t, released, "concurrent reservation release did not return") | |||
| 160 | + | } | |||
| 161 | + | awaitMoveTeardownState(t, sess, 0, 0) | |||
| 162 | + | awaitDaemonMoveActive(t, d, 0) | |||
| 163 | + | } | |||
| 164 | + | ||||
| 165 | + | func TestMoveLifecycleReservationWaitCanBeCancelledWithoutChangingCounts(t *testing.T) { | |||
| 166 | + | d := newTestDaemon(t, nil, stubClock{}) | |||
| 167 | + | sess := &session{id: "source"} | |||
| 168 | + | reservation, err := d.reserveMoveLifecycles(sess, sess) | |||
| 169 | + | require.NoError(t, err) | |||
| 170 | + | deadline := &snapshotShutdownDeadline{done: make(chan struct{})} | |||
| 171 | + | ||||
| 172 | + | teardown := make(chan bool, 1) | |||
| 173 | + | go func() { teardown <- sess.beginTeardown(deadline) }() | |||
| 174 | + | awaitMoveTeardownState(t, sess, 1, 1) | |||
| 175 | + | close(deadline.done) | |||
| 176 | + | require.False(t, awaitTestValue(t, teardown, "cancelled teardown waiter did not return")) | |||
| 177 | + | awaitMoveTeardownState(t, sess, 1, 0) | |||
| 178 | + | awaitDaemonMoveActive(t, d, 1) | |||
| 179 | + | ||||
| 180 | + | reservation.Release() | |||
| 181 | + | awaitMoveTeardownState(t, sess, 0, 0) | |||
| 182 | + | awaitDaemonMoveActive(t, d, 0) | |||
| 183 | + | } | |||
| 184 | + | ||||
| 185 | + | func TestMoveLifecycleReservationWakesAllTeardownWaiters(t *testing.T) { | |||
| 186 | + | d := newTestDaemon(t, nil, stubClock{}) | |||
| 187 | + | sess := &session{id: "source"} | |||
| 188 | + | reservation, err := d.reserveMoveLifecycles(sess, sess) | |||
| 189 | + | require.NoError(t, err) | |||
| 190 | + | ||||
| 191 | + | claimed := make(chan struct{}, 2) | |||
| 192 | + | finish := make(chan struct{}, 2) | |||
| 193 | + | done := make(chan struct{}, 2) | |||
| 194 | + | for range 2 { | |||
| 195 | + | go func() { | |||
| 196 | + | if sess.beginTeardown(nil) { | |||
| 197 | + | claimed <- struct{}{} | |||
| 198 | + | <-finish | |||
| 199 | + | sess.finishTeardown() | |||
| 200 | + | } | |||
| 201 | + | done <- struct{}{} | |||
| 202 | + | }() | |||
| 203 | + | } | |||
| 204 | + | awaitMoveTeardownState(t, sess, 1, 2) | |||
| 205 | + | reservation.Release() | |||
| 206 | + | for range 2 { | |||
| 207 | + | awaitTestCompletion(t, claimed, "teardown waiter did not claim ownership") | |||
| 208 | + | finish <- struct{}{} | |||
| 209 | + | } | |||
| 210 | + | for range 2 { | |||
| 211 | + | awaitTestCompletion(t, done, "teardown waiter did not complete") | |||
| 212 | + | } | |||
| 213 | + | awaitMoveTeardownState(t, sess, 0, 0) | |||
| 214 | + | } | |||
| 215 | + | ||||
| 216 | + | func TestDaemonShutdownDrainsMoveBeforePaneLifetimeCancellation(t *testing.T) { | |||
| 217 | + | d := newTestDaemon(t, nil, stubClock{}) | |||
| 218 | + | source := &session{id: "source"} | |||
| 219 | + | destination := &session{id: "destination"} | |||
| 220 | + | reservation, err := d.reserveMoveLifecycles(source, destination) | |||
| 221 | + | require.NoError(t, err) | |||
| 222 | + | paneLifetime := d.paneProcessCtx | |||
| 223 | + | require.NotNil(t, paneLifetime) | |||
| 224 | + | ||||
| 225 | + | shutdown := make(chan bool, 1) | |||
| 226 | + | go func() { shutdown <- d.shutdownAll(ports.ReasonServerShutdown) }() | |||
| 227 | + | awaitDaemonMoveClosing(t, d) | |||
| 228 | + | select { | |||
| 229 | + | case <-paneLifetime.Done(): | |||
| 230 | + | t.Fatal("shutdown cancelled transferable pane lifetime before the move released") | |||
| 231 | + | default: | |||
| 232 | + | } | |||
| 233 | + | select { | |||
| 234 | + | case <-shutdown: | |||
| 235 | + | t.Fatal("shutdown completed while a move lifecycle remained reserved") | |||
| 236 | + | default: | |||
| 237 | + | } | |||
| 238 | + | ||||
| 239 | + | reservation.Release() | |||
| 240 | + | awaitTestCompletion(t, paneLifetime.Done(), "shutdown did not cancel pane lifetime after move drain") | |||
| 241 | + | require.False(t, awaitTestValue(t, shutdown, "shutdown did not continue after move drain")) | |||
| 242 | + | ||||
| 243 | + | reservation, err = d.reserveMoveLifecycles(source, destination) | |||
| 244 | + | require.ErrorIs(t, err, errMoveLifecycleUnavailable) | |||
| 245 | + | require.Nil(t, reservation) | |||
| 246 | + | awaitDaemonMoveActive(t, d, 0) | |||
| 247 | + | } | |||
M internal/usecase/daemon/pane.go +132 -17
| @@ -3,6 +3,7 @@ package daemon | |||
| 3 | 3 | import ( | |
| 4 | 4 | "context" | |
| 5 | 5 | "sync" | |
| 6 | + | "sync/atomic" | |
| 6 | 7 | ||
| 7 | 8 | "github.com/bnema/vev/internal/domain" | |
| 8 | 9 | "github.com/bnema/vev/internal/ports" | |
| @@ -10,6 +11,97 @@ import ( | |||
| 10 | 11 | "github.com/bnema/vev/pkg/vt" | |
| 11 | 12 | ) | |
| 12 | 13 | ||
| 14 | + | // paneProcessLifetime owns the context passed to PTYFactory.Open. Its daemon | |
| 15 | + | // parent lets an installed pane move between sessions without changing process | |
| 16 | + | // lifetime. Temporary opening parents bound an unpublished Open; publish | |
| 17 | + | // atomically detaches those parents before transferring cancellation to pane. | |
| 18 | + | type paneProcessLifetime struct { | |
| 19 | + | ctx context.Context | |
| 20 | + | cancel context.CancelFunc | |
| 21 | + | ||
| 22 | + | mu sync.Mutex | |
| 23 | + | opening bool | |
| 24 | + | openingParents []context.Context | |
| 25 | + | stopOpening []func() bool | |
| 26 | + | } | |
| 27 | + | ||
| 28 | + | func (d *Daemon) newPaneProcessLifetime(openingParents ...context.Context) *paneProcessLifetime { | |
| 29 | + | parent := d.paneProcessCtx | |
| 30 | + | if parent == nil { | |
| 31 | + | parent = context.Background() | |
| 32 | + | } | |
| 33 | + | ctx, rawCancel := context.WithCancel(parent) | |
| 34 | + | var cancelOnce sync.Once | |
| 35 | + | lifetime := &paneProcessLifetime{ | |
| 36 | + | ctx: ctx, | |
| 37 | + | cancel: func() { cancelOnce.Do(rawCancel) }, | |
| 38 | + | opening: true, | |
| 39 | + | openingParents: append([]context.Context(nil), openingParents...), | |
| 40 | + | } | |
| 41 | + | for _, openingParent := range lifetime.openingParents { | |
| 42 | + | if openingParent == nil { | |
| 43 | + | continue | |
| 44 | + | } | |
| 45 | + | lifetime.stopOpening = append(lifetime.stopOpening, context.AfterFunc(openingParent, func() { | |
| 46 | + | lifetime.mu.Lock() | |
| 47 | + | if lifetime.opening { | |
| 48 | + | lifetime.cancel() | |
| 49 | + | } | |
| 50 | + | lifetime.mu.Unlock() | |
| 51 | + | })) | |
| 52 | + | } | |
| 53 | + | return lifetime | |
| 54 | + | } | |
| 55 | + | ||
| 56 | + | // publish transfers a successfully opened process to p. A cancellation that | |
| 57 | + | // wins before this call prevents publication; one that starts afterward no | |
| 58 | + | // longer belongs to the source session or tab. | |
| 59 | + | func (l *paneProcessLifetime) publish(p *pane) bool { | |
| 60 | + | if l == nil || p == nil { | |
| 61 | + | return false | |
| 62 | + | } | |
| 63 | + | l.mu.Lock() | |
| 64 | + | defer l.mu.Unlock() | |
| 65 | + | for _, parent := range l.openingParents { | |
| 66 | + | if parent != nil && parent.Err() != nil { | |
| 67 | + | l.opening = false | |
| 68 | + | l.stopOpeningParentsLocked() | |
| 69 | + | l.cancel() | |
| 70 | + | return false | |
| 71 | + | } | |
| 72 | + | } | |
| 73 | + | if l.ctx.Err() != nil { | |
| 74 | + | l.opening = false | |
| 75 | + | l.stopOpeningParentsLocked() | |
| 76 | + | l.cancel() | |
| 77 | + | return false | |
| 78 | + | } | |
| 79 | + | l.opening = false | |
| 80 | + | l.stopOpeningParentsLocked() | |
| 81 | + | p.ctx = l.ctx | |
| 82 | + | p.cancel = l.cancel | |
| 83 | + | return true | |
| 84 | + | } | |
| 85 | + | ||
| 86 | + | func (l *paneProcessLifetime) abort() { | |
| 87 | + | if l == nil { | |
| 88 | + | return | |
| 89 | + | } | |
| 90 | + | l.mu.Lock() | |
| 91 | + | l.opening = false | |
| 92 | + | l.stopOpeningParentsLocked() | |
| 93 | + | l.cancel() | |
| 94 | + | l.mu.Unlock() | |
| 95 | + | } | |
| 96 | + | ||
| 97 | + | func (l *paneProcessLifetime) stopOpeningParentsLocked() { | |
| 98 | + | for _, stop := range l.stopOpening { | |
| 99 | + | stop() | |
| 100 | + | } | |
| 101 | + | l.stopOpening = nil | |
| 102 | + | l.openingParents = nil | |
| 103 | + | } | |
| 104 | + | ||
| 13 | 105 | // pane owns one PTY and its terminal screen state. | |
| 14 | 106 | // Lock order when multiple locks are held is: | |
| 15 | 107 | // attachedClient.sendMu > Daemon.mu > session.mu > tab.mu > pane.mu. | |
| @@ -18,31 +110,54 @@ type pane struct { | |||
| 18 | 110 | id layout.PaneID | |
| 19 | 111 | stableID string | |
| 20 | 112 | pty ports.PTY | |
| 21 | - | mu sync.Mutex // guards screen, history, syncGen, rect, resizeApplying, resizeRetry, resizePending, PTY side effects, and title | |
| 22 | - | resizeMu sync.Mutex // serializes PTY resizes without holding mu | |
| 23 | - | screen *vt.Screen | |
| 24 | - | history *vt.History | |
| 25 | - | syncGen uint64 | |
| 26 | - | rect domain.Rect | |
| 113 | + | mu sync.Mutex // guards screen, history, syncGen, rect, resizeApplying, resizeRetry, resizePending, PTY side effects, title, and owner publication | |
| 114 | + | owner atomic.Pointer[paneOwner] | |
| 115 | + | // ownerGeneration is advanced only while mu is held. Readers consume the | |
| 116 | + | // generation through the immutable owner pointer above. | |
| 117 | + | ownerGeneration uint64 | |
| 118 | + | resizeMu sync.Mutex // serializes PTY resizes without holding mu | |
| 119 | + | screen *vt.Screen | |
| 120 | + | history *vt.History | |
| 121 | + | syncGen uint64 | |
| 122 | + | rect domain.Rect | |
| 27 | 123 | // resizeApplying gates VT parsing across PTY.Resize. The reader continues | |
| 28 | 124 | // draining into resizePending so output is replayed against the target (or | |
| 29 | 125 | // retained old) screen only after apply resolves. | |
| 30 | - | resizeApplying bool | |
| 31 | - | resizeRetry bool // PTY resize failed for the committed rectangle; retry on the next accepted plan. | |
| 32 | - | resizePending []byte | |
| 33 | - | ptyResponses []byte | |
| 34 | - | ptyClipboards []string | |
| 35 | - | ptyAttention bool | |
| 36 | - | popupGeometry floatingGeometry // last geometry committed after a successful floating resize | |
| 37 | - | title paneTitleState | |
| 38 | - | ctx context.Context | |
| 39 | - | cancel context.CancelFunc | |
| 40 | - | floatingCloseOnce sync.Once // used only by floating lifecycle teardown paths | |
| 126 | + | resizeApplying bool | |
| 127 | + | resizeRetry bool // PTY resize failed for the committed rectangle; retry on the next accepted plan. | |
| 128 | + | resizePending []byte | |
| 129 | + | ptyResponses []byte | |
| 130 | + | ptyClipboards []string | |
| 131 | + | ptyAttention bool | |
| 132 | + | popupGeometry floatingGeometry // last geometry committed after a successful floating resize | |
| 133 | + | title paneTitleState | |
| 134 | + | ctx context.Context | |
| 135 | + | cancel context.CancelFunc | |
| 136 | + | processCloseOnce sync.Once | |
| 41 | 137 | // onExit is set before the reader starts and never changed. Floating panes | |
| 42 | 138 | // use it to reap their independent slot without touching the layout tree. | |
| 43 | 139 | onExit func() | |
| 44 | 140 | } | |
| 45 | 141 | ||
| 142 | + | func closePaneProcess(p *pane) { | |
| 143 | + | if p == nil { | |
| 144 | + | return | |
| 145 | + | } | |
| 146 | + | p.processCloseOnce.Do(func() { | |
| 147 | + | // Revocation shares pane.mu with VT parsing and owner publication, so no | |
| 148 | + | // effect lease remains current once terminal teardown begins. | |
| 149 | + | p.mu.Lock() | |
| 150 | + | p.clearOwnerLocked() | |
| 151 | + | p.mu.Unlock() | |
| 152 | + | if p.cancel != nil { | |
| 153 | + | p.cancel() | |
| 154 | + | } | |
| 155 | + | if p.pty != nil { | |
| 156 | + | _ = p.pty.Close() | |
| 157 | + | } | |
| 158 | + | }) | |
| 159 | + | } | |
| 160 | + | ||
| 46 | 161 | func newPane(id layout.PaneID, pty ports.PTY, sz domain.Size) *pane { | |
| 47 | 162 | return newPaneWithStableID(id, fallbackStableID("p"), pty, sz) | |
| 48 | 163 | } | |
| @@ -3,6 +3,7 @@ package daemon | |||||
| 3 | import ( | 3 | import ( | ||
| 4 | "context" | 4 | "context" | ||
| 5 | "sync" | 5 | "sync" | ||
| 6 | + | "sync/atomic" | |||
| 6 | 7 | ||||
| 7 | "github.com/bnema/vev/internal/domain" | 8 | "github.com/bnema/vev/internal/domain" | ||
| 8 | "github.com/bnema/vev/internal/ports" | 9 | "github.com/bnema/vev/internal/ports" | ||
| @@ -10,6 +11,97 @@ import ( | |||||
| 10 | "github.com/bnema/vev/pkg/vt" | 11 | "github.com/bnema/vev/pkg/vt" | ||
| 11 | ) | 12 | ) | ||
| 12 | 13 | ||||
| 14 | + | // paneProcessLifetime owns the context passed to PTYFactory.Open. Its daemon | |||
| 15 | + | // parent lets an installed pane move between sessions without changing process | |||
| 16 | + | // lifetime. Temporary opening parents bound an unpublished Open; publish | |||
| 17 | + | // atomically detaches those parents before transferring cancellation to pane. | |||
| 18 | + | type paneProcessLifetime struct { | |||
| 19 | + | ctx context.Context | |||
| 20 | + | cancel context.CancelFunc | |||
| 21 | + | ||||
| 22 | + | mu sync.Mutex | |||
| 23 | + | opening bool | |||
| 24 | + | openingParents []context.Context | |||
| 25 | + | stopOpening []func() bool | |||
| 26 | + | } | |||
| 27 | + | ||||
| 28 | + | func (d *Daemon) newPaneProcessLifetime(openingParents ...context.Context) *paneProcessLifetime { | |||
| 29 | + | parent := d.paneProcessCtx | |||
| 30 | + | if parent == nil { | |||
| 31 | + | parent = context.Background() | |||
| 32 | + | } | |||
| 33 | + | ctx, rawCancel := context.WithCancel(parent) | |||
| 34 | + | var cancelOnce sync.Once | |||
| 35 | + | lifetime := &paneProcessLifetime{ | |||
| 36 | + | ctx: ctx, | |||
| 37 | + | cancel: func() { cancelOnce.Do(rawCancel) }, | |||
| 38 | + | opening: true, | |||
| 39 | + | openingParents: append([]context.Context(nil), openingParents...), | |||
| 40 | + | } | |||
| 41 | + | for _, openingParent := range lifetime.openingParents { | |||
| 42 | + | if openingParent == nil { | |||
| 43 | + | continue | |||
| 44 | + | } | |||
| 45 | + | lifetime.stopOpening = append(lifetime.stopOpening, context.AfterFunc(openingParent, func() { | |||
| 46 | + | lifetime.mu.Lock() | |||
| 47 | + | if lifetime.opening { | |||
| 48 | + | lifetime.cancel() | |||
| 49 | + | } | |||
| 50 | + | lifetime.mu.Unlock() | |||
| 51 | + | })) | |||
| 52 | + | } | |||
| 53 | + | return lifetime | |||
| 54 | + | } | |||
| 55 | + | ||||
| 56 | + | // publish transfers a successfully opened process to p. A cancellation that | |||
| 57 | + | // wins before this call prevents publication; one that starts afterward no | |||
| 58 | + | // longer belongs to the source session or tab. | |||
| 59 | + | func (l *paneProcessLifetime) publish(p *pane) bool { | |||
| 60 | + | if l == nil || p == nil { | |||
| 61 | + | return false | |||
| 62 | + | } | |||
| 63 | + | l.mu.Lock() | |||
| 64 | + | defer l.mu.Unlock() | |||
| 65 | + | for _, parent := range l.openingParents { | |||
| 66 | + | if parent != nil && parent.Err() != nil { | |||
| 67 | + | l.opening = false | |||
| 68 | + | l.stopOpeningParentsLocked() | |||
| 69 | + | l.cancel() | |||
| 70 | + | return false | |||
| 71 | + | } | |||
| 72 | + | } | |||
| 73 | + | if l.ctx.Err() != nil { | |||
| 74 | + | l.opening = false | |||
| 75 | + | l.stopOpeningParentsLocked() | |||
| 76 | + | l.cancel() | |||
| 77 | + | return false | |||
| 78 | + | } | |||
| 79 | + | l.opening = false | |||
| 80 | + | l.stopOpeningParentsLocked() | |||
| 81 | + | p.ctx = l.ctx | |||
| 82 | + | p.cancel = l.cancel | |||
| 83 | + | return true | |||
| 84 | + | } | |||
| 85 | + | ||||
| 86 | + | func (l *paneProcessLifetime) abort() { | |||
| 87 | + | if l == nil { | |||
| 88 | + | return | |||
| 89 | + | } | |||
| 90 | + | l.mu.Lock() | |||
| 91 | + | l.opening = false | |||
| 92 | + | l.stopOpeningParentsLocked() | |||
| 93 | + | l.cancel() | |||
| 94 | + | l.mu.Unlock() | |||
| 95 | + | } | |||
| 96 | + | ||||
| 97 | + | func (l *paneProcessLifetime) stopOpeningParentsLocked() { | |||
| 98 | + | for _, stop := range l.stopOpening { | |||
| 99 | + | stop() | |||
| 100 | + | } | |||
| 101 | + | l.stopOpening = nil | |||
| 102 | + | l.openingParents = nil | |||
| 103 | + | } | |||
| 104 | + | ||||
| 13 | // pane owns one PTY and its terminal screen state. | 105 | // pane owns one PTY and its terminal screen state. | ||
| 14 | // Lock order when multiple locks are held is: | 106 | // Lock order when multiple locks are held is: | ||
| 15 | // attachedClient.sendMu > Daemon.mu > session.mu > tab.mu > pane.mu. | 107 | // attachedClient.sendMu > Daemon.mu > session.mu > tab.mu > pane.mu. | ||
| @@ -18,31 +110,54 @@ type pane struct { | |||||
| 18 | id layout.PaneID | 110 | id layout.PaneID | ||
| 19 | stableID string | 111 | stableID string | ||
| 20 | pty ports.PTY | 112 | pty ports.PTY | ||
| 21 | - | mu sync.Mutex // guards screen, history, syncGen, rect, resizeApplying, resizeRetry, resizePending, PTY side effects, and title | 113 | + | mu sync.Mutex // guards screen, history, syncGen, rect, resizeApplying, resizeRetry, resizePending, PTY side effects, title, and owner publication |
| 22 | - | resizeMu sync.Mutex // serializes PTY resizes without holding mu | 114 | + | owner atomic.Pointer[paneOwner] |
| 23 | - | screen *vt.Screen | 115 | + | // ownerGeneration is advanced only while mu is held. Readers consume the |
| 24 | - | history *vt.History | 116 | + | // generation through the immutable owner pointer above. |
| 25 | - | syncGen uint64 | 117 | + | ownerGeneration uint64 |
| 26 | - | rect domain.Rect | 118 | + | resizeMu sync.Mutex // serializes PTY resizes without holding mu |
| 119 | + | screen *vt.Screen | |||
| 120 | + | history *vt.History | |||
| 121 | + | syncGen uint64 | |||
| 122 | + | rect domain.Rect | |||
| 27 | // resizeApplying gates VT parsing across PTY.Resize. The reader continues | 123 | // resizeApplying gates VT parsing across PTY.Resize. The reader continues | ||
| 28 | // draining into resizePending so output is replayed against the target (or | 124 | // draining into resizePending so output is replayed against the target (or | ||
| 29 | // retained old) screen only after apply resolves. | 125 | // retained old) screen only after apply resolves. | ||
| 30 | - | resizeApplying bool | 126 | + | resizeApplying bool |
| 31 | - | resizeRetry bool // PTY resize failed for the committed rectangle; retry on the next accepted plan. | 127 | + | resizeRetry bool // PTY resize failed for the committed rectangle; retry on the next accepted plan. |
| 32 | - | resizePending []byte | 128 | + | resizePending []byte |
| 33 | - | ptyResponses []byte | 129 | + | ptyResponses []byte |
| 34 | - | ptyClipboards []string | 130 | + | ptyClipboards []string |
| 35 | - | ptyAttention bool | 131 | + | ptyAttention bool |
| 36 | - | popupGeometry floatingGeometry // last geometry committed after a successful floating resize | 132 | + | popupGeometry floatingGeometry // last geometry committed after a successful floating resize |
| 37 | - | title paneTitleState | 133 | + | title paneTitleState |
| 38 | - | ctx context.Context | 134 | + | ctx context.Context |
| 39 | - | cancel context.CancelFunc | 135 | + | cancel context.CancelFunc |
| 40 | - | floatingCloseOnce sync.Once // used only by floating lifecycle teardown paths | 136 | + | processCloseOnce sync.Once |
| 41 | // onExit is set before the reader starts and never changed. Floating panes | 137 | // onExit is set before the reader starts and never changed. Floating panes | ||
| 42 | // use it to reap their independent slot without touching the layout tree. | 138 | // use it to reap their independent slot without touching the layout tree. | ||
| 43 | onExit func() | 139 | onExit func() | ||
| 44 | } | 140 | } | ||
| 45 | 141 | ||||
| 142 | + | func closePaneProcess(p *pane) { | |||
| 143 | + | if p == nil { | |||
| 144 | + | return | |||
| 145 | + | } | |||
| 146 | + | p.processCloseOnce.Do(func() { | |||
| 147 | + | // Revocation shares pane.mu with VT parsing and owner publication, so no | |||
| 148 | + | // effect lease remains current once terminal teardown begins. | |||
| 149 | + | p.mu.Lock() | |||
| 150 | + | p.clearOwnerLocked() | |||
| 151 | + | p.mu.Unlock() | |||
| 152 | + | if p.cancel != nil { | |||
| 153 | + | p.cancel() | |||
| 154 | + | } | |||
| 155 | + | if p.pty != nil { | |||
| 156 | + | _ = p.pty.Close() | |||
| 157 | + | } | |||
| 158 | + | }) | |||
| 159 | + | } | |||
| 160 | + | ||||
| 46 | func newPane(id layout.PaneID, pty ports.PTY, sz domain.Size) *pane { | 161 | func newPane(id layout.PaneID, pty ports.PTY, sz domain.Size) *pane { | ||
| 47 | return newPaneWithStableID(id, fallbackStableID("p"), pty, sz) | 162 | return newPaneWithStableID(id, fallbackStableID("p"), pty, sz) | ||
| 48 | } | 163 | } | ||
M internal/usecase/daemon/pane_actions.go +17 -12
| @@ -1,7 +1,6 @@ | |||
| 1 | 1 | package daemon | |
| 2 | 2 | ||
| 3 | 3 | import ( | |
| 4 | - | "context" | |
| 5 | 4 | "errors" | |
| 6 | 5 | "fmt" | |
| 7 | 6 | ||
| @@ -80,24 +79,35 @@ func (d *Daemon) spawnPaneOpAt( | |||
| 80 | 79 | return fmt.Errorf("daemon: generating pane identity: %w", err) | |
| 81 | 80 | } | |
| 82 | 81 | command, args := d.ptyCommand(env) | |
| 83 | - | pty, err := d.ptys.Open(sess.ctx, command, args, childEnvFrom(env, name, tabStableID, paneStableID, term), cwd, rectSize(newRect)) | |
| 82 | + | lifetime := d.newPaneProcessLifetime(tb.ctx) | |
| 83 | + | pty, err := d.ptys.Open(lifetime.ctx, command, args, childEnvFrom(env, name, tabStableID, paneStableID, term), cwd, rectSize(newRect)) | |
| 84 | 84 | if err != nil { | |
| 85 | + | lifetime.abort() | |
| 86 | + | if pty != nil { | |
| 87 | + | _ = pty.Close() | |
| 88 | + | } | |
| 85 | 89 | d.log.Warn("pty spawn failed", "err", err, "session", name, "pane", newID, "kind", "pane") | |
| 86 | 90 | return domain.UserErr(domain.NoticePaneSpawn, "couldn't open pane: shell failed to start", err) | |
| 87 | 91 | } | |
| 88 | 92 | ||
| 89 | - | pctx, cancel := context.WithCancel(tb.ctx) | |
| 90 | 93 | p := newPaneWithStableID(newID, paneStableID, pty, rectSize(newRect)) | |
| 91 | - | p.ctx, p.cancel = pctx, cancel | |
| 92 | 94 | p.rect = newRect | |
| 93 | 95 | ||
| 94 | 96 | tb.mu.Lock() | |
| 95 | - | if tb.layoutGeneration != generation || tb.panes[oldFocus] != target || tb.tree == nil || !layout.ContainsLeaf(tb.tree.Root, oldFocus) { | |
| 97 | + | if tb.layoutGeneration != generation || tb.panes[oldFocus] != target || tb.tree == nil || !layout.ContainsLeaf(tb.tree.Root, oldFocus) || tb.ctx != nil && tb.ctx.Err() != nil { | |
| 96 | 98 | tb.mu.Unlock() | |
| 97 | - | cancel() | |
| 99 | + | lifetime.abort() | |
| 98 | 100 | _ = pty.Close() | |
| 99 | 101 | return layout.ErrNotFound | |
| 100 | 102 | } | |
| 103 | + | if !lifetime.publish(p) { | |
| 104 | + | tb.mu.Unlock() | |
| 105 | + | _ = pty.Close() | |
| 106 | + | return layout.ErrNotFound | |
| 107 | + | } | |
| 108 | + | // The tab lock excludes membership observers while pane.mu publishes the | |
| 109 | + | // initial owner generation. | |
| 110 | + | publishPaneOwner(p, sess, tb, 0) | |
| 101 | 111 | tb.tree = candidate | |
| 102 | 112 | tb.panes[newID] = p | |
| 103 | 113 | tb.bumpLayoutGenerationLocked() | |
| @@ -239,12 +249,7 @@ func (d *Daemon) closePane(sess *session, tb *tab, id layout.PaneID, ac *attache | |||
| 239 | 249 | if rc := sess.renderCoordinator(); rc != nil { | |
| 240 | 250 | rc.noteSyncPaneRemoved(p) | |
| 241 | 251 | } | |
| 242 | - | if p.cancel != nil { | |
| 243 | - | p.cancel() | |
| 244 | - | } | |
| 245 | - | if p.pty != nil { | |
| 246 | - | _ = p.pty.Close() | |
| 247 | - | } | |
| 252 | + | closePaneProcess(p) | |
| 248 | 253 | d.log.Info("pane closed", "session", sess.name, "pane", id) | |
| 249 | 254 | if repaint { | |
| 250 | 255 | if ac != nil { | |
| @@ -1,7 +1,6 @@ | |||||
| 1 | package daemon | 1 | package daemon | ||
| 2 | 2 | ||||
| 3 | import ( | 3 | import ( | ||
| 4 | - | "context" | |||
| 5 | "errors" | 4 | "errors" | ||
| 6 | "fmt" | 5 | "fmt" | ||
| 7 | 6 | ||||
| @@ -80,24 +79,35 @@ func (d *Daemon) spawnPaneOpAt( | |||||
| 80 | return fmt.Errorf("daemon: generating pane identity: %w", err) | 79 | return fmt.Errorf("daemon: generating pane identity: %w", err) | ||
| 81 | } | 80 | } | ||
| 82 | command, args := d.ptyCommand(env) | 81 | command, args := d.ptyCommand(env) | ||
| 83 | - | pty, err := d.ptys.Open(sess.ctx, command, args, childEnvFrom(env, name, tabStableID, paneStableID, term), cwd, rectSize(newRect)) | 82 | + | lifetime := d.newPaneProcessLifetime(tb.ctx) |
| 83 | + | pty, err := d.ptys.Open(lifetime.ctx, command, args, childEnvFrom(env, name, tabStableID, paneStableID, term), cwd, rectSize(newRect)) | |||
| 84 | if err != nil { | 84 | if err != nil { | ||
| 85 | + | lifetime.abort() | |||
| 86 | + | if pty != nil { | |||
| 87 | + | _ = pty.Close() | |||
| 88 | + | } | |||
| 85 | d.log.Warn("pty spawn failed", "err", err, "session", name, "pane", newID, "kind", "pane") | 89 | d.log.Warn("pty spawn failed", "err", err, "session", name, "pane", newID, "kind", "pane") | ||
| 86 | return domain.UserErr(domain.NoticePaneSpawn, "couldn't open pane: shell failed to start", err) | 90 | return domain.UserErr(domain.NoticePaneSpawn, "couldn't open pane: shell failed to start", err) | ||
| 87 | } | 91 | } | ||
| 88 | 92 | ||||
| 89 | - | pctx, cancel := context.WithCancel(tb.ctx) | |||
| 90 | p := newPaneWithStableID(newID, paneStableID, pty, rectSize(newRect)) | 93 | p := newPaneWithStableID(newID, paneStableID, pty, rectSize(newRect)) | ||
| 91 | - | p.ctx, p.cancel = pctx, cancel | |||
| 92 | p.rect = newRect | 94 | p.rect = newRect | ||
| 93 | 95 | ||||
| 94 | tb.mu.Lock() | 96 | tb.mu.Lock() | ||
| 95 | - | if tb.layoutGeneration != generation || tb.panes[oldFocus] != target || tb.tree == nil || !layout.ContainsLeaf(tb.tree.Root, oldFocus) { | 97 | + | if tb.layoutGeneration != generation || tb.panes[oldFocus] != target || tb.tree == nil || !layout.ContainsLeaf(tb.tree.Root, oldFocus) || tb.ctx != nil && tb.ctx.Err() != nil { |
| 96 | tb.mu.Unlock() | 98 | tb.mu.Unlock() | ||
| 97 | - | cancel() | 99 | + | lifetime.abort() |
| 98 | _ = pty.Close() | 100 | _ = pty.Close() | ||
| 99 | return layout.ErrNotFound | 101 | return layout.ErrNotFound | ||
| 100 | } | 102 | } | ||
| 103 | + | if !lifetime.publish(p) { | |||
| 104 | + | tb.mu.Unlock() | |||
| 105 | + | _ = pty.Close() | |||
| 106 | + | return layout.ErrNotFound | |||
| 107 | + | } | |||
| 108 | + | // The tab lock excludes membership observers while pane.mu publishes the | |||
| 109 | + | // initial owner generation. | |||
| 110 | + | publishPaneOwner(p, sess, tb, 0) | |||
| 101 | tb.tree = candidate | 111 | tb.tree = candidate | ||
| 102 | tb.panes[newID] = p | 112 | tb.panes[newID] = p | ||
| 103 | tb.bumpLayoutGenerationLocked() | 113 | tb.bumpLayoutGenerationLocked() | ||
| @@ -239,12 +249,7 @@ func (d *Daemon) closePane(sess *session, tb *tab, id layout.PaneID, ac *attache | |||||
| 239 | if rc := sess.renderCoordinator(); rc != nil { | 249 | if rc := sess.renderCoordinator(); rc != nil { | ||
| 240 | rc.noteSyncPaneRemoved(p) | 250 | rc.noteSyncPaneRemoved(p) | ||
| 241 | } | 251 | } | ||
| 242 | - | if p.cancel != nil { | 252 | + | closePaneProcess(p) |
| 243 | - | p.cancel() | |||
| 244 | - | } | |||
| 245 | - | if p.pty != nil { | |||
| 246 | - | _ = p.pty.Close() | |||
| 247 | - | } | |||
| 248 | d.log.Info("pane closed", "session", sess.name, "pane", id) | 253 | d.log.Info("pane closed", "session", sess.name, "pane", id) | ||
| 249 | if repaint { | 254 | if repaint { | ||
| 250 | if ac != nil { | 255 | if ac != nil { | ||
A internal/usecase/daemon/pane_lifetime_test.go +381 -0
| @@ -0,0 +1,381 @@ | |||
| 1 | + | package daemon | |
| 2 | + | ||
| 3 | + | import ( | |
| 4 | + | "context" | |
| 5 | + | "errors" | |
| 6 | + | "io" | |
| 7 | + | "sync" | |
| 8 | + | "sync/atomic" | |
| 9 | + | "testing" | |
| 10 | + | "time" | |
| 11 | + | ||
| 12 | + | "github.com/stretchr/testify/require" | |
| 13 | + | ||
| 14 | + | "github.com/bnema/vev/internal/domain" | |
| 15 | + | "github.com/bnema/vev/internal/ports" | |
| 16 | + | "github.com/bnema/vev/internal/usecase/layout" | |
| 17 | + | ) | |
| 18 | + | ||
| 19 | + | type lifetimePTY struct { | |
| 20 | + | ctx context.Context | |
| 21 | + | closed chan struct{} | |
| 22 | + | closeOnce sync.Once | |
| 23 | + | closes atomic.Int32 | |
| 24 | + | } | |
| 25 | + | ||
| 26 | + | func newLifetimePTY() *lifetimePTY { | |
| 27 | + | return &lifetimePTY{closed: make(chan struct{})} | |
| 28 | + | } | |
| 29 | + | ||
| 30 | + | func (p *lifetimePTY) Read([]byte) (int, error) { | |
| 31 | + | select { | |
| 32 | + | case <-p.ctx.Done(): | |
| 33 | + | return 0, p.ctx.Err() | |
| 34 | + | case <-p.closed: | |
| 35 | + | return 0, io.EOF | |
| 36 | + | } | |
| 37 | + | } | |
| 38 | + | ||
| 39 | + | func (*lifetimePTY) Write(b []byte) (int, error) { return len(b), nil } | |
| 40 | + | func (*lifetimePTY) Resize(domain.Size) error { return nil } | |
| 41 | + | func (*lifetimePTY) Pid() int { return 1 } | |
| 42 | + | func (*lifetimePTY) ForegroundPgid() (int, error) { | |
| 43 | + | return 1, nil | |
| 44 | + | } | |
| 45 | + | func (p *lifetimePTY) Close() error { | |
| 46 | + | p.closes.Add(1) | |
| 47 | + | p.closeOnce.Do(func() { close(p.closed) }) | |
| 48 | + | return nil | |
| 49 | + | } | |
| 50 | + | ||
| 51 | + | type lifetimePTYFactory struct { | |
| 52 | + | mu sync.Mutex | |
| 53 | + | ptys []*lifetimePTY | |
| 54 | + | contexts []context.Context | |
| 55 | + | } | |
| 56 | + | ||
| 57 | + | func (f *lifetimePTYFactory) Open(ctx context.Context, _ string, _ []string, _ []string, _ string, _ domain.Size) (ports.PTY, error) { | |
| 58 | + | p := newLifetimePTY() | |
| 59 | + | p.ctx = ctx | |
| 60 | + | f.mu.Lock() | |
| 61 | + | f.ptys = append(f.ptys, p) | |
| 62 | + | f.contexts = append(f.contexts, ctx) | |
| 63 | + | f.mu.Unlock() | |
| 64 | + | return p, nil | |
| 65 | + | } | |
| 66 | + | ||
| 67 | + | func (f *lifetimePTYFactory) opened(index int) (*lifetimePTY, context.Context) { | |
| 68 | + | f.mu.Lock() | |
| 69 | + | defer f.mu.Unlock() | |
| 70 | + | return f.ptys[index], f.contexts[index] | |
| 71 | + | } | |
| 72 | + | ||
| 73 | + | func TestPaneOpenErrorsReleaseReturnedPTYWithoutPublication(t *testing.T) { | |
| 74 | + | type openCase struct { | |
| 75 | + | name string | |
| 76 | + | wantCode domain.NoticeCode | |
| 77 | + | run func(*testing.T, *Daemon) ([]*tab, func(), error) | |
| 78 | + | } | |
| 79 | + | tests := []openCase{ | |
| 80 | + | { | |
| 81 | + | name: "initial pane", | |
| 82 | + | wantCode: domain.NoticeSessionSpawn, | |
| 83 | + | run: func(_ *testing.T, d *Daemon) ([]*tab, func(), error) { | |
| 84 | + | sess, err := createSessionForTest(d, "work", true, "/tmp", domain.Size{Cols: 80, Rows: 24}, terminalEnv{}, nil) | |
| 85 | + | if sess != nil { | |
| 86 | + | return sess.tabs, func() { closeTabs(sess.tabs) }, err | |
| 87 | + | } | |
| 88 | + | return nil, d.serveCancel, err | |
| 89 | + | }, | |
| 90 | + | }, | |
| 91 | + | { | |
| 92 | + | name: "created tab", | |
| 93 | + | wantCode: domain.NoticeTabSpawn, | |
| 94 | + | run: func(t *testing.T, d *Daemon) ([]*tab, func(), error) { | |
| 95 | + | sessCtx, cancelSession := context.WithCancel(t.Context()) | |
| 96 | + | base := newTab(newQuietPTY(), domain.Size{Cols: 80, Rows: 22}) | |
| 97 | + | base.ctx, base.cancel = context.WithCancel(sessCtx) | |
| 98 | + | sess := &session{id: "work", name: "work", ctx: sessCtx, cancel: cancelSession, tabs: []*tab{base}} | |
| 99 | + | d.mu.Lock() | |
| 100 | + | d.sessions[sess.id] = sess | |
| 101 | + | d.mu.Unlock() | |
| 102 | + | err := d.createTab(sess, domain.Size{Cols: 80, Rows: 24}) | |
| 103 | + | return sess.tabs, func() { | |
| 104 | + | cancelSession() | |
| 105 | + | closeTabs(sess.tabs) | |
| 106 | + | }, err | |
| 107 | + | }, | |
| 108 | + | }, | |
| 109 | + | { | |
| 110 | + | name: "split pane", | |
| 111 | + | wantCode: domain.NoticePaneSpawn, | |
| 112 | + | run: func(t *testing.T, d *Daemon) ([]*tab, func(), error) { | |
| 113 | + | sessCtx, cancelSession := context.WithCancel(t.Context()) | |
| 114 | + | tb := newTab(newQuietPTY(), domain.Size{Cols: 41, Rows: 10}) | |
| 115 | + | tb.ctx, tb.cancel = context.WithCancel(sessCtx) | |
| 116 | + | sess := &session{id: "work", name: "work", cwd: "/tmp", ctx: sessCtx, cancel: cancelSession, tabs: []*tab{tb}} | |
| 117 | + | err := d.splitPaneAt(sess, tb, tb.focusedPane(), layout.Right) | |
| 118 | + | return []*tab{tb}, func() { | |
| 119 | + | cancelSession() | |
| 120 | + | tb.closeAllPanes() | |
| 121 | + | }, err | |
| 122 | + | }, | |
| 123 | + | }, | |
| 124 | + | { | |
| 125 | + | name: "restored pane", | |
| 126 | + | run: func(t *testing.T, d *Daemon) ([]*tab, func(), error) { | |
| 127 | + | restoreCtx, cancelRestore := context.WithCancel(t.Context()) | |
| 128 | + | sessCtx, cancelSession := context.WithCancel(t.Context()) | |
| 129 | + | tabs, err := d.restoreSnapshotTabs(restoreCtx, sessCtx, restoreAcceptanceSession(t, "work")) | |
| 130 | + | return tabs, func() { | |
| 131 | + | cancelRestore() | |
| 132 | + | cancelSession() | |
| 133 | + | closeRestoredTabs(tabs) | |
| 134 | + | }, err | |
| 135 | + | }, | |
| 136 | + | }, | |
| 137 | + | } | |
| 138 | + | ||
| 139 | + | for _, tt := range tests { | |
| 140 | + | t.Run(tt.name, func(t *testing.T) { | |
| 141 | + | cause := errors.New("open failed") | |
| 142 | + | pty := newFailedOpenPTY() | |
| 143 | + | cancelCallbacks := atomic.Int32{} | |
| 144 | + | cancelled := make(chan struct{}) | |
| 145 | + | openFactory := paneErrorFactory{ | |
| 146 | + | pty: pty, | |
| 147 | + | err: cause, | |
| 148 | + | onOpen: func(ctx context.Context) { | |
| 149 | + | pty.ctx = ctx | |
| 150 | + | context.AfterFunc(ctx, func() { | |
| 151 | + | cancelCallbacks.Add(1) | |
| 152 | + | close(cancelled) | |
| 153 | + | }) | |
| 154 | + | }, | |
| 155 | + | } | |
| 156 | + | d := newTestDaemon(t, &openFactory, stubClock{}) | |
| 157 | + | ||
| 158 | + | tabs, cleanup, err := tt.run(t, d) | |
| 159 | + | require.ErrorIs(t, err, cause) | |
| 160 | + | if tt.wantCode != 0 { | |
| 161 | + | var userErr *domain.UserError | |
| 162 | + | require.ErrorAs(t, err, &userErr) | |
| 163 | + | require.Equal(t, tt.wantCode, userErr.Code) | |
| 164 | + | } | |
| 165 | + | select { | |
| 166 | + | case <-cancelled: | |
| 167 | + | case <-time.After(time.Second): | |
| 168 | + | t.Fatal("failed pane Open process context was not cancelled") | |
| 169 | + | } | |
| 170 | + | ||
| 171 | + | for _, tb := range tabs { | |
| 172 | + | if tb == nil { | |
| 173 | + | continue | |
| 174 | + | } | |
| 175 | + | tb.mu.Lock() | |
| 176 | + | floating := tb.floating.pane | |
| 177 | + | publishedFailedPTY := false | |
| 178 | + | for _, published := range tb.panes { | |
| 179 | + | publishedFailedPTY = publishedFailedPTY || published.pty == pty | |
| 180 | + | } | |
| 181 | + | tb.mu.Unlock() | |
| 182 | + | require.NotSame(t, pty, floating) | |
| 183 | + | require.False(t, publishedFailedPTY, "failed Open must not publish a pane or owner") | |
| 184 | + | } | |
| 185 | + | require.Zero(t, pty.readerStarts.Load(), "failed Open must not start a reader") | |
| 186 | + | require.Equal(t, int32(1), pty.closes.Load(), "failed Open PTY must close exactly once") | |
| 187 | + | require.Equal(t, int32(1), cancelCallbacks.Load(), "failed Open context must cancel exactly once") | |
| 188 | + | ||
| 189 | + | cleanup() | |
| 190 | + | require.Equal(t, int32(1), pty.closes.Load(), "later teardown must not close an unpublished PTY again") | |
| 191 | + | require.Equal(t, int32(1), cancelCallbacks.Load(), "later teardown must not recancel an unpublished process") | |
| 192 | + | }) | |
| 193 | + | } | |
| 194 | + | } | |
| 195 | + | ||
| 196 | + | type paneErrorFactory struct { | |
| 197 | + | pty ports.PTY | |
| 198 | + | err error | |
| 199 | + | onOpen func(context.Context) | |
| 200 | + | } | |
| 201 | + | ||
| 202 | + | func (f *paneErrorFactory) Open(ctx context.Context, _ string, _ []string, _ []string, _ string, _ domain.Size) (ports.PTY, error) { | |
| 203 | + | if f.onOpen != nil { | |
| 204 | + | f.onOpen(ctx) | |
| 205 | + | } | |
| 206 | + | return f.pty, f.err | |
| 207 | + | } | |
| 208 | + | ||
| 209 | + | func TestInitialPaneProcessLifetimeIsDaemonRooted(t *testing.T) { | |
| 210 | + | factory := &lifetimePTYFactory{} | |
| 211 | + | d := newTestDaemon(t, factory, stubClock{}) | |
| 212 | + | sess, err := createSessionForTest(d, "work", true, "/tmp", domain.Size{Cols: 80, Rows: 24}, terminalEnv{}, nil) | |
| 213 | + | require.NoError(t, err) | |
| 214 | + | ||
| 215 | + | p := sess.activeTab().focusedPane() | |
| 216 | + | pty, openCtx := factory.opened(0) | |
| 217 | + | require.NotNil(t, p.ctx) | |
| 218 | + | require.Equal(t, openCtx, p.ctx) | |
| 219 | + | require.NotNil(t, p.cancel) | |
| 220 | + | ||
| 221 | + | sess.cancel() | |
| 222 | + | select { | |
| 223 | + | case <-openCtx.Done(): | |
| 224 | + | t.Fatal("session cancellation killed a published pane process") | |
| 225 | + | default: | |
| 226 | + | } | |
| 227 | + | ||
| 228 | + | require.NoError(t, d.killSession(sess, ports.ReasonSessionKilled, false)) | |
| 229 | + | select { | |
| 230 | + | case <-openCtx.Done(): | |
| 231 | + | case <-time.After(time.Second): | |
| 232 | + | t.Fatal("session teardown did not cancel the pane process") | |
| 233 | + | } | |
| 234 | + | d.sessWg.Wait() | |
| 235 | + | require.Equal(t, int32(1), pty.closes.Load()) | |
| 236 | + | } | |
| 237 | + | ||
| 238 | + | func TestCreatedTabPaneProcessLifetimeIsDaemonRooted(t *testing.T) { | |
| 239 | + | factory := &lifetimePTYFactory{} | |
| 240 | + | d := newTestDaemon(t, factory, stubClock{}) | |
| 241 | + | sessCtx, cancelSession := context.WithCancel(t.Context()) | |
| 242 | + | sess := &session{ | |
| 243 | + | id: "work", | |
| 244 | + | name: "work", | |
| 245 | + | ctx: sessCtx, | |
| 246 | + | cancel: cancelSession, | |
| 247 | + | tabs: []*tab{newTab(newQuietPTY(), domain.Size{Cols: 80, Rows: 22})}, | |
| 248 | + | } | |
| 249 | + | d.mu.Lock() | |
| 250 | + | d.sessions[sess.id] = sess | |
| 251 | + | d.mu.Unlock() | |
| 252 | + | ||
| 253 | + | require.NoError(t, d.createTab(sess, domain.Size{Cols: 80, Rows: 24})) | |
| 254 | + | created := sess.activeTab() | |
| 255 | + | p := created.focusedPane() | |
| 256 | + | pty, openCtx := factory.opened(0) | |
| 257 | + | require.Equal(t, openCtx, p.ctx) | |
| 258 | + | require.NotNil(t, p.cancel) | |
| 259 | + | ||
| 260 | + | cancelSession() | |
| 261 | + | select { | |
| 262 | + | case <-openCtx.Done(): | |
| 263 | + | t.Fatal("session cancellation killed a published tab pane process") | |
| 264 | + | default: | |
| 265 | + | } | |
| 266 | + | ||
| 267 | + | created.closeAllPanes() | |
| 268 | + | select { | |
| 269 | + | case <-openCtx.Done(): | |
| 270 | + | case <-time.After(time.Second): | |
| 271 | + | t.Fatal("tab teardown did not cancel the pane process") | |
| 272 | + | } | |
| 273 | + | d.sessWg.Wait() | |
| 274 | + | require.Equal(t, int32(1), pty.closes.Load()) | |
| 275 | + | } | |
| 276 | + | ||
| 277 | + | func TestSplitPaneProcessLifetimeIsDaemonRooted(t *testing.T) { | |
| 278 | + | factory := &lifetimePTYFactory{} | |
| 279 | + | d := newTestDaemon(t, factory, stubClock{}) | |
| 280 | + | sessCtx, cancelSession := context.WithCancel(t.Context()) | |
| 281 | + | tb := newTab(newQuietPTY(), domain.Size{Cols: 80, Rows: 22}) | |
| 282 | + | tb.ctx, tb.cancel = context.WithCancel(sessCtx) | |
| 283 | + | sess := &session{id: "work", name: "work", ctx: sessCtx, cancel: cancelSession, tabs: []*tab{tb}} | |
| 284 | + | target := tb.focusedPane() | |
| 285 | + | ||
| 286 | + | require.NoError(t, d.splitPaneAt(sess, tb, target, layout.Right)) | |
| 287 | + | tb.mu.Lock() | |
| 288 | + | p := tb.panes["pane-2"] | |
| 289 | + | tb.mu.Unlock() | |
| 290 | + | require.NotNil(t, p) | |
| 291 | + | pty, openCtx := factory.opened(0) | |
| 292 | + | require.Equal(t, openCtx, p.ctx) | |
| 293 | + | require.NotNil(t, p.cancel) | |
| 294 | + | ||
| 295 | + | cancelSession() | |
| 296 | + | select { | |
| 297 | + | case <-openCtx.Done(): | |
| 298 | + | t.Fatal("session cancellation killed a published split pane process") | |
| 299 | + | default: | |
| 300 | + | } | |
| 301 | + | ||
| 302 | + | tb.closeAllPanes() | |
| 303 | + | select { | |
| 304 | + | case <-openCtx.Done(): | |
| 305 | + | case <-time.After(time.Second): | |
| 306 | + | t.Fatal("tab teardown did not cancel the split pane process") | |
| 307 | + | } | |
| 308 | + | d.sessWg.Wait() | |
| 309 | + | require.Equal(t, int32(1), pty.closes.Load()) | |
| 310 | + | } | |
| 311 | + | ||
| 312 | + | func TestFloatingPaneProcessLifetimeIsDaemonRooted(t *testing.T) { | |
| 313 | + | factory := &lifetimePTYFactory{} | |
| 314 | + | d := newTestDaemon(t, factory, stubClock{}) | |
| 315 | + | sessCtx, cancelSession := context.WithCancel(t.Context()) | |
| 316 | + | tb := newFloatingTestTab(t) | |
| 317 | + | tb.ctx, tb.cancel = context.WithCancel(sessCtx) | |
| 318 | + | sess := &session{id: "work", name: "work", ctx: sessCtx, cancel: cancelSession, tabs: []*tab{tb}} | |
| 319 | + | tb.mu.Lock() | |
| 320 | + | generation := tb.beginFloatingWarmLocked(true) | |
| 321 | + | tb.mu.Unlock() | |
| 322 | + | ||
| 323 | + | d.openAndInstallFloating(sess, tb, floatingLaunchSpec{ | |
| 324 | + | sessionName: "work", | |
| 325 | + | size: domain.Size{Cols: 20, Rows: 8}, | |
| 326 | + | geometry: floatingGeometry{Inner: domain.Rect{Width: 20, Height: 8}}, | |
| 327 | + | paneStableID: "p_floating", | |
| 328 | + | parentCtx: tb.ctx, | |
| 329 | + | }, generation) | |
| 330 | + | tb.mu.Lock() | |
| 331 | + | p := tb.floating.pane | |
| 332 | + | tb.mu.Unlock() | |
| 333 | + | require.NotNil(t, p) | |
| 334 | + | pty, openCtx := factory.opened(0) | |
| 335 | + | require.Equal(t, openCtx, p.ctx) | |
| 336 | + | require.NotNil(t, p.cancel) | |
| 337 | + | ||
| 338 | + | cancelSession() | |
| 339 | + | select { | |
| 340 | + | case <-openCtx.Done(): | |
| 341 | + | t.Fatal("session cancellation killed a published floating pane process") | |
| 342 | + | default: | |
| 343 | + | } | |
| 344 | + | ||
| 345 | + | d.teardownFloating(tb, nil) | |
| 346 | + | select { | |
| 347 | + | case <-openCtx.Done(): | |
| 348 | + | case <-time.After(time.Second): | |
| 349 | + | t.Fatal("floating teardown did not cancel the pane process") | |
| 350 | + | } | |
| 351 | + | d.sessWg.Wait() | |
| 352 | + | require.Equal(t, int32(1), pty.closes.Load()) | |
| 353 | + | } | |
| 354 | + | ||
| 355 | + | func TestRestoredPaneProcessLifetimeIsDaemonRooted(t *testing.T) { | |
| 356 | + | factory := &lifetimePTYFactory{} | |
| 357 | + | d := newTestDaemon(t, factory, stubClock{}) | |
| 358 | + | sessCtx, cancelSession := context.WithCancel(t.Context()) | |
| 359 | + | tabs, err := d.restoreSnapshotTabs(t.Context(), sessCtx, restoreAcceptanceSession(t, "work")) | |
| 360 | + | require.NoError(t, err) | |
| 361 | + | require.Len(t, tabs, 1) | |
| 362 | + | p := tabs[0].focusedPane() | |
| 363 | + | pty, openCtx := factory.opened(0) | |
| 364 | + | require.Equal(t, openCtx, p.ctx) | |
| 365 | + | require.NotNil(t, p.cancel) | |
| 366 | + | ||
| 367 | + | cancelSession() | |
| 368 | + | select { | |
| 369 | + | case <-openCtx.Done(): | |
| 370 | + | t.Fatal("session cancellation killed a restored pane process") | |
| 371 | + | default: | |
| 372 | + | } | |
| 373 | + | ||
| 374 | + | closeRestoredTabs(tabs) | |
| 375 | + | select { | |
| 376 | + | case <-openCtx.Done(): | |
| 377 | + | case <-time.After(time.Second): | |
| 378 | + | t.Fatal("restore teardown did not cancel the pane process") | |
| 379 | + | } | |
| 380 | + | require.Equal(t, int32(1), pty.closes.Load()) | |
| 381 | + | } | |
| @@ -0,0 +1,381 @@ | |||||
| 1 | + | package daemon | |||
| 2 | + | ||||
| 3 | + | import ( | |||
| 4 | + | "context" | |||
| 5 | + | "errors" | |||
| 6 | + | "io" | |||
| 7 | + | "sync" | |||
| 8 | + | "sync/atomic" | |||
| 9 | + | "testing" | |||
| 10 | + | "time" | |||
| 11 | + | ||||
| 12 | + | "github.com/stretchr/testify/require" | |||
| 13 | + | ||||
| 14 | + | "github.com/bnema/vev/internal/domain" | |||
| 15 | + | "github.com/bnema/vev/internal/ports" | |||
| 16 | + | "github.com/bnema/vev/internal/usecase/layout" | |||
| 17 | + | ) | |||
| 18 | + | ||||
| 19 | + | type lifetimePTY struct { | |||
| 20 | + | ctx context.Context | |||
| 21 | + | closed chan struct{} | |||
| 22 | + | closeOnce sync.Once | |||
| 23 | + | closes atomic.Int32 | |||
| 24 | + | } | |||
| 25 | + | ||||
| 26 | + | func newLifetimePTY() *lifetimePTY { | |||
| 27 | + | return &lifetimePTY{closed: make(chan struct{})} | |||
| 28 | + | } | |||
| 29 | + | ||||
| 30 | + | func (p *lifetimePTY) Read([]byte) (int, error) { | |||
| 31 | + | select { | |||
| 32 | + | case <-p.ctx.Done(): | |||
| 33 | + | return 0, p.ctx.Err() | |||
| 34 | + | case <-p.closed: | |||
| 35 | + | return 0, io.EOF | |||
| 36 | + | } | |||
| 37 | + | } | |||
| 38 | + | ||||
| 39 | + | func (*lifetimePTY) Write(b []byte) (int, error) { return len(b), nil } | |||
| 40 | + | func (*lifetimePTY) Resize(domain.Size) error { return nil } | |||
| 41 | + | func (*lifetimePTY) Pid() int { return 1 } | |||
| 42 | + | func (*lifetimePTY) ForegroundPgid() (int, error) { | |||
| 43 | + | return 1, nil | |||
| 44 | + | } | |||
| 45 | + | func (p *lifetimePTY) Close() error { | |||
| 46 | + | p.closes.Add(1) | |||
| 47 | + | p.closeOnce.Do(func() { close(p.closed) }) | |||
| 48 | + | return nil | |||
| 49 | + | } | |||
| 50 | + | ||||
| 51 | + | type lifetimePTYFactory struct { | |||
| 52 | + | mu sync.Mutex | |||
| 53 | + | ptys []*lifetimePTY | |||
| 54 | + | contexts []context.Context | |||
| 55 | + | } | |||
| 56 | + | ||||
| 57 | + | func (f *lifetimePTYFactory) Open(ctx context.Context, _ string, _ []string, _ []string, _ string, _ domain.Size) (ports.PTY, error) { | |||
| 58 | + | p := newLifetimePTY() | |||
| 59 | + | p.ctx = ctx | |||
| 60 | + | f.mu.Lock() | |||
| 61 | + | f.ptys = append(f.ptys, p) | |||
| 62 | + | f.contexts = append(f.contexts, ctx) | |||
| 63 | + | f.mu.Unlock() | |||
| 64 | + | return p, nil | |||
| 65 | + | } | |||
| 66 | + | ||||
| 67 | + | func (f *lifetimePTYFactory) opened(index int) (*lifetimePTY, context.Context) { | |||
| 68 | + | f.mu.Lock() | |||
| 69 | + | defer f.mu.Unlock() | |||
| 70 | + | return f.ptys[index], f.contexts[index] | |||
| 71 | + | } | |||
| 72 | + | ||||
| 73 | + | func TestPaneOpenErrorsReleaseReturnedPTYWithoutPublication(t *testing.T) { | |||
| 74 | + | type openCase struct { | |||
| 75 | + | name string | |||
| 76 | + | wantCode domain.NoticeCode | |||
| 77 | + | run func(*testing.T, *Daemon) ([]*tab, func(), error) | |||
| 78 | + | } | |||
| 79 | + | tests := []openCase{ | |||
| 80 | + | { | |||
| 81 | + | name: "initial pane", | |||
| 82 | + | wantCode: domain.NoticeSessionSpawn, | |||
| 83 | + | run: func(_ *testing.T, d *Daemon) ([]*tab, func(), error) { | |||
| 84 | + | sess, err := createSessionForTest(d, "work", true, "/tmp", domain.Size{Cols: 80, Rows: 24}, terminalEnv{}, nil) | |||
| 85 | + | if sess != nil { | |||
| 86 | + | return sess.tabs, func() { closeTabs(sess.tabs) }, err | |||
| 87 | + | } | |||
| 88 | + | return nil, d.serveCancel, err | |||
| 89 | + | }, | |||
| 90 | + | }, | |||
| 91 | + | { | |||
| 92 | + | name: "created tab", | |||
| 93 | + | wantCode: domain.NoticeTabSpawn, | |||
| 94 | + | run: func(t *testing.T, d *Daemon) ([]*tab, func(), error) { | |||
| 95 | + | sessCtx, cancelSession := context.WithCancel(t.Context()) | |||
| 96 | + | base := newTab(newQuietPTY(), domain.Size{Cols: 80, Rows: 22}) | |||
| 97 | + | base.ctx, base.cancel = context.WithCancel(sessCtx) | |||
| 98 | + | sess := &session{id: "work", name: "work", ctx: sessCtx, cancel: cancelSession, tabs: []*tab{base}} | |||
| 99 | + | d.mu.Lock() | |||
| 100 | + | d.sessions[sess.id] = sess | |||
| 101 | + | d.mu.Unlock() | |||
| 102 | + | err := d.createTab(sess, domain.Size{Cols: 80, Rows: 24}) | |||
| 103 | + | return sess.tabs, func() { | |||
| 104 | + | cancelSession() | |||
| 105 | + | closeTabs(sess.tabs) | |||
| 106 | + | }, err | |||
| 107 | + | }, | |||
| 108 | + | }, | |||
| 109 | + | { | |||
| 110 | + | name: "split pane", | |||
| 111 | + | wantCode: domain.NoticePaneSpawn, | |||
| 112 | + | run: func(t *testing.T, d *Daemon) ([]*tab, func(), error) { | |||
| 113 | + | sessCtx, cancelSession := context.WithCancel(t.Context()) | |||
| 114 | + | tb := newTab(newQuietPTY(), domain.Size{Cols: 41, Rows: 10}) | |||
| 115 | + | tb.ctx, tb.cancel = context.WithCancel(sessCtx) | |||
| 116 | + | sess := &session{id: "work", name: "work", cwd: "/tmp", ctx: sessCtx, cancel: cancelSession, tabs: []*tab{tb}} | |||
| 117 | + | err := d.splitPaneAt(sess, tb, tb.focusedPane(), layout.Right) | |||
| 118 | + | return []*tab{tb}, func() { | |||
| 119 | + | cancelSession() | |||
| 120 | + | tb.closeAllPanes() | |||
| 121 | + | }, err | |||
| 122 | + | }, | |||
| 123 | + | }, | |||
| 124 | + | { | |||
| 125 | + | name: "restored pane", | |||
| 126 | + | run: func(t *testing.T, d *Daemon) ([]*tab, func(), error) { | |||
| 127 | + | restoreCtx, cancelRestore := context.WithCancel(t.Context()) | |||
| 128 | + | sessCtx, cancelSession := context.WithCancel(t.Context()) | |||
| 129 | + | tabs, err := d.restoreSnapshotTabs(restoreCtx, sessCtx, restoreAcceptanceSession(t, "work")) | |||
| 130 | + | return tabs, func() { | |||
| 131 | + | cancelRestore() | |||
| 132 | + | cancelSession() | |||
| 133 | + | closeRestoredTabs(tabs) | |||
| 134 | + | }, err | |||
| 135 | + | }, | |||
| 136 | + | }, | |||
| 137 | + | } | |||
| 138 | + | ||||
| 139 | + | for _, tt := range tests { | |||
| 140 | + | t.Run(tt.name, func(t *testing.T) { | |||
| 141 | + | cause := errors.New("open failed") | |||
| 142 | + | pty := newFailedOpenPTY() | |||
| 143 | + | cancelCallbacks := atomic.Int32{} | |||
| 144 | + | cancelled := make(chan struct{}) | |||
| 145 | + | openFactory := paneErrorFactory{ | |||
| 146 | + | pty: pty, | |||
| 147 | + | err: cause, | |||
| 148 | + | onOpen: func(ctx context.Context) { | |||
| 149 | + | pty.ctx = ctx | |||
| 150 | + | context.AfterFunc(ctx, func() { | |||
| 151 | + | cancelCallbacks.Add(1) | |||
| 152 | + | close(cancelled) | |||
| 153 | + | }) | |||
| 154 | + | }, | |||
| 155 | + | } | |||
| 156 | + | d := newTestDaemon(t, &openFactory, stubClock{}) | |||
| 157 | + | ||||
| 158 | + | tabs, cleanup, err := tt.run(t, d) | |||
| 159 | + | require.ErrorIs(t, err, cause) | |||
| 160 | + | if tt.wantCode != 0 { | |||
| 161 | + | var userErr *domain.UserError | |||
| 162 | + | require.ErrorAs(t, err, &userErr) | |||
| 163 | + | require.Equal(t, tt.wantCode, userErr.Code) | |||
| 164 | + | } | |||
| 165 | + | select { | |||
| 166 | + | case <-cancelled: | |||
| 167 | + | case <-time.After(time.Second): | |||
| 168 | + | t.Fatal("failed pane Open process context was not cancelled") | |||
| 169 | + | } | |||
| 170 | + | ||||
| 171 | + | for _, tb := range tabs { | |||
| 172 | + | if tb == nil { | |||
| 173 | + | continue | |||
| 174 | + | } | |||
| 175 | + | tb.mu.Lock() | |||
| 176 | + | floating := tb.floating.pane | |||
| 177 | + | publishedFailedPTY := false | |||
| 178 | + | for _, published := range tb.panes { | |||
| 179 | + | publishedFailedPTY = publishedFailedPTY || published.pty == pty | |||
| 180 | + | } | |||
| 181 | + | tb.mu.Unlock() | |||
| 182 | + | require.NotSame(t, pty, floating) | |||
| 183 | + | require.False(t, publishedFailedPTY, "failed Open must not publish a pane or owner") | |||
| 184 | + | } | |||
| 185 | + | require.Zero(t, pty.readerStarts.Load(), "failed Open must not start a reader") | |||
| 186 | + | require.Equal(t, int32(1), pty.closes.Load(), "failed Open PTY must close exactly once") | |||
| 187 | + | require.Equal(t, int32(1), cancelCallbacks.Load(), "failed Open context must cancel exactly once") | |||
| 188 | + | ||||
| 189 | + | cleanup() | |||
| 190 | + | require.Equal(t, int32(1), pty.closes.Load(), "later teardown must not close an unpublished PTY again") | |||
| 191 | + | require.Equal(t, int32(1), cancelCallbacks.Load(), "later teardown must not recancel an unpublished process") | |||
| 192 | + | }) | |||
| 193 | + | } | |||
| 194 | + | } | |||
| 195 | + | ||||
| 196 | + | type paneErrorFactory struct { | |||
| 197 | + | pty ports.PTY | |||
| 198 | + | err error | |||
| 199 | + | onOpen func(context.Context) | |||
| 200 | + | } | |||
| 201 | + | ||||
| 202 | + | func (f *paneErrorFactory) Open(ctx context.Context, _ string, _ []string, _ []string, _ string, _ domain.Size) (ports.PTY, error) { | |||
| 203 | + | if f.onOpen != nil { | |||
| 204 | + | f.onOpen(ctx) | |||
| 205 | + | } | |||
| 206 | + | return f.pty, f.err | |||
| 207 | + | } | |||
| 208 | + | ||||
| 209 | + | func TestInitialPaneProcessLifetimeIsDaemonRooted(t *testing.T) { | |||
| 210 | + | factory := &lifetimePTYFactory{} | |||
| 211 | + | d := newTestDaemon(t, factory, stubClock{}) | |||
| 212 | + | sess, err := createSessionForTest(d, "work", true, "/tmp", domain.Size{Cols: 80, Rows: 24}, terminalEnv{}, nil) | |||
| 213 | + | require.NoError(t, err) | |||
| 214 | + | ||||
| 215 | + | p := sess.activeTab().focusedPane() | |||
| 216 | + | pty, openCtx := factory.opened(0) | |||
| 217 | + | require.NotNil(t, p.ctx) | |||
| 218 | + | require.Equal(t, openCtx, p.ctx) | |||
| 219 | + | require.NotNil(t, p.cancel) | |||
| 220 | + | ||||
| 221 | + | sess.cancel() | |||
| 222 | + | select { | |||
| 223 | + | case <-openCtx.Done(): | |||
| 224 | + | t.Fatal("session cancellation killed a published pane process") | |||
| 225 | + | default: | |||
| 226 | + | } | |||
| 227 | + | ||||
| 228 | + | require.NoError(t, d.killSession(sess, ports.ReasonSessionKilled, false)) | |||
| 229 | + | select { | |||
| 230 | + | case <-openCtx.Done(): | |||
| 231 | + | case <-time.After(time.Second): | |||
| 232 | + | t.Fatal("session teardown did not cancel the pane process") | |||
| 233 | + | } | |||
| 234 | + | d.sessWg.Wait() | |||
| 235 | + | require.Equal(t, int32(1), pty.closes.Load()) | |||
| 236 | + | } | |||
| 237 | + | ||||
| 238 | + | func TestCreatedTabPaneProcessLifetimeIsDaemonRooted(t *testing.T) { | |||
| 239 | + | factory := &lifetimePTYFactory{} | |||
| 240 | + | d := newTestDaemon(t, factory, stubClock{}) | |||
| 241 | + | sessCtx, cancelSession := context.WithCancel(t.Context()) | |||
| 242 | + | sess := &session{ | |||
| 243 | + | id: "work", | |||
| 244 | + | name: "work", | |||
| 245 | + | ctx: sessCtx, | |||
| 246 | + | cancel: cancelSession, | |||
| 247 | + | tabs: []*tab{newTab(newQuietPTY(), domain.Size{Cols: 80, Rows: 22})}, | |||
| 248 | + | } | |||
| 249 | + | d.mu.Lock() | |||
| 250 | + | d.sessions[sess.id] = sess | |||
| 251 | + | d.mu.Unlock() | |||
| 252 | + | ||||
| 253 | + | require.NoError(t, d.createTab(sess, domain.Size{Cols: 80, Rows: 24})) | |||
| 254 | + | created := sess.activeTab() | |||
| 255 | + | p := created.focusedPane() | |||
| 256 | + | pty, openCtx := factory.opened(0) | |||
| 257 | + | require.Equal(t, openCtx, p.ctx) | |||
| 258 | + | require.NotNil(t, p.cancel) | |||
| 259 | + | ||||
| 260 | + | cancelSession() | |||
| 261 | + | select { | |||
| 262 | + | case <-openCtx.Done(): | |||
| 263 | + | t.Fatal("session cancellation killed a published tab pane process") | |||
| 264 | + | default: | |||
| 265 | + | } | |||
| 266 | + | ||||
| 267 | + | created.closeAllPanes() | |||
| 268 | + | select { | |||
| 269 | + | case <-openCtx.Done(): | |||
| 270 | + | case <-time.After(time.Second): | |||
| 271 | + | t.Fatal("tab teardown did not cancel the pane process") | |||
| 272 | + | } | |||
| 273 | + | d.sessWg.Wait() | |||
| 274 | + | require.Equal(t, int32(1), pty.closes.Load()) | |||
| 275 | + | } | |||
| 276 | + | ||||
| 277 | + | func TestSplitPaneProcessLifetimeIsDaemonRooted(t *testing.T) { | |||
| 278 | + | factory := &lifetimePTYFactory{} | |||
| 279 | + | d := newTestDaemon(t, factory, stubClock{}) | |||
| 280 | + | sessCtx, cancelSession := context.WithCancel(t.Context()) | |||
| 281 | + | tb := newTab(newQuietPTY(), domain.Size{Cols: 80, Rows: 22}) | |||
| 282 | + | tb.ctx, tb.cancel = context.WithCancel(sessCtx) | |||
| 283 | + | sess := &session{id: "work", name: "work", ctx: sessCtx, cancel: cancelSession, tabs: []*tab{tb}} | |||
| 284 | + | target := tb.focusedPane() | |||
| 285 | + | ||||
| 286 | + | require.NoError(t, d.splitPaneAt(sess, tb, target, layout.Right)) | |||
| 287 | + | tb.mu.Lock() | |||
| 288 | + | p := tb.panes["pane-2"] | |||
| 289 | + | tb.mu.Unlock() | |||
| 290 | + | require.NotNil(t, p) | |||
| 291 | + | pty, openCtx := factory.opened(0) | |||
| 292 | + | require.Equal(t, openCtx, p.ctx) | |||
| 293 | + | require.NotNil(t, p.cancel) | |||
| 294 | + | ||||
| 295 | + | cancelSession() | |||
| 296 | + | select { | |||
| 297 | + | case <-openCtx.Done(): | |||
| 298 | + | t.Fatal("session cancellation killed a published split pane process") | |||
| 299 | + | default: | |||
| 300 | + | } | |||
| 301 | + | ||||
| 302 | + | tb.closeAllPanes() | |||
| 303 | + | select { | |||
| 304 | + | case <-openCtx.Done(): | |||
| 305 | + | case <-time.After(time.Second): | |||
| 306 | + | t.Fatal("tab teardown did not cancel the split pane process") | |||
| 307 | + | } | |||
| 308 | + | d.sessWg.Wait() | |||
| 309 | + | require.Equal(t, int32(1), pty.closes.Load()) | |||
| 310 | + | } | |||
| 311 | + | ||||
| 312 | + | func TestFloatingPaneProcessLifetimeIsDaemonRooted(t *testing.T) { | |||
| 313 | + | factory := &lifetimePTYFactory{} | |||
| 314 | + | d := newTestDaemon(t, factory, stubClock{}) | |||
| 315 | + | sessCtx, cancelSession := context.WithCancel(t.Context()) | |||
| 316 | + | tb := newFloatingTestTab(t) | |||
| 317 | + | tb.ctx, tb.cancel = context.WithCancel(sessCtx) | |||
| 318 | + | sess := &session{id: "work", name: "work", ctx: sessCtx, cancel: cancelSession, tabs: []*tab{tb}} | |||
| 319 | + | tb.mu.Lock() | |||
| 320 | + | generation := tb.beginFloatingWarmLocked(true) | |||
| 321 | + | tb.mu.Unlock() | |||
| 322 | + | ||||
| 323 | + | d.openAndInstallFloating(sess, tb, floatingLaunchSpec{ | |||
| 324 | + | sessionName: "work", | |||
| 325 | + | size: domain.Size{Cols: 20, Rows: 8}, | |||
| 326 | + | geometry: floatingGeometry{Inner: domain.Rect{Width: 20, Height: 8}}, | |||
| 327 | + | paneStableID: "p_floating", | |||
| 328 | + | parentCtx: tb.ctx, | |||
| 329 | + | }, generation) | |||
| 330 | + | tb.mu.Lock() | |||
| 331 | + | p := tb.floating.pane | |||
| 332 | + | tb.mu.Unlock() | |||
| 333 | + | require.NotNil(t, p) | |||
| 334 | + | pty, openCtx := factory.opened(0) | |||
| 335 | + | require.Equal(t, openCtx, p.ctx) | |||
| 336 | + | require.NotNil(t, p.cancel) | |||
| 337 | + | ||||
| 338 | + | cancelSession() | |||
| 339 | + | select { | |||
| 340 | + | case <-openCtx.Done(): | |||
| 341 | + | t.Fatal("session cancellation killed a published floating pane process") | |||
| 342 | + | default: | |||
| 343 | + | } | |||
| 344 | + | ||||
| 345 | + | d.teardownFloating(tb, nil) | |||
| 346 | + | select { | |||
| 347 | + | case <-openCtx.Done(): | |||
| 348 | + | case <-time.After(time.Second): | |||
| 349 | + | t.Fatal("floating teardown did not cancel the pane process") | |||
| 350 | + | } | |||
| 351 | + | d.sessWg.Wait() | |||
| 352 | + | require.Equal(t, int32(1), pty.closes.Load()) | |||
| 353 | + | } | |||
| 354 | + | ||||
| 355 | + | func TestRestoredPaneProcessLifetimeIsDaemonRooted(t *testing.T) { | |||
| 356 | + | factory := &lifetimePTYFactory{} | |||
| 357 | + | d := newTestDaemon(t, factory, stubClock{}) | |||
| 358 | + | sessCtx, cancelSession := context.WithCancel(t.Context()) | |||
| 359 | + | tabs, err := d.restoreSnapshotTabs(t.Context(), sessCtx, restoreAcceptanceSession(t, "work")) | |||
| 360 | + | require.NoError(t, err) | |||
| 361 | + | require.Len(t, tabs, 1) | |||
| 362 | + | p := tabs[0].focusedPane() | |||
| 363 | + | pty, openCtx := factory.opened(0) | |||
| 364 | + | require.Equal(t, openCtx, p.ctx) | |||
| 365 | + | require.NotNil(t, p.cancel) | |||
| 366 | + | ||||
| 367 | + | cancelSession() | |||
| 368 | + | select { | |||
| 369 | + | case <-openCtx.Done(): | |||
| 370 | + | t.Fatal("session cancellation killed a restored pane process") | |||
| 371 | + | default: | |||
| 372 | + | } | |||
| 373 | + | ||||
| 374 | + | closeRestoredTabs(tabs) | |||
| 375 | + | select { | |||
| 376 | + | case <-openCtx.Done(): | |||
| 377 | + | case <-time.After(time.Second): | |||
| 378 | + | t.Fatal("restore teardown did not cancel the pane process") | |||
| 379 | + | } | |||
| 380 | + | require.Equal(t, int32(1), pty.closes.Load()) | |||
| 381 | + | } | |||
A internal/usecase/daemon/pane_owner.go +117 -0
| @@ -0,0 +1,117 @@ | |||
| 1 | + | package daemon | |
| 2 | + | ||
| 3 | + | // paneOwner is an immutable routing snapshot for one published pane. A zero | |
| 4 | + | // floatingSlotGeneration identifies a tiled pane; installed floating panes | |
| 5 | + | // carry the tab slot generation that owns them. | |
| 6 | + | type paneOwner struct { | |
| 7 | + | session *session | |
| 8 | + | tab *tab | |
| 9 | + | generation uint64 | |
| 10 | + | floatingSlotGeneration uint64 | |
| 11 | + | } | |
| 12 | + | ||
| 13 | + | // paneEffectLease binds an effect to the exact immutable owner generation that | |
| 14 | + | // existed while the pane was parsed or otherwise inspected under pane.mu. | |
| 15 | + | type paneEffectLease struct { | |
| 16 | + | pane *pane | |
| 17 | + | owner *paneOwner | |
| 18 | + | } | |
| 19 | + | ||
| 20 | + | // Current reports whether the lease still names the published pane owner. For | |
| 21 | + | // floating panes it also verifies that the same tab slot and slot generation | |
| 22 | + | // remain installed. Callers must not hold pane.mu or tab.mu. | |
| 23 | + | func (l paneEffectLease) Current() bool { | |
| 24 | + | return l.pane != nil && l.pane.ownerLeaseCurrent(l) | |
| 25 | + | } | |
| 26 | + | ||
| 27 | + | // ownerSnapshot returns the immutable owner visible to lock-free routing | |
| 28 | + | // readers. Ownership publication itself is serialized under pane.mu. | |
| 29 | + | func (p *pane) ownerSnapshot() *paneOwner { | |
| 30 | + | if p == nil { | |
| 31 | + | return nil | |
| 32 | + | } | |
| 33 | + | return p.owner.Load() | |
| 34 | + | } | |
| 35 | + | ||
| 36 | + | // effectLeaseLocked captures the current owner while pane.mu linearizes VT | |
| 37 | + | // parsing against transfer. The caller must hold pane.mu. | |
| 38 | + | func (p *pane) effectLeaseLocked() paneEffectLease { | |
| 39 | + | if p == nil { | |
| 40 | + | return paneEffectLease{} | |
| 41 | + | } | |
| 42 | + | return paneEffectLease{pane: p, owner: p.owner.Load()} | |
| 43 | + | } | |
| 44 | + | ||
| 45 | + | // 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. | |
| 47 | + | func (p *pane) publishOwnerLocked(sess *session, tb *tab, floatingSlotGeneration uint64) (paneEffectLease, paneEffectLease) { | |
| 48 | + | if p == nil { | |
| 49 | + | return paneEffectLease{}, paneEffectLease{} | |
| 50 | + | } | |
| 51 | + | old := paneEffectLease{pane: p, owner: p.owner.Load()} | |
| 52 | + | p.ownerGeneration++ | |
| 53 | + | next := &paneOwner{ | |
| 54 | + | session: sess, | |
| 55 | + | tab: tb, | |
| 56 | + | generation: p.ownerGeneration, | |
| 57 | + | floatingSlotGeneration: floatingSlotGeneration, | |
| 58 | + | } | |
| 59 | + | p.owner.Store(next) | |
| 60 | + | return old, paneEffectLease{pane: p, owner: next} | |
| 61 | + | } | |
| 62 | + | ||
| 63 | + | // clearOwnerLocked revokes the published owner during terminal teardown. The | |
| 64 | + | // caller must hold pane.mu. The returned lease identifies the retired owner. | |
| 65 | + | func (p *pane) clearOwnerLocked() paneEffectLease { | |
| 66 | + | if p == nil { | |
| 67 | + | return paneEffectLease{} | |
| 68 | + | } | |
| 69 | + | old := paneEffectLease{pane: p, owner: p.owner.Load()} | |
| 70 | + | if old.owner != nil { | |
| 71 | + | p.ownerGeneration++ | |
| 72 | + | p.owner.Store(nil) | |
| 73 | + | } | |
| 74 | + | return old | |
| 75 | + | } | |
| 76 | + | ||
| 77 | + | func (p *pane) ownerLeaseCurrent(lease paneEffectLease) bool { | |
| 78 | + | if p == nil || lease.pane != p || lease.owner == nil || p.owner.Load() != lease.owner { | |
| 79 | + | return false | |
| 80 | + | } | |
| 81 | + | owner := lease.owner | |
| 82 | + | if owner.session == nil || owner.tab == nil { | |
| 83 | + | return false | |
| 84 | + | } | |
| 85 | + | if owner.floatingSlotGeneration == 0 { | |
| 86 | + | return true | |
| 87 | + | } | |
| 88 | + | owner.tab.mu.Lock() | |
| 89 | + | current := owner.tab.floating.pane == p && | |
| 90 | + | owner.tab.floating.generation == owner.floatingSlotGeneration && | |
| 91 | + | (owner.tab.floating.state == floatingHidden || owner.tab.floating.state == floatingVisible) | |
| 92 | + | owner.tab.mu.Unlock() | |
| 93 | + | return current && p.owner.Load() == owner | |
| 94 | + | } | |
| 95 | + | ||
| 96 | + | func publishPaneOwner(p *pane, sess *session, tb *tab, floatingSlotGeneration uint64) paneEffectLease { | |
| 97 | + | if p == nil { | |
| 98 | + | return paneEffectLease{} | |
| 99 | + | } | |
| 100 | + | p.mu.Lock() | |
| 101 | + | _, lease := p.publishOwnerLocked(sess, tb, floatingSlotGeneration) | |
| 102 | + | p.mu.Unlock() | |
| 103 | + | return lease | |
| 104 | + | } | |
| 105 | + | ||
| 106 | + | // publishTiledPaneOwners initializes every pane in an unpublished tab before | |
| 107 | + | // that tab is made visible or any reader starts. | |
| 108 | + | func publishTiledPaneOwners(sess *session, tb *tab) { | |
| 109 | + | if sess == nil || tb == nil { | |
| 110 | + | return | |
| 111 | + | } | |
| 112 | + | tb.mu.Lock() | |
| 113 | + | for _, p := range tb.panes { | |
| 114 | + | publishPaneOwner(p, sess, tb, 0) | |
| 115 | + | } | |
| 116 | + | tb.mu.Unlock() | |
| 117 | + | } | |
| @@ -0,0 +1,117 @@ | |||||
| 1 | + | package daemon | |||
| 2 | + | ||||
| 3 | + | // paneOwner is an immutable routing snapshot for one published pane. A zero | |||
| 4 | + | // floatingSlotGeneration identifies a tiled pane; installed floating panes | |||
| 5 | + | // carry the tab slot generation that owns them. | |||
| 6 | + | type paneOwner struct { | |||
| 7 | + | session *session | |||
| 8 | + | tab *tab | |||
| 9 | + | generation uint64 | |||
| 10 | + | floatingSlotGeneration uint64 | |||
| 11 | + | } | |||
| 12 | + | ||||
| 13 | + | // paneEffectLease binds an effect to the exact immutable owner generation that | |||
| 14 | + | // existed while the pane was parsed or otherwise inspected under pane.mu. | |||
| 15 | + | type paneEffectLease struct { | |||
| 16 | + | pane *pane | |||
| 17 | + | owner *paneOwner | |||
| 18 | + | } | |||
| 19 | + | ||||
| 20 | + | // Current reports whether the lease still names the published pane owner. For | |||
| 21 | + | // floating panes it also verifies that the same tab slot and slot generation | |||
| 22 | + | // remain installed. Callers must not hold pane.mu or tab.mu. | |||
| 23 | + | func (l paneEffectLease) Current() bool { | |||
| 24 | + | return l.pane != nil && l.pane.ownerLeaseCurrent(l) | |||
| 25 | + | } | |||
| 26 | + | ||||
| 27 | + | // ownerSnapshot returns the immutable owner visible to lock-free routing | |||
| 28 | + | // readers. Ownership publication itself is serialized under pane.mu. | |||
| 29 | + | func (p *pane) ownerSnapshot() *paneOwner { | |||
| 30 | + | if p == nil { | |||
| 31 | + | return nil | |||
| 32 | + | } | |||
| 33 | + | return p.owner.Load() | |||
| 34 | + | } | |||
| 35 | + | ||||
| 36 | + | // effectLeaseLocked captures the current owner while pane.mu linearizes VT | |||
| 37 | + | // parsing against transfer. The caller must hold pane.mu. | |||
| 38 | + | func (p *pane) effectLeaseLocked() paneEffectLease { | |||
| 39 | + | if p == nil { | |||
| 40 | + | return paneEffectLease{} | |||
| 41 | + | } | |||
| 42 | + | return paneEffectLease{pane: p, owner: p.owner.Load()} | |||
| 43 | + | } | |||
| 44 | + | ||||
| 45 | + | // 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. | |||
| 47 | + | func (p *pane) publishOwnerLocked(sess *session, tb *tab, floatingSlotGeneration uint64) (paneEffectLease, paneEffectLease) { | |||
| 48 | + | if p == nil { | |||
| 49 | + | return paneEffectLease{}, paneEffectLease{} | |||
| 50 | + | } | |||
| 51 | + | old := paneEffectLease{pane: p, owner: p.owner.Load()} | |||
| 52 | + | p.ownerGeneration++ | |||
| 53 | + | next := &paneOwner{ | |||
| 54 | + | session: sess, | |||
| 55 | + | tab: tb, | |||
| 56 | + | generation: p.ownerGeneration, | |||
| 57 | + | floatingSlotGeneration: floatingSlotGeneration, | |||
| 58 | + | } | |||
| 59 | + | p.owner.Store(next) | |||
| 60 | + | return old, paneEffectLease{pane: p, owner: next} | |||
| 61 | + | } | |||
| 62 | + | ||||
| 63 | + | // clearOwnerLocked revokes the published owner during terminal teardown. The | |||
| 64 | + | // caller must hold pane.mu. The returned lease identifies the retired owner. | |||
| 65 | + | func (p *pane) clearOwnerLocked() paneEffectLease { | |||
| 66 | + | if p == nil { | |||
| 67 | + | return paneEffectLease{} | |||
| 68 | + | } | |||
| 69 | + | old := paneEffectLease{pane: p, owner: p.owner.Load()} | |||
| 70 | + | if old.owner != nil { | |||
| 71 | + | p.ownerGeneration++ | |||
| 72 | + | p.owner.Store(nil) | |||
| 73 | + | } | |||
| 74 | + | return old | |||
| 75 | + | } | |||
| 76 | + | ||||
| 77 | + | func (p *pane) ownerLeaseCurrent(lease paneEffectLease) bool { | |||
| 78 | + | if p == nil || lease.pane != p || lease.owner == nil || p.owner.Load() != lease.owner { | |||
| 79 | + | return false | |||
| 80 | + | } | |||
| 81 | + | owner := lease.owner | |||
| 82 | + | if owner.session == nil || owner.tab == nil { | |||
| 83 | + | return false | |||
| 84 | + | } | |||
| 85 | + | if owner.floatingSlotGeneration == 0 { | |||
| 86 | + | return true | |||
| 87 | + | } | |||
| 88 | + | owner.tab.mu.Lock() | |||
| 89 | + | current := owner.tab.floating.pane == p && | |||
| 90 | + | owner.tab.floating.generation == owner.floatingSlotGeneration && | |||
| 91 | + | (owner.tab.floating.state == floatingHidden || owner.tab.floating.state == floatingVisible) | |||
| 92 | + | owner.tab.mu.Unlock() | |||
| 93 | + | return current && p.owner.Load() == owner | |||
| 94 | + | } | |||
| 95 | + | ||||
| 96 | + | func publishPaneOwner(p *pane, sess *session, tb *tab, floatingSlotGeneration uint64) paneEffectLease { | |||
| 97 | + | if p == nil { | |||
| 98 | + | return paneEffectLease{} | |||
| 99 | + | } | |||
| 100 | + | p.mu.Lock() | |||
| 101 | + | _, lease := p.publishOwnerLocked(sess, tb, floatingSlotGeneration) | |||
| 102 | + | p.mu.Unlock() | |||
| 103 | + | return lease | |||
| 104 | + | } | |||
| 105 | + | ||||
| 106 | + | // publishTiledPaneOwners initializes every pane in an unpublished tab before | |||
| 107 | + | // that tab is made visible or any reader starts. | |||
| 108 | + | func publishTiledPaneOwners(sess *session, tb *tab) { | |||
| 109 | + | if sess == nil || tb == nil { | |||
| 110 | + | return | |||
| 111 | + | } | |||
| 112 | + | tb.mu.Lock() | |||
| 113 | + | for _, p := range tb.panes { | |||
| 114 | + | publishPaneOwner(p, sess, tb, 0) | |||
| 115 | + | } | |||
| 116 | + | tb.mu.Unlock() | |||
| 117 | + | } | |||
A internal/usecase/daemon/pane_owner_test.go +92 -0
| @@ -0,0 +1,92 @@ | |||
| 1 | + | package daemon | |
| 2 | + | ||
| 3 | + | import ( | |
| 4 | + | "testing" | |
| 5 | + | ||
| 6 | + | "github.com/bnema/vev/internal/domain" | |
| 7 | + | "github.com/bnema/vev/internal/usecase/layout" | |
| 8 | + | "github.com/stretchr/testify/require" | |
| 9 | + | ) | |
| 10 | + | ||
| 11 | + | func TestPaneOwnerPublicationAdvancesGenerationAndInvalidatesLease(t *testing.T) { | |
| 12 | + | p := newPaneWithStableID(layout.PaneID("pane-1"), "p_stable", nil, domain.Size{Cols: 80, Rows: 23}) | |
| 13 | + | firstSession := &session{id: "first"} | |
| 14 | + | firstTab := newTab(nil, domain.Size{Cols: 80, Rows: 23}) | |
| 15 | + | secondSession := &session{id: "second"} | |
| 16 | + | secondTab := newTab(nil, domain.Size{Cols: 80, Rows: 23}) | |
| 17 | + | ||
| 18 | + | p.mu.Lock() | |
| 19 | + | oldLease, _ := p.publishOwnerLocked(firstSession, firstTab, 0) | |
| 20 | + | firstLease := p.effectLeaseLocked() | |
| 21 | + | p.mu.Unlock() | |
| 22 | + | require.False(t, oldLease.Current()) | |
| 23 | + | require.True(t, firstLease.Current()) | |
| 24 | + | require.Equal(t, uint64(1), firstLease.owner.generation) | |
| 25 | + | require.Same(t, firstSession, firstLease.owner.session) | |
| 26 | + | require.Same(t, firstTab, firstLease.owner.tab) | |
| 27 | + | ||
| 28 | + | p.mu.Lock() | |
| 29 | + | retiredLease, _ := p.publishOwnerLocked(secondSession, secondTab, 0) | |
| 30 | + | secondLease := p.effectLeaseLocked() | |
| 31 | + | p.mu.Unlock() | |
| 32 | + | require.Equal(t, firstLease, retiredLease) | |
| 33 | + | require.False(t, firstLease.Current()) | |
| 34 | + | require.True(t, secondLease.Current()) | |
| 35 | + | require.Equal(t, uint64(2), secondLease.owner.generation) | |
| 36 | + | require.Equal(t, "p_stable", p.stableID, "owner publication must preserve stable pane identity") | |
| 37 | + | ||
| 38 | + | p.mu.Lock() | |
| 39 | + | clearedLease := p.clearOwnerLocked() | |
| 40 | + | p.mu.Unlock() | |
| 41 | + | require.Equal(t, secondLease, clearedLease) | |
| 42 | + | require.False(t, secondLease.Current()) | |
| 43 | + | require.Nil(t, p.ownerSnapshot()) | |
| 44 | + | require.Equal(t, "p_stable", p.stableID, "terminal owner clearing must preserve stable pane identity") | |
| 45 | + | // Leases retain immutable owner records even after later publication and | |
| 46 | + | // terminal clearing. | |
| 47 | + | require.Same(t, firstSession, firstLease.owner.session) | |
| 48 | + | require.Same(t, firstTab, firstLease.owner.tab) | |
| 49 | + | require.Equal(t, uint64(1), firstLease.owner.generation) | |
| 50 | + | } | |
| 51 | + | ||
| 52 | + | func TestFloatingPaneOwnerLeaseRequiresCurrentSlotGeneration(t *testing.T) { | |
| 53 | + | p := newPaneWithStableID(layout.PaneID("floating"), "p_float", nil, domain.Size{Cols: 20, Rows: 8}) | |
| 54 | + | sess := &session{id: "owner"} | |
| 55 | + | tb := newTab(nil, domain.Size{Cols: 80, Rows: 23}) | |
| 56 | + | ||
| 57 | + | tb.mu.Lock() | |
| 58 | + | generation := tb.beginFloatingWarmLocked(true) | |
| 59 | + | p.mu.Lock() | |
| 60 | + | p.publishOwnerLocked(sess, tb, generation) | |
| 61 | + | lease := p.effectLeaseLocked() | |
| 62 | + | p.mu.Unlock() | |
| 63 | + | require.True(t, tb.installFloatingLocked(p, generation)) | |
| 64 | + | tb.mu.Unlock() | |
| 65 | + | require.True(t, lease.Current()) | |
| 66 | + | ||
| 67 | + | tb.mu.Lock() | |
| 68 | + | tb.floating.generation++ | |
| 69 | + | tb.mu.Unlock() | |
| 70 | + | require.False(t, lease.Current(), "a reused floating slot must reject the old pane effect lease") | |
| 71 | + | require.Equal(t, "p_float", p.stableID) | |
| 72 | + | } | |
| 73 | + | ||
| 74 | + | func TestPublishTiledPaneOwnersInitializesAllPanesBeforePublication(t *testing.T) { | |
| 75 | + | sess := &session{id: "owner"} | |
| 76 | + | tb := newTabWithStableID("t_stable", "p_one", nil, domain.Size{Cols: 80, Rows: 23}) | |
| 77 | + | second := newPaneWithStableID("pane-2", "p_two", nil, domain.Size{Cols: 40, Rows: 23}) | |
| 78 | + | tb.panes[second.id] = second | |
| 79 | + | ||
| 80 | + | publishTiledPaneOwners(sess, tb) | |
| 81 | + | ||
| 82 | + | for _, p := range []*pane{tb.panes["pane-1"], second} { | |
| 83 | + | owner := p.ownerSnapshot() | |
| 84 | + | require.NotNil(t, owner) | |
| 85 | + | require.Same(t, sess, owner.session) | |
| 86 | + | require.Same(t, tb, owner.tab) | |
| 87 | + | require.Equal(t, uint64(1), owner.generation) | |
| 88 | + | require.Zero(t, owner.floatingSlotGeneration) | |
| 89 | + | } | |
| 90 | + | require.Equal(t, "p_one", tb.panes["pane-1"].stableID) | |
| 91 | + | require.Equal(t, "p_two", second.stableID) | |
| 92 | + | } | |
| @@ -0,0 +1,92 @@ | |||||
| 1 | + | package daemon | |||
| 2 | + | ||||
| 3 | + | import ( | |||
| 4 | + | "testing" | |||
| 5 | + | ||||
| 6 | + | "github.com/bnema/vev/internal/domain" | |||
| 7 | + | "github.com/bnema/vev/internal/usecase/layout" | |||
| 8 | + | "github.com/stretchr/testify/require" | |||
| 9 | + | ) | |||
| 10 | + | ||||
| 11 | + | func TestPaneOwnerPublicationAdvancesGenerationAndInvalidatesLease(t *testing.T) { | |||
| 12 | + | p := newPaneWithStableID(layout.PaneID("pane-1"), "p_stable", nil, domain.Size{Cols: 80, Rows: 23}) | |||
| 13 | + | firstSession := &session{id: "first"} | |||
| 14 | + | firstTab := newTab(nil, domain.Size{Cols: 80, Rows: 23}) | |||
| 15 | + | secondSession := &session{id: "second"} | |||
| 16 | + | secondTab := newTab(nil, domain.Size{Cols: 80, Rows: 23}) | |||
| 17 | + | ||||
| 18 | + | p.mu.Lock() | |||
| 19 | + | oldLease, _ := p.publishOwnerLocked(firstSession, firstTab, 0) | |||
| 20 | + | firstLease := p.effectLeaseLocked() | |||
| 21 | + | p.mu.Unlock() | |||
| 22 | + | require.False(t, oldLease.Current()) | |||
| 23 | + | require.True(t, firstLease.Current()) | |||
| 24 | + | require.Equal(t, uint64(1), firstLease.owner.generation) | |||
| 25 | + | require.Same(t, firstSession, firstLease.owner.session) | |||
| 26 | + | require.Same(t, firstTab, firstLease.owner.tab) | |||
| 27 | + | ||||
| 28 | + | p.mu.Lock() | |||
| 29 | + | retiredLease, _ := p.publishOwnerLocked(secondSession, secondTab, 0) | |||
| 30 | + | secondLease := p.effectLeaseLocked() | |||
| 31 | + | p.mu.Unlock() | |||
| 32 | + | require.Equal(t, firstLease, retiredLease) | |||
| 33 | + | require.False(t, firstLease.Current()) | |||
| 34 | + | require.True(t, secondLease.Current()) | |||
| 35 | + | require.Equal(t, uint64(2), secondLease.owner.generation) | |||
| 36 | + | require.Equal(t, "p_stable", p.stableID, "owner publication must preserve stable pane identity") | |||
| 37 | + | ||||
| 38 | + | p.mu.Lock() | |||
| 39 | + | clearedLease := p.clearOwnerLocked() | |||
| 40 | + | p.mu.Unlock() | |||
| 41 | + | require.Equal(t, secondLease, clearedLease) | |||
| 42 | + | require.False(t, secondLease.Current()) | |||
| 43 | + | require.Nil(t, p.ownerSnapshot()) | |||
| 44 | + | require.Equal(t, "p_stable", p.stableID, "terminal owner clearing must preserve stable pane identity") | |||
| 45 | + | // Leases retain immutable owner records even after later publication and | |||
| 46 | + | // terminal clearing. | |||
| 47 | + | require.Same(t, firstSession, firstLease.owner.session) | |||
| 48 | + | require.Same(t, firstTab, firstLease.owner.tab) | |||
| 49 | + | require.Equal(t, uint64(1), firstLease.owner.generation) | |||
| 50 | + | } | |||
| 51 | + | ||||
| 52 | + | func TestFloatingPaneOwnerLeaseRequiresCurrentSlotGeneration(t *testing.T) { | |||
| 53 | + | p := newPaneWithStableID(layout.PaneID("floating"), "p_float", nil, domain.Size{Cols: 20, Rows: 8}) | |||
| 54 | + | sess := &session{id: "owner"} | |||
| 55 | + | tb := newTab(nil, domain.Size{Cols: 80, Rows: 23}) | |||
| 56 | + | ||||
| 57 | + | tb.mu.Lock() | |||
| 58 | + | generation := tb.beginFloatingWarmLocked(true) | |||
| 59 | + | p.mu.Lock() | |||
| 60 | + | p.publishOwnerLocked(sess, tb, generation) | |||
| 61 | + | lease := p.effectLeaseLocked() | |||
| 62 | + | p.mu.Unlock() | |||
| 63 | + | require.True(t, tb.installFloatingLocked(p, generation)) | |||
| 64 | + | tb.mu.Unlock() | |||
| 65 | + | require.True(t, lease.Current()) | |||
| 66 | + | ||||
| 67 | + | tb.mu.Lock() | |||
| 68 | + | tb.floating.generation++ | |||
| 69 | + | tb.mu.Unlock() | |||
| 70 | + | require.False(t, lease.Current(), "a reused floating slot must reject the old pane effect lease") | |||
| 71 | + | require.Equal(t, "p_float", p.stableID) | |||
| 72 | + | } | |||
| 73 | + | ||||
| 74 | + | func TestPublishTiledPaneOwnersInitializesAllPanesBeforePublication(t *testing.T) { | |||
| 75 | + | sess := &session{id: "owner"} | |||
| 76 | + | tb := newTabWithStableID("t_stable", "p_one", nil, domain.Size{Cols: 80, Rows: 23}) | |||
| 77 | + | second := newPaneWithStableID("pane-2", "p_two", nil, domain.Size{Cols: 40, Rows: 23}) | |||
| 78 | + | tb.panes[second.id] = second | |||
| 79 | + | ||||
| 80 | + | publishTiledPaneOwners(sess, tb) | |||
| 81 | + | ||||
| 82 | + | for _, p := range []*pane{tb.panes["pane-1"], second} { | |||
| 83 | + | owner := p.ownerSnapshot() | |||
| 84 | + | require.NotNil(t, owner) | |||
| 85 | + | require.Same(t, sess, owner.session) | |||
| 86 | + | require.Same(t, tb, owner.tab) | |||
| 87 | + | require.Equal(t, uint64(1), owner.generation) | |||
| 88 | + | require.Zero(t, owner.floatingSlotGeneration) | |||
| 89 | + | } | |||
| 90 | + | require.Equal(t, "p_one", tb.panes["pane-1"].stableID) | |||
| 91 | + | require.Equal(t, "p_two", second.stableID) | |||
| 92 | + | } | |||
M internal/usecase/daemon/session.go +54 -23
| @@ -49,6 +49,7 @@ type session struct { | |||
| 49 | 49 | teardownMu sync.Mutex | |
| 50 | 50 | teardownActive bool | |
| 51 | 51 | teardownDone chan struct{} | |
| 52 | + | moveReservations uint | |
| 52 | 53 | teardownWaiters uint | |
| 53 | 54 | teardownChanged chan struct{} | |
| 54 | 55 | lifecycleStopOnce sync.Once | |
| @@ -251,13 +252,23 @@ func (d *Daemon) createSessionLocked(name string, ephemeral bool, cwd string, sz | |||
| 251 | 252 | return nil, domain.UserErr(domain.NoticeSessionSpawn, "couldn't create session", err) | |
| 252 | 253 | } | |
| 253 | 254 | command, args := d.ptyCommand(env) | |
| 254 | - | pty, err := d.ptys.Open(d.serveCtx, command, args, childEnvFrom(env, name, tabStableID, paneStableID, term), cwd, tbSize) | |
| 255 | + | lifetime := d.newPaneProcessLifetime(d.serveCtx) | |
| 256 | + | pty, err := d.ptys.Open(lifetime.ctx, command, args, childEnvFrom(env, name, tabStableID, paneStableID, term), cwd, tbSize) | |
| 255 | 257 | if err != nil { | |
| 258 | + | lifetime.abort() | |
| 259 | + | if pty != nil { | |
| 260 | + | _ = pty.Close() | |
| 261 | + | } | |
| 256 | 262 | closeTabs(tabs) | |
| 257 | 263 | d.log.Warn("pty spawn failed", "err", err, "session", name, "kind", "session") | |
| 258 | 264 | return nil, domain.UserErr(domain.NoticeSessionSpawn, "couldn't create session: shell failed to start", err) | |
| 259 | 265 | } | |
| 260 | 266 | tb := newTabWithStableID(tabStableID, paneStableID, pty, tbSize) | |
| 267 | + | if !lifetime.publish(tb.focusedPane()) { | |
| 268 | + | _ = pty.Close() | |
| 269 | + | closeTabs(tabs) | |
| 270 | + | return nil, domain.UserErr(domain.NoticeSessionSpawn, "couldn't create session: shell failed to start", lifetime.ctx.Err()) | |
| 271 | + | } | |
| 261 | 272 | if i < len(names) { | |
| 262 | 273 | tb.name = names[i] | |
| 263 | 274 | } | |
| @@ -297,6 +308,11 @@ func (d *Daemon) createSessionLocked(name string, ephemeral bool, cwd string, sz | |||
| 297 | 308 | } | |
| 298 | 309 | sess.mruAt.Store(lastUsedSeq) | |
| 299 | 310 | sess.snapEligible.Store(!ephemeral && name != "") | |
| 311 | + | // These tabs are still private construction state. Publish every tiled | |
| 312 | + | // owner before the session enters d.sessions or any PTY reader starts. | |
| 313 | + | for _, tb := range tabs { | |
| 314 | + | publishTiledPaneOwners(sess, tb) | |
| 315 | + | } | |
| 300 | 316 | if !ephemeral { | |
| 301 | 317 | if d.persistEnabled { | |
| 302 | 318 | record := domain.CatalogueRecord{Name: name, IncarnationID: incarnation, Cwd: cwd, CreatedAt: createdAt, UpdatedAt: createdAt, LastUsedSeq: lastUsedSeq, TabNames: names} | |
| @@ -339,9 +355,7 @@ func (d *Daemon) createSessionLocked(name string, ephemeral bool, cwd string, sz | |||
| 339 | 355 | ||
| 340 | 356 | func closeTabs(tabs []*tab) { | |
| 341 | 357 | for _, tb := range tabs { | |
| 342 | - | for _, p := range tb.panes { | |
| 343 | - | _ = p.pty.Close() | |
| 344 | - | } | |
| 358 | + | tb.closeAllPanes() | |
| 345 | 359 | } | |
| 346 | 360 | } | |
| 347 | 361 | ||
| @@ -468,8 +482,13 @@ func (d *Daemon) createTab(sess *session, sz domain.Size) error { | |||
| 468 | 482 | return err | |
| 469 | 483 | } | |
| 470 | 484 | command, args := d.ptyCommand(env) | |
| 471 | - | pty, err := d.ptys.Open(sess.ctx, command, args, childEnvFrom(env, name, tabStableID, paneStableID, term), cwd, tbSize) | |
| 485 | + | lifetime := d.newPaneProcessLifetime(sess.ctx) | |
| 486 | + | pty, err := d.ptys.Open(lifetime.ctx, command, args, childEnvFrom(env, name, tabStableID, paneStableID, term), cwd, tbSize) | |
| 472 | 487 | if err != nil { | |
| 488 | + | lifetime.abort() | |
| 489 | + | if pty != nil { | |
| 490 | + | _ = pty.Close() | |
| 491 | + | } | |
| 473 | 492 | d.log.Warn("pty spawn failed", "err", err, "session", name, "kind", "tab") | |
| 474 | 493 | return domain.UserErr(domain.NoticeTabSpawn, "couldn't open tab: shell failed to start", err) | |
| 475 | 494 | } | |
| @@ -488,14 +507,21 @@ func (d *Daemon) createTab(sess *session, sz domain.Size) error { | |||
| 488 | 507 | d.mu.Lock() | |
| 489 | 508 | if d.closing || d.sessions[sess.id] != sess || sess.ctx.Err() != nil { | |
| 490 | 509 | d.mu.Unlock() | |
| 510 | + | lifetime.abort() | |
| 491 | 511 | _ = pty.Close() | |
| 492 | 512 | return errors.New("daemon: session closed") | |
| 493 | 513 | } | |
| 494 | 514 | tb.ctx, tb.cancel = context.WithCancel(sess.ctx) | |
| 495 | - | for _, p := range tb.panes { | |
| 496 | - | p.ctx, p.cancel = context.WithCancel(tb.ctx) | |
| 497 | - | } | |
| 498 | 515 | sess.mu.Lock() | |
| 516 | + | p := tb.focusedPane() | |
| 517 | + | if !lifetime.publish(p) { | |
| 518 | + | sess.mu.Unlock() | |
| 519 | + | d.mu.Unlock() | |
| 520 | + | _ = pty.Close() | |
| 521 | + | return errors.New("daemon: session closed") | |
| 522 | + | } | |
| 523 | + | // Publish ownership before appending the tab to the visible session. | |
| 524 | + | publishPaneOwner(p, sess, tb, 0) | |
| 499 | 525 | sess.tabs = append(sess.tabs, tb) | |
| 500 | 526 | sess.active = len(sess.tabs) - 1 | |
| 501 | 527 | tabIndex := sess.active | |
| @@ -560,12 +586,7 @@ func (tb *tab) closeAllPanes() { | |||
| 560 | 586 | panes = append(panes, floating) | |
| 561 | 587 | } | |
| 562 | 588 | for _, p := range panes { | |
| 563 | - | if p.cancel != nil { | |
| 564 | - | p.cancel() | |
| 565 | - | } | |
| 566 | - | if p.pty != nil { | |
| 567 | - | _ = p.pty.Close() | |
| 568 | - | } | |
| 589 | + | closePaneProcess(p) | |
| 569 | 590 | } | |
| 570 | 591 | } | |
| 571 | 592 | ||
| @@ -614,12 +635,22 @@ func (d *Daemon) startTabGoroutines(sess *session, tb *tab) { | |||
| 614 | 635 | } | |
| 615 | 636 | ||
| 616 | 637 | func (d *Daemon) startPaneGoroutines(sess *session, tb *tab, p *pane) { | |
| 617 | - | if p != nil { | |
| 618 | - | sess.mu.Lock() | |
| 619 | - | name := sess.name | |
| 620 | - | sess.mu.Unlock() | |
| 621 | - | d.log.Info("pane created", "session", name, "pane", p.id) | |
| 638 | + | if p == nil { | |
| 639 | + | return | |
| 640 | + | } | |
| 641 | + | owner := p.ownerSnapshot() | |
| 642 | + | if owner == nil { | |
| 643 | + | // Teardown may win after publication but before reader startup. In that | |
| 644 | + | // case the process is already closed and no ownerless reader may start. | |
| 645 | + | return | |
| 646 | + | } | |
| 647 | + | if owner.session != sess || owner.tab != tb { | |
| 648 | + | panic("daemon: starting pane reader for a different published owner") | |
| 622 | 649 | } | |
| 650 | + | sess.mu.Lock() | |
| 651 | + | name := sess.name | |
| 652 | + | sess.mu.Unlock() | |
| 653 | + | d.log.Info("pane created", "session", name, "pane", p.id) | |
| 623 | 654 | // Scheduler ownership was removed; this launch creates exactly one reader. | |
| 624 | 655 | d.sessWg.Add(1) | |
| 625 | 656 | go d.ptyReader(sess, tb, p) | |
| @@ -1215,7 +1246,7 @@ func (s *session) beginTeardown(deadline *snapshotShutdownDeadline) bool { | |||
| 1215 | 1246 | } | |
| 1216 | 1247 | ||
| 1217 | 1248 | s.teardownMu.Lock() | |
| 1218 | - | if !s.teardownActive { | |
| 1249 | + | if !s.teardownActive && s.moveReservations == 0 { | |
| 1219 | 1250 | if deadline != nil { | |
| 1220 | 1251 | select { | |
| 1221 | 1252 | case <-deadline.Done(): | |
| @@ -1230,17 +1261,17 @@ func (s *session) beginTeardown(deadline *snapshotShutdownDeadline) bool { | |||
| 1230 | 1261 | s.teardownMu.Unlock() | |
| 1231 | 1262 | return true | |
| 1232 | 1263 | } | |
| 1233 | - | done := s.teardownDone | |
| 1234 | 1264 | s.teardownWaiters++ | |
| 1235 | 1265 | s.signalTeardownChangedLocked() | |
| 1266 | + | changed := s.teardownChangeLocked() | |
| 1236 | 1267 | s.teardownMu.Unlock() | |
| 1237 | 1268 | ||
| 1238 | 1269 | timedOut := false | |
| 1239 | 1270 | if deadline == nil { | |
| 1240 | - | <-done | |
| 1271 | + | <-changed | |
| 1241 | 1272 | } else { | |
| 1242 | 1273 | select { | |
| 1243 | - | case <-done: | |
| 1274 | + | case <-changed: | |
| 1244 | 1275 | case <-deadline.Done(): | |
| 1245 | 1276 | timedOut = true | |
| 1246 | 1277 | } | |
| @@ -49,6 +49,7 @@ type session struct { | |||||
| 49 | teardownMu sync.Mutex | 49 | teardownMu sync.Mutex | ||
| 50 | teardownActive bool | 50 | teardownActive bool | ||
| 51 | teardownDone chan struct{} | 51 | teardownDone chan struct{} | ||
| 52 | + | moveReservations uint | |||
| 52 | teardownWaiters uint | 53 | teardownWaiters uint | ||
| 53 | teardownChanged chan struct{} | 54 | teardownChanged chan struct{} | ||
| 54 | lifecycleStopOnce sync.Once | 55 | lifecycleStopOnce sync.Once | ||
| @@ -251,13 +252,23 @@ func (d *Daemon) createSessionLocked(name string, ephemeral bool, cwd string, sz | |||||
| 251 | return nil, domain.UserErr(domain.NoticeSessionSpawn, "couldn't create session", err) | 252 | return nil, domain.UserErr(domain.NoticeSessionSpawn, "couldn't create session", err) | ||
| 252 | } | 253 | } | ||
| 253 | command, args := d.ptyCommand(env) | 254 | command, args := d.ptyCommand(env) | ||
| 254 | - | pty, err := d.ptys.Open(d.serveCtx, command, args, childEnvFrom(env, name, tabStableID, paneStableID, term), cwd, tbSize) | 255 | + | lifetime := d.newPaneProcessLifetime(d.serveCtx) |
| 256 | + | pty, err := d.ptys.Open(lifetime.ctx, command, args, childEnvFrom(env, name, tabStableID, paneStableID, term), cwd, tbSize) | |||
| 255 | if err != nil { | 257 | if err != nil { | ||
| 258 | + | lifetime.abort() | |||
| 259 | + | if pty != nil { | |||
| 260 | + | _ = pty.Close() | |||
| 261 | + | } | |||
| 256 | closeTabs(tabs) | 262 | closeTabs(tabs) | ||
| 257 | d.log.Warn("pty spawn failed", "err", err, "session", name, "kind", "session") | 263 | d.log.Warn("pty spawn failed", "err", err, "session", name, "kind", "session") | ||
| 258 | return nil, domain.UserErr(domain.NoticeSessionSpawn, "couldn't create session: shell failed to start", err) | 264 | return nil, domain.UserErr(domain.NoticeSessionSpawn, "couldn't create session: shell failed to start", err) | ||
| 259 | } | 265 | } | ||
| 260 | tb := newTabWithStableID(tabStableID, paneStableID, pty, tbSize) | 266 | tb := newTabWithStableID(tabStableID, paneStableID, pty, tbSize) | ||
| 267 | + | if !lifetime.publish(tb.focusedPane()) { | |||
| 268 | + | _ = pty.Close() | |||
| 269 | + | closeTabs(tabs) | |||
| 270 | + | return nil, domain.UserErr(domain.NoticeSessionSpawn, "couldn't create session: shell failed to start", lifetime.ctx.Err()) | |||
| 271 | + | } | |||
| 261 | if i < len(names) { | 272 | if i < len(names) { | ||
| 262 | tb.name = names[i] | 273 | tb.name = names[i] | ||
| 263 | } | 274 | } | ||
| @@ -297,6 +308,11 @@ func (d *Daemon) createSessionLocked(name string, ephemeral bool, cwd string, sz | |||||
| 297 | } | 308 | } | ||
| 298 | sess.mruAt.Store(lastUsedSeq) | 309 | sess.mruAt.Store(lastUsedSeq) | ||
| 299 | sess.snapEligible.Store(!ephemeral && name != "") | 310 | sess.snapEligible.Store(!ephemeral && name != "") | ||
| 311 | + | // These tabs are still private construction state. Publish every tiled | |||
| 312 | + | // owner before the session enters d.sessions or any PTY reader starts. | |||
| 313 | + | for _, tb := range tabs { | |||
| 314 | + | publishTiledPaneOwners(sess, tb) | |||
| 315 | + | } | |||
| 300 | if !ephemeral { | 316 | if !ephemeral { | ||
| 301 | if d.persistEnabled { | 317 | if d.persistEnabled { | ||
| 302 | record := domain.CatalogueRecord{Name: name, IncarnationID: incarnation, Cwd: cwd, CreatedAt: createdAt, UpdatedAt: createdAt, LastUsedSeq: lastUsedSeq, TabNames: names} | 318 | record := domain.CatalogueRecord{Name: name, IncarnationID: incarnation, Cwd: cwd, CreatedAt: createdAt, UpdatedAt: createdAt, LastUsedSeq: lastUsedSeq, TabNames: names} | ||
| @@ -339,9 +355,7 @@ func (d *Daemon) createSessionLocked(name string, ephemeral bool, cwd string, sz | |||||
| 339 | 355 | ||||
| 340 | func closeTabs(tabs []*tab) { | 356 | func closeTabs(tabs []*tab) { | ||
| 341 | for _, tb := range tabs { | 357 | for _, tb := range tabs { | ||
| 342 | - | for _, p := range tb.panes { | 358 | + | tb.closeAllPanes() |
| 343 | - | _ = p.pty.Close() | |||
| 344 | - | } | |||
| 345 | } | 359 | } | ||
| 346 | } | 360 | } | ||
| 347 | 361 | ||||
| @@ -468,8 +482,13 @@ func (d *Daemon) createTab(sess *session, sz domain.Size) error { | |||||
| 468 | return err | 482 | return err | ||
| 469 | } | 483 | } | ||
| 470 | command, args := d.ptyCommand(env) | 484 | command, args := d.ptyCommand(env) | ||
| 471 | - | pty, err := d.ptys.Open(sess.ctx, command, args, childEnvFrom(env, name, tabStableID, paneStableID, term), cwd, tbSize) | 485 | + | lifetime := d.newPaneProcessLifetime(sess.ctx) |
| 486 | + | pty, err := d.ptys.Open(lifetime.ctx, command, args, childEnvFrom(env, name, tabStableID, paneStableID, term), cwd, tbSize) | |||
| 472 | if err != nil { | 487 | if err != nil { | ||
| 488 | + | lifetime.abort() | |||
| 489 | + | if pty != nil { | |||
| 490 | + | _ = pty.Close() | |||
| 491 | + | } | |||
| 473 | d.log.Warn("pty spawn failed", "err", err, "session", name, "kind", "tab") | 492 | d.log.Warn("pty spawn failed", "err", err, "session", name, "kind", "tab") | ||
| 474 | return domain.UserErr(domain.NoticeTabSpawn, "couldn't open tab: shell failed to start", err) | 493 | return domain.UserErr(domain.NoticeTabSpawn, "couldn't open tab: shell failed to start", err) | ||
| 475 | } | 494 | } | ||
| @@ -488,14 +507,21 @@ func (d *Daemon) createTab(sess *session, sz domain.Size) error { | |||||
| 488 | d.mu.Lock() | 507 | d.mu.Lock() | ||
| 489 | if d.closing || d.sessions[sess.id] != sess || sess.ctx.Err() != nil { | 508 | if d.closing || d.sessions[sess.id] != sess || sess.ctx.Err() != nil { | ||
| 490 | d.mu.Unlock() | 509 | d.mu.Unlock() | ||
| 510 | + | lifetime.abort() | |||
| 491 | _ = pty.Close() | 511 | _ = pty.Close() | ||
| 492 | return errors.New("daemon: session closed") | 512 | return errors.New("daemon: session closed") | ||
| 493 | } | 513 | } | ||
| 494 | tb.ctx, tb.cancel = context.WithCancel(sess.ctx) | 514 | tb.ctx, tb.cancel = context.WithCancel(sess.ctx) | ||
| 495 | - | for _, p := range tb.panes { | |||
| 496 | - | p.ctx, p.cancel = context.WithCancel(tb.ctx) | |||
| 497 | - | } | |||
| 498 | sess.mu.Lock() | 515 | sess.mu.Lock() | ||
| 516 | + | p := tb.focusedPane() | |||
| 517 | + | if !lifetime.publish(p) { | |||
| 518 | + | sess.mu.Unlock() | |||
| 519 | + | d.mu.Unlock() | |||
| 520 | + | _ = pty.Close() | |||
| 521 | + | return errors.New("daemon: session closed") | |||
| 522 | + | } | |||
| 523 | + | // Publish ownership before appending the tab to the visible session. | |||
| 524 | + | publishPaneOwner(p, sess, tb, 0) | |||
| 499 | sess.tabs = append(sess.tabs, tb) | 525 | sess.tabs = append(sess.tabs, tb) | ||
| 500 | sess.active = len(sess.tabs) - 1 | 526 | sess.active = len(sess.tabs) - 1 | ||
| 501 | tabIndex := sess.active | 527 | tabIndex := sess.active | ||
| @@ -560,12 +586,7 @@ func (tb *tab) closeAllPanes() { | |||||
| 560 | panes = append(panes, floating) | 586 | panes = append(panes, floating) | ||
| 561 | } | 587 | } | ||
| 562 | for _, p := range panes { | 588 | for _, p := range panes { | ||
| 563 | - | if p.cancel != nil { | 589 | + | closePaneProcess(p) |
| 564 | - | p.cancel() | |||
| 565 | - | } | |||
| 566 | - | if p.pty != nil { | |||
| 567 | - | _ = p.pty.Close() | |||
| 568 | - | } | |||
| 569 | } | 590 | } | ||
| 570 | } | 591 | } | ||
| 571 | 592 | ||||
| @@ -614,12 +635,22 @@ func (d *Daemon) startTabGoroutines(sess *session, tb *tab) { | |||||
| 614 | } | 635 | } | ||
| 615 | 636 | ||||
| 616 | func (d *Daemon) startPaneGoroutines(sess *session, tb *tab, p *pane) { | 637 | func (d *Daemon) startPaneGoroutines(sess *session, tb *tab, p *pane) { | ||
| 617 | - | if p != nil { | 638 | + | if p == nil { |
| 618 | - | sess.mu.Lock() | 639 | + | return |
| 619 | - | name := sess.name | 640 | + | } |
| 620 | - | sess.mu.Unlock() | 641 | + | owner := p.ownerSnapshot() |
| 621 | - | d.log.Info("pane created", "session", name, "pane", p.id) | 642 | + | if owner == nil { |
| 643 | + | // Teardown may win after publication but before reader startup. In that | |||
| 644 | + | // case the process is already closed and no ownerless reader may start. | |||
| 645 | + | return | |||
| 646 | + | } | |||
| 647 | + | if owner.session != sess || owner.tab != tb { | |||
| 648 | + | panic("daemon: starting pane reader for a different published owner") | |||
| 622 | } | 649 | } | ||
| 650 | + | sess.mu.Lock() | |||
| 651 | + | name := sess.name | |||
| 652 | + | sess.mu.Unlock() | |||
| 653 | + | d.log.Info("pane created", "session", name, "pane", p.id) | |||
| 623 | // Scheduler ownership was removed; this launch creates exactly one reader. | 654 | // Scheduler ownership was removed; this launch creates exactly one reader. | ||
| 624 | d.sessWg.Add(1) | 655 | d.sessWg.Add(1) | ||
| 625 | go d.ptyReader(sess, tb, p) | 656 | go d.ptyReader(sess, tb, p) | ||
| @@ -1215,7 +1246,7 @@ func (s *session) beginTeardown(deadline *snapshotShutdownDeadline) bool { | |||||
| 1215 | } | 1246 | } | ||
| 1216 | 1247 | ||||
| 1217 | s.teardownMu.Lock() | 1248 | s.teardownMu.Lock() | ||
| 1218 | - | if !s.teardownActive { | 1249 | + | if !s.teardownActive && s.moveReservations == 0 { |
| 1219 | if deadline != nil { | 1250 | if deadline != nil { | ||
| 1220 | select { | 1251 | select { | ||
| 1221 | case <-deadline.Done(): | 1252 | case <-deadline.Done(): | ||
| @@ -1230,17 +1261,17 @@ func (s *session) beginTeardown(deadline *snapshotShutdownDeadline) bool { | |||||
| 1230 | s.teardownMu.Unlock() | 1261 | s.teardownMu.Unlock() | ||
| 1231 | return true | 1262 | return true | ||
| 1232 | } | 1263 | } | ||
| 1233 | - | done := s.teardownDone | |||
| 1234 | s.teardownWaiters++ | 1264 | s.teardownWaiters++ | ||
| 1235 | s.signalTeardownChangedLocked() | 1265 | s.signalTeardownChangedLocked() | ||
| 1266 | + | changed := s.teardownChangeLocked() | |||
| 1236 | s.teardownMu.Unlock() | 1267 | s.teardownMu.Unlock() | ||
| 1237 | 1268 | ||||
| 1238 | timedOut := false | 1269 | timedOut := false | ||
| 1239 | if deadline == nil { | 1270 | if deadline == nil { | ||
| 1240 | - | <-done | 1271 | + | <-changed |
| 1241 | } else { | 1272 | } else { | ||
| 1242 | select { | 1273 | select { | ||
| 1243 | - | case <-done: | 1274 | + | case <-changed: |
| 1244 | case <-deadline.Done(): | 1275 | case <-deadline.Done(): | ||
| 1245 | timedOut = true | 1276 | timedOut = true | ||
| 1246 | } | 1277 | } | ||
M internal/usecase/daemon/snapshot_restore.go +17 -4
| @@ -261,16 +261,23 @@ func (d *Daemon) restoreSnapshotPane(ctx context.Context, sessionName, tabStable | |||
| 261 | 261 | } | |
| 262 | 262 | } | |
| 263 | 263 | command, args := d.ptyCommand(restoreEnv) | |
| 264 | - | pty, err := d.ptys.Open(ctx, command, args, childEnvFrom(restoreEnv, sessionName, tabStableID, paneStableID, restoreTerm), paneSnap.Cwd, restorePTYSize(contentRect, tabSize)) | |
| 264 | + | lifetime := d.newPaneProcessLifetime(ctx, tabCtx) | |
| 265 | + | pty, err := d.ptys.Open(lifetime.ctx, command, args, childEnvFrom(restoreEnv, sessionName, tabStableID, paneStableID, restoreTerm), paneSnap.Cwd, restorePTYSize(contentRect, tabSize)) | |
| 265 | 266 | if err != nil { | |
| 267 | + | lifetime.abort() | |
| 268 | + | if pty != nil { | |
| 269 | + | _ = pty.Close() | |
| 270 | + | } | |
| 266 | 271 | return nil, err | |
| 267 | 272 | } | |
| 268 | 273 | p := newPaneWithStableID(paneSnap.ID, paneStableID, pty, restorePTYSize(contentRect, tabSize)) | |
| 269 | - | p.ctx, p.cancel = context.WithCancel(tabCtx) | |
| 274 | + | if !lifetime.publish(p) { | |
| 275 | + | _ = pty.Close() | |
| 276 | + | return nil, lifetime.ctx.Err() | |
| 277 | + | } | |
| 270 | 278 | p.rect = contentRect | |
| 271 | 279 | if err := restorePaneTerminal(p, paneSnap); err != nil { | |
| 272 | - | p.cancel() | |
| 273 | - | _ = pty.Close() | |
| 280 | + | closePaneProcess(p) | |
| 274 | 281 | return nil, err | |
| 275 | 282 | } | |
| 276 | 283 | if decision := planProcessRestore(paneSnap.Process, allowlist); decision.Restore { | |
| @@ -284,6 +291,12 @@ func (d *Daemon) restoreSnapshotPane(ctx context.Context, sessionName, tabStable | |||
| 284 | 291 | ||
| 285 | 292 | func (d *Daemon) newRestoredSession(snap snapcodec.Session, sctx context.Context, cancel context.CancelFunc, tabs []*tab) *session { | |
| 286 | 293 | sess := &session{name: snap.Name, ctx: sctx, cancel: cancel, tabs: tabs, active: int(snap.Active), terminal: terminalEnv{}, env: copyEnvironment(d.baseEnv), createdAt: int64(snap.CreatedAt), snapshotWake: d.snapshotWake, snapshotChunkCache: newSnapshotChunkCache(snapshotChunkCacheLimit)} | |
| 294 | + | // Restored tabs remain private until persistAndRegisterRestoredSession. | |
| 295 | + | // Initialize owners now so registration and reader startup cannot expose an | |
| 296 | + | // ownerless pane. | |
| 297 | + | for _, tb := range tabs { | |
| 298 | + | publishTiledPaneOwners(sess, tb) | |
| 299 | + | } | |
| 287 | 300 | sess.snapEligible.Store(true) | |
| 288 | 301 | if len(snap.Tabs) > 0 && len(snap.Tabs[0].Panes) > 0 { | |
| 289 | 302 | sess.cwd = snap.Tabs[0].Panes[0].Cwd | |
| @@ -261,16 +261,23 @@ func (d *Daemon) restoreSnapshotPane(ctx context.Context, sessionName, tabStable | |||||
| 261 | } | 261 | } | ||
| 262 | } | 262 | } | ||
| 263 | command, args := d.ptyCommand(restoreEnv) | 263 | command, args := d.ptyCommand(restoreEnv) | ||
| 264 | - | pty, err := d.ptys.Open(ctx, command, args, childEnvFrom(restoreEnv, sessionName, tabStableID, paneStableID, restoreTerm), paneSnap.Cwd, restorePTYSize(contentRect, tabSize)) | 264 | + | lifetime := d.newPaneProcessLifetime(ctx, tabCtx) |
| 265 | + | pty, err := d.ptys.Open(lifetime.ctx, command, args, childEnvFrom(restoreEnv, sessionName, tabStableID, paneStableID, restoreTerm), paneSnap.Cwd, restorePTYSize(contentRect, tabSize)) | |||
| 265 | if err != nil { | 266 | if err != nil { | ||
| 267 | + | lifetime.abort() | |||
| 268 | + | if pty != nil { | |||
| 269 | + | _ = pty.Close() | |||
| 270 | + | } | |||
| 266 | return nil, err | 271 | return nil, err | ||
| 267 | } | 272 | } | ||
| 268 | p := newPaneWithStableID(paneSnap.ID, paneStableID, pty, restorePTYSize(contentRect, tabSize)) | 273 | p := newPaneWithStableID(paneSnap.ID, paneStableID, pty, restorePTYSize(contentRect, tabSize)) | ||
| 269 | - | p.ctx, p.cancel = context.WithCancel(tabCtx) | 274 | + | if !lifetime.publish(p) { |
| 275 | + | _ = pty.Close() | |||
| 276 | + | return nil, lifetime.ctx.Err() | |||
| 277 | + | } | |||
| 270 | p.rect = contentRect | 278 | p.rect = contentRect | ||
| 271 | if err := restorePaneTerminal(p, paneSnap); err != nil { | 279 | if err := restorePaneTerminal(p, paneSnap); err != nil { | ||
| 272 | - | p.cancel() | 280 | + | closePaneProcess(p) |
| 273 | - | _ = pty.Close() | |||
| 274 | return nil, err | 281 | return nil, err | ||
| 275 | } | 282 | } | ||
| 276 | if decision := planProcessRestore(paneSnap.Process, allowlist); decision.Restore { | 283 | if decision := planProcessRestore(paneSnap.Process, allowlist); decision.Restore { | ||
| @@ -284,6 +291,12 @@ func (d *Daemon) restoreSnapshotPane(ctx context.Context, sessionName, tabStable | |||||
| 284 | 291 | ||||
| 285 | func (d *Daemon) newRestoredSession(snap snapcodec.Session, sctx context.Context, cancel context.CancelFunc, tabs []*tab) *session { | 292 | func (d *Daemon) newRestoredSession(snap snapcodec.Session, sctx context.Context, cancel context.CancelFunc, tabs []*tab) *session { | ||
| 286 | sess := &session{name: snap.Name, ctx: sctx, cancel: cancel, tabs: tabs, active: int(snap.Active), terminal: terminalEnv{}, env: copyEnvironment(d.baseEnv), createdAt: int64(snap.CreatedAt), snapshotWake: d.snapshotWake, snapshotChunkCache: newSnapshotChunkCache(snapshotChunkCacheLimit)} | 293 | sess := &session{name: snap.Name, ctx: sctx, cancel: cancel, tabs: tabs, active: int(snap.Active), terminal: terminalEnv{}, env: copyEnvironment(d.baseEnv), createdAt: int64(snap.CreatedAt), snapshotWake: d.snapshotWake, snapshotChunkCache: newSnapshotChunkCache(snapshotChunkCacheLimit)} | ||
| 294 | + | // Restored tabs remain private until persistAndRegisterRestoredSession. | |||
| 295 | + | // Initialize owners now so registration and reader startup cannot expose an | |||
| 296 | + | // ownerless pane. | |||
| 297 | + | for _, tb := range tabs { | |||
| 298 | + | publishTiledPaneOwners(sess, tb) | |||
| 299 | + | } | |||
| 287 | sess.snapEligible.Store(true) | 300 | sess.snapEligible.Store(true) | ||
| 288 | if len(snap.Tabs) > 0 && len(snap.Tabs[0].Panes) > 0 { | 301 | if len(snap.Tabs) > 0 && len(snap.Tabs[0].Panes) > 0 { | ||
| 289 | sess.cwd = snap.Tabs[0].Panes[0].Cwd | 302 | sess.cwd = snap.Tabs[0].Panes[0].Cwd | ||