fix(daemon): harden move concurrency edges
- commit
- dad29baf4fb6088d4987cff7f0de8d99edd6d066
- author
- brice · · 2026-07-28 13:27 UTC
- parent
- 8f53268
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email
22 files · +436 -86
M internal/usecase/daemon/client.go +4 -4
| @@ -413,12 +413,9 @@ func (d *Daemon) notifyDetachedSnapshotAsync(snapshot detachedAttachmentSnapshot | |||
| 413 | 413 | ||
| 414 | 414 | go func() { | |
| 415 | 415 | defer close(done) | |
| 416 | - | _, err := d.boundedSendWith(snapshot.transport.transport, func() error { | |
| 416 | + | _, _ = d.boundedSendWith(snapshot.transport.transport, func() error { | |
| 417 | 417 | return snapshot.ac.sendExpectedTransport(snapshot.transport, frameDetached(reason)) | |
| 418 | 418 | }) | |
| 419 | - | if errors.Is(err, errSendTimedOut) { | |
| 420 | - | _ = snapshot.ac.closeCapturedTransport(snapshot.transport.transport) | |
| 421 | - | } | |
| 422 | 419 | revoked := snapshot.ac.revokeTransport(snapshot.transport.transport) | |
| 423 | 420 | if revoked == nil { | |
| 424 | 421 | revoked = snapshot.transport.transport | |
| @@ -598,6 +595,9 @@ func (d *Daemon) firstPaintForTransition(token attachmentRoleToken) bool { | |||
| 598 | 595 | d.afterRoleEffectAdmitted(token) | |
| 599 | 596 | } | |
| 600 | 597 | if token.rebase { | |
| 598 | + | if d.beforeFirstPaintSendWait != nil { | |
| 599 | + | d.beforeFirstPaintSendWait(token) | |
| 600 | + | } | |
| 601 | 601 | token.ac.sendMu.Lock() | |
| 602 | 602 | if token.ac.renderStages.handoffRebase != nil { | |
| 603 | 603 | token.ac.renderStages.handoffRebase() | |
| @@ -413,12 +413,9 @@ func (d *Daemon) notifyDetachedSnapshotAsync(snapshot detachedAttachmentSnapshot | |||||
| 413 | 413 | ||||
| 414 | go func() { | 414 | go func() { | ||
| 415 | defer close(done) | 415 | defer close(done) | ||
| 416 | - | _, err := d.boundedSendWith(snapshot.transport.transport, func() error { | 416 | + | _, _ = d.boundedSendWith(snapshot.transport.transport, func() error { |
| 417 | return snapshot.ac.sendExpectedTransport(snapshot.transport, frameDetached(reason)) | 417 | return snapshot.ac.sendExpectedTransport(snapshot.transport, frameDetached(reason)) | ||
| 418 | }) | 418 | }) | ||
| 419 | - | if errors.Is(err, errSendTimedOut) { | |||
| 420 | - | _ = snapshot.ac.closeCapturedTransport(snapshot.transport.transport) | |||
| 421 | - | } | |||
| 422 | revoked := snapshot.ac.revokeTransport(snapshot.transport.transport) | 419 | revoked := snapshot.ac.revokeTransport(snapshot.transport.transport) | ||
| 423 | if revoked == nil { | 420 | if revoked == nil { | ||
| 424 | revoked = snapshot.transport.transport | 421 | revoked = snapshot.transport.transport | ||
| @@ -598,6 +595,9 @@ func (d *Daemon) firstPaintForTransition(token attachmentRoleToken) bool { | |||||
| 598 | d.afterRoleEffectAdmitted(token) | 595 | d.afterRoleEffectAdmitted(token) | ||
| 599 | } | 596 | } | ||
| 600 | if token.rebase { | 597 | if token.rebase { | ||
| 598 | + | if d.beforeFirstPaintSendWait != nil { | |||
| 599 | + | d.beforeFirstPaintSendWait(token) | |||
| 600 | + | } | |||
| 601 | token.ac.sendMu.Lock() | 601 | token.ac.sendMu.Lock() | ||
| 602 | if token.ac.renderStages.handoffRebase != nil { | 602 | if token.ac.renderStages.handoffRebase != nil { | ||
| 603 | token.ac.renderStages.handoffRebase() | 603 | token.ac.renderStages.handoffRebase() | ||
M internal/usecase/daemon/control.go +4 -1
| @@ -448,7 +448,10 @@ func (d *Daemon) liveSessionByName(name string) *session { | |||
| 448 | 448 | } | |
| 449 | 449 | ||
| 450 | 450 | func sessionMoveLocator(sess *session) moveSessionLocator { | |
| 451 | - | return moveSessionLocator{ID: sess.id, Incarnation: sess.incarnation, Name: sess.name} | |
| 451 | + | sess.mu.Lock() | |
| 452 | + | name := sess.name | |
| 453 | + | sess.mu.Unlock() | |
| 454 | + | return moveSessionLocator{ID: sess.id, Incarnation: sess.incarnation, Name: name} | |
| 452 | 455 | } | |
| 453 | 456 | func (e controlExec) split(direction layout.Direction) error { | |
| 454 | 457 | return e.runAction(daemonActionRequest{kind: daemonActionSplitPane, direction: direction}) | |
| @@ -448,7 +448,10 @@ func (d *Daemon) liveSessionByName(name string) *session { | |||||
| 448 | } | 448 | } | ||
| 449 | 449 | ||||
| 450 | func sessionMoveLocator(sess *session) moveSessionLocator { | 450 | func sessionMoveLocator(sess *session) moveSessionLocator { | ||
| 451 | - | return moveSessionLocator{ID: sess.id, Incarnation: sess.incarnation, Name: sess.name} | 451 | + | sess.mu.Lock() |
| 452 | + | name := sess.name | |||
| 453 | + | sess.mu.Unlock() | |||
| 454 | + | return moveSessionLocator{ID: sess.id, Incarnation: sess.incarnation, Name: name} | |||
| 452 | } | 455 | } | ||
| 453 | func (e controlExec) split(direction layout.Direction) error { | 456 | func (e controlExec) split(direction layout.Direction) error { | ||
| 454 | return e.runAction(daemonActionRequest{kind: daemonActionSplitPane, direction: direction}) | 457 | return e.runAction(daemonActionRequest{kind: daemonActionSplitPane, direction: direction}) | ||
M internal/usecase/daemon/control_test.go +8 -0
| @@ -657,7 +657,9 @@ func TestHandleCommandMovePaneUsesActiveFocusedSourceWithSessionFlag(t *testing. | |||
| 657 | 657 | require.Len(t, source.tabs, 1) | |
| 658 | 658 | require.Equal(t, "t_inactive", source.tabs[0].stableID) | |
| 659 | 659 | source.mu.Unlock() | |
| 660 | + | destination.mu.Lock() | |
| 660 | 661 | destTab := destination.tabs[0] | |
| 662 | + | destination.mu.Unlock() | |
| 661 | 663 | destTab.mu.Lock() | |
| 662 | 664 | var moved *pane | |
| 663 | 665 | for _, candidate := range destTab.panes { | |
| @@ -692,7 +694,9 @@ func TestHandleCommandMovePaneSelfUsesStableSourceIDs(t *testing.T) { | |||
| 692 | 694 | require.Len(t, active.panes, 1) | |
| 693 | 695 | require.Equal(t, "p_active", active.panes[layout.PaneID("pane-1")].stableID) | |
| 694 | 696 | active.mu.Unlock() | |
| 697 | + | destination.mu.Lock() | |
| 695 | 698 | destTab := destination.tabs[0] | |
| 699 | + | destination.mu.Unlock() | |
| 696 | 700 | destTab.mu.Lock() | |
| 697 | 701 | var moved *pane | |
| 698 | 702 | for _, candidate := range destTab.panes { | |
| @@ -893,7 +897,9 @@ func TestHandleCommandMovePaneRelocatedStableIDsOverrideAdvisorySessionName(t *t | |||
| 893 | 897 | advisory.mu.Lock() | |
| 894 | 898 | require.Len(t, advisory.tabs, 1, "the advisory session-name match must remain untouched") | |
| 895 | 899 | advisory.mu.Unlock() | |
| 900 | + | destination.mu.Lock() | |
| 896 | 901 | destTab := destination.tabs[0] | |
| 902 | + | destination.mu.Unlock() | |
| 897 | 903 | destTab.mu.Lock() | |
| 898 | 904 | require.Len(t, destTab.panes, 2) | |
| 899 | 905 | destTab.mu.Unlock() | |
| @@ -921,7 +927,9 @@ func TestHandleCommandMovePaneStableIDsLocateSessionWithoutSelfRedirect(t *testi | |||
| 921 | 927 | require.Len(t, source.tabs, 1) | |
| 922 | 928 | require.Equal(t, "t_active", source.tabs[0].stableID) | |
| 923 | 929 | source.mu.Unlock() | |
| 930 | + | destination.mu.Lock() | |
| 924 | 931 | destTab := destination.tabs[0] | |
| 932 | + | destination.mu.Unlock() | |
| 925 | 933 | destTab.mu.Lock() | |
| 926 | 934 | var moved *pane | |
| 927 | 935 | for _, candidate := range destTab.panes { | |
| @@ -657,7 +657,9 @@ func TestHandleCommandMovePaneUsesActiveFocusedSourceWithSessionFlag(t *testing. | |||||
| 657 | require.Len(t, source.tabs, 1) | 657 | require.Len(t, source.tabs, 1) | ||
| 658 | require.Equal(t, "t_inactive", source.tabs[0].stableID) | 658 | require.Equal(t, "t_inactive", source.tabs[0].stableID) | ||
| 659 | source.mu.Unlock() | 659 | source.mu.Unlock() | ||
| 660 | + | destination.mu.Lock() | |||
| 660 | destTab := destination.tabs[0] | 661 | destTab := destination.tabs[0] | ||
| 662 | + | destination.mu.Unlock() | |||
| 661 | destTab.mu.Lock() | 663 | destTab.mu.Lock() | ||
| 662 | var moved *pane | 664 | var moved *pane | ||
| 663 | for _, candidate := range destTab.panes { | 665 | for _, candidate := range destTab.panes { | ||
| @@ -692,7 +694,9 @@ func TestHandleCommandMovePaneSelfUsesStableSourceIDs(t *testing.T) { | |||||
| 692 | require.Len(t, active.panes, 1) | 694 | require.Len(t, active.panes, 1) | ||
| 693 | require.Equal(t, "p_active", active.panes[layout.PaneID("pane-1")].stableID) | 695 | require.Equal(t, "p_active", active.panes[layout.PaneID("pane-1")].stableID) | ||
| 694 | active.mu.Unlock() | 696 | active.mu.Unlock() | ||
| 697 | + | destination.mu.Lock() | |||
| 695 | destTab := destination.tabs[0] | 698 | destTab := destination.tabs[0] | ||
| 699 | + | destination.mu.Unlock() | |||
| 696 | destTab.mu.Lock() | 700 | destTab.mu.Lock() | ||
| 697 | var moved *pane | 701 | var moved *pane | ||
| 698 | for _, candidate := range destTab.panes { | 702 | for _, candidate := range destTab.panes { | ||
| @@ -893,7 +897,9 @@ func TestHandleCommandMovePaneRelocatedStableIDsOverrideAdvisorySessionName(t *t | |||||
| 893 | advisory.mu.Lock() | 897 | advisory.mu.Lock() | ||
| 894 | require.Len(t, advisory.tabs, 1, "the advisory session-name match must remain untouched") | 898 | require.Len(t, advisory.tabs, 1, "the advisory session-name match must remain untouched") | ||
| 895 | advisory.mu.Unlock() | 899 | advisory.mu.Unlock() | ||
| 900 | + | destination.mu.Lock() | |||
| 896 | destTab := destination.tabs[0] | 901 | destTab := destination.tabs[0] | ||
| 902 | + | destination.mu.Unlock() | |||
| 897 | destTab.mu.Lock() | 903 | destTab.mu.Lock() | ||
| 898 | require.Len(t, destTab.panes, 2) | 904 | require.Len(t, destTab.panes, 2) | ||
| 899 | destTab.mu.Unlock() | 905 | destTab.mu.Unlock() | ||
| @@ -921,7 +927,9 @@ func TestHandleCommandMovePaneStableIDsLocateSessionWithoutSelfRedirect(t *testi | |||||
| 921 | require.Len(t, source.tabs, 1) | 927 | require.Len(t, source.tabs, 1) | ||
| 922 | require.Equal(t, "t_active", source.tabs[0].stableID) | 928 | require.Equal(t, "t_active", source.tabs[0].stableID) | ||
| 923 | source.mu.Unlock() | 929 | source.mu.Unlock() | ||
| 930 | + | destination.mu.Lock() | |||
| 924 | destTab := destination.tabs[0] | 931 | destTab := destination.tabs[0] | ||
| 932 | + | destination.mu.Unlock() | |||
| 925 | destTab.mu.Lock() | 933 | destTab.mu.Lock() | ||
| 926 | var moved *pane | 934 | var moved *pane | ||
| 927 | for _, candidate := range destTab.panes { | 935 | for _, candidate := range destTab.panes { | ||
M internal/usecase/daemon/copymode_lifecycle_test.go +28 -0
| @@ -55,6 +55,34 @@ func TestCopyModeLifecycleActivateTabClearsRuntime(t *testing.T) { | |||
| 55 | 55 | ac.overlays.copyMu.Unlock() | |
| 56 | 56 | } | |
| 57 | 57 | ||
| 58 | + | func TestActivateTabAfterResizeWithClientAndNoLeaseUsesDirectPath(t *testing.T) { | |
| 59 | + | d, sess, ac, _, releases := newManualTabSession(t, 2) | |
| 60 | + | defer releaseAll(releases) | |
| 61 | + | d.enterCopyMode(sess, ac) | |
| 62 | + | require.True(t, ac.overlays.copyActive()) | |
| 63 | + | ||
| 64 | + | d.activateTabAfterResizeForLease(sess, sess.activeTab(), true, ac, nil) | |
| 65 | + | ||
| 66 | + | require.False(t, ac.overlays.copyActive()) | |
| 67 | + | } | |
| 68 | + | ||
| 69 | + | func TestActivateTabAfterResizeWithCurrentLeaseUsesLeaseBackedPath(t *testing.T) { | |
| 70 | + | d, sess, ac, _, releases := newManualTabSession(t, 2) | |
| 71 | + | defer releaseAll(releases) | |
| 72 | + | d.enterCopyMode(sess, ac) | |
| 73 | + | require.True(t, ac.overlays.copyActive()) | |
| 74 | + | rc := newRenderCoordinator(renderCoordinatorOptions{clock: d.clock}) | |
| 75 | + | rc.attach(ac) | |
| 76 | + | sess.installRenderCoordinator(rc) | |
| 77 | + | defer func() { rc.beginSessionTeardown().finish() }() | |
| 78 | + | lease := rc.attachmentLease(ac) | |
| 79 | + | require.NotNil(t, lease) | |
| 80 | + | ||
| 81 | + | d.activateTabAfterResizeForLease(sess, sess.activeTab(), true, ac, lease) | |
| 82 | + | ||
| 83 | + | require.False(t, ac.overlays.copyActive()) | |
| 84 | + | } | |
| 85 | + | ||
| 58 | 86 | func TestCopyModeLifecycleClosePaneClearsRecoveredClientState(t *testing.T) { | |
| 59 | 87 | d, sess, _, _ := newSplitTestDaemon(t, domain.Size{Cols: 41, Rows: 10}) | |
| 60 | 88 | tb := sess.activeTab() | |
| @@ -55,6 +55,34 @@ func TestCopyModeLifecycleActivateTabClearsRuntime(t *testing.T) { | |||||
| 55 | ac.overlays.copyMu.Unlock() | 55 | ac.overlays.copyMu.Unlock() | ||
| 56 | } | 56 | } | ||
| 57 | 57 | ||||
| 58 | + | func TestActivateTabAfterResizeWithClientAndNoLeaseUsesDirectPath(t *testing.T) { | |||
| 59 | + | d, sess, ac, _, releases := newManualTabSession(t, 2) | |||
| 60 | + | defer releaseAll(releases) | |||
| 61 | + | d.enterCopyMode(sess, ac) | |||
| 62 | + | require.True(t, ac.overlays.copyActive()) | |||
| 63 | + | ||||
| 64 | + | d.activateTabAfterResizeForLease(sess, sess.activeTab(), true, ac, nil) | |||
| 65 | + | ||||
| 66 | + | require.False(t, ac.overlays.copyActive()) | |||
| 67 | + | } | |||
| 68 | + | ||||
| 69 | + | func TestActivateTabAfterResizeWithCurrentLeaseUsesLeaseBackedPath(t *testing.T) { | |||
| 70 | + | d, sess, ac, _, releases := newManualTabSession(t, 2) | |||
| 71 | + | defer releaseAll(releases) | |||
| 72 | + | d.enterCopyMode(sess, ac) | |||
| 73 | + | require.True(t, ac.overlays.copyActive()) | |||
| 74 | + | rc := newRenderCoordinator(renderCoordinatorOptions{clock: d.clock}) | |||
| 75 | + | rc.attach(ac) | |||
| 76 | + | sess.installRenderCoordinator(rc) | |||
| 77 | + | defer func() { rc.beginSessionTeardown().finish() }() | |||
| 78 | + | lease := rc.attachmentLease(ac) | |||
| 79 | + | require.NotNil(t, lease) | |||
| 80 | + | ||||
| 81 | + | d.activateTabAfterResizeForLease(sess, sess.activeTab(), true, ac, lease) | |||
| 82 | + | ||||
| 83 | + | require.False(t, ac.overlays.copyActive()) | |||
| 84 | + | } | |||
| 85 | + | ||||
| 58 | func TestCopyModeLifecycleClosePaneClearsRecoveredClientState(t *testing.T) { | 86 | func TestCopyModeLifecycleClosePaneClearsRecoveredClientState(t *testing.T) { | ||
| 59 | d, sess, _, _ := newSplitTestDaemon(t, domain.Size{Cols: 41, Rows: 10}) | 87 | d, sess, _, _ := newSplitTestDaemon(t, domain.Size{Cols: 41, Rows: 10}) | ||
| 60 | tb := sess.activeTab() | 88 | tb := sess.activeTab() | ||
M internal/usecase/daemon/daemon.go +3 -0
| @@ -177,6 +177,9 @@ type Daemon struct { | |||
| 177 | 177 | // afterRoleEffectAdmitted is a deterministic test seam after a frame/paint | |
| 178 | 178 | // reserves its exact capability and before its first observable mutation. | |
| 179 | 179 | afterRoleEffectAdmitted func(attachmentRoleToken) | |
| 180 | + | // beforeFirstPaintSendWait is a test-only seam immediately before a | |
| 181 | + | // transition first paint waits for the attachment send lock. | |
| 182 | + | beforeFirstPaintSendWait func(attachmentRoleToken) | |
| 180 | 183 | // afterDelayedKeyEffectAttempt observes whether a timer callback acquired a | |
| 181 | 184 | // fresh exact capability before producing PTY, action, or overlay effects. | |
| 182 | 185 | afterDelayedKeyEffectAttempt func(bool) | |
| @@ -177,6 +177,9 @@ type Daemon struct { | |||||
| 177 | // afterRoleEffectAdmitted is a deterministic test seam after a frame/paint | 177 | // afterRoleEffectAdmitted is a deterministic test seam after a frame/paint | ||
| 178 | // reserves its exact capability and before its first observable mutation. | 178 | // reserves its exact capability and before its first observable mutation. | ||
| 179 | afterRoleEffectAdmitted func(attachmentRoleToken) | 179 | afterRoleEffectAdmitted func(attachmentRoleToken) | ||
| 180 | + | // beforeFirstPaintSendWait is a test-only seam immediately before a | |||
| 181 | + | // transition first paint waits for the attachment send lock. | |||
| 182 | + | beforeFirstPaintSendWait func(attachmentRoleToken) | |||
| 180 | // afterDelayedKeyEffectAttempt observes whether a timer callback acquired a | 183 | // afterDelayedKeyEffectAttempt observes whether a timer callback acquired a | ||
| 181 | // fresh exact capability before producing PTY, action, or overlay effects. | 184 | // fresh exact capability before producing PTY, action, or overlay effects. | ||
| 182 | afterDelayedKeyEffectAttempt func(bool) | 185 | afterDelayedKeyEffectAttempt func(bool) | ||
M internal/usecase/daemon/daemon_test.go +12 -2
| @@ -381,12 +381,22 @@ func newCapturingTransport(t testing.TB) (*portsmocks.MockTransport, chan ports. | |||
| 381 | 381 | ||
| 382 | 382 | func newManualSessionWithPTYs(t testing.TB, ptys ...ports.PTY) (*Daemon, *session, *attachedClient, chan ports.Frame) { | |
| 383 | 383 | t.Helper() | |
| 384 | - | return newManualSessionWithPTYsCleanup(t, true, ptys...) | |
| 384 | + | return newManualSessionWithPTYsClock(t, stubClock{}, ptys...) | |
| 385 | + | } | |
| 386 | + | ||
| 387 | + | func newManualSessionWithPTYsClock(t testing.TB, clock ports.Clock, ptys ...ports.PTY) (*Daemon, *session, *attachedClient, chan ports.Frame) { | |
| 388 | + | t.Helper() | |
| 389 | + | return newManualSessionWithPTYsClockCleanup(t, clock, true, ptys...) | |
| 385 | 390 | } | |
| 386 | 391 | ||
| 387 | 392 | func newManualSessionWithPTYsCleanup(t testing.TB, registerCleanup bool, ptys ...ports.PTY) (*Daemon, *session, *attachedClient, chan ports.Frame) { | |
| 388 | 393 | t.Helper() | |
| 389 | - | d := newTestDaemonWithCleanup(t, nil, stubClock{}, registerCleanup) | |
| 394 | + | return newManualSessionWithPTYsClockCleanup(t, stubClock{}, registerCleanup, ptys...) | |
| 395 | + | } | |
| 396 | + | ||
| 397 | + | func newManualSessionWithPTYsClockCleanup(t testing.TB, clock ports.Clock, registerCleanup bool, ptys ...ports.PTY) (*Daemon, *session, *attachedClient, chan ports.Frame) { | |
| 398 | + | t.Helper() | |
| 399 | + | d := newTestDaemonWithCleanup(t, nil, clock, registerCleanup) | |
| 390 | 400 | tr, sends := newCapturingTransport(t) | |
| 391 | 401 | ac := &attachedClient{tr: tr, output: newOutputStateStream(), size: domain.Size{Cols: 80, Rows: 24}} | |
| 392 | 402 | ac.initOverlays() | |
| @@ -381,12 +381,22 @@ func newCapturingTransport(t testing.TB) (*portsmocks.MockTransport, chan ports. | |||||
| 381 | 381 | ||||
| 382 | func newManualSessionWithPTYs(t testing.TB, ptys ...ports.PTY) (*Daemon, *session, *attachedClient, chan ports.Frame) { | 382 | func newManualSessionWithPTYs(t testing.TB, ptys ...ports.PTY) (*Daemon, *session, *attachedClient, chan ports.Frame) { | ||
| 383 | t.Helper() | 383 | t.Helper() | ||
| 384 | - | return newManualSessionWithPTYsCleanup(t, true, ptys...) | 384 | + | return newManualSessionWithPTYsClock(t, stubClock{}, ptys...) |
| 385 | + | } | |||
| 386 | + | ||||
| 387 | + | func newManualSessionWithPTYsClock(t testing.TB, clock ports.Clock, ptys ...ports.PTY) (*Daemon, *session, *attachedClient, chan ports.Frame) { | |||
| 388 | + | t.Helper() | |||
| 389 | + | return newManualSessionWithPTYsClockCleanup(t, clock, true, ptys...) | |||
| 385 | } | 390 | } | ||
| 386 | 391 | ||||
| 387 | func newManualSessionWithPTYsCleanup(t testing.TB, registerCleanup bool, ptys ...ports.PTY) (*Daemon, *session, *attachedClient, chan ports.Frame) { | 392 | func newManualSessionWithPTYsCleanup(t testing.TB, registerCleanup bool, ptys ...ports.PTY) (*Daemon, *session, *attachedClient, chan ports.Frame) { | ||
| 388 | t.Helper() | 393 | t.Helper() | ||
| 389 | - | d := newTestDaemonWithCleanup(t, nil, stubClock{}, registerCleanup) | 394 | + | return newManualSessionWithPTYsClockCleanup(t, stubClock{}, registerCleanup, ptys...) |
| 395 | + | } | |||
| 396 | + | ||||
| 397 | + | func newManualSessionWithPTYsClockCleanup(t testing.TB, clock ports.Clock, registerCleanup bool, ptys ...ports.PTY) (*Daemon, *session, *attachedClient, chan ports.Frame) { | |||
| 398 | + | t.Helper() | |||
| 399 | + | d := newTestDaemonWithCleanup(t, nil, clock, registerCleanup) | |||
| 390 | tr, sends := newCapturingTransport(t) | 400 | tr, sends := newCapturingTransport(t) | ||
| 391 | ac := &attachedClient{tr: tr, output: newOutputStateStream(), size: domain.Size{Cols: 80, Rows: 24}} | 401 | ac := &attachedClient{tr: tr, output: newOutputStateStream(), size: domain.Size{Cols: 80, Rows: 24}} | ||
| 392 | ac.initOverlays() | 402 | ac.initOverlays() | ||
M internal/usecase/daemon/floating.go +2 -1
| @@ -185,7 +185,8 @@ func (d *Daemon) activateTabAfterResizeForLease(sess *session, tb *tab, outerRes | |||
| 185 | 185 | sess.mu.Lock() | |
| 186 | 186 | ac = sess.client | |
| 187 | 187 | sess.mu.Unlock() | |
| 188 | - | } else { | |
| 188 | + | } | |
| 189 | + | if lease != nil { | |
| 189 | 190 | rc := sess.renderCoordinator() | |
| 190 | 191 | if rc == nil || !rc.leaseCurrent(lease, true) { | |
| 191 | 192 | return false | |
| @@ -185,7 +185,8 @@ func (d *Daemon) activateTabAfterResizeForLease(sess *session, tb *tab, outerRes | |||||
| 185 | sess.mu.Lock() | 185 | sess.mu.Lock() | ||
| 186 | ac = sess.client | 186 | ac = sess.client | ||
| 187 | sess.mu.Unlock() | 187 | sess.mu.Unlock() | ||
| 188 | - | } else { | 188 | + | } |
| 189 | + | if lease != nil { | |||
| 189 | rc := sess.renderCoordinator() | 190 | rc := sess.renderCoordinator() | ||
| 190 | if rc == nil || !rc.leaseCurrent(lease, true) { | 191 | if rc == nil || !rc.leaseCurrent(lease, true) { | ||
| 191 | return false | 192 | return false | ||
M internal/usecase/daemon/move_acceptance_test.go +63 -16
| @@ -76,6 +76,7 @@ func TestMovePaneAcceptanceRetainsSourceFocusAndDestinationActivity(t *testing.T | |||
| 76 | 76 | sourceTab.stableID = "source-tab" | |
| 77 | 77 | moved := sourceTab.focusedPane() | |
| 78 | 78 | remaining := newPaneWithStableID("pane-2", "remaining-pane", remainingPTY, domain.Size{Cols: 80, Rows: 23}) | |
| 79 | + | sourceTab.mu.Lock() | |
| 79 | 80 | sourceTab.tree = &layout.Tree{ | |
| 80 | 81 | Root: &layout.Node{Kind: layout.Split, Dir: layout.Horizontal, Children: []*layout.Node{ | |
| 81 | 82 | layout.NewLeaf("pane-1"), layout.NewLeaf("pane-2"), | |
| @@ -83,6 +84,7 @@ func TestMovePaneAcceptanceRetainsSourceFocusAndDestinationActivity(t *testing.T | |||
| 83 | 84 | Focus: "pane-1", | |
| 84 | 85 | } | |
| 85 | 86 | sourceTab.panes[remaining.id] = remaining | |
| 87 | + | sourceTab.mu.Unlock() | |
| 86 | 88 | publishPaneOwner(remaining, source, sourceTab, 0) | |
| 87 | 89 | ||
| 88 | 90 | destinationActive := newTabWithStableID("destination-active", "active-pane", activePTY, domain.Size{Cols: 80, Rows: 23}) | |
| @@ -110,9 +112,15 @@ func TestMovePaneAcceptanceRetainsSourceFocusAndDestinationActivity(t *testing.T | |||
| 110 | 112 | beforeDestinationSnapshot := destination.snapshotGeneration | |
| 111 | 113 | destination.snapshotMu.Unlock() | |
| 112 | 114 | ||
| 115 | + | sourceTab.mu.Lock() | |
| 113 | 116 | beforeSourceGeneration := sourceTab.layoutGeneration | |
| 117 | + | sourceTab.mu.Unlock() | |
| 118 | + | destinationTab.mu.Lock() | |
| 114 | 119 | beforeDestinationGeneration := destinationTab.layoutGeneration | |
| 120 | + | destinationTab.mu.Unlock() | |
| 121 | + | moved.mu.Lock() | |
| 115 | 122 | beforeOwnerGeneration := moved.ownerGeneration | |
| 123 | + | moved.mu.Unlock() | |
| 116 | 124 | ||
| 117 | 125 | require.NoError(t, d.movePane(movePaneRequest{ | |
| 118 | 126 | Source: moveSessionLocator{ID: source.id, Incarnation: source.incarnation}, | |
| @@ -122,16 +130,35 @@ func TestMovePaneAcceptanceRetainsSourceFocusAndDestinationActivity(t *testing.T | |||
| 122 | 130 | DestinationTabID: domain.TabStableID(destinationTab.stableID), | |
| 123 | 131 | })) | |
| 124 | 132 | ||
| 125 | - | require.Len(t, source.tabs, 1) | |
| 126 | - | require.Same(t, sourceTab, source.tabs[0]) | |
| 127 | - | require.Same(t, remaining, sourceTab.panes[remaining.id]) | |
| 128 | - | require.Equal(t, layout.PaneID("pane-2"), sourceTab.tree.Focus, "source focus must use the valid fallback leaf") | |
| 129 | - | require.Same(t, destinationActive, destination.tabs[destination.active], "destination active tab must remain unchanged") | |
| 130 | - | require.Same(t, moved, destinationTab.panes[moved.id]) | |
| 131 | - | require.Equal(t, destinationTab.tree.Focus, moved.id) | |
| 132 | - | require.Equal(t, beforeOwnerGeneration+1, moved.ownerGeneration, "move publishes exactly one new owner generation") | |
| 133 | - | require.Equal(t, beforeSourceGeneration+1, sourceTab.layoutGeneration) | |
| 134 | - | require.Equal(t, beforeDestinationGeneration+1, destinationTab.layoutGeneration) | |
| 133 | + | source.mu.Lock() | |
| 134 | + | sourceTabs := append([]*tab(nil), source.tabs...) | |
| 135 | + | source.mu.Unlock() | |
| 136 | + | require.Len(t, sourceTabs, 1) | |
| 137 | + | require.Same(t, sourceTab, sourceTabs[0]) | |
| 138 | + | sourceTab.mu.Lock() | |
| 139 | + | remainingPane := sourceTab.panes[remaining.id] | |
| 140 | + | sourceFocus := sourceTab.tree.Focus | |
| 141 | + | sourceGeneration := sourceTab.layoutGeneration | |
| 142 | + | sourceTab.mu.Unlock() | |
| 143 | + | require.Same(t, remaining, remainingPane) | |
| 144 | + | require.Equal(t, layout.PaneID("pane-2"), sourceFocus, "source focus must use the valid fallback leaf") | |
| 145 | + | destination.mu.Lock() | |
| 146 | + | destinationActiveTab := destination.tabs[destination.active] | |
| 147 | + | destination.mu.Unlock() | |
| 148 | + | require.Same(t, destinationActive, destinationActiveTab, "destination active tab must remain unchanged") | |
| 149 | + | destinationTab.mu.Lock() | |
| 150 | + | destinationMovedPane := destinationTab.panes[moved.id] | |
| 151 | + | destinationFocus := destinationTab.tree.Focus | |
| 152 | + | destinationGeneration := destinationTab.layoutGeneration | |
| 153 | + | destinationTab.mu.Unlock() | |
| 154 | + | require.Same(t, moved, destinationMovedPane) | |
| 155 | + | require.Equal(t, destinationFocus, moved.id) | |
| 156 | + | moved.mu.Lock() | |
| 157 | + | ownerGeneration := moved.ownerGeneration | |
| 158 | + | moved.mu.Unlock() | |
| 159 | + | require.Equal(t, beforeOwnerGeneration+1, ownerGeneration, "move publishes exactly one new owner generation") | |
| 160 | + | require.Equal(t, beforeSourceGeneration+1, sourceGeneration) | |
| 161 | + | require.Equal(t, beforeDestinationGeneration+1, destinationGeneration) | |
| 135 | 162 | source.snapshotMu.Lock() | |
| 136 | 163 | require.GreaterOrEqual(t, source.snapshotGeneration, beforeSourceSnapshot+1) | |
| 137 | 164 | require.True(t, source.snapDirty.Load()) | |
| @@ -166,8 +193,12 @@ func TestMovePaneRejectsStaleIncarnationWithoutMutation(t *testing.T) { | |||
| 166 | 193 | d.sessions[destination.id] = destination | |
| 167 | 194 | d.mu.Unlock() | |
| 168 | 195 | ||
| 196 | + | sourceTab.mu.Lock() | |
| 169 | 197 | beforeSource := sourceTab.tree.Clone() | |
| 198 | + | sourceTab.mu.Unlock() | |
| 199 | + | destinationTab.mu.Lock() | |
| 170 | 200 | beforeDestination := destinationTab.tree.Clone() | |
| 201 | + | destinationTab.mu.Unlock() | |
| 171 | 202 | beforeOwner := moved.ownerSnapshot() | |
| 172 | 203 | staleIncarnation := source.incarnation | |
| 173 | 204 | staleIncarnation[0]++ | |
| @@ -179,11 +210,19 @@ func TestMovePaneRejectsStaleIncarnationWithoutMutation(t *testing.T) { | |||
| 179 | 210 | DestinationTabID: domain.TabStableID(destinationTab.stableID), | |
| 180 | 211 | }) | |
| 181 | 212 | require.ErrorIs(t, err, errMovePaneInvalid) | |
| 182 | - | require.Equal(t, beforeSource, sourceTab.tree) | |
| 183 | - | require.Equal(t, beforeDestination, destinationTab.tree) | |
| 213 | + | sourceTab.mu.Lock() | |
| 214 | + | afterSource := sourceTab.tree.Clone() | |
| 215 | + | sourceMovedPane := sourceTab.panes[moved.id] | |
| 216 | + | sourceTab.mu.Unlock() | |
| 217 | + | destinationTab.mu.Lock() | |
| 218 | + | afterDestination := destinationTab.tree.Clone() | |
| 219 | + | destinationMovedPane := destinationTab.panes[moved.id] | |
| 220 | + | destinationTab.mu.Unlock() | |
| 221 | + | require.Equal(t, beforeSource, afterSource) | |
| 222 | + | require.Equal(t, beforeDestination, afterDestination) | |
| 184 | 223 | require.Same(t, beforeOwner, moved.ownerSnapshot()) | |
| 185 | - | require.Same(t, moved, sourceTab.panes[moved.id]) | |
| 186 | - | require.NotSame(t, moved, destinationTab.panes[moved.id]) | |
| 224 | + | require.Same(t, moved, sourceMovedPane) | |
| 225 | + | require.NotSame(t, moved, destinationMovedPane) | |
| 187 | 226 | } | |
| 188 | 227 | ||
| 189 | 228 | func TestMovePaneRejectsStaleLayoutGenerationBeforeMutation(t *testing.T) { | |
| @@ -206,7 +245,9 @@ func TestMovePaneRejectsStaleLayoutGenerationBeforeMutation(t *testing.T) { | |||
| 206 | 245 | d.sessions[destination.id] = destination | |
| 207 | 246 | d.mu.Unlock() | |
| 208 | 247 | ||
| 248 | + | destinationTab.mu.Lock() | |
| 209 | 249 | beforeDestination := destinationTab.tree.Clone() | |
| 250 | + | destinationTab.mu.Unlock() | |
| 210 | 251 | beforeOwner := moved.ownerSnapshot() | |
| 211 | 252 | d.afterMovePaneSourceSnapshot = func() { | |
| 212 | 253 | sourceTab.mu.Lock() | |
| @@ -223,7 +264,13 @@ func TestMovePaneRejectsStaleLayoutGenerationBeforeMutation(t *testing.T) { | |||
| 223 | 264 | DestinationTabID: domain.TabStableID(destinationTab.stableID), | |
| 224 | 265 | }) | |
| 225 | 266 | require.Error(t, err) | |
| 226 | - | require.Equal(t, beforeDestination, destinationTab.tree) | |
| 267 | + | destinationTab.mu.Lock() | |
| 268 | + | afterDestination := destinationTab.tree.Clone() | |
| 269 | + | destinationTab.mu.Unlock() | |
| 270 | + | sourceTab.mu.Lock() | |
| 271 | + | sourceMovedPane := sourceTab.panes[moved.id] | |
| 272 | + | sourceTab.mu.Unlock() | |
| 273 | + | require.Equal(t, beforeDestination, afterDestination) | |
| 227 | 274 | require.Same(t, beforeOwner, moved.ownerSnapshot()) | |
| 228 | - | require.Same(t, moved, sourceTab.panes[moved.id]) | |
| 275 | + | require.Same(t, moved, sourceMovedPane) | |
| 229 | 276 | } | |
| @@ -76,6 +76,7 @@ func TestMovePaneAcceptanceRetainsSourceFocusAndDestinationActivity(t *testing.T | |||||
| 76 | sourceTab.stableID = "source-tab" | 76 | sourceTab.stableID = "source-tab" | ||
| 77 | moved := sourceTab.focusedPane() | 77 | moved := sourceTab.focusedPane() | ||
| 78 | remaining := newPaneWithStableID("pane-2", "remaining-pane", remainingPTY, domain.Size{Cols: 80, Rows: 23}) | 78 | remaining := newPaneWithStableID("pane-2", "remaining-pane", remainingPTY, domain.Size{Cols: 80, Rows: 23}) | ||
| 79 | + | sourceTab.mu.Lock() | |||
| 79 | sourceTab.tree = &layout.Tree{ | 80 | sourceTab.tree = &layout.Tree{ | ||
| 80 | Root: &layout.Node{Kind: layout.Split, Dir: layout.Horizontal, Children: []*layout.Node{ | 81 | Root: &layout.Node{Kind: layout.Split, Dir: layout.Horizontal, Children: []*layout.Node{ | ||
| 81 | layout.NewLeaf("pane-1"), layout.NewLeaf("pane-2"), | 82 | layout.NewLeaf("pane-1"), layout.NewLeaf("pane-2"), | ||
| @@ -83,6 +84,7 @@ func TestMovePaneAcceptanceRetainsSourceFocusAndDestinationActivity(t *testing.T | |||||
| 83 | Focus: "pane-1", | 84 | Focus: "pane-1", | ||
| 84 | } | 85 | } | ||
| 85 | sourceTab.panes[remaining.id] = remaining | 86 | sourceTab.panes[remaining.id] = remaining | ||
| 87 | + | sourceTab.mu.Unlock() | |||
| 86 | publishPaneOwner(remaining, source, sourceTab, 0) | 88 | publishPaneOwner(remaining, source, sourceTab, 0) | ||
| 87 | 89 | ||||
| 88 | destinationActive := newTabWithStableID("destination-active", "active-pane", activePTY, domain.Size{Cols: 80, Rows: 23}) | 90 | destinationActive := newTabWithStableID("destination-active", "active-pane", activePTY, domain.Size{Cols: 80, Rows: 23}) | ||
| @@ -110,9 +112,15 @@ func TestMovePaneAcceptanceRetainsSourceFocusAndDestinationActivity(t *testing.T | |||||
| 110 | beforeDestinationSnapshot := destination.snapshotGeneration | 112 | beforeDestinationSnapshot := destination.snapshotGeneration | ||
| 111 | destination.snapshotMu.Unlock() | 113 | destination.snapshotMu.Unlock() | ||
| 112 | 114 | ||||
| 115 | + | sourceTab.mu.Lock() | |||
| 113 | beforeSourceGeneration := sourceTab.layoutGeneration | 116 | beforeSourceGeneration := sourceTab.layoutGeneration | ||
| 117 | + | sourceTab.mu.Unlock() | |||
| 118 | + | destinationTab.mu.Lock() | |||
| 114 | beforeDestinationGeneration := destinationTab.layoutGeneration | 119 | beforeDestinationGeneration := destinationTab.layoutGeneration | ||
| 120 | + | destinationTab.mu.Unlock() | |||
| 121 | + | moved.mu.Lock() | |||
| 115 | beforeOwnerGeneration := moved.ownerGeneration | 122 | beforeOwnerGeneration := moved.ownerGeneration | ||
| 123 | + | moved.mu.Unlock() | |||
| 116 | 124 | ||||
| 117 | require.NoError(t, d.movePane(movePaneRequest{ | 125 | require.NoError(t, d.movePane(movePaneRequest{ | ||
| 118 | Source: moveSessionLocator{ID: source.id, Incarnation: source.incarnation}, | 126 | Source: moveSessionLocator{ID: source.id, Incarnation: source.incarnation}, | ||
| @@ -122,16 +130,35 @@ func TestMovePaneAcceptanceRetainsSourceFocusAndDestinationActivity(t *testing.T | |||||
| 122 | DestinationTabID: domain.TabStableID(destinationTab.stableID), | 130 | DestinationTabID: domain.TabStableID(destinationTab.stableID), | ||
| 123 | })) | 131 | })) | ||
| 124 | 132 | ||||
| 125 | - | require.Len(t, source.tabs, 1) | 133 | + | source.mu.Lock() |
| 126 | - | require.Same(t, sourceTab, source.tabs[0]) | 134 | + | sourceTabs := append([]*tab(nil), source.tabs...) |
| 127 | - | require.Same(t, remaining, sourceTab.panes[remaining.id]) | 135 | + | source.mu.Unlock() |
| 128 | - | require.Equal(t, layout.PaneID("pane-2"), sourceTab.tree.Focus, "source focus must use the valid fallback leaf") | 136 | + | require.Len(t, sourceTabs, 1) |
| 129 | - | require.Same(t, destinationActive, destination.tabs[destination.active], "destination active tab must remain unchanged") | 137 | + | require.Same(t, sourceTab, sourceTabs[0]) |
| 130 | - | require.Same(t, moved, destinationTab.panes[moved.id]) | 138 | + | sourceTab.mu.Lock() |
| 131 | - | require.Equal(t, destinationTab.tree.Focus, moved.id) | 139 | + | remainingPane := sourceTab.panes[remaining.id] |
| 132 | - | require.Equal(t, beforeOwnerGeneration+1, moved.ownerGeneration, "move publishes exactly one new owner generation") | 140 | + | sourceFocus := sourceTab.tree.Focus |
| 133 | - | require.Equal(t, beforeSourceGeneration+1, sourceTab.layoutGeneration) | 141 | + | sourceGeneration := sourceTab.layoutGeneration |
| 134 | - | require.Equal(t, beforeDestinationGeneration+1, destinationTab.layoutGeneration) | 142 | + | sourceTab.mu.Unlock() |
| 143 | + | require.Same(t, remaining, remainingPane) | |||
| 144 | + | require.Equal(t, layout.PaneID("pane-2"), sourceFocus, "source focus must use the valid fallback leaf") | |||
| 145 | + | destination.mu.Lock() | |||
| 146 | + | destinationActiveTab := destination.tabs[destination.active] | |||
| 147 | + | destination.mu.Unlock() | |||
| 148 | + | require.Same(t, destinationActive, destinationActiveTab, "destination active tab must remain unchanged") | |||
| 149 | + | destinationTab.mu.Lock() | |||
| 150 | + | destinationMovedPane := destinationTab.panes[moved.id] | |||
| 151 | + | destinationFocus := destinationTab.tree.Focus | |||
| 152 | + | destinationGeneration := destinationTab.layoutGeneration | |||
| 153 | + | destinationTab.mu.Unlock() | |||
| 154 | + | require.Same(t, moved, destinationMovedPane) | |||
| 155 | + | require.Equal(t, destinationFocus, moved.id) | |||
| 156 | + | moved.mu.Lock() | |||
| 157 | + | ownerGeneration := moved.ownerGeneration | |||
| 158 | + | moved.mu.Unlock() | |||
| 159 | + | require.Equal(t, beforeOwnerGeneration+1, ownerGeneration, "move publishes exactly one new owner generation") | |||
| 160 | + | require.Equal(t, beforeSourceGeneration+1, sourceGeneration) | |||
| 161 | + | require.Equal(t, beforeDestinationGeneration+1, destinationGeneration) | |||
| 135 | source.snapshotMu.Lock() | 162 | source.snapshotMu.Lock() | ||
| 136 | require.GreaterOrEqual(t, source.snapshotGeneration, beforeSourceSnapshot+1) | 163 | require.GreaterOrEqual(t, source.snapshotGeneration, beforeSourceSnapshot+1) | ||
| 137 | require.True(t, source.snapDirty.Load()) | 164 | require.True(t, source.snapDirty.Load()) | ||
| @@ -166,8 +193,12 @@ func TestMovePaneRejectsStaleIncarnationWithoutMutation(t *testing.T) { | |||||
| 166 | d.sessions[destination.id] = destination | 193 | d.sessions[destination.id] = destination | ||
| 167 | d.mu.Unlock() | 194 | d.mu.Unlock() | ||
| 168 | 195 | ||||
| 196 | + | sourceTab.mu.Lock() | |||
| 169 | beforeSource := sourceTab.tree.Clone() | 197 | beforeSource := sourceTab.tree.Clone() | ||
| 198 | + | sourceTab.mu.Unlock() | |||
| 199 | + | destinationTab.mu.Lock() | |||
| 170 | beforeDestination := destinationTab.tree.Clone() | 200 | beforeDestination := destinationTab.tree.Clone() | ||
| 201 | + | destinationTab.mu.Unlock() | |||
| 171 | beforeOwner := moved.ownerSnapshot() | 202 | beforeOwner := moved.ownerSnapshot() | ||
| 172 | staleIncarnation := source.incarnation | 203 | staleIncarnation := source.incarnation | ||
| 173 | staleIncarnation[0]++ | 204 | staleIncarnation[0]++ | ||
| @@ -179,11 +210,19 @@ func TestMovePaneRejectsStaleIncarnationWithoutMutation(t *testing.T) { | |||||
| 179 | DestinationTabID: domain.TabStableID(destinationTab.stableID), | 210 | DestinationTabID: domain.TabStableID(destinationTab.stableID), | ||
| 180 | }) | 211 | }) | ||
| 181 | require.ErrorIs(t, err, errMovePaneInvalid) | 212 | require.ErrorIs(t, err, errMovePaneInvalid) | ||
| 182 | - | require.Equal(t, beforeSource, sourceTab.tree) | 213 | + | sourceTab.mu.Lock() |
| 183 | - | require.Equal(t, beforeDestination, destinationTab.tree) | 214 | + | afterSource := sourceTab.tree.Clone() |
| 215 | + | sourceMovedPane := sourceTab.panes[moved.id] | |||
| 216 | + | sourceTab.mu.Unlock() | |||
| 217 | + | destinationTab.mu.Lock() | |||
| 218 | + | afterDestination := destinationTab.tree.Clone() | |||
| 219 | + | destinationMovedPane := destinationTab.panes[moved.id] | |||
| 220 | + | destinationTab.mu.Unlock() | |||
| 221 | + | require.Equal(t, beforeSource, afterSource) | |||
| 222 | + | require.Equal(t, beforeDestination, afterDestination) | |||
| 184 | require.Same(t, beforeOwner, moved.ownerSnapshot()) | 223 | require.Same(t, beforeOwner, moved.ownerSnapshot()) | ||
| 185 | - | require.Same(t, moved, sourceTab.panes[moved.id]) | 224 | + | require.Same(t, moved, sourceMovedPane) |
| 186 | - | require.NotSame(t, moved, destinationTab.panes[moved.id]) | 225 | + | require.NotSame(t, moved, destinationMovedPane) |
| 187 | } | 226 | } | ||
| 188 | 227 | ||||
| 189 | func TestMovePaneRejectsStaleLayoutGenerationBeforeMutation(t *testing.T) { | 228 | func TestMovePaneRejectsStaleLayoutGenerationBeforeMutation(t *testing.T) { | ||
| @@ -206,7 +245,9 @@ func TestMovePaneRejectsStaleLayoutGenerationBeforeMutation(t *testing.T) { | |||||
| 206 | d.sessions[destination.id] = destination | 245 | d.sessions[destination.id] = destination | ||
| 207 | d.mu.Unlock() | 246 | d.mu.Unlock() | ||
| 208 | 247 | ||||
| 248 | + | destinationTab.mu.Lock() | |||
| 209 | beforeDestination := destinationTab.tree.Clone() | 249 | beforeDestination := destinationTab.tree.Clone() | ||
| 250 | + | destinationTab.mu.Unlock() | |||
| 210 | beforeOwner := moved.ownerSnapshot() | 251 | beforeOwner := moved.ownerSnapshot() | ||
| 211 | d.afterMovePaneSourceSnapshot = func() { | 252 | d.afterMovePaneSourceSnapshot = func() { | ||
| 212 | sourceTab.mu.Lock() | 253 | sourceTab.mu.Lock() | ||
| @@ -223,7 +264,13 @@ func TestMovePaneRejectsStaleLayoutGenerationBeforeMutation(t *testing.T) { | |||||
| 223 | DestinationTabID: domain.TabStableID(destinationTab.stableID), | 264 | DestinationTabID: domain.TabStableID(destinationTab.stableID), | ||
| 224 | }) | 265 | }) | ||
| 225 | require.Error(t, err) | 266 | require.Error(t, err) | ||
| 226 | - | require.Equal(t, beforeDestination, destinationTab.tree) | 267 | + | destinationTab.mu.Lock() |
| 268 | + | afterDestination := destinationTab.tree.Clone() | |||
| 269 | + | destinationTab.mu.Unlock() | |||
| 270 | + | sourceTab.mu.Lock() | |||
| 271 | + | sourceMovedPane := sourceTab.panes[moved.id] | |||
| 272 | + | sourceTab.mu.Unlock() | |||
| 273 | + | require.Equal(t, beforeDestination, afterDestination) | |||
| 227 | require.Same(t, beforeOwner, moved.ownerSnapshot()) | 274 | require.Same(t, beforeOwner, moved.ownerSnapshot()) | ||
| 228 | - | require.Same(t, moved, sourceTab.panes[moved.id]) | 275 | + | require.Same(t, moved, sourceMovedPane) |
| 229 | } | 276 | } | ||
M internal/usecase/daemon/move_atomicity_test.go +29 -31
| @@ -10,21 +10,20 @@ import ( | |||
| 10 | 10 | ) | |
| 11 | 11 | ||
| 12 | 12 | type moveCommitObservation struct { | |
| 13 | - | sourceRegistered bool | |
| 14 | - | sourceTabMember bool | |
| 15 | - | sourcePaneMember bool | |
| 16 | - | destinationTabMember bool | |
| 17 | - | destinationPaneMember bool | |
| 18 | - | sourceActive *tab | |
| 19 | - | destinationActive *tab | |
| 20 | - | sourceFocus layout.PaneID | |
| 21 | - | destinationFocus layout.PaneID | |
| 22 | - | ownerSession *session | |
| 23 | - | ownerTab *tab | |
| 24 | - | sourceRole attachmentRole | |
| 25 | - | destinationRole attachmentRole | |
| 26 | - | sourceClientSession *session | |
| 27 | - | destinationClientSession *session | |
| 13 | + | sourceRegistered bool | |
| 14 | + | sourceTabMember bool | |
| 15 | + | sourcePaneMember bool | |
| 16 | + | destinationTabMember bool | |
| 17 | + | destinationPaneMember bool | |
| 18 | + | sourceActive *tab | |
| 19 | + | destinationActive *tab | |
| 20 | + | sourceFocus layout.PaneID | |
| 21 | + | destinationFocus layout.PaneID | |
| 22 | + | ownerSession *session | |
| 23 | + | ownerTab *tab | |
| 24 | + | sourceRole attachmentRole | |
| 25 | + | destinationRole attachmentRole | |
| 26 | + | sourceClientSession *session | |
| 28 | 27 | } | |
| 29 | 28 | ||
| 30 | 29 | // readMoveCommitObservation follows the same architecture lock order as the | |
| @@ -40,21 +39,20 @@ func readMoveCommitObservation(d *Daemon, source, destination *session, sourceTa | |||
| 40 | 39 | ||
| 41 | 40 | owner := movedPane.ownerSnapshot() | |
| 42 | 41 | observation := moveCommitObservation{ | |
| 43 | - | sourceRegistered: d.sessions[source.id] == source, | |
| 44 | - | sourceTabMember: moveTabMemberLocked(source, sourceTab), | |
| 45 | - | sourcePaneMember: sourceTab.panes[movedPane.id] == movedPane, | |
| 46 | - | destinationTabMember: moveTabMemberLocked(destination, destinationTab), | |
| 47 | - | destinationPaneMember: destinationTab.panes[movedPane.id] == movedPane, | |
| 48 | - | sourceActive: activeMoveTabLocked(source), | |
| 49 | - | destinationActive: activeMoveTabLocked(destination), | |
| 50 | - | sourceFocus: sourceTab.tree.Focus, | |
| 51 | - | destinationFocus: destinationTab.tree.Focus, | |
| 52 | - | ownerSession: nil, | |
| 53 | - | ownerTab: nil, | |
| 54 | - | sourceRole: source.attachmentRoleLocked(sourceClient), | |
| 55 | - | destinationRole: destination.attachmentRoleLocked(sourceClient), | |
| 56 | - | sourceClientSession: sourceClient.currentSession(), | |
| 57 | - | destinationClientSession: sourceClient.currentSession(), | |
| 42 | + | sourceRegistered: d.sessions[source.id] == source, | |
| 43 | + | sourceTabMember: moveTabMemberLocked(source, sourceTab), | |
| 44 | + | sourcePaneMember: sourceTab.panes[movedPane.id] == movedPane, | |
| 45 | + | destinationTabMember: moveTabMemberLocked(destination, destinationTab), | |
| 46 | + | destinationPaneMember: destinationTab.panes[movedPane.id] == movedPane, | |
| 47 | + | sourceActive: activeMoveTabLocked(source), | |
| 48 | + | destinationActive: activeMoveTabLocked(destination), | |
| 49 | + | sourceFocus: sourceTab.tree.Focus, | |
| 50 | + | destinationFocus: destinationTab.tree.Focus, | |
| 51 | + | ownerSession: nil, | |
| 52 | + | ownerTab: nil, | |
| 53 | + | sourceRole: source.attachmentRoleLocked(sourceClient), | |
| 54 | + | destinationRole: destination.attachmentRoleLocked(sourceClient), | |
| 55 | + | sourceClientSession: sourceClient.currentSession(), | |
| 58 | 56 | } | |
| 59 | 57 | if owner != nil { | |
| 60 | 58 | observation.ownerSession = owner.session | |
| @@ -133,7 +131,7 @@ func TestMovePaneCommitPointHidesPartialPublication(t *testing.T) { | |||
| 133 | 131 | return o.sourceActive == sourceTab || o.sourceFocus != layout.PaneID("pane-1") || o.destinationActive != destinationTab || o.destinationFocus != layout.PaneID("pane-2") | |
| 134 | 132 | }}, | |
| 135 | 133 | {name: "attachment roles", read: func(o moveCommitObservation) bool { | |
| 136 | - | return o.sourceRole == attachmentActive && o.destinationRole == attachmentDetached && o.sourceClientSession == source && o.destinationClientSession == source | |
| 134 | + | return o.sourceRole == attachmentActive && o.destinationRole == attachmentDetached && o.sourceClientSession == source | |
| 137 | 135 | }}, | |
| 138 | 136 | } | |
| 139 | 137 | started := make(chan string, len(readers)) | |
| @@ -10,21 +10,20 @@ import ( | |||||
| 10 | ) | 10 | ) | ||
| 11 | 11 | ||||
| 12 | type moveCommitObservation struct { | 12 | type moveCommitObservation struct { | ||
| 13 | - | sourceRegistered bool | 13 | + | sourceRegistered bool |
| 14 | - | sourceTabMember bool | 14 | + | sourceTabMember bool |
| 15 | - | sourcePaneMember bool | 15 | + | sourcePaneMember bool |
| 16 | - | destinationTabMember bool | 16 | + | destinationTabMember bool |
| 17 | - | destinationPaneMember bool | 17 | + | destinationPaneMember bool |
| 18 | - | sourceActive *tab | 18 | + | sourceActive *tab |
| 19 | - | destinationActive *tab | 19 | + | destinationActive *tab |
| 20 | - | sourceFocus layout.PaneID | 20 | + | sourceFocus layout.PaneID |
| 21 | - | destinationFocus layout.PaneID | 21 | + | destinationFocus layout.PaneID |
| 22 | - | ownerSession *session | 22 | + | ownerSession *session |
| 23 | - | ownerTab *tab | 23 | + | ownerTab *tab |
| 24 | - | sourceRole attachmentRole | 24 | + | sourceRole attachmentRole |
| 25 | - | destinationRole attachmentRole | 25 | + | destinationRole attachmentRole |
| 26 | - | sourceClientSession *session | 26 | + | sourceClientSession *session |
| 27 | - | destinationClientSession *session | |||
| 28 | } | 27 | } | ||
| 29 | 28 | ||||
| 30 | // readMoveCommitObservation follows the same architecture lock order as the | 29 | // readMoveCommitObservation follows the same architecture lock order as the | ||
| @@ -40,21 +39,20 @@ func readMoveCommitObservation(d *Daemon, source, destination *session, sourceTa | |||||
| 40 | 39 | ||||
| 41 | owner := movedPane.ownerSnapshot() | 40 | owner := movedPane.ownerSnapshot() | ||
| 42 | observation := moveCommitObservation{ | 41 | observation := moveCommitObservation{ | ||
| 43 | - | sourceRegistered: d.sessions[source.id] == source, | 42 | + | sourceRegistered: d.sessions[source.id] == source, |
| 44 | - | sourceTabMember: moveTabMemberLocked(source, sourceTab), | 43 | + | sourceTabMember: moveTabMemberLocked(source, sourceTab), |
| 45 | - | sourcePaneMember: sourceTab.panes[movedPane.id] == movedPane, | 44 | + | sourcePaneMember: sourceTab.panes[movedPane.id] == movedPane, |
| 46 | - | destinationTabMember: moveTabMemberLocked(destination, destinationTab), | 45 | + | destinationTabMember: moveTabMemberLocked(destination, destinationTab), |
| 47 | - | destinationPaneMember: destinationTab.panes[movedPane.id] == movedPane, | 46 | + | destinationPaneMember: destinationTab.panes[movedPane.id] == movedPane, |
| 48 | - | sourceActive: activeMoveTabLocked(source), | 47 | + | sourceActive: activeMoveTabLocked(source), |
| 49 | - | destinationActive: activeMoveTabLocked(destination), | 48 | + | destinationActive: activeMoveTabLocked(destination), |
| 50 | - | sourceFocus: sourceTab.tree.Focus, | 49 | + | sourceFocus: sourceTab.tree.Focus, |
| 51 | - | destinationFocus: destinationTab.tree.Focus, | 50 | + | destinationFocus: destinationTab.tree.Focus, |
| 52 | - | ownerSession: nil, | 51 | + | ownerSession: nil, |
| 53 | - | ownerTab: nil, | 52 | + | ownerTab: nil, |
| 54 | - | sourceRole: source.attachmentRoleLocked(sourceClient), | 53 | + | sourceRole: source.attachmentRoleLocked(sourceClient), |
| 55 | - | destinationRole: destination.attachmentRoleLocked(sourceClient), | 54 | + | destinationRole: destination.attachmentRoleLocked(sourceClient), |
| 56 | - | sourceClientSession: sourceClient.currentSession(), | 55 | + | sourceClientSession: sourceClient.currentSession(), |
| 57 | - | destinationClientSession: sourceClient.currentSession(), | |||
| 58 | } | 56 | } | ||
| 59 | if owner != nil { | 57 | if owner != nil { | ||
| 60 | observation.ownerSession = owner.session | 58 | observation.ownerSession = owner.session | ||
| @@ -133,7 +131,7 @@ func TestMovePaneCommitPointHidesPartialPublication(t *testing.T) { | |||||
| 133 | return o.sourceActive == sourceTab || o.sourceFocus != layout.PaneID("pane-1") || o.destinationActive != destinationTab || o.destinationFocus != layout.PaneID("pane-2") | 131 | return o.sourceActive == sourceTab || o.sourceFocus != layout.PaneID("pane-1") || o.destinationActive != destinationTab || o.destinationFocus != layout.PaneID("pane-2") | ||
| 134 | }}, | 132 | }}, | ||
| 135 | {name: "attachment roles", read: func(o moveCommitObservation) bool { | 133 | {name: "attachment roles", read: func(o moveCommitObservation) bool { | ||
| 136 | - | return o.sourceRole == attachmentActive && o.destinationRole == attachmentDetached && o.sourceClientSession == source && o.destinationClientSession == source | 134 | + | return o.sourceRole == attachmentActive && o.destinationRole == attachmentDetached && o.sourceClientSession == source |
| 137 | }}, | 135 | }}, | ||
| 138 | } | 136 | } | ||
| 139 | started := make(chan string, len(readers)) | 137 | started := make(chan string, len(readers)) | ||
M internal/usecase/daemon/move_handoff_failure_test.go +16 -8
| @@ -80,6 +80,7 @@ type moveState struct { | |||
| 80 | 80 | } | |
| 81 | 81 | ||
| 82 | 82 | func captureMoveState(d *Daemon, source, destination *session, sourceTab, destinationTab *tab, movedPane *pane, client *attachedClient) moveState { | |
| 83 | + | clientState := captureMoveClientState(source, destination, client) | |
| 83 | 84 | d.mu.Lock() | |
| 84 | 85 | d.notices.routingMu.Lock() | |
| 85 | 86 | unlockSessions := lockAttachmentSessions(source, destination) | |
| @@ -102,7 +103,7 @@ func captureMoveState(d *Daemon, source, destination *session, sourceTab, destin | |||
| 102 | 103 | ownerGeneration: movedPane.ownerGeneration, | |
| 103 | 104 | ctxDone: moveContextDone(movedPane.ctx), | |
| 104 | 105 | }, | |
| 105 | - | client: captureMoveClientStateLocked(source, destination, client), | |
| 106 | + | client: clientState, | |
| 106 | 107 | } | |
| 107 | 108 | ||
| 108 | 109 | movedPane.mu.Unlock() | |
| @@ -152,16 +153,22 @@ func captureMoveTabStateLocked(tb *tab) moveTabState { | |||
| 152 | 153 | } | |
| 153 | 154 | } | |
| 154 | 155 | ||
| 155 | - | func captureMoveClientStateLocked(source, destination *session, client *attachedClient) moveClientState { | |
| 156 | + | // captureMoveClientState snapshots coordinator and role-effect state before | |
| 157 | + | // callers acquire daemon/session/tab move-state locks. This keeps coordinator | |
| 158 | + | // and role-gate locks above, never beneath, the outer move fixture locks. | |
| 159 | + | func captureMoveClientState(source, destination *session, client *attachedClient) moveClientState { | |
| 160 | + | sourceLease := captureMoveLeaseState(source, client) | |
| 161 | + | destinationLease := captureMoveLeaseState(destination, client) | |
| 162 | + | roleEffectPhase := captureMoveRoleEffectPhase(client) | |
| 156 | 163 | return moveClientState{ | |
| 157 | 164 | transport: client.transportSnapshot(), | |
| 158 | 165 | roleGeneration: client.roleGeneration.Load(), | |
| 159 | 166 | currentSession: client.currentSession(), | |
| 160 | - | sourceRole: source.attachmentRoleLocked(client), | |
| 161 | - | destinationRole: destination.attachmentRoleLocked(client), | |
| 162 | - | sourceLease: captureMoveLeaseState(source, client), | |
| 163 | - | destinationLease: captureMoveLeaseState(destination, client), | |
| 164 | - | roleEffectPhase: captureMoveRoleEffectPhase(client), | |
| 167 | + | sourceRole: source.attachmentRole(client), | |
| 168 | + | destinationRole: destination.attachmentRole(client), | |
| 169 | + | sourceLease: sourceLease, | |
| 170 | + | destinationLease: destinationLease, | |
| 171 | + | roleEffectPhase: roleEffectPhase, | |
| 165 | 172 | } | |
| 166 | 173 | } | |
| 167 | 174 | ||
| @@ -262,6 +269,7 @@ type moveTabHandoffState struct { | |||
| 262 | 269 | } | |
| 263 | 270 | ||
| 264 | 271 | func captureMoveTabHandoffState(d *Daemon, source, destination *session, moved *tab, follower *attachedClient) moveTabHandoffState { | |
| 272 | + | followerState := captureMoveClientState(source, destination, follower) | |
| 265 | 273 | d.mu.Lock() | |
| 266 | 274 | d.notices.routingMu.Lock() | |
| 267 | 275 | unlockSessions := lockAttachmentSessions(source, destination) | |
| @@ -275,7 +283,7 @@ func captureMoveTabHandoffState(d *Daemon, source, destination *session, moved * | |||
| 275 | 283 | source: captureMoveSessionStateLocked(source), | |
| 276 | 284 | destination: captureMoveSessionStateLocked(destination), | |
| 277 | 285 | movedTab: captureMoveTabStateLocked(moved), | |
| 278 | - | follower: captureMoveClientStateLocked(source, destination, follower), | |
| 286 | + | follower: followerState, | |
| 279 | 287 | } | |
| 280 | 288 | ||
| 281 | 289 | moved.mu.Unlock() | |
| @@ -80,6 +80,7 @@ type moveState struct { | |||||
| 80 | } | 80 | } | ||
| 81 | 81 | ||||
| 82 | func captureMoveState(d *Daemon, source, destination *session, sourceTab, destinationTab *tab, movedPane *pane, client *attachedClient) moveState { | 82 | func captureMoveState(d *Daemon, source, destination *session, sourceTab, destinationTab *tab, movedPane *pane, client *attachedClient) moveState { | ||
| 83 | + | clientState := captureMoveClientState(source, destination, client) | |||
| 83 | d.mu.Lock() | 84 | d.mu.Lock() | ||
| 84 | d.notices.routingMu.Lock() | 85 | d.notices.routingMu.Lock() | ||
| 85 | unlockSessions := lockAttachmentSessions(source, destination) | 86 | unlockSessions := lockAttachmentSessions(source, destination) | ||
| @@ -102,7 +103,7 @@ func captureMoveState(d *Daemon, source, destination *session, sourceTab, destin | |||||
| 102 | ownerGeneration: movedPane.ownerGeneration, | 103 | ownerGeneration: movedPane.ownerGeneration, | ||
| 103 | ctxDone: moveContextDone(movedPane.ctx), | 104 | ctxDone: moveContextDone(movedPane.ctx), | ||
| 104 | }, | 105 | }, | ||
| 105 | - | client: captureMoveClientStateLocked(source, destination, client), | 106 | + | client: clientState, |
| 106 | } | 107 | } | ||
| 107 | 108 | ||||
| 108 | movedPane.mu.Unlock() | 109 | movedPane.mu.Unlock() | ||
| @@ -152,16 +153,22 @@ func captureMoveTabStateLocked(tb *tab) moveTabState { | |||||
| 152 | } | 153 | } | ||
| 153 | } | 154 | } | ||
| 154 | 155 | ||||
| 155 | - | func captureMoveClientStateLocked(source, destination *session, client *attachedClient) moveClientState { | 156 | + | // captureMoveClientState snapshots coordinator and role-effect state before |
| 157 | + | // callers acquire daemon/session/tab move-state locks. This keeps coordinator | |||
| 158 | + | // and role-gate locks above, never beneath, the outer move fixture locks. | |||
| 159 | + | func captureMoveClientState(source, destination *session, client *attachedClient) moveClientState { | |||
| 160 | + | sourceLease := captureMoveLeaseState(source, client) | |||
| 161 | + | destinationLease := captureMoveLeaseState(destination, client) | |||
| 162 | + | roleEffectPhase := captureMoveRoleEffectPhase(client) | |||
| 156 | return moveClientState{ | 163 | return moveClientState{ | ||
| 157 | transport: client.transportSnapshot(), | 164 | transport: client.transportSnapshot(), | ||
| 158 | roleGeneration: client.roleGeneration.Load(), | 165 | roleGeneration: client.roleGeneration.Load(), | ||
| 159 | currentSession: client.currentSession(), | 166 | currentSession: client.currentSession(), | ||
| 160 | - | sourceRole: source.attachmentRoleLocked(client), | 167 | + | sourceRole: source.attachmentRole(client), |
| 161 | - | destinationRole: destination.attachmentRoleLocked(client), | 168 | + | destinationRole: destination.attachmentRole(client), |
| 162 | - | sourceLease: captureMoveLeaseState(source, client), | 169 | + | sourceLease: sourceLease, |
| 163 | - | destinationLease: captureMoveLeaseState(destination, client), | 170 | + | destinationLease: destinationLease, |
| 164 | - | roleEffectPhase: captureMoveRoleEffectPhase(client), | 171 | + | roleEffectPhase: roleEffectPhase, |
| 165 | } | 172 | } | ||
| 166 | } | 173 | } | ||
| 167 | 174 | ||||
| @@ -262,6 +269,7 @@ type moveTabHandoffState struct { | |||||
| 262 | } | 269 | } | ||
| 263 | 270 | ||||
| 264 | func captureMoveTabHandoffState(d *Daemon, source, destination *session, moved *tab, follower *attachedClient) moveTabHandoffState { | 271 | func captureMoveTabHandoffState(d *Daemon, source, destination *session, moved *tab, follower *attachedClient) moveTabHandoffState { | ||
| 272 | + | followerState := captureMoveClientState(source, destination, follower) | |||
| 265 | d.mu.Lock() | 273 | d.mu.Lock() | ||
| 266 | d.notices.routingMu.Lock() | 274 | d.notices.routingMu.Lock() | ||
| 267 | unlockSessions := lockAttachmentSessions(source, destination) | 275 | unlockSessions := lockAttachmentSessions(source, destination) | ||
| @@ -275,7 +283,7 @@ func captureMoveTabHandoffState(d *Daemon, source, destination *session, moved * | |||||
| 275 | source: captureMoveSessionStateLocked(source), | 283 | source: captureMoveSessionStateLocked(source), | ||
| 276 | destination: captureMoveSessionStateLocked(destination), | 284 | destination: captureMoveSessionStateLocked(destination), | ||
| 277 | movedTab: captureMoveTabStateLocked(moved), | 285 | movedTab: captureMoveTabStateLocked(moved), | ||
| 278 | - | follower: captureMoveClientStateLocked(source, destination, follower), | 286 | + | follower: followerState, |
| 279 | } | 287 | } | ||
| 280 | 288 | ||||
| 281 | moved.mu.Unlock() | 289 | moved.mu.Unlock() | ||
M internal/usecase/daemon/move_picker_test.go +13 -4
| @@ -12,12 +12,19 @@ import ( | |||
| 12 | 12 | ) | |
| 13 | 13 | ||
| 14 | 14 | func setupMovePickerSessions(t *testing.T, extraDestinationTabs int) (*Daemon, *session, *attachedClient, *session, *tab, []func()) { | |
| 15 | + | t.Helper() | |
| 16 | + | return setupMovePickerSessionsWithClock(t, stubClock{}, extraDestinationTabs) | |
| 17 | + | } | |
| 18 | + | ||
| 19 | + | func setupMovePickerSessionsWithClock(t *testing.T, clock ports.Clock, extraDestinationTabs int) (*Daemon, *session, *attachedClient, *session, *tab, []func()) { | |
| 15 | 20 | t.Helper() | |
| 16 | 21 | sourcePTY, releaseSource := newBlockingPTY(t) | |
| 17 | - | d, source, ac, _ := newManualSessionWithPTYs(t, sourcePTY) | |
| 22 | + | d, source, ac, _ := newManualSessionWithPTYsClock(t, clock, sourcePTY) | |
| 18 | 23 | source.id, source.name, source.incarnation = "source", "source", domain.IncarnationID{1} | |
| 24 | + | d.mu.Lock() | |
| 19 | 25 | delete(d.sessions, domain.SessionID("manual")) | |
| 20 | 26 | d.sessions[source.id] = source | |
| 27 | + | d.mu.Unlock() | |
| 21 | 28 | sourceTab := source.tabs[0] | |
| 22 | 29 | sourceTab.stableID = "source-tab" | |
| 23 | 30 | sourcePane := sourceTab.focusedPane() | |
| @@ -171,7 +178,8 @@ func TestMovePickerEnterCommitsMoveTab(t *testing.T) { | |||
| 171 | 178 | } | |
| 172 | 179 | ||
| 173 | 180 | func TestMovePickerEscapePerformsNoMutation(t *testing.T) { | |
| 174 | - | d, source, ac, _, _, releases := setupMovePickerSessions(t, 0) | |
| 181 | + | clk := &signalClock{timers: make(chan *signalTimer, 16)} | |
| 182 | + | d, source, ac, _, _, releases := setupMovePickerSessionsWithClock(t, clk, 0) | |
| 175 | 183 | defer releaseAll(releases) | |
| 176 | 184 | before := len(source.tabs) | |
| 177 | 185 | ||
| @@ -179,8 +187,9 @@ func TestMovePickerEscapePerformsNoMutation(t *testing.T) { | |||
| 179 | 187 | Session: moveSessionLocator{ID: source.id, Incarnation: source.incarnation, Name: source.name}, | |
| 180 | 188 | TabID: "source-tab", PaneID: "source-pane", | |
| 181 | 189 | })) | |
| 182 | - | clk := &signalClock{timers: make(chan *signalTimer, 1)} | |
| 183 | - | d.clock = clk | |
| 190 | + | for len(clk.timers) > 0 { | |
| 191 | + | <-clk.timers | |
| 192 | + | } | |
| 184 | 193 | d.handlePickerInput(ac, []byte("\x1b")) | |
| 185 | 194 | timer := <-clk.timers | |
| 186 | 195 | timer.ch <- time.Now() | |
| @@ -12,12 +12,19 @@ import ( | |||||
| 12 | ) | 12 | ) | ||
| 13 | 13 | ||||
| 14 | func setupMovePickerSessions(t *testing.T, extraDestinationTabs int) (*Daemon, *session, *attachedClient, *session, *tab, []func()) { | 14 | func setupMovePickerSessions(t *testing.T, extraDestinationTabs int) (*Daemon, *session, *attachedClient, *session, *tab, []func()) { | ||
| 15 | + | t.Helper() | |||
| 16 | + | return setupMovePickerSessionsWithClock(t, stubClock{}, extraDestinationTabs) | |||
| 17 | + | } | |||
| 18 | + | ||||
| 19 | + | func setupMovePickerSessionsWithClock(t *testing.T, clock ports.Clock, extraDestinationTabs int) (*Daemon, *session, *attachedClient, *session, *tab, []func()) { | |||
| 15 | t.Helper() | 20 | t.Helper() | ||
| 16 | sourcePTY, releaseSource := newBlockingPTY(t) | 21 | sourcePTY, releaseSource := newBlockingPTY(t) | ||
| 17 | - | d, source, ac, _ := newManualSessionWithPTYs(t, sourcePTY) | 22 | + | d, source, ac, _ := newManualSessionWithPTYsClock(t, clock, sourcePTY) |
| 18 | source.id, source.name, source.incarnation = "source", "source", domain.IncarnationID{1} | 23 | source.id, source.name, source.incarnation = "source", "source", domain.IncarnationID{1} | ||
| 24 | + | d.mu.Lock() | |||
| 19 | delete(d.sessions, domain.SessionID("manual")) | 25 | delete(d.sessions, domain.SessionID("manual")) | ||
| 20 | d.sessions[source.id] = source | 26 | d.sessions[source.id] = source | ||
| 27 | + | d.mu.Unlock() | |||
| 21 | sourceTab := source.tabs[0] | 28 | sourceTab := source.tabs[0] | ||
| 22 | sourceTab.stableID = "source-tab" | 29 | sourceTab.stableID = "source-tab" | ||
| 23 | sourcePane := sourceTab.focusedPane() | 30 | sourcePane := sourceTab.focusedPane() | ||
| @@ -171,7 +178,8 @@ func TestMovePickerEnterCommitsMoveTab(t *testing.T) { | |||||
| 171 | } | 178 | } | ||
| 172 | 179 | ||||
| 173 | func TestMovePickerEscapePerformsNoMutation(t *testing.T) { | 180 | func TestMovePickerEscapePerformsNoMutation(t *testing.T) { | ||
| 174 | - | d, source, ac, _, _, releases := setupMovePickerSessions(t, 0) | 181 | + | clk := &signalClock{timers: make(chan *signalTimer, 16)} |
| 182 | + | d, source, ac, _, _, releases := setupMovePickerSessionsWithClock(t, clk, 0) | |||
| 175 | defer releaseAll(releases) | 183 | defer releaseAll(releases) | ||
| 176 | before := len(source.tabs) | 184 | before := len(source.tabs) | ||
| 177 | 185 | ||||
| @@ -179,8 +187,9 @@ func TestMovePickerEscapePerformsNoMutation(t *testing.T) { | |||||
| 179 | Session: moveSessionLocator{ID: source.id, Incarnation: source.incarnation, Name: source.name}, | 187 | Session: moveSessionLocator{ID: source.id, Incarnation: source.incarnation, Name: source.name}, | ||
| 180 | TabID: "source-tab", PaneID: "source-pane", | 188 | TabID: "source-tab", PaneID: "source-pane", | ||
| 181 | })) | 189 | })) | ||
| 182 | - | clk := &signalClock{timers: make(chan *signalTimer, 1)} | 190 | + | for len(clk.timers) > 0 { |
| 183 | - | d.clock = clk | 191 | + | <-clk.timers |
| 192 | + | } | |||
| 184 | d.handlePickerInput(ac, []byte("\x1b")) | 193 | d.handlePickerInput(ac, []byte("\x1b")) | ||
| 185 | timer := <-clk.timers | 194 | timer := <-clk.timers | ||
| 186 | timer.ch <- time.Now() | 195 | timer.ch <- time.Now() | ||
M internal/usecase/daemon/move_resize_fences.go +1 -1
| @@ -181,8 +181,8 @@ func (d *Daemon) publishResizeOwnerPostEffect(members []resizeMember, effect res | |||
| 181 | 181 | if fences == nil { | |
| 182 | 182 | return false | |
| 183 | 183 | } | |
| 184 | + | defer fences.Release() | |
| 184 | 185 | publish() | |
| 185 | - | fences.Release() | |
| 186 | 186 | return true | |
| 187 | 187 | } | |
| 188 | 188 | ||
| @@ -181,8 +181,8 @@ func (d *Daemon) publishResizeOwnerPostEffect(members []resizeMember, effect res | |||||
| 181 | if fences == nil { | 181 | if fences == nil { | ||
| 182 | return false | 182 | return false | ||
| 183 | } | 183 | } | ||
| 184 | + | defer fences.Release() | |||
| 184 | publish() | 185 | publish() | ||
| 185 | - | fences.Release() | |||
| 186 | return true | 186 | return true | ||
| 187 | } | 187 | } | ||
| 188 | 188 | ||||
M internal/usecase/daemon/move_resize_fences_test.go +29 -0
| @@ -185,6 +185,35 @@ func TestMoveResizeFencesCleanUpWhenPublicationPanics(t *testing.T) { | |||
| 185 | 185 | } | |
| 186 | 186 | } | |
| 187 | 187 | ||
| 188 | + | func TestPublishResizeOwnerPostEffectReleasesAllFencesWhenCallbackPanics(t *testing.T) { | |
| 189 | + | d := &Daemon{} | |
| 190 | + | sess := &session{id: "session"} | |
| 191 | + | tb := &tab{stableID: "tab"} | |
| 192 | + | p := &pane{stableID: "pane"} | |
| 193 | + | members := []resizeMember{{session: sess, tab: tb, pane: p}} | |
| 194 | + | ||
| 195 | + | var recovered any | |
| 196 | + | func() { | |
| 197 | + | defer func() { recovered = recover() }() | |
| 198 | + | d.publishResizeOwnerPostEffect(members, resizeOwnerPostSnapshotDirty, func() { | |
| 199 | + | panic("post effect failed") | |
| 200 | + | }) | |
| 201 | + | }() | |
| 202 | + | require.Equal(t, "post effect failed", recovered) | |
| 203 | + | ||
| 204 | + | for _, fence := range []struct { | |
| 205 | + | name string | |
| 206 | + | mu *sync.Mutex | |
| 207 | + | }{ | |
| 208 | + | {name: "session", mu: &sess.layoutApplyMu}, | |
| 209 | + | {name: "tab", mu: &tb.layoutApplyMu}, | |
| 210 | + | {name: "pane", mu: &p.resizeMu}, | |
| 211 | + | } { | |
| 212 | + | require.True(t, fence.mu.TryLock(), "%s fence leaked after panic recovery", fence.name) | |
| 213 | + | fence.mu.Unlock() | |
| 214 | + | } | |
| 215 | + | } | |
| 216 | + | ||
| 188 | 217 | func TestMoveResizeFencesWaitBeforeTakingArchitectureLocks(t *testing.T) { | |
| 189 | 218 | d := &Daemon{} | |
| 190 | 219 | sess := &session{id: "session"} | |
| @@ -185,6 +185,35 @@ func TestMoveResizeFencesCleanUpWhenPublicationPanics(t *testing.T) { | |||||
| 185 | } | 185 | } | ||
| 186 | } | 186 | } | ||
| 187 | 187 | ||||
| 188 | + | func TestPublishResizeOwnerPostEffectReleasesAllFencesWhenCallbackPanics(t *testing.T) { | |||
| 189 | + | d := &Daemon{} | |||
| 190 | + | sess := &session{id: "session"} | |||
| 191 | + | tb := &tab{stableID: "tab"} | |||
| 192 | + | p := &pane{stableID: "pane"} | |||
| 193 | + | members := []resizeMember{{session: sess, tab: tb, pane: p}} | |||
| 194 | + | ||||
| 195 | + | var recovered any | |||
| 196 | + | func() { | |||
| 197 | + | defer func() { recovered = recover() }() | |||
| 198 | + | d.publishResizeOwnerPostEffect(members, resizeOwnerPostSnapshotDirty, func() { | |||
| 199 | + | panic("post effect failed") | |||
| 200 | + | }) | |||
| 201 | + | }() | |||
| 202 | + | require.Equal(t, "post effect failed", recovered) | |||
| 203 | + | ||||
| 204 | + | for _, fence := range []struct { | |||
| 205 | + | name string | |||
| 206 | + | mu *sync.Mutex | |||
| 207 | + | }{ | |||
| 208 | + | {name: "session", mu: &sess.layoutApplyMu}, | |||
| 209 | + | {name: "tab", mu: &tb.layoutApplyMu}, | |||
| 210 | + | {name: "pane", mu: &p.resizeMu}, | |||
| 211 | + | } { | |||
| 212 | + | require.True(t, fence.mu.TryLock(), "%s fence leaked after panic recovery", fence.name) | |||
| 213 | + | fence.mu.Unlock() | |||
| 214 | + | } | |||
| 215 | + | } | |||
| 216 | + | ||||
| 188 | func TestMoveResizeFencesWaitBeforeTakingArchitectureLocks(t *testing.T) { | 217 | func TestMoveResizeFencesWaitBeforeTakingArchitectureLocks(t *testing.T) { | ||
| 189 | d := &Daemon{} | 218 | d := &Daemon{} | ||
| 190 | sess := &session{id: "session"} | 219 | sess := &session{id: "session"} | ||
M internal/usecase/daemon/move_stale_destination_test.go +1 -1
| @@ -40,7 +40,7 @@ func TestMovePaneRejectsStaleDestinationIncarnationWithoutMutation(t *testing.T) | |||
| 40 | 40 | Destination: moveSessionLocator{ID: destination.id, Incarnation: staleIncarnation}, | |
| 41 | 41 | DestinationTabID: domain.TabStableID(destinationTab.stableID), | |
| 42 | 42 | }) | |
| 43 | - | require.ErrorIs(t, err, errMovePaneInvalid) | |
| 43 | + | require.ErrorIs(t, err, errMoveStaleTarget) | |
| 44 | 44 | require.Equal(t, beforeSource, sourceTab.tree) | |
| 45 | 45 | require.Equal(t, beforeDestination, destinationTab.tree) | |
| 46 | 46 | require.Same(t, beforeOwner, moved.ownerSnapshot()) | |
| @@ -40,7 +40,7 @@ func TestMovePaneRejectsStaleDestinationIncarnationWithoutMutation(t *testing.T) | |||||
| 40 | Destination: moveSessionLocator{ID: destination.id, Incarnation: staleIncarnation}, | 40 | Destination: moveSessionLocator{ID: destination.id, Incarnation: staleIncarnation}, | ||
| 41 | DestinationTabID: domain.TabStableID(destinationTab.stableID), | 41 | DestinationTabID: domain.TabStableID(destinationTab.stableID), | ||
| 42 | }) | 42 | }) | ||
| 43 | - | require.ErrorIs(t, err, errMovePaneInvalid) | 43 | + | require.ErrorIs(t, err, errMoveStaleTarget) |
| 44 | require.Equal(t, beforeSource, sourceTab.tree) | 44 | require.Equal(t, beforeSource, sourceTab.tree) | ||
| 45 | require.Equal(t, beforeDestination, destinationTab.tree) | 45 | require.Equal(t, beforeDestination, destinationTab.tree) | ||
| 46 | require.Same(t, beforeOwner, moved.ownerSnapshot()) | 46 | require.Same(t, beforeOwner, moved.ownerSnapshot()) | ||
M internal/usecase/daemon/palette.go +7 -7
| @@ -324,23 +324,23 @@ func (d *Daemon) handlePaletteInput(ac *attachedClient, data []byte, effects ... | |||
| 324 | 324 | sess.dispatchMu.Lock() | |
| 325 | 325 | err := cmd.Run(paletteExec{d: d, sess: sess, ac: ac, effect: effect}, args) | |
| 326 | 326 | sess.dispatchMu.Unlock() | |
| 327 | - | if errors.Is(err, errAttachmentTransition) { | |
| 328 | - | return | |
| 329 | - | } | |
| 330 | - | if err == nil && roleHandoff { | |
| 327 | + | if roleHandoff { | |
| 331 | 328 | if current := ac.currentSession(); current != nil { | |
| 332 | 329 | currentToken := current.attachmentToken(ac, ac.transport()) | |
| 333 | 330 | fresh, admitted := ac.beginRoleEffect(currentToken) | |
| 331 | + | if ac.closeExecutedPalette(generation, rawQuery) { | |
| 332 | + | d.invalidateRender(current, ac, true, "palette.go") | |
| 333 | + | } | |
| 334 | 334 | if admitted { | |
| 335 | - | if ac.closeExecutedPalette(generation, rawQuery) { | |
| 336 | - | d.invalidateRender(current, ac, true, "palette.go") | |
| 337 | - | } | |
| 338 | 335 | fresh.End() | |
| 339 | 336 | } | |
| 340 | 337 | } else { | |
| 341 | 338 | ac.closeExecutedPalette(generation, rawQuery) | |
| 342 | 339 | } | |
| 343 | 340 | } | |
| 341 | + | if errors.Is(err, errAttachmentTransition) { | |
| 342 | + | return | |
| 343 | + | } | |
| 344 | 344 | if err != nil { | |
| 345 | 345 | d.log.Error("palette command failed", "err", err, "command", cmd.Code) | |
| 346 | 346 | d.reportError(sess, paletteCommandNoticeError(cmd, err)) | |
| @@ -324,23 +324,23 @@ func (d *Daemon) handlePaletteInput(ac *attachedClient, data []byte, effects ... | |||||
| 324 | sess.dispatchMu.Lock() | 324 | sess.dispatchMu.Lock() | ||
| 325 | err := cmd.Run(paletteExec{d: d, sess: sess, ac: ac, effect: effect}, args) | 325 | err := cmd.Run(paletteExec{d: d, sess: sess, ac: ac, effect: effect}, args) | ||
| 326 | sess.dispatchMu.Unlock() | 326 | sess.dispatchMu.Unlock() | ||
| 327 | - | if errors.Is(err, errAttachmentTransition) { | 327 | + | if roleHandoff { |
| 328 | - | return | |||
| 329 | - | } | |||
| 330 | - | if err == nil && roleHandoff { | |||
| 331 | if current := ac.currentSession(); current != nil { | 328 | if current := ac.currentSession(); current != nil { | ||
| 332 | currentToken := current.attachmentToken(ac, ac.transport()) | 329 | currentToken := current.attachmentToken(ac, ac.transport()) | ||
| 333 | fresh, admitted := ac.beginRoleEffect(currentToken) | 330 | fresh, admitted := ac.beginRoleEffect(currentToken) | ||
| 331 | + | if ac.closeExecutedPalette(generation, rawQuery) { | |||
| 332 | + | d.invalidateRender(current, ac, true, "palette.go") | |||
| 333 | + | } | |||
| 334 | if admitted { | 334 | if admitted { | ||
| 335 | - | if ac.closeExecutedPalette(generation, rawQuery) { | |||
| 336 | - | d.invalidateRender(current, ac, true, "palette.go") | |||
| 337 | - | } | |||
| 338 | fresh.End() | 335 | fresh.End() | ||
| 339 | } | 336 | } | ||
| 340 | } else { | 337 | } else { | ||
| 341 | ac.closeExecutedPalette(generation, rawQuery) | 338 | ac.closeExecutedPalette(generation, rawQuery) | ||
| 342 | } | 339 | } | ||
| 343 | } | 340 | } | ||
| 341 | + | if errors.Is(err, errAttachmentTransition) { | |||
| 342 | + | return | |||
| 343 | + | } | |||
| 344 | if err != nil { | 344 | if err != nil { | ||
| 345 | d.log.Error("palette command failed", "err", err, "command", cmd.Code) | 345 | d.log.Error("palette command failed", "err", err, "command", cmd.Code) | ||
| 346 | d.reportError(sess, paletteCommandNoticeError(cmd, err)) | 346 | d.reportError(sess, paletteCommandNoticeError(cmd, err)) | ||
M internal/usecase/daemon/palette_test.go +52 -0
| @@ -453,6 +453,58 @@ func TestPaletteJRSThenBSKReversesJump(t *testing.T) { | |||
| 453 | 453 | require.Same(t, current, ac.currentSession()) | |
| 454 | 454 | } | |
| 455 | 455 | ||
| 456 | + | func TestPaletteFailedRoleHandoffClosesExecutedInteraction(t *testing.T) { | |
| 457 | + | d, current, ac, _, releases := newRecentNavigationTestSessions(t) | |
| 458 | + | defer releaseAll(releases) | |
| 459 | + | ||
| 460 | + | // Establish a valid back-session target, then remove that target immediately | |
| 461 | + | // after the command releases its role admission. | |
| 462 | + | target := d.sessions[domain.SessionID("recent")] | |
| 463 | + | ac.previousSession.Set(target) | |
| 464 | + | invalidations := installPaletteInvalidationObserver(current) | |
| 465 | + | d.enterPalette(current, ac) | |
| 466 | + | d.handlePaletteInput(ac, []byte("BSK")) | |
| 467 | + | d.afterActionRoleEffectEnded = func(action string) { | |
| 468 | + | if action == "back-session" { | |
| 469 | + | d.mu.Lock() | |
| 470 | + | delete(d.sessions, target.id) | |
| 471 | + | d.mu.Unlock() | |
| 472 | + | } | |
| 473 | + | } | |
| 474 | + | ||
| 475 | + | token := current.attachmentToken(ac, ac.transport()) | |
| 476 | + | effect, admitted := ac.beginRoleEffect(token) | |
| 477 | + | require.True(t, admitted) | |
| 478 | + | d.handlePaletteInput(ac, []byte("\r"), effect) | |
| 479 | + | ||
| 480 | + | require.False(t, ac.overlays.paletteActive(), "a failed executed handoff must not leave a stale palette") | |
| 481 | + | require.Same(t, current, ac.currentSession(), "failed handoff must preserve the source attachment") | |
| 482 | + | invalidation := awaitTestValue(t, invalidations, "failed palette handoff did not invalidate rendering") | |
| 483 | + | require.True(t, invalidation.reset) | |
| 484 | + | } | |
| 485 | + | ||
| 486 | + | func TestPaletteDeniedPostHandoffRoleEffectClosesAndInvalidates(t *testing.T) { | |
| 487 | + | p, release := newBlockingPTY(t) | |
| 488 | + | defer release() | |
| 489 | + | d, sess, ac, _ := newManualSessionWithPTYs(t, p) | |
| 490 | + | invalidations := installPaletteInvalidationObserver(sess) | |
| 491 | + | ||
| 492 | + | d.enterPalette(sess, ac) | |
| 493 | + | d.handlePaletteInput(ac, []byte("BSK")) | |
| 494 | + | // Make the attachment token detached while retaining currentSession. The | |
| 495 | + | // no-op BSK command succeeds, but its post-execution beginRoleEffect is | |
| 496 | + | // deterministically denied. | |
| 497 | + | sess.mu.Lock() | |
| 498 | + | sess.client = nil | |
| 499 | + | sess.mu.Unlock() | |
| 500 | + | ||
| 501 | + | d.handlePaletteInput(ac, []byte("\r")) | |
| 502 | + | ||
| 503 | + | require.False(t, ac.overlays.paletteActive(), "denied cleanup admission must still close the executed palette") | |
| 504 | + | invalidation := awaitTestValue(t, invalidations, "denied palette cleanup did not invalidate rendering") | |
| 505 | + | require.True(t, invalidation.reset) | |
| 506 | + | } | |
| 507 | + | ||
| 456 | 508 | func TestPaletteJRSDisplacedTargetKeepsInteractionOpen(t *testing.T) { | |
| 457 | 509 | p, release := newBlockingPTY(t) | |
| 458 | 510 | defer release() | |
| @@ -453,6 +453,58 @@ func TestPaletteJRSThenBSKReversesJump(t *testing.T) { | |||||
| 453 | require.Same(t, current, ac.currentSession()) | 453 | require.Same(t, current, ac.currentSession()) | ||
| 454 | } | 454 | } | ||
| 455 | 455 | ||||
| 456 | + | func TestPaletteFailedRoleHandoffClosesExecutedInteraction(t *testing.T) { | |||
| 457 | + | d, current, ac, _, releases := newRecentNavigationTestSessions(t) | |||
| 458 | + | defer releaseAll(releases) | |||
| 459 | + | ||||
| 460 | + | // Establish a valid back-session target, then remove that target immediately | |||
| 461 | + | // after the command releases its role admission. | |||
| 462 | + | target := d.sessions[domain.SessionID("recent")] | |||
| 463 | + | ac.previousSession.Set(target) | |||
| 464 | + | invalidations := installPaletteInvalidationObserver(current) | |||
| 465 | + | d.enterPalette(current, ac) | |||
| 466 | + | d.handlePaletteInput(ac, []byte("BSK")) | |||
| 467 | + | d.afterActionRoleEffectEnded = func(action string) { | |||
| 468 | + | if action == "back-session" { | |||
| 469 | + | d.mu.Lock() | |||
| 470 | + | delete(d.sessions, target.id) | |||
| 471 | + | d.mu.Unlock() | |||
| 472 | + | } | |||
| 473 | + | } | |||
| 474 | + | ||||
| 475 | + | token := current.attachmentToken(ac, ac.transport()) | |||
| 476 | + | effect, admitted := ac.beginRoleEffect(token) | |||
| 477 | + | require.True(t, admitted) | |||
| 478 | + | d.handlePaletteInput(ac, []byte("\r"), effect) | |||
| 479 | + | ||||
| 480 | + | require.False(t, ac.overlays.paletteActive(), "a failed executed handoff must not leave a stale palette") | |||
| 481 | + | require.Same(t, current, ac.currentSession(), "failed handoff must preserve the source attachment") | |||
| 482 | + | invalidation := awaitTestValue(t, invalidations, "failed palette handoff did not invalidate rendering") | |||
| 483 | + | require.True(t, invalidation.reset) | |||
| 484 | + | } | |||
| 485 | + | ||||
| 486 | + | func TestPaletteDeniedPostHandoffRoleEffectClosesAndInvalidates(t *testing.T) { | |||
| 487 | + | p, release := newBlockingPTY(t) | |||
| 488 | + | defer release() | |||
| 489 | + | d, sess, ac, _ := newManualSessionWithPTYs(t, p) | |||
| 490 | + | invalidations := installPaletteInvalidationObserver(sess) | |||
| 491 | + | ||||
| 492 | + | d.enterPalette(sess, ac) | |||
| 493 | + | d.handlePaletteInput(ac, []byte("BSK")) | |||
| 494 | + | // Make the attachment token detached while retaining currentSession. The | |||
| 495 | + | // no-op BSK command succeeds, but its post-execution beginRoleEffect is | |||
| 496 | + | // deterministically denied. | |||
| 497 | + | sess.mu.Lock() | |||
| 498 | + | sess.client = nil | |||
| 499 | + | sess.mu.Unlock() | |||
| 500 | + | ||||
| 501 | + | d.handlePaletteInput(ac, []byte("\r")) | |||
| 502 | + | ||||
| 503 | + | require.False(t, ac.overlays.paletteActive(), "denied cleanup admission must still close the executed palette") | |||
| 504 | + | invalidation := awaitTestValue(t, invalidations, "denied palette cleanup did not invalidate rendering") | |||
| 505 | + | require.True(t, invalidation.reset) | |||
| 506 | + | } | |||
| 507 | + | ||||
| 456 | func TestPaletteJRSDisplacedTargetKeepsInteractionOpen(t *testing.T) { | 508 | func TestPaletteJRSDisplacedTargetKeepsInteractionOpen(t *testing.T) { | ||
| 457 | p, release := newBlockingPTY(t) | 509 | p, release := newBlockingPTY(t) | ||
| 458 | defer release() | 510 | defer release() | ||
M internal/usecase/daemon/picker.go +8 -2
| @@ -194,9 +194,15 @@ func (d *Daemon) handlePickerInput(ac *attachedClient, data []byte, effects ...* | |||
| 194 | 194 | if len(effects) != 0 { | |
| 195 | 195 | effect = effects[0] | |
| 196 | 196 | } | |
| 197 | - | effect.bindActionEnd(d, "picker-delete") | |
| 198 | 197 | if ok { | |
| 199 | - | if err := d.killPickerTargetForRole(target, effect.roleToken()); err != nil { | |
| 198 | + | var err error | |
| 199 | + | if effect != nil { | |
| 200 | + | effect.bindActionEnd(d, "picker-delete") | |
| 201 | + | err = d.killPickerTargetForRole(target, effect.roleToken()) | |
| 202 | + | } else { | |
| 203 | + | err = d.killPickerTarget(target) | |
| 204 | + | } | |
| 205 | + | if err != nil { | |
| 200 | 206 | d.reportError(sess, err) | |
| 201 | 207 | } | |
| 202 | 208 | } | |
| @@ -194,9 +194,15 @@ func (d *Daemon) handlePickerInput(ac *attachedClient, data []byte, effects ...* | |||||
| 194 | if len(effects) != 0 { | 194 | if len(effects) != 0 { | ||
| 195 | effect = effects[0] | 195 | effect = effects[0] | ||
| 196 | } | 196 | } | ||
| 197 | - | effect.bindActionEnd(d, "picker-delete") | |||
| 198 | if ok { | 197 | if ok { | ||
| 199 | - | if err := d.killPickerTargetForRole(target, effect.roleToken()); err != nil { | 198 | + | var err error |
| 199 | + | if effect != nil { | |||
| 200 | + | effect.bindActionEnd(d, "picker-delete") | |||
| 201 | + | err = d.killPickerTargetForRole(target, effect.roleToken()) | |||
| 202 | + | } else { | |||
| 203 | + | err = d.killPickerTarget(target) | |||
| 204 | + | } | |||
| 205 | + | if err != nil { | |||
| 200 | d.reportError(sess, err) | 206 | d.reportError(sess, err) | ||
| 201 | } | 207 | } | ||
| 202 | } | 208 | } | ||
M internal/usecase/daemon/picker_test.go +44 -0
| @@ -1492,6 +1492,11 @@ func TestPickerNavigationRefreshAfterSuccessfulDeleteSelectsAttachedActiveStable | |||
| 1492 | 1492 | ||
| 1493 | 1493 | d.handlePickerInput(ac, []byte("x")) | |
| 1494 | 1494 | ||
| 1495 | + | d.mu.Lock() | |
| 1496 | + | _, targetStillLive := d.sessions[target.id] | |
| 1497 | + | d.mu.Unlock() | |
| 1498 | + | require.False(t, targetStillLive, "ordinary picker deletion must remove the selected session") | |
| 1499 | + | ||
| 1495 | 1500 | ac.overlays.pickerMu.Lock() | |
| 1496 | 1501 | selected, ok = ac.overlays.picker.Selected() | |
| 1497 | 1502 | ac.overlays.pickerMu.Unlock() | |
| @@ -1501,6 +1506,45 @@ func TestPickerNavigationRefreshAfterSuccessfulDeleteSelectsAttachedActiveStable | |||
| 1501 | 1506 | require.Equal(t, 1, selected.TabIndex) | |
| 1502 | 1507 | } | |
| 1503 | 1508 | ||
| 1509 | + | func TestPickerRoleEffectDeleteRemovesSelectedSessionAndRefreshes(t *testing.T) { | |
| 1510 | + | d, current, ac, _, currentReleases := newManualTabSession(t, 1) | |
| 1511 | + | defer releaseAll(currentReleases) | |
| 1512 | + | current.id, current.name = "current", "z-current" | |
| 1513 | + | delete(d.sessions, domain.SessionID("manual")) | |
| 1514 | + | d.sessions[current.id] = current | |
| 1515 | + | ||
| 1516 | + | targetPTY, releaseTarget := newBlockingPTY(t) | |
| 1517 | + | defer releaseTarget() | |
| 1518 | + | targetTab := newTestTabWithContext(targetPTY, current.ctx, current.cancel) | |
| 1519 | + | target := &session{id: "target", name: "a-target", ephemeral: true, ctx: current.ctx, cancel: func() {}, tabs: []*tab{targetTab}} | |
| 1520 | + | d.sessions[target.id] = target | |
| 1521 | + | current.mu.Lock() | |
| 1522 | + | current.client = ac | |
| 1523 | + | current.mu.Unlock() | |
| 1524 | + | require.NotNil(t, d.attachCoordinator(current, nil, ac, true)) | |
| 1525 | + | ||
| 1526 | + | d.enterPicker(current, ac) | |
| 1527 | + | d.handlePickerInput(ac, []byte("k")) | |
| 1528 | + | ac.overlays.pickerMu.Lock() | |
| 1529 | + | selected, ok := ac.overlays.picker.Selected() | |
| 1530 | + | ac.overlays.pickerMu.Unlock() | |
| 1531 | + | require.True(t, ok) | |
| 1532 | + | require.Equal(t, target.id, selected.Session) | |
| 1533 | + | ||
| 1534 | + | token := current.attachmentToken(ac, ac.transport()) | |
| 1535 | + | token.lease = current.renderCoordinator().attachmentLease(ac) | |
| 1536 | + | ac.publishRoleCapability(token) | |
| 1537 | + | effect, admitted := ac.beginRoleEffect(token) | |
| 1538 | + | require.True(t, admitted) | |
| 1539 | + | d.handlePickerInput(ac, []byte("x"), effect) | |
| 1540 | + | ||
| 1541 | + | d.mu.Lock() | |
| 1542 | + | _, targetStillLive := d.sessions[target.id] | |
| 1543 | + | d.mu.Unlock() | |
| 1544 | + | require.False(t, targetStillLive, "effect-backed picker deletion must remove the selected session") | |
| 1545 | + | require.True(t, ac.overlays.pickerActive(), "successful deletion refreshes rather than closes the picker") | |
| 1546 | + | } | |
| 1547 | + | ||
| 1504 | 1548 | func TestPickerKillActiveSessionSnapshotDeleteRefusalReportsOnceAndKeepsPicker(t *testing.T) { | |
| 1505 | 1549 | d, from, ac, sends, releases := newRecentNavigationTestSessions(t) | |
| 1506 | 1550 | defer releaseAll(releases) | |
| @@ -1492,6 +1492,11 @@ func TestPickerNavigationRefreshAfterSuccessfulDeleteSelectsAttachedActiveStable | |||||
| 1492 | 1492 | ||||
| 1493 | d.handlePickerInput(ac, []byte("x")) | 1493 | d.handlePickerInput(ac, []byte("x")) | ||
| 1494 | 1494 | ||||
| 1495 | + | d.mu.Lock() | |||
| 1496 | + | _, targetStillLive := d.sessions[target.id] | |||
| 1497 | + | d.mu.Unlock() | |||
| 1498 | + | require.False(t, targetStillLive, "ordinary picker deletion must remove the selected session") | |||
| 1499 | + | ||||
| 1495 | ac.overlays.pickerMu.Lock() | 1500 | ac.overlays.pickerMu.Lock() | ||
| 1496 | selected, ok = ac.overlays.picker.Selected() | 1501 | selected, ok = ac.overlays.picker.Selected() | ||
| 1497 | ac.overlays.pickerMu.Unlock() | 1502 | ac.overlays.pickerMu.Unlock() | ||
| @@ -1501,6 +1506,45 @@ func TestPickerNavigationRefreshAfterSuccessfulDeleteSelectsAttachedActiveStable | |||||
| 1501 | require.Equal(t, 1, selected.TabIndex) | 1506 | require.Equal(t, 1, selected.TabIndex) | ||
| 1502 | } | 1507 | } | ||
| 1503 | 1508 | ||||
| 1509 | + | func TestPickerRoleEffectDeleteRemovesSelectedSessionAndRefreshes(t *testing.T) { | |||
| 1510 | + | d, current, ac, _, currentReleases := newManualTabSession(t, 1) | |||
| 1511 | + | defer releaseAll(currentReleases) | |||
| 1512 | + | current.id, current.name = "current", "z-current" | |||
| 1513 | + | delete(d.sessions, domain.SessionID("manual")) | |||
| 1514 | + | d.sessions[current.id] = current | |||
| 1515 | + | ||||
| 1516 | + | targetPTY, releaseTarget := newBlockingPTY(t) | |||
| 1517 | + | defer releaseTarget() | |||
| 1518 | + | targetTab := newTestTabWithContext(targetPTY, current.ctx, current.cancel) | |||
| 1519 | + | target := &session{id: "target", name: "a-target", ephemeral: true, ctx: current.ctx, cancel: func() {}, tabs: []*tab{targetTab}} | |||
| 1520 | + | d.sessions[target.id] = target | |||
| 1521 | + | current.mu.Lock() | |||
| 1522 | + | current.client = ac | |||
| 1523 | + | current.mu.Unlock() | |||
| 1524 | + | require.NotNil(t, d.attachCoordinator(current, nil, ac, true)) | |||
| 1525 | + | ||||
| 1526 | + | d.enterPicker(current, ac) | |||
| 1527 | + | d.handlePickerInput(ac, []byte("k")) | |||
| 1528 | + | ac.overlays.pickerMu.Lock() | |||
| 1529 | + | selected, ok := ac.overlays.picker.Selected() | |||
| 1530 | + | ac.overlays.pickerMu.Unlock() | |||
| 1531 | + | require.True(t, ok) | |||
| 1532 | + | require.Equal(t, target.id, selected.Session) | |||
| 1533 | + | ||||
| 1534 | + | token := current.attachmentToken(ac, ac.transport()) | |||
| 1535 | + | token.lease = current.renderCoordinator().attachmentLease(ac) | |||
| 1536 | + | ac.publishRoleCapability(token) | |||
| 1537 | + | effect, admitted := ac.beginRoleEffect(token) | |||
| 1538 | + | require.True(t, admitted) | |||
| 1539 | + | d.handlePickerInput(ac, []byte("x"), effect) | |||
| 1540 | + | ||||
| 1541 | + | d.mu.Lock() | |||
| 1542 | + | _, targetStillLive := d.sessions[target.id] | |||
| 1543 | + | d.mu.Unlock() | |||
| 1544 | + | require.False(t, targetStillLive, "effect-backed picker deletion must remove the selected session") | |||
| 1545 | + | require.True(t, ac.overlays.pickerActive(), "successful deletion refreshes rather than closes the picker") | |||
| 1546 | + | } | |||
| 1547 | + | ||||
| 1504 | func TestPickerKillActiveSessionSnapshotDeleteRefusalReportsOnceAndKeepsPicker(t *testing.T) { | 1548 | func TestPickerKillActiveSessionSnapshotDeleteRefusalReportsOnceAndKeepsPicker(t *testing.T) { | ||
| 1505 | d, from, ac, sends, releases := newRecentNavigationTestSessions(t) | 1549 | d, from, ac, sends, releases := newRecentNavigationTestSessions(t) | ||
| 1506 | defer releaseAll(releases) | 1550 | defer releaseAll(releases) | ||
M internal/usecase/daemon/resume_test.go +52 -0
| @@ -51,6 +51,40 @@ func (t *closeTrackingTransport) Sends() []ports.Frame { | |||
| 51 | 51 | return append([]ports.Frame(nil), t.sends...) | |
| 52 | 52 | } | |
| 53 | 53 | ||
| 54 | + | type closeCountingBlockedTransport struct { | |
| 55 | + | closeOnce sync.Once | |
| 56 | + | closed chan struct{} | |
| 57 | + | count int | |
| 58 | + | mu sync.Mutex | |
| 59 | + | } | |
| 60 | + | ||
| 61 | + | func newCloseCountingBlockedTransport() *closeCountingBlockedTransport { | |
| 62 | + | return &closeCountingBlockedTransport{closed: make(chan struct{})} | |
| 63 | + | } | |
| 64 | + | ||
| 65 | + | func (t *closeCountingBlockedTransport) Send(ports.Frame) error { | |
| 66 | + | <-t.closed | |
| 67 | + | return errors.New("closed") | |
| 68 | + | } | |
| 69 | + | ||
| 70 | + | func (t *closeCountingBlockedTransport) Recv() (ports.Frame, error) { | |
| 71 | + | return ports.Frame{}, errors.New("closed") | |
| 72 | + | } | |
| 73 | + | ||
| 74 | + | func (t *closeCountingBlockedTransport) Close() error { | |
| 75 | + | t.mu.Lock() | |
| 76 | + | t.count++ | |
| 77 | + | t.mu.Unlock() | |
| 78 | + | t.closeOnce.Do(func() { close(t.closed) }) | |
| 79 | + | return nil | |
| 80 | + | } | |
| 81 | + | ||
| 82 | + | func (t *closeCountingBlockedTransport) CloseCount() int { | |
| 83 | + | t.mu.Lock() | |
| 84 | + | defer t.mu.Unlock() | |
| 85 | + | return t.count | |
| 86 | + | } | |
| 87 | + | ||
| 54 | 88 | func helloResumeCapable(intent uint8, name string, token uint64) ports.Hello { | |
| 55 | 89 | return ports.Hello{ | |
| 56 | 90 | Version: ports.ProtocolVersion, | |
| @@ -139,6 +173,24 @@ func TestBoundedSendTimeoutCannotTargetResumedTransport(t *testing.T) { | |||
| 139 | 173 | require.False(t, newTransport.Closed(), "orphaned send must not close the resumed transport") | |
| 140 | 174 | } | |
| 141 | 175 | ||
| 176 | + | func TestDetachedNotificationTimeoutClosesCapturedTransportOnce(t *testing.T) { | |
| 177 | + | clock := &signalClock{timers: make(chan *signalTimer, 1)} | |
| 178 | + | d := newTestDaemon(t, nil, clock) | |
| 179 | + | tr := newCloseCountingBlockedTransport() | |
| 180 | + | ac := &attachedClient{tr: tr, output: newOutputStateStream()} | |
| 181 | + | ||
| 182 | + | d.notifyDetachedSnapshotAsync(detachedAttachmentSnapshot{ | |
| 183 | + | ac: ac, | |
| 184 | + | transport: ac.transportSnapshot(), | |
| 185 | + | }, ports.ReasonDetach) | |
| 186 | + | timer := <-clock.timers | |
| 187 | + | timer.ch <- time.Time{} | |
| 188 | + | d.waitNotifies() | |
| 189 | + | ||
| 190 | + | require.Equal(t, 1, tr.CloseCount(), "timeout revocation must close the captured transport at most once") | |
| 191 | + | require.Nil(t, ac.transport(), "timeout cleanup must revoke the captured transport") | |
| 192 | + | } | |
| 193 | + | ||
| 142 | 194 | func TestHandleHelloResumeDefersFreshOutputUntilWelcome(t *testing.T) { | |
| 143 | 195 | pty, releasePTY := newBlockingPTY(t) | |
| 144 | 196 | defer releasePTY() | |
| @@ -51,6 +51,40 @@ func (t *closeTrackingTransport) Sends() []ports.Frame { | |||||
| 51 | return append([]ports.Frame(nil), t.sends...) | 51 | return append([]ports.Frame(nil), t.sends...) | ||
| 52 | } | 52 | } | ||
| 53 | 53 | ||||
| 54 | + | type closeCountingBlockedTransport struct { | |||
| 55 | + | closeOnce sync.Once | |||
| 56 | + | closed chan struct{} | |||
| 57 | + | count int | |||
| 58 | + | mu sync.Mutex | |||
| 59 | + | } | |||
| 60 | + | ||||
| 61 | + | func newCloseCountingBlockedTransport() *closeCountingBlockedTransport { | |||
| 62 | + | return &closeCountingBlockedTransport{closed: make(chan struct{})} | |||
| 63 | + | } | |||
| 64 | + | ||||
| 65 | + | func (t *closeCountingBlockedTransport) Send(ports.Frame) error { | |||
| 66 | + | <-t.closed | |||
| 67 | + | return errors.New("closed") | |||
| 68 | + | } | |||
| 69 | + | ||||
| 70 | + | func (t *closeCountingBlockedTransport) Recv() (ports.Frame, error) { | |||
| 71 | + | return ports.Frame{}, errors.New("closed") | |||
| 72 | + | } | |||
| 73 | + | ||||
| 74 | + | func (t *closeCountingBlockedTransport) Close() error { | |||
| 75 | + | t.mu.Lock() | |||
| 76 | + | t.count++ | |||
| 77 | + | t.mu.Unlock() | |||
| 78 | + | t.closeOnce.Do(func() { close(t.closed) }) | |||
| 79 | + | return nil | |||
| 80 | + | } | |||
| 81 | + | ||||
| 82 | + | func (t *closeCountingBlockedTransport) CloseCount() int { | |||
| 83 | + | t.mu.Lock() | |||
| 84 | + | defer t.mu.Unlock() | |||
| 85 | + | return t.count | |||
| 86 | + | } | |||
| 87 | + | ||||
| 54 | func helloResumeCapable(intent uint8, name string, token uint64) ports.Hello { | 88 | func helloResumeCapable(intent uint8, name string, token uint64) ports.Hello { | ||
| 55 | return ports.Hello{ | 89 | return ports.Hello{ | ||
| 56 | Version: ports.ProtocolVersion, | 90 | Version: ports.ProtocolVersion, | ||
| @@ -139,6 +173,24 @@ func TestBoundedSendTimeoutCannotTargetResumedTransport(t *testing.T) { | |||||
| 139 | require.False(t, newTransport.Closed(), "orphaned send must not close the resumed transport") | 173 | require.False(t, newTransport.Closed(), "orphaned send must not close the resumed transport") | ||
| 140 | } | 174 | } | ||
| 141 | 175 | ||||
| 176 | + | func TestDetachedNotificationTimeoutClosesCapturedTransportOnce(t *testing.T) { | |||
| 177 | + | clock := &signalClock{timers: make(chan *signalTimer, 1)} | |||
| 178 | + | d := newTestDaemon(t, nil, clock) | |||
| 179 | + | tr := newCloseCountingBlockedTransport() | |||
| 180 | + | ac := &attachedClient{tr: tr, output: newOutputStateStream()} | |||
| 181 | + | ||||
| 182 | + | d.notifyDetachedSnapshotAsync(detachedAttachmentSnapshot{ | |||
| 183 | + | ac: ac, | |||
| 184 | + | transport: ac.transportSnapshot(), | |||
| 185 | + | }, ports.ReasonDetach) | |||
| 186 | + | timer := <-clock.timers | |||
| 187 | + | timer.ch <- time.Time{} | |||
| 188 | + | d.waitNotifies() | |||
| 189 | + | ||||
| 190 | + | require.Equal(t, 1, tr.CloseCount(), "timeout revocation must close the captured transport at most once") | |||
| 191 | + | require.Nil(t, ac.transport(), "timeout cleanup must revoke the captured transport") | |||
| 192 | + | } | |||
| 193 | + | ||||
| 142 | func TestHandleHelloResumeDefersFreshOutputUntilWelcome(t *testing.T) { | 194 | func TestHandleHelloResumeDefersFreshOutputUntilWelcome(t *testing.T) { | ||
| 143 | pty, releasePTY := newBlockingPTY(t) | 195 | pty, releasePTY := newBlockingPTY(t) | ||
| 144 | defer releasePTY() | 196 | defer releasePTY() | ||
A internal/usecase/daemon/session_move_locator_test.go +44 -0
| @@ -0,0 +1,44 @@ | |||
| 1 | + | package daemon | |
| 2 | + | ||
| 3 | + | import ( | |
| 4 | + | "fmt" | |
| 5 | + | "sync" | |
| 6 | + | "testing" | |
| 7 | + | ||
| 8 | + | "github.com/stretchr/testify/require" | |
| 9 | + | ||
| 10 | + | "github.com/bnema/vev/internal/domain" | |
| 11 | + | ) | |
| 12 | + | ||
| 13 | + | func TestSessionMoveLocatorSynchronizesMutableNameRead(t *testing.T) { | |
| 14 | + | sess := &session{ | |
| 15 | + | id: domain.SessionID("session-id"), | |
| 16 | + | incarnation: domain.IncarnationID{1}, | |
| 17 | + | name: "initial", | |
| 18 | + | } | |
| 19 | + | start := make(chan struct{}) | |
| 20 | + | var wg sync.WaitGroup | |
| 21 | + | wg.Add(2) | |
| 22 | + | go func() { | |
| 23 | + | defer wg.Done() | |
| 24 | + | <-start | |
| 25 | + | for i := range 10_000 { | |
| 26 | + | sess.mu.Lock() | |
| 27 | + | sess.name = fmt.Sprintf("session-%d", i) | |
| 28 | + | sess.mu.Unlock() | |
| 29 | + | } | |
| 30 | + | }() | |
| 31 | + | go func() { | |
| 32 | + | defer wg.Done() | |
| 33 | + | <-start | |
| 34 | + | for range 10_000 { | |
| 35 | + | _ = sessionMoveLocator(sess) | |
| 36 | + | } | |
| 37 | + | }() | |
| 38 | + | close(start) | |
| 39 | + | wg.Wait() | |
| 40 | + | ||
| 41 | + | locator := sessionMoveLocator(sess) | |
| 42 | + | require.Equal(t, sess.id, locator.ID) | |
| 43 | + | require.Equal(t, sess.incarnation, locator.Incarnation) | |
| 44 | + | } | |
| @@ -0,0 +1,44 @@ | |||||
| 1 | + | package daemon | |||
| 2 | + | ||||
| 3 | + | import ( | |||
| 4 | + | "fmt" | |||
| 5 | + | "sync" | |||
| 6 | + | "testing" | |||
| 7 | + | ||||
| 8 | + | "github.com/stretchr/testify/require" | |||
| 9 | + | ||||
| 10 | + | "github.com/bnema/vev/internal/domain" | |||
| 11 | + | ) | |||
| 12 | + | ||||
| 13 | + | func TestSessionMoveLocatorSynchronizesMutableNameRead(t *testing.T) { | |||
| 14 | + | sess := &session{ | |||
| 15 | + | id: domain.SessionID("session-id"), | |||
| 16 | + | incarnation: domain.IncarnationID{1}, | |||
| 17 | + | name: "initial", | |||
| 18 | + | } | |||
| 19 | + | start := make(chan struct{}) | |||
| 20 | + | var wg sync.WaitGroup | |||
| 21 | + | wg.Add(2) | |||
| 22 | + | go func() { | |||
| 23 | + | defer wg.Done() | |||
| 24 | + | <-start | |||
| 25 | + | for i := range 10_000 { | |||
| 26 | + | sess.mu.Lock() | |||
| 27 | + | sess.name = fmt.Sprintf("session-%d", i) | |||
| 28 | + | sess.mu.Unlock() | |||
| 29 | + | } | |||
| 30 | + | }() | |||
| 31 | + | go func() { | |||
| 32 | + | defer wg.Done() | |||
| 33 | + | <-start | |||
| 34 | + | for range 10_000 { | |||
| 35 | + | _ = sessionMoveLocator(sess) | |||
| 36 | + | } | |||
| 37 | + | }() | |||
| 38 | + | close(start) | |||
| 39 | + | wg.Wait() | |||
| 40 | + | ||||
| 41 | + | locator := sessionMoveLocator(sess) | |||
| 42 | + | require.Equal(t, sess.id, locator.ID) | |||
| 43 | + | require.Equal(t, sess.incarnation, locator.Incarnation) | |||
| 44 | + | } | |||
M internal/usecase/daemon/snatched_client_test.go +7 -3
| @@ -1047,13 +1047,17 @@ func TestCrossSessionTransitionPublishesCoherentOwnershipBeforeBlockedFirstPaint | |||
| 1047 | 1047 | targetCoordinator.mu.Unlock() | |
| 1048 | 1048 | require.True(t, result.published.activeCurrent()) | |
| 1049 | 1049 | ||
| 1050 | - | paintStarted := make(chan struct{}) | |
| 1050 | + | beforeSendWait := make(chan struct{}) | |
| 1051 | + | d.beforeFirstPaintSendWait = func(token attachmentRoleToken) { | |
| 1052 | + | if token.ac == moving { | |
| 1053 | + | close(beforeSendWait) | |
| 1054 | + | } | |
| 1055 | + | } | |
| 1051 | 1056 | paintDone := make(chan bool, 1) | |
| 1052 | 1057 | go func() { | |
| 1053 | - | close(paintStarted) | |
| 1054 | 1058 | paintDone <- d.firstPaintForTransition(result.published) | |
| 1055 | 1059 | }() | |
| 1056 | - | <-paintStarted | |
| 1060 | + | <-beforeSendWait | |
| 1057 | 1061 | require.True(t, d.mu.TryLock(), "first paint retained daemon lock while waiting for sendMu") | |
| 1058 | 1062 | d.mu.Unlock() | |
| 1059 | 1063 | require.True(t, d.notices.routingMu.TryLock(), "first paint retained routing lock while waiting for sendMu") | |
| @@ -1047,13 +1047,17 @@ func TestCrossSessionTransitionPublishesCoherentOwnershipBeforeBlockedFirstPaint | |||||
| 1047 | targetCoordinator.mu.Unlock() | 1047 | targetCoordinator.mu.Unlock() | ||
| 1048 | require.True(t, result.published.activeCurrent()) | 1048 | require.True(t, result.published.activeCurrent()) | ||
| 1049 | 1049 | ||||
| 1050 | - | paintStarted := make(chan struct{}) | 1050 | + | beforeSendWait := make(chan struct{}) |
| 1051 | + | d.beforeFirstPaintSendWait = func(token attachmentRoleToken) { | |||
| 1052 | + | if token.ac == moving { | |||
| 1053 | + | close(beforeSendWait) | |||
| 1054 | + | } | |||
| 1055 | + | } | |||
| 1051 | paintDone := make(chan bool, 1) | 1056 | paintDone := make(chan bool, 1) | ||
| 1052 | go func() { | 1057 | go func() { | ||
| 1053 | - | close(paintStarted) | |||
| 1054 | paintDone <- d.firstPaintForTransition(result.published) | 1058 | paintDone <- d.firstPaintForTransition(result.published) | ||
| 1055 | }() | 1059 | }() | ||
| 1056 | - | <-paintStarted | 1060 | + | <-beforeSendWait |
| 1057 | require.True(t, d.mu.TryLock(), "first paint retained daemon lock while waiting for sendMu") | 1061 | require.True(t, d.mu.TryLock(), "first paint retained daemon lock while waiting for sendMu") | ||
| 1058 | d.mu.Unlock() | 1062 | d.mu.Unlock() | ||
| 1059 | require.True(t, d.notices.routingMu.TryLock(), "first paint retained routing lock while waiting for sendMu") | 1063 | require.True(t, d.notices.routingMu.TryLock(), "first paint retained routing lock while waiting for sendMu") | ||
M internal/usecase/daemon/stalled_attachment_test.go +9 -5
| @@ -52,12 +52,15 @@ func TestFinishAttachWaitsForAdmittedOldRender(t *testing.T) { | |||
| 52 | 52 | replacement := portsmocks.NewMockTransport(t) | |
| 53 | 53 | replacement.EXPECT().Send(mock.Anything).Return(nil).Maybe() | |
| 54 | 54 | replacement.EXPECT().Close().Return(nil).Maybe() | |
| 55 | - | attached := make(chan *attachedClient, 1) | |
| 55 | + | type attachResult struct { | |
| 56 | + | client *attachedClient | |
| 57 | + | err error | |
| 58 | + | } | |
| 59 | + | attached := make(chan attachResult, 1) | |
| 56 | 60 | go func() { | |
| 57 | 61 | d.mu.Lock() | |
| 58 | 62 | ac, err := d.finishAttach(sess, replacement, hello.Size, terminalEnv{}, hello) | |
| 59 | - | require.NoError(t, err) | |
| 60 | - | attached <- ac | |
| 63 | + | attached <- attachResult{client: ac, err: err} | |
| 61 | 64 | }() | |
| 62 | 65 | ||
| 63 | 66 | select { | |
| @@ -67,8 +70,9 @@ func TestFinishAttachWaitsForAdmittedOldRender(t *testing.T) { | |||
| 67 | 70 | } | |
| 68 | 71 | close(releaseOldSend) | |
| 69 | 72 | select { | |
| 70 | - | case ac := <-attached: | |
| 71 | - | require.Same(t, replacement, ac.transport()) | |
| 73 | + | case result := <-attached: | |
| 74 | + | require.NoError(t, result.err) | |
| 75 | + | require.Same(t, replacement, result.client.transport()) | |
| 72 | 76 | case <-time.After(time.Second): | |
| 73 | 77 | t.Fatal("replacement did not publish after the admitted render completed") | |
| 74 | 78 | } | |
| @@ -52,12 +52,15 @@ func TestFinishAttachWaitsForAdmittedOldRender(t *testing.T) { | |||||
| 52 | replacement := portsmocks.NewMockTransport(t) | 52 | replacement := portsmocks.NewMockTransport(t) | ||
| 53 | replacement.EXPECT().Send(mock.Anything).Return(nil).Maybe() | 53 | replacement.EXPECT().Send(mock.Anything).Return(nil).Maybe() | ||
| 54 | replacement.EXPECT().Close().Return(nil).Maybe() | 54 | replacement.EXPECT().Close().Return(nil).Maybe() | ||
| 55 | - | attached := make(chan *attachedClient, 1) | 55 | + | type attachResult struct { |
| 56 | + | client *attachedClient | |||
| 57 | + | err error | |||
| 58 | + | } | |||
| 59 | + | attached := make(chan attachResult, 1) | |||
| 56 | go func() { | 60 | go func() { | ||
| 57 | d.mu.Lock() | 61 | d.mu.Lock() | ||
| 58 | ac, err := d.finishAttach(sess, replacement, hello.Size, terminalEnv{}, hello) | 62 | ac, err := d.finishAttach(sess, replacement, hello.Size, terminalEnv{}, hello) | ||
| 59 | - | require.NoError(t, err) | 63 | + | attached <- attachResult{client: ac, err: err} |
| 60 | - | attached <- ac | |||
| 61 | }() | 64 | }() | ||
| 62 | 65 | ||||
| 63 | select { | 66 | select { | ||
| @@ -67,8 +70,9 @@ func TestFinishAttachWaitsForAdmittedOldRender(t *testing.T) { | |||||
| 67 | } | 70 | } | ||
| 68 | close(releaseOldSend) | 71 | close(releaseOldSend) | ||
| 69 | select { | 72 | select { | ||
| 70 | - | case ac := <-attached: | 73 | + | case result := <-attached: |
| 71 | - | require.Same(t, replacement, ac.transport()) | 74 | + | require.NoError(t, result.err) |
| 75 | + | require.Same(t, replacement, result.client.transport()) | |||
| 72 | case <-time.After(time.Second): | 76 | case <-time.After(time.Second): | ||
| 73 | t.Fatal("replacement did not publish after the admitted render completed") | 77 | t.Fatal("replacement did not publish after the admitted render completed") | ||
| 74 | } | 78 | } | ||