fix(daemon): harden pane resize retry transactions
- commit
- edadec9fd4096089b44303b956a65269e6fe143b
- author
- brice · · 2026-07-25 06:07 UTC
- parent
- 50d230a
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email
7 files · +317 -50
M internal/usecase/daemon/pane_actions.go +4 -5
| @@ -286,17 +286,16 @@ func (d *Daemon) focusDirAt(sess *session, tb *tab, target *pane, dir layout.Dir | |||
| 286 | 286 | tb.mu.Unlock() | |
| 287 | 287 | return layout.ErrNotFound | |
| 288 | 288 | } | |
| 289 | - | oldFocus := target.id | |
| 290 | 289 | candidate := tb.tree.Clone() | |
| 291 | - | candidate.Focus = oldFocus | |
| 290 | + | candidate.Focus = target.id | |
| 292 | 291 | err := candidate.FocusDir(dir, domain.Rect{Width: tb.size.Cols, Height: tb.size.Rows}) | |
| 293 | - | newFocus := candidate.Focus | |
| 294 | - | if err == nil && newFocus != tb.tree.Focus { | |
| 292 | + | committed := err == nil && candidate.Focus != tb.tree.Focus | |
| 293 | + | if committed { | |
| 295 | 294 | tb.tree = candidate | |
| 296 | 295 | tb.bumpLayoutGenerationLocked() | |
| 297 | 296 | } | |
| 298 | 297 | tb.mu.Unlock() | |
| 299 | - | if err == nil && newFocus != oldFocus { | |
| 298 | + | if committed { | |
| 300 | 299 | d.applyTabLayout(sess, tb) | |
| 301 | 300 | } | |
| 302 | 301 | if errors.Is(err, layout.ErrNoPane) { | |
| @@ -286,17 +286,16 @@ func (d *Daemon) focusDirAt(sess *session, tb *tab, target *pane, dir layout.Dir | |||||
| 286 | tb.mu.Unlock() | 286 | tb.mu.Unlock() | ||
| 287 | return layout.ErrNotFound | 287 | return layout.ErrNotFound | ||
| 288 | } | 288 | } | ||
| 289 | - | oldFocus := target.id | |||
| 290 | candidate := tb.tree.Clone() | 289 | candidate := tb.tree.Clone() | ||
| 291 | - | candidate.Focus = oldFocus | 290 | + | candidate.Focus = target.id |
| 292 | err := candidate.FocusDir(dir, domain.Rect{Width: tb.size.Cols, Height: tb.size.Rows}) | 291 | err := candidate.FocusDir(dir, domain.Rect{Width: tb.size.Cols, Height: tb.size.Rows}) | ||
| 293 | - | newFocus := candidate.Focus | 292 | + | committed := err == nil && candidate.Focus != tb.tree.Focus |
| 294 | - | if err == nil && newFocus != tb.tree.Focus { | 293 | + | if committed { |
| 295 | tb.tree = candidate | 294 | tb.tree = candidate | ||
| 296 | tb.bumpLayoutGenerationLocked() | 295 | tb.bumpLayoutGenerationLocked() | ||
| 297 | } | 296 | } | ||
| 298 | tb.mu.Unlock() | 297 | tb.mu.Unlock() | ||
| 299 | - | if err == nil && newFocus != oldFocus { | 298 | + | if committed { |
| 300 | d.applyTabLayout(sess, tb) | 299 | d.applyTabLayout(sess, tb) | ||
| 301 | } | 300 | } | ||
| 302 | if errors.Is(err, layout.ErrNoPane) { | 301 | if errors.Is(err, layout.ErrNoPane) { | ||
M internal/usecase/daemon/pane_actions_test.go +86 -0
| @@ -72,6 +72,92 @@ func (*layoutRetryTimer) Reset(time.Duration) bool { return false } | |||
| 72 | 72 | func (*layoutRetryTimer) Stop() bool { return true } | |
| 73 | 73 | func (t *layoutRetryTimer) fire() { t.ch <- time.Unix(0, 0) } | |
| 74 | 74 | ||
| 75 | + | func TestLayoutRetryIsBoundedDeduplicatedAndCanceled(t *testing.T) { | |
| 76 | + | t.Run("persistent failure is bounded and reports once", func(t *testing.T) { | |
| 77 | + | pty := &transactionalResizePTY{errs: []error{errors.New("initial"), errors.New("retry 1"), errors.New("retry 2"), errors.New("retry 3")}} | |
| 78 | + | d, sess, _, _ := newManualSessionWithPTYs(t, pty) | |
| 79 | + | clock := &layoutRetryClock{timers: make(chan *layoutRetryTimer, 16)} | |
| 80 | + | d.clock = clock | |
| 81 | + | tb := sess.activeTab() | |
| 82 | + | tb.mu.Lock() | |
| 83 | + | tb.size = domain.Size{Cols: 100, Rows: 20} | |
| 84 | + | tb.bumpLayoutGenerationLocked() | |
| 85 | + | tb.mu.Unlock() | |
| 86 | + | ||
| 87 | + | d.applyTabLayout(sess, tb) | |
| 88 | + | // A second accepted failure joins the existing worker rather than adding | |
| 89 | + | // another timer/goroutine. | |
| 90 | + | d.scheduleAcceptedTabLayoutRetry(sess, tb) | |
| 91 | + | for range maxAcceptedTabLayoutRetries { | |
| 92 | + | var timer *layoutRetryTimer | |
| 93 | + | for timer == nil { | |
| 94 | + | candidate := <-clock.timers | |
| 95 | + | if candidate.delay == minOutputRenderDeadline { | |
| 96 | + | timer = candidate | |
| 97 | + | } | |
| 98 | + | } | |
| 99 | + | timer.fire() | |
| 100 | + | } | |
| 101 | + | require.Eventually(t, func() bool { | |
| 102 | + | tb.layoutRetryMu.Lock() | |
| 103 | + | defer tb.layoutRetryMu.Unlock() | |
| 104 | + | return !tb.layoutRetryRunning | |
| 105 | + | }, time.Second, time.Millisecond) | |
| 106 | + | require.Len(t, pty.requested(), 1+maxAcceptedTabLayoutRetries) | |
| 107 | + | count := 0 | |
| 108 | + | for _, notice := range d.notices.history() { | |
| 109 | + | if notice.Code == domain.NoticeResizeFailed { | |
| 110 | + | count++ | |
| 111 | + | } | |
| 112 | + | } | |
| 113 | + | require.Equal(t, 1, count, "background retries must not create a notification storm") | |
| 114 | + | }) | |
| 115 | + | ||
| 116 | + | t.Run("tab cancellation stops a waiting retry", func(t *testing.T) { | |
| 117 | + | pty := &transactionalResizePTY{errs: []error{errors.New("initial")}} | |
| 118 | + | d, sess, _, _ := newManualSessionWithPTYs(t, pty) | |
| 119 | + | clock := &layoutRetryClock{timers: make(chan *layoutRetryTimer, 16)} | |
| 120 | + | d.clock = clock | |
| 121 | + | tb := sess.activeTab() | |
| 122 | + | tb.mu.Lock() | |
| 123 | + | tb.size = domain.Size{Cols: 100, Rows: 20} | |
| 124 | + | tb.bumpLayoutGenerationLocked() | |
| 125 | + | tb.mu.Unlock() | |
| 126 | + | d.applyTabLayout(sess, tb) | |
| 127 | + | var timer *layoutRetryTimer | |
| 128 | + | for timer == nil { | |
| 129 | + | candidate := <-clock.timers | |
| 130 | + | if candidate.delay == minOutputRenderDeadline { | |
| 131 | + | timer = candidate | |
| 132 | + | } | |
| 133 | + | } | |
| 134 | + | tb.cancel() | |
| 135 | + | timer.fire() | |
| 136 | + | require.Eventually(t, func() bool { | |
| 137 | + | tb.layoutRetryMu.Lock() | |
| 138 | + | defer tb.layoutRetryMu.Unlock() | |
| 139 | + | return !tb.layoutRetryRunning | |
| 140 | + | }, time.Second, time.Millisecond) | |
| 141 | + | require.Len(t, pty.requested(), 1) | |
| 142 | + | }) | |
| 143 | + | } | |
| 144 | + | ||
| 145 | + | func TestFocusDirAtDoesNotApplyUncommittedCandidate(t *testing.T) { | |
| 146 | + | pty := &transactionalResizePTY{} | |
| 147 | + | d, sess, _, _ := newManualSessionWithPTYs(t, pty) | |
| 148 | + | tb := sess.activeTab() | |
| 149 | + | second := newPane("pane-2", nil, domain.Size{Cols: 80, Rows: 23}) | |
| 150 | + | tb.mu.Lock() | |
| 151 | + | tb.panes[second.id] = second | |
| 152 | + | tb.tree = &layout.Tree{Root: &layout.Node{Kind: layout.Split, Dir: layout.Horizontal, Children: []*layout.Node{layout.NewLeaf("pane-1"), layout.NewLeaf("pane-2")}}, Focus: "pane-2"} | |
| 153 | + | tb.panes["pane-1"].resizeRetry = true | |
| 154 | + | target := tb.panes["pane-1"] | |
| 155 | + | tb.mu.Unlock() | |
| 156 | + | ||
| 157 | + | require.NoError(t, d.focusDirAt(sess, tb, target, layout.Right)) | |
| 158 | + | require.Empty(t, pty.requested(), "an equality-mismatched candidate was not committed and must not apply") | |
| 159 | + | } | |
| 160 | + | ||
| 75 | 161 | func TestLayoutApplicationRetriesOnlyAcceptedFailedGeometry(t *testing.T) { | |
| 76 | 162 | pty := &transactionalResizePTY{errs: []error{errors.New("first resize fails")}} | |
| 77 | 163 | d, sess, _, _ := newManualSessionWithPTYs(t, pty) | |
| @@ -72,6 +72,92 @@ func (*layoutRetryTimer) Reset(time.Duration) bool { return false } | |||||
| 72 | func (*layoutRetryTimer) Stop() bool { return true } | 72 | func (*layoutRetryTimer) Stop() bool { return true } | ||
| 73 | func (t *layoutRetryTimer) fire() { t.ch <- time.Unix(0, 0) } | 73 | func (t *layoutRetryTimer) fire() { t.ch <- time.Unix(0, 0) } | ||
| 74 | 74 | ||||
| 75 | + | func TestLayoutRetryIsBoundedDeduplicatedAndCanceled(t *testing.T) { | |||
| 76 | + | t.Run("persistent failure is bounded and reports once", func(t *testing.T) { | |||
| 77 | + | pty := &transactionalResizePTY{errs: []error{errors.New("initial"), errors.New("retry 1"), errors.New("retry 2"), errors.New("retry 3")}} | |||
| 78 | + | d, sess, _, _ := newManualSessionWithPTYs(t, pty) | |||
| 79 | + | clock := &layoutRetryClock{timers: make(chan *layoutRetryTimer, 16)} | |||
| 80 | + | d.clock = clock | |||
| 81 | + | tb := sess.activeTab() | |||
| 82 | + | tb.mu.Lock() | |||
| 83 | + | tb.size = domain.Size{Cols: 100, Rows: 20} | |||
| 84 | + | tb.bumpLayoutGenerationLocked() | |||
| 85 | + | tb.mu.Unlock() | |||
| 86 | + | ||||
| 87 | + | d.applyTabLayout(sess, tb) | |||
| 88 | + | // A second accepted failure joins the existing worker rather than adding | |||
| 89 | + | // another timer/goroutine. | |||
| 90 | + | d.scheduleAcceptedTabLayoutRetry(sess, tb) | |||
| 91 | + | for range maxAcceptedTabLayoutRetries { | |||
| 92 | + | var timer *layoutRetryTimer | |||
| 93 | + | for timer == nil { | |||
| 94 | + | candidate := <-clock.timers | |||
| 95 | + | if candidate.delay == minOutputRenderDeadline { | |||
| 96 | + | timer = candidate | |||
| 97 | + | } | |||
| 98 | + | } | |||
| 99 | + | timer.fire() | |||
| 100 | + | } | |||
| 101 | + | require.Eventually(t, func() bool { | |||
| 102 | + | tb.layoutRetryMu.Lock() | |||
| 103 | + | defer tb.layoutRetryMu.Unlock() | |||
| 104 | + | return !tb.layoutRetryRunning | |||
| 105 | + | }, time.Second, time.Millisecond) | |||
| 106 | + | require.Len(t, pty.requested(), 1+maxAcceptedTabLayoutRetries) | |||
| 107 | + | count := 0 | |||
| 108 | + | for _, notice := range d.notices.history() { | |||
| 109 | + | if notice.Code == domain.NoticeResizeFailed { | |||
| 110 | + | count++ | |||
| 111 | + | } | |||
| 112 | + | } | |||
| 113 | + | require.Equal(t, 1, count, "background retries must not create a notification storm") | |||
| 114 | + | }) | |||
| 115 | + | ||||
| 116 | + | t.Run("tab cancellation stops a waiting retry", func(t *testing.T) { | |||
| 117 | + | pty := &transactionalResizePTY{errs: []error{errors.New("initial")}} | |||
| 118 | + | d, sess, _, _ := newManualSessionWithPTYs(t, pty) | |||
| 119 | + | clock := &layoutRetryClock{timers: make(chan *layoutRetryTimer, 16)} | |||
| 120 | + | d.clock = clock | |||
| 121 | + | tb := sess.activeTab() | |||
| 122 | + | tb.mu.Lock() | |||
| 123 | + | tb.size = domain.Size{Cols: 100, Rows: 20} | |||
| 124 | + | tb.bumpLayoutGenerationLocked() | |||
| 125 | + | tb.mu.Unlock() | |||
| 126 | + | d.applyTabLayout(sess, tb) | |||
| 127 | + | var timer *layoutRetryTimer | |||
| 128 | + | for timer == nil { | |||
| 129 | + | candidate := <-clock.timers | |||
| 130 | + | if candidate.delay == minOutputRenderDeadline { | |||
| 131 | + | timer = candidate | |||
| 132 | + | } | |||
| 133 | + | } | |||
| 134 | + | tb.cancel() | |||
| 135 | + | timer.fire() | |||
| 136 | + | require.Eventually(t, func() bool { | |||
| 137 | + | tb.layoutRetryMu.Lock() | |||
| 138 | + | defer tb.layoutRetryMu.Unlock() | |||
| 139 | + | return !tb.layoutRetryRunning | |||
| 140 | + | }, time.Second, time.Millisecond) | |||
| 141 | + | require.Len(t, pty.requested(), 1) | |||
| 142 | + | }) | |||
| 143 | + | } | |||
| 144 | + | ||||
| 145 | + | func TestFocusDirAtDoesNotApplyUncommittedCandidate(t *testing.T) { | |||
| 146 | + | pty := &transactionalResizePTY{} | |||
| 147 | + | d, sess, _, _ := newManualSessionWithPTYs(t, pty) | |||
| 148 | + | tb := sess.activeTab() | |||
| 149 | + | second := newPane("pane-2", nil, domain.Size{Cols: 80, Rows: 23}) | |||
| 150 | + | tb.mu.Lock() | |||
| 151 | + | tb.panes[second.id] = second | |||
| 152 | + | tb.tree = &layout.Tree{Root: &layout.Node{Kind: layout.Split, Dir: layout.Horizontal, Children: []*layout.Node{layout.NewLeaf("pane-1"), layout.NewLeaf("pane-2")}}, Focus: "pane-2"} | |||
| 153 | + | tb.panes["pane-1"].resizeRetry = true | |||
| 154 | + | target := tb.panes["pane-1"] | |||
| 155 | + | tb.mu.Unlock() | |||
| 156 | + | ||||
| 157 | + | require.NoError(t, d.focusDirAt(sess, tb, target, layout.Right)) | |||
| 158 | + | require.Empty(t, pty.requested(), "an equality-mismatched candidate was not committed and must not apply") | |||
| 159 | + | } | |||
| 160 | + | ||||
| 75 | func TestLayoutApplicationRetriesOnlyAcceptedFailedGeometry(t *testing.T) { | 161 | func TestLayoutApplicationRetriesOnlyAcceptedFailedGeometry(t *testing.T) { | ||
| 76 | pty := &transactionalResizePTY{errs: []error{errors.New("first resize fails")}} | 162 | pty := &transactionalResizePTY{errs: []error{errors.New("first resize fails")}} | ||
| 77 | d, sess, _, _ := newManualSessionWithPTYs(t, pty) | 163 | d, sess, _, _ := newManualSessionWithPTYs(t, pty) | ||
M internal/usecase/daemon/session.go +6 -0
| @@ -130,6 +130,12 @@ type tab struct { | |||
| 130 | 130 | // state changes. layoutApplyMu serializes the lock-free PTY apply boundary. | |
| 131 | 131 | layoutGeneration uint64 | |
| 132 | 132 | layoutApplyMu sync.Mutex | |
| 133 | + | // layoutRetryMu owns the one bounded delayed retry worker for accepted | |
| 134 | + | // tiled-layout degradation. Its context is derived from ctx, so tab/session | |
| 135 | + | // teardown cancels a waiting retry before it can touch PTY state. | |
| 136 | + | layoutRetryMu sync.Mutex | |
| 137 | + | layoutRetryCancel context.CancelFunc | |
| 138 | + | layoutRetryRunning bool | |
| 133 | 139 | ||
| 134 | 140 | stableID string | |
| 135 | 141 | name string | |
| @@ -130,6 +130,12 @@ type tab struct { | |||||
| 130 | // state changes. layoutApplyMu serializes the lock-free PTY apply boundary. | 130 | // state changes. layoutApplyMu serializes the lock-free PTY apply boundary. | ||
| 131 | layoutGeneration uint64 | 131 | layoutGeneration uint64 | ||
| 132 | layoutApplyMu sync.Mutex | 132 | layoutApplyMu sync.Mutex | ||
| 133 | + | // layoutRetryMu owns the one bounded delayed retry worker for accepted | |||
| 134 | + | // tiled-layout degradation. Its context is derived from ctx, so tab/session | |||
| 135 | + | // teardown cancels a waiting retry before it can touch PTY state. | |||
| 136 | + | layoutRetryMu sync.Mutex | |||
| 137 | + | layoutRetryCancel context.CancelFunc | |||
| 138 | + | layoutRetryRunning bool | |||
| 133 | 139 | ||||
| 134 | stableID string | 140 | stableID string | ||
| 135 | name string | 141 | name string | ||
M internal/usecase/daemon/transactional_resize.go +128 -44
| @@ -1,6 +1,8 @@ | |||
| 1 | 1 | package daemon | |
| 2 | 2 | ||
| 3 | 3 | import ( | |
| 4 | + | "context" | |
| 5 | + | ||
| 4 | 6 | "github.com/bnema/vev/internal/domain" | |
| 5 | 7 | "github.com/bnema/vev/internal/ports" | |
| 6 | 8 | "github.com/bnema/vev/internal/usecase/layout" | |
| @@ -9,16 +11,20 @@ import ( | |||
| 9 | 11 | // resizeMember is an unpublished layout decision. Prepare only reads guarded | |
| 10 | 12 | // state; apply performs the external PTY call; commit is the sole publisher. | |
| 11 | 13 | type resizeMember struct { | |
| 12 | - | session *session | |
| 13 | - | tab *tab | |
| 14 | - | pane *pane | |
| 15 | - | rect domain.Rect | |
| 16 | - | floating floatingGeometry | |
| 17 | - | isFloating bool | |
| 18 | - | retry bool | |
| 19 | - | ok bool | |
| 20 | - | err error | |
| 21 | - | screenResized bool | |
| 14 | + | session *session | |
| 15 | + | tab *tab | |
| 16 | + | pane *pane | |
| 17 | + | rect domain.Rect | |
| 18 | + | floating floatingGeometry | |
| 19 | + | isFloating bool | |
| 20 | + | // floatingGeneration identifies the exact popup slot accepted by a failed | |
| 21 | + | // resize. Floating panes are intentionally outside tb.panes, so retries | |
| 22 | + | // must validate this generation and pointer rather than use tiled lookup. | |
| 23 | + | floatingGeneration uint64 | |
| 24 | + | retry bool | |
| 25 | + | ok bool | |
| 26 | + | err error | |
| 27 | + | screenResized bool | |
| 22 | 28 | } | |
| 23 | 29 | ||
| 24 | 30 | type resizePlan struct { | |
| @@ -153,19 +159,17 @@ func (d *Daemon) cancelStalePreparedGates(sess *session, tb *tab, plan *prepared | |||
| 153 | 159 | } | |
| 154 | 160 | ||
| 155 | 161 | func (d *Daemon) finishPreparedTabMembers(plan *preparedTabLayout, accepted bool) { | |
| 162 | + | if !accepted { | |
| 163 | + | // cancelStalePreparedGates owns stale-plan cancellation. Keeping this | |
| 164 | + | // path empty ensures a removed member's pending bytes are replayed once, | |
| 165 | + | // after the fresh layout determines it cannot survive the retry. | |
| 166 | + | return | |
| 167 | + | } | |
| 156 | 168 | for i := range plan.members { | |
| 157 | 169 | member := &plan.members[i] | |
| 158 | 170 | if !member.screenResized { | |
| 159 | 171 | continue | |
| 160 | 172 | } | |
| 161 | - | if !accepted { | |
| 162 | - | member.tab.mu.Lock() | |
| 163 | - | current := member.tab.panes[member.pane.id] == member.pane | |
| 164 | - | member.tab.mu.Unlock() | |
| 165 | - | if current { | |
| 166 | - | continue | |
| 167 | - | } | |
| 168 | - | } | |
| 169 | 173 | member.pane.resizeMu.Lock() | |
| 170 | 174 | d.replayResizePending(member.session, member.tab, member.pane, false, member.rect) | |
| 171 | 175 | member.pane.resizeMu.Unlock() | |
| @@ -176,6 +180,10 @@ func (d *Daemon) finishPreparedTabMembers(plan *preparedTabLayout, accepted bool | |||
| 176 | 180 | // one per-tab single-writer loop, but never holds tab or pane state locks while | |
| 177 | 181 | // invoking PTY.Resize. | |
| 178 | 182 | func (d *Daemon) applyTabLayoutTransaction(sess *session, tb *tab, current ...func() bool) ([]resizeMember, bool) { | |
| 183 | + | return d.applyTabLayoutTransactionWithNotice(sess, tb, true, current...) | |
| 184 | + | } | |
| 185 | + | ||
| 186 | + | func (d *Daemon) applyTabLayoutTransactionWithNotice(sess *session, tb *tab, reportFailure bool, current ...func() bool) ([]resizeMember, bool) { | |
| 179 | 187 | if tb == nil { | |
| 180 | 188 | return nil, false | |
| 181 | 189 | } | |
| @@ -225,7 +233,7 @@ func (d *Daemon) applyTabLayoutTransaction(sess *session, tb *tab, current ...fu | |||
| 225 | 233 | failed = append(failed, member) | |
| 226 | 234 | } | |
| 227 | 235 | } | |
| 228 | - | if len(failed) != 0 { | |
| 236 | + | if reportFailure && len(failed) != 0 { | |
| 229 | 237 | d.notify(sess, domain.NoticeWarn, domain.NoticeResizeFailed, | |
| 230 | 238 | "pane resize failed; retrying in background", failed[len(failed)-1].err) | |
| 231 | 239 | } | |
| @@ -245,34 +253,57 @@ func (d *Daemon) applyTabLayout(sess *session, tb *tab) bool { | |||
| 245 | 253 | return true | |
| 246 | 254 | } | |
| 247 | 255 | ||
| 248 | - | // scheduleAcceptedTabLayoutRetry retries a failed PTY resize only after its | |
| 249 | - | // geometry has passed the tab generation and pane-membership validation. The | |
| 250 | - | // retry prepares a new canonical plan, so a later layout mutation supersedes | |
| 251 | - | // this target rather than replaying stale rectangles. | |
| 256 | + | const maxAcceptedTabLayoutRetries = 3 | |
| 257 | + | ||
| 258 | + | // scheduleAcceptedTabLayoutRetry owns one bounded retry worker per tab. The | |
| 259 | + | // worker is deduplicated, derives cancellation from the tab lifecycle, and | |
| 260 | + | // suppresses repeat degradation notices after the accepted initial failure. | |
| 252 | 261 | func (d *Daemon) scheduleAcceptedTabLayoutRetry(sess *session, tb *tab) { | |
| 253 | 262 | if d.clock == nil || tb == nil || tb.ctx == nil { | |
| 254 | 263 | return | |
| 255 | 264 | } | |
| 256 | - | timer := d.clock.NewTimer(minOutputRenderDeadline) | |
| 257 | - | if timer == nil { | |
| 258 | - | return | |
| 259 | - | } | |
| 260 | - | timerC := timer.C() | |
| 261 | - | if timerC == nil { | |
| 262 | - | timer.Stop() | |
| 265 | + | tb.layoutRetryMu.Lock() | |
| 266 | + | if tb.layoutRetryRunning { | |
| 267 | + | tb.layoutRetryMu.Unlock() | |
| 263 | 268 | return | |
| 264 | 269 | } | |
| 270 | + | ctx, cancel := context.WithCancel(tb.ctx) | |
| 271 | + | tb.layoutRetryRunning = true | |
| 272 | + | tb.layoutRetryCancel = cancel | |
| 273 | + | tb.layoutRetryMu.Unlock() | |
| 274 | + | ||
| 265 | 275 | go func() { | |
| 266 | - | select { | |
| 267 | - | case <-tb.ctx.Done(): | |
| 268 | - | timer.Stop() | |
| 269 | - | return | |
| 270 | - | case <-timerC: | |
| 271 | - | } | |
| 272 | - | if tb.ctx.Err() != nil { | |
| 273 | - | return | |
| 274 | - | } | |
| 275 | - | if d.applyTabLayout(sess, tb) { | |
| 276 | + | defer func() { | |
| 277 | + | cancel() | |
| 278 | + | tb.layoutRetryMu.Lock() | |
| 279 | + | tb.layoutRetryRunning = false | |
| 280 | + | tb.layoutRetryCancel = nil | |
| 281 | + | tb.layoutRetryMu.Unlock() | |
| 282 | + | }() | |
| 283 | + | for range maxAcceptedTabLayoutRetries { | |
| 284 | + | timer := d.clock.NewTimer(minOutputRenderDeadline) | |
| 285 | + | if timer == nil { | |
| 286 | + | return | |
| 287 | + | } | |
| 288 | + | timerC := timer.C() | |
| 289 | + | if timerC == nil { | |
| 290 | + | timer.Stop() | |
| 291 | + | return | |
| 292 | + | } | |
| 293 | + | select { | |
| 294 | + | case <-ctx.Done(): | |
| 295 | + | timer.Stop() | |
| 296 | + | return | |
| 297 | + | case <-timerC: | |
| 298 | + | } | |
| 299 | + | if ctx.Err() != nil { | |
| 300 | + | return | |
| 301 | + | } | |
| 302 | + | failed, ok := d.applyTabLayoutTransactionWithNotice(sess, tb, false, func() bool { return ctx.Err() == nil }) | |
| 303 | + | if !ok || len(failed) == 0 { | |
| 304 | + | return | |
| 305 | + | } | |
| 306 | + | markSnapshotDirty(sess) | |
| 276 | 307 | sess.mu.Lock() | |
| 277 | 308 | ac := sess.client | |
| 278 | 309 | sess.mu.Unlock() | |
| @@ -413,11 +444,19 @@ func (d *Daemon) replayResizePending(sess *session, tb *tab, p *pane, resized bo | |||
| 413 | 444 | // generation, so its slot generation and exact pane identity validate its | |
| 414 | 445 | // publication instead. | |
| 415 | 446 | func (d *Daemon) applyVisibleFloatingLayout(sess *session, tb *tab, current func() bool) ([]resizeMember, bool) { | |
| 447 | + | return d.applyVisibleFloatingLayoutForMember(sess, tb, current, nil) | |
| 448 | + | } | |
| 449 | + | ||
| 450 | + | // applyVisibleFloatingLayoutForMember optionally fences a delayed retry to the | |
| 451 | + | // exact failed floating slot. Unlike tiled panes, a popup is not in tb.panes, | |
| 452 | + | // so accepting a replacement here would silently retry unrelated geometry. | |
| 453 | + | func (d *Daemon) applyVisibleFloatingLayoutForMember(sess *session, tb *tab, current func() bool, expected *resizeMember) ([]resizeMember, bool) { | |
| 416 | 454 | if tb == nil { | |
| 417 | 455 | return nil, true | |
| 418 | 456 | } | |
| 419 | 457 | tb.mu.Lock() | |
| 420 | - | if tb.floating.state != floatingVisible || tb.floating.pane == nil { | |
| 458 | + | if tb.floating.state != floatingVisible || tb.floating.pane == nil || | |
| 459 | + | (expected != nil && (tb.floating.generation != expected.floatingGeneration || tb.floating.pane != expected.pane)) { | |
| 421 | 460 | tb.mu.Unlock() | |
| 422 | 461 | return nil, true | |
| 423 | 462 | } | |
| @@ -434,7 +473,7 @@ func (d *Daemon) applyVisibleFloatingLayout(sess *session, tb *tab, current func | |||
| 434 | 473 | // floating slot and tab size are revalidated. A newer client resize may | |
| 435 | 474 | // otherwise publish this obsolete popup geometry after its PTY call returns. | |
| 436 | 475 | plan := preparedTabLayout{members: []resizeMember{{ | |
| 437 | - | session: sess, tab: tb, pane: p, rect: geometry.Inner, floating: geometry, isFloating: true, | |
| 476 | + | session: sess, tab: tb, pane: p, rect: geometry.Inner, floating: geometry, isFloating: true, floatingGeneration: generation, | |
| 438 | 477 | }}} | |
| 439 | 478 | d.applyPreparedTabMembers(&plan) | |
| 440 | 479 | ||
| @@ -569,7 +608,10 @@ func (d *Daemon) applySessionLayout(sess *session, size domain.Size, current, ad | |||
| 569 | 608 | } | |
| 570 | 609 | if !valid { | |
| 571 | 610 | d.releasePreparedSessionGates(plans) | |
| 572 | - | if current == nil || !current() { | |
| 611 | + | // Headless requests have no attachment epoch. A live tab mutation | |
| 612 | + | // still invalidates their plan, but must trigger a fresh prepare rather | |
| 613 | + | // than being mistaken for a canceled request. | |
| 614 | + | if current != nil && !current() { | |
| 573 | 615 | return nil, false | |
| 574 | 616 | } | |
| 575 | 617 | // A layout mutation invalidated the plans while this epoch remains | |
| @@ -654,7 +696,7 @@ func (d *Daemon) retryResizeMembers(sess *session, ac *attachedClient, lease *at | |||
| 654 | 696 | tabs := make([]*tab, 0, len(members)) | |
| 655 | 697 | seen := make(map[*tab]struct{}, len(members)) | |
| 656 | 698 | for _, member := range members { | |
| 657 | - | if member.tab == nil { | |
| 699 | + | if member.tab == nil || member.isFloating { | |
| 658 | 700 | continue | |
| 659 | 701 | } | |
| 660 | 702 | if _, ok := seen[member.tab]; ok { | |
| @@ -713,6 +755,48 @@ func (d *Daemon) retryResizeMembers(sess *session, ac *attachedClient, lease *at | |||
| 713 | 755 | } | |
| 714 | 756 | tb.mu.Unlock() | |
| 715 | 757 | } | |
| 758 | + | for _, member := range members { | |
| 759 | + | if !member.isFloating || member.tab == nil || member.pane == nil { | |
| 760 | + | continue | |
| 761 | + | } | |
| 762 | + | if !rc.retryCurrentForLease(epoch, ac, lease) { | |
| 763 | + | return | |
| 764 | + | } | |
| 765 | + | // Floating panes are outside tb.panes. Validate the exact accepted slot | |
| 766 | + | // before preparing a fresh geometry; applyVisibleFloatingLayout repeats | |
| 767 | + | // the same pointer/generation check around external PTY.Resize. | |
| 768 | + | member.tab.mu.Lock() | |
| 769 | + | currentSlot := member.tab.floating.state == floatingVisible && | |
| 770 | + | member.tab.floating.generation == member.floatingGeneration && | |
| 771 | + | member.tab.floating.pane == member.pane | |
| 772 | + | retryPending := false | |
| 773 | + | if currentSlot { | |
| 774 | + | member.pane.mu.Lock() | |
| 775 | + | retryPending = member.pane.resizeRetry | |
| 776 | + | member.pane.mu.Unlock() | |
| 777 | + | } | |
| 778 | + | member.tab.mu.Unlock() | |
| 779 | + | if !currentSlot || !retryPending { | |
| 780 | + | continue | |
| 781 | + | } | |
| 782 | + | freshFailed, ok := d.applyVisibleFloatingLayoutForMember(sess, member.tab, func() bool { | |
| 783 | + | return rc.retryCurrentForLease(epoch, ac, lease) | |
| 784 | + | }, &member) | |
| 785 | + | if !ok || !rc.retryCurrentForLease(epoch, ac, lease) { | |
| 786 | + | return | |
| 787 | + | } | |
| 788 | + | failed = append(failed, freshFailed...) | |
| 789 | + | member.tab.mu.Lock() | |
| 790 | + | stillCurrent := member.tab.floating.state == floatingVisible && | |
| 791 | + | member.tab.floating.generation == member.floatingGeneration && | |
| 792 | + | member.tab.floating.pane == member.pane | |
| 793 | + | member.tab.mu.Unlock() | |
| 794 | + | if stillCurrent && len(freshFailed) == 0 { | |
| 795 | + | member.pane.mu.Lock() | |
| 796 | + | succeeded = succeeded || !member.pane.resizeRetry | |
| 797 | + | member.pane.mu.Unlock() | |
| 798 | + | } | |
| 799 | + | } | |
| 716 | 800 | if len(failed) != 0 { | |
| 717 | 801 | rc.scheduleResizeRetryForLease(epoch, ac, lease, func() { d.retryResizeMembers(sess, ac, lease, epoch, failed) }) | |
| 718 | 802 | } | |
| @@ -1,6 +1,8 @@ | |||||
| 1 | package daemon | 1 | package daemon | ||
| 2 | 2 | ||||
| 3 | import ( | 3 | import ( | ||
| 4 | + | "context" | |||
| 5 | + | ||||
| 4 | "github.com/bnema/vev/internal/domain" | 6 | "github.com/bnema/vev/internal/domain" | ||
| 5 | "github.com/bnema/vev/internal/ports" | 7 | "github.com/bnema/vev/internal/ports" | ||
| 6 | "github.com/bnema/vev/internal/usecase/layout" | 8 | "github.com/bnema/vev/internal/usecase/layout" | ||
| @@ -9,16 +11,20 @@ import ( | |||||
| 9 | // resizeMember is an unpublished layout decision. Prepare only reads guarded | 11 | // resizeMember is an unpublished layout decision. Prepare only reads guarded | ||
| 10 | // state; apply performs the external PTY call; commit is the sole publisher. | 12 | // state; apply performs the external PTY call; commit is the sole publisher. | ||
| 11 | type resizeMember struct { | 13 | type resizeMember struct { | ||
| 12 | - | session *session | 14 | + | session *session |
| 13 | - | tab *tab | 15 | + | tab *tab |
| 14 | - | pane *pane | 16 | + | pane *pane |
| 15 | - | rect domain.Rect | 17 | + | rect domain.Rect |
| 16 | - | floating floatingGeometry | 18 | + | floating floatingGeometry |
| 17 | - | isFloating bool | 19 | + | isFloating bool |
| 18 | - | retry bool | 20 | + | // floatingGeneration identifies the exact popup slot accepted by a failed |
| 19 | - | ok bool | 21 | + | // resize. Floating panes are intentionally outside tb.panes, so retries |
| 20 | - | err error | 22 | + | // must validate this generation and pointer rather than use tiled lookup. |
| 21 | - | screenResized bool | 23 | + | floatingGeneration uint64 |
| 24 | + | retry bool | |||
| 25 | + | ok bool | |||
| 26 | + | err error | |||
| 27 | + | screenResized bool | |||
| 22 | } | 28 | } | ||
| 23 | 29 | ||||
| 24 | type resizePlan struct { | 30 | type resizePlan struct { | ||
| @@ -153,19 +159,17 @@ func (d *Daemon) cancelStalePreparedGates(sess *session, tb *tab, plan *prepared | |||||
| 153 | } | 159 | } | ||
| 154 | 160 | ||||
| 155 | func (d *Daemon) finishPreparedTabMembers(plan *preparedTabLayout, accepted bool) { | 161 | func (d *Daemon) finishPreparedTabMembers(plan *preparedTabLayout, accepted bool) { | ||
| 162 | + | if !accepted { | |||
| 163 | + | // cancelStalePreparedGates owns stale-plan cancellation. Keeping this | |||
| 164 | + | // path empty ensures a removed member's pending bytes are replayed once, | |||
| 165 | + | // after the fresh layout determines it cannot survive the retry. | |||
| 166 | + | return | |||
| 167 | + | } | |||
| 156 | for i := range plan.members { | 168 | for i := range plan.members { | ||
| 157 | member := &plan.members[i] | 169 | member := &plan.members[i] | ||
| 158 | if !member.screenResized { | 170 | if !member.screenResized { | ||
| 159 | continue | 171 | continue | ||
| 160 | } | 172 | } | ||
| 161 | - | if !accepted { | |||
| 162 | - | member.tab.mu.Lock() | |||
| 163 | - | current := member.tab.panes[member.pane.id] == member.pane | |||
| 164 | - | member.tab.mu.Unlock() | |||
| 165 | - | if current { | |||
| 166 | - | continue | |||
| 167 | - | } | |||
| 168 | - | } | |||
| 169 | member.pane.resizeMu.Lock() | 173 | member.pane.resizeMu.Lock() | ||
| 170 | d.replayResizePending(member.session, member.tab, member.pane, false, member.rect) | 174 | d.replayResizePending(member.session, member.tab, member.pane, false, member.rect) | ||
| 171 | member.pane.resizeMu.Unlock() | 175 | member.pane.resizeMu.Unlock() | ||
| @@ -176,6 +180,10 @@ func (d *Daemon) finishPreparedTabMembers(plan *preparedTabLayout, accepted bool | |||||
| 176 | // one per-tab single-writer loop, but never holds tab or pane state locks while | 180 | // one per-tab single-writer loop, but never holds tab or pane state locks while | ||
| 177 | // invoking PTY.Resize. | 181 | // invoking PTY.Resize. | ||
| 178 | func (d *Daemon) applyTabLayoutTransaction(sess *session, tb *tab, current ...func() bool) ([]resizeMember, bool) { | 182 | func (d *Daemon) applyTabLayoutTransaction(sess *session, tb *tab, current ...func() bool) ([]resizeMember, bool) { | ||
| 183 | + | return d.applyTabLayoutTransactionWithNotice(sess, tb, true, current...) | |||
| 184 | + | } | |||
| 185 | + | ||||
| 186 | + | func (d *Daemon) applyTabLayoutTransactionWithNotice(sess *session, tb *tab, reportFailure bool, current ...func() bool) ([]resizeMember, bool) { | |||
| 179 | if tb == nil { | 187 | if tb == nil { | ||
| 180 | return nil, false | 188 | return nil, false | ||
| 181 | } | 189 | } | ||
| @@ -225,7 +233,7 @@ func (d *Daemon) applyTabLayoutTransaction(sess *session, tb *tab, current ...fu | |||||
| 225 | failed = append(failed, member) | 233 | failed = append(failed, member) | ||
| 226 | } | 234 | } | ||
| 227 | } | 235 | } | ||
| 228 | - | if len(failed) != 0 { | 236 | + | if reportFailure && len(failed) != 0 { |
| 229 | d.notify(sess, domain.NoticeWarn, domain.NoticeResizeFailed, | 237 | d.notify(sess, domain.NoticeWarn, domain.NoticeResizeFailed, | ||
| 230 | "pane resize failed; retrying in background", failed[len(failed)-1].err) | 238 | "pane resize failed; retrying in background", failed[len(failed)-1].err) | ||
| 231 | } | 239 | } | ||
| @@ -245,34 +253,57 @@ func (d *Daemon) applyTabLayout(sess *session, tb *tab) bool { | |||||
| 245 | return true | 253 | return true | ||
| 246 | } | 254 | } | ||
| 247 | 255 | ||||
| 248 | - | // scheduleAcceptedTabLayoutRetry retries a failed PTY resize only after its | 256 | + | const maxAcceptedTabLayoutRetries = 3 |
| 249 | - | // geometry has passed the tab generation and pane-membership validation. The | 257 | + | |
| 250 | - | // retry prepares a new canonical plan, so a later layout mutation supersedes | 258 | + | // scheduleAcceptedTabLayoutRetry owns one bounded retry worker per tab. The |
| 251 | - | // this target rather than replaying stale rectangles. | 259 | + | // worker is deduplicated, derives cancellation from the tab lifecycle, and |
| 260 | + | // suppresses repeat degradation notices after the accepted initial failure. | |||
| 252 | func (d *Daemon) scheduleAcceptedTabLayoutRetry(sess *session, tb *tab) { | 261 | func (d *Daemon) scheduleAcceptedTabLayoutRetry(sess *session, tb *tab) { | ||
| 253 | if d.clock == nil || tb == nil || tb.ctx == nil { | 262 | if d.clock == nil || tb == nil || tb.ctx == nil { | ||
| 254 | return | 263 | return | ||
| 255 | } | 264 | } | ||
| 256 | - | timer := d.clock.NewTimer(minOutputRenderDeadline) | 265 | + | tb.layoutRetryMu.Lock() |
| 257 | - | if timer == nil { | 266 | + | if tb.layoutRetryRunning { |
| 258 | - | return | 267 | + | tb.layoutRetryMu.Unlock() |
| 259 | - | } | |||
| 260 | - | timerC := timer.C() | |||
| 261 | - | if timerC == nil { | |||
| 262 | - | timer.Stop() | |||
| 263 | return | 268 | return | ||
| 264 | } | 269 | } | ||
| 270 | + | ctx, cancel := context.WithCancel(tb.ctx) | |||
| 271 | + | tb.layoutRetryRunning = true | |||
| 272 | + | tb.layoutRetryCancel = cancel | |||
| 273 | + | tb.layoutRetryMu.Unlock() | |||
| 274 | + | ||||
| 265 | go func() { | 275 | go func() { | ||
| 266 | - | select { | 276 | + | defer func() { |
| 267 | - | case <-tb.ctx.Done(): | 277 | + | cancel() |
| 268 | - | timer.Stop() | 278 | + | tb.layoutRetryMu.Lock() |
| 269 | - | return | 279 | + | tb.layoutRetryRunning = false |
| 270 | - | case <-timerC: | 280 | + | tb.layoutRetryCancel = nil |
| 271 | - | } | 281 | + | tb.layoutRetryMu.Unlock() |
| 272 | - | if tb.ctx.Err() != nil { | 282 | + | }() |
| 273 | - | return | 283 | + | for range maxAcceptedTabLayoutRetries { |
| 274 | - | } | 284 | + | timer := d.clock.NewTimer(minOutputRenderDeadline) |
| 275 | - | if d.applyTabLayout(sess, tb) { | 285 | + | if timer == nil { |
| 286 | + | return | |||
| 287 | + | } | |||
| 288 | + | timerC := timer.C() | |||
| 289 | + | if timerC == nil { | |||
| 290 | + | timer.Stop() | |||
| 291 | + | return | |||
| 292 | + | } | |||
| 293 | + | select { | |||
| 294 | + | case <-ctx.Done(): | |||
| 295 | + | timer.Stop() | |||
| 296 | + | return | |||
| 297 | + | case <-timerC: | |||
| 298 | + | } | |||
| 299 | + | if ctx.Err() != nil { | |||
| 300 | + | return | |||
| 301 | + | } | |||
| 302 | + | failed, ok := d.applyTabLayoutTransactionWithNotice(sess, tb, false, func() bool { return ctx.Err() == nil }) | |||
| 303 | + | if !ok || len(failed) == 0 { | |||
| 304 | + | return | |||
| 305 | + | } | |||
| 306 | + | markSnapshotDirty(sess) | |||
| 276 | sess.mu.Lock() | 307 | sess.mu.Lock() | ||
| 277 | ac := sess.client | 308 | ac := sess.client | ||
| 278 | sess.mu.Unlock() | 309 | sess.mu.Unlock() | ||
| @@ -413,11 +444,19 @@ func (d *Daemon) replayResizePending(sess *session, tb *tab, p *pane, resized bo | |||||
| 413 | // generation, so its slot generation and exact pane identity validate its | 444 | // generation, so its slot generation and exact pane identity validate its | ||
| 414 | // publication instead. | 445 | // publication instead. | ||
| 415 | func (d *Daemon) applyVisibleFloatingLayout(sess *session, tb *tab, current func() bool) ([]resizeMember, bool) { | 446 | func (d *Daemon) applyVisibleFloatingLayout(sess *session, tb *tab, current func() bool) ([]resizeMember, bool) { | ||
| 447 | + | return d.applyVisibleFloatingLayoutForMember(sess, tb, current, nil) | |||
| 448 | + | } | |||
| 449 | + | ||||
| 450 | + | // applyVisibleFloatingLayoutForMember optionally fences a delayed retry to the | |||
| 451 | + | // exact failed floating slot. Unlike tiled panes, a popup is not in tb.panes, | |||
| 452 | + | // so accepting a replacement here would silently retry unrelated geometry. | |||
| 453 | + | func (d *Daemon) applyVisibleFloatingLayoutForMember(sess *session, tb *tab, current func() bool, expected *resizeMember) ([]resizeMember, bool) { | |||
| 416 | if tb == nil { | 454 | if tb == nil { | ||
| 417 | return nil, true | 455 | return nil, true | ||
| 418 | } | 456 | } | ||
| 419 | tb.mu.Lock() | 457 | tb.mu.Lock() | ||
| 420 | - | if tb.floating.state != floatingVisible || tb.floating.pane == nil { | 458 | + | if tb.floating.state != floatingVisible || tb.floating.pane == nil || |
| 459 | + | (expected != nil && (tb.floating.generation != expected.floatingGeneration || tb.floating.pane != expected.pane)) { | |||
| 421 | tb.mu.Unlock() | 460 | tb.mu.Unlock() | ||
| 422 | return nil, true | 461 | return nil, true | ||
| 423 | } | 462 | } | ||
| @@ -434,7 +473,7 @@ func (d *Daemon) applyVisibleFloatingLayout(sess *session, tb *tab, current func | |||||
| 434 | // floating slot and tab size are revalidated. A newer client resize may | 473 | // floating slot and tab size are revalidated. A newer client resize may | ||
| 435 | // otherwise publish this obsolete popup geometry after its PTY call returns. | 474 | // otherwise publish this obsolete popup geometry after its PTY call returns. | ||
| 436 | plan := preparedTabLayout{members: []resizeMember{{ | 475 | plan := preparedTabLayout{members: []resizeMember{{ | ||
| 437 | - | session: sess, tab: tb, pane: p, rect: geometry.Inner, floating: geometry, isFloating: true, | 476 | + | session: sess, tab: tb, pane: p, rect: geometry.Inner, floating: geometry, isFloating: true, floatingGeneration: generation, |
| 438 | }}} | 477 | }}} | ||
| 439 | d.applyPreparedTabMembers(&plan) | 478 | d.applyPreparedTabMembers(&plan) | ||
| 440 | 479 | ||||
| @@ -569,7 +608,10 @@ func (d *Daemon) applySessionLayout(sess *session, size domain.Size, current, ad | |||||
| 569 | } | 608 | } | ||
| 570 | if !valid { | 609 | if !valid { | ||
| 571 | d.releasePreparedSessionGates(plans) | 610 | d.releasePreparedSessionGates(plans) | ||
| 572 | - | if current == nil || !current() { | 611 | + | // Headless requests have no attachment epoch. A live tab mutation |
| 612 | + | // still invalidates their plan, but must trigger a fresh prepare rather | |||
| 613 | + | // than being mistaken for a canceled request. | |||
| 614 | + | if current != nil && !current() { | |||
| 573 | return nil, false | 615 | return nil, false | ||
| 574 | } | 616 | } | ||
| 575 | // A layout mutation invalidated the plans while this epoch remains | 617 | // A layout mutation invalidated the plans while this epoch remains | ||
| @@ -654,7 +696,7 @@ func (d *Daemon) retryResizeMembers(sess *session, ac *attachedClient, lease *at | |||||
| 654 | tabs := make([]*tab, 0, len(members)) | 696 | tabs := make([]*tab, 0, len(members)) | ||
| 655 | seen := make(map[*tab]struct{}, len(members)) | 697 | seen := make(map[*tab]struct{}, len(members)) | ||
| 656 | for _, member := range members { | 698 | for _, member := range members { | ||
| 657 | - | if member.tab == nil { | 699 | + | if member.tab == nil || member.isFloating { |
| 658 | continue | 700 | continue | ||
| 659 | } | 701 | } | ||
| 660 | if _, ok := seen[member.tab]; ok { | 702 | if _, ok := seen[member.tab]; ok { | ||
| @@ -713,6 +755,48 @@ func (d *Daemon) retryResizeMembers(sess *session, ac *attachedClient, lease *at | |||||
| 713 | } | 755 | } | ||
| 714 | tb.mu.Unlock() | 756 | tb.mu.Unlock() | ||
| 715 | } | 757 | } | ||
| 758 | + | for _, member := range members { | |||
| 759 | + | if !member.isFloating || member.tab == nil || member.pane == nil { | |||
| 760 | + | continue | |||
| 761 | + | } | |||
| 762 | + | if !rc.retryCurrentForLease(epoch, ac, lease) { | |||
| 763 | + | return | |||
| 764 | + | } | |||
| 765 | + | // Floating panes are outside tb.panes. Validate the exact accepted slot | |||
| 766 | + | // before preparing a fresh geometry; applyVisibleFloatingLayout repeats | |||
| 767 | + | // the same pointer/generation check around external PTY.Resize. | |||
| 768 | + | member.tab.mu.Lock() | |||
| 769 | + | currentSlot := member.tab.floating.state == floatingVisible && | |||
| 770 | + | member.tab.floating.generation == member.floatingGeneration && | |||
| 771 | + | member.tab.floating.pane == member.pane | |||
| 772 | + | retryPending := false | |||
| 773 | + | if currentSlot { | |||
| 774 | + | member.pane.mu.Lock() | |||
| 775 | + | retryPending = member.pane.resizeRetry | |||
| 776 | + | member.pane.mu.Unlock() | |||
| 777 | + | } | |||
| 778 | + | member.tab.mu.Unlock() | |||
| 779 | + | if !currentSlot || !retryPending { | |||
| 780 | + | continue | |||
| 781 | + | } | |||
| 782 | + | freshFailed, ok := d.applyVisibleFloatingLayoutForMember(sess, member.tab, func() bool { | |||
| 783 | + | return rc.retryCurrentForLease(epoch, ac, lease) | |||
| 784 | + | }, &member) | |||
| 785 | + | if !ok || !rc.retryCurrentForLease(epoch, ac, lease) { | |||
| 786 | + | return | |||
| 787 | + | } | |||
| 788 | + | failed = append(failed, freshFailed...) | |||
| 789 | + | member.tab.mu.Lock() | |||
| 790 | + | stillCurrent := member.tab.floating.state == floatingVisible && | |||
| 791 | + | member.tab.floating.generation == member.floatingGeneration && | |||
| 792 | + | member.tab.floating.pane == member.pane | |||
| 793 | + | member.tab.mu.Unlock() | |||
| 794 | + | if stillCurrent && len(freshFailed) == 0 { | |||
| 795 | + | member.pane.mu.Lock() | |||
| 796 | + | succeeded = succeeded || !member.pane.resizeRetry | |||
| 797 | + | member.pane.mu.Unlock() | |||
| 798 | + | } | |||
| 799 | + | } | |||
| 716 | if len(failed) != 0 { | 800 | if len(failed) != 0 { | ||
| 717 | rc.scheduleResizeRetryForLease(epoch, ac, lease, func() { d.retryResizeMembers(sess, ac, lease, epoch, failed) }) | 801 | rc.scheduleResizeRetryForLease(epoch, ac, lease, func() { d.retryResizeMembers(sess, ac, lease, epoch, failed) }) | ||
| 718 | } | 802 | } | ||
M internal/usecase/daemon/transactional_resize_test.go +79 -0
| @@ -478,6 +478,85 @@ func TestTransactionalResizeRejectsNewerEpochBeforeSessionPublication(t *testing | |||
| 478 | 478 | require.Equal(t, []domain.Size{{Cols: 100, Rows: 28}, {Cols: 120, Rows: 32}}, second.requested()) | |
| 479 | 479 | } | |
| 480 | 480 | ||
| 481 | + | func TestStaleRemovedMemberGateIsCanceledOnceByFreshPlan(t *testing.T) { | |
| 482 | + | pty := &transactionalResizePTY{} | |
| 483 | + | d, sess, _, _ := newManualSessionWithPTYs(t, pty) | |
| 484 | + | tb := sess.activeTab() | |
| 485 | + | p := tb.focusedPane() | |
| 486 | + | p.resizeApplying = true | |
| 487 | + | p.resizePending = []byte("x") | |
| 488 | + | plan := preparedTabLayout{members: []resizeMember{{session: sess, tab: tb, pane: p, rect: p.rect, screenResized: true}}} | |
| 489 | + | tb.mu.Lock() | |
| 490 | + | delete(tb.panes, p.id) | |
| 491 | + | tb.bumpLayoutGenerationLocked() | |
| 492 | + | tb.mu.Unlock() | |
| 493 | + | ||
| 494 | + | d.finishPreparedTabMembers(&plan, false) | |
| 495 | + | p.mu.Lock() | |
| 496 | + | require.True(t, p.resizeApplying, "stale finalization must leave cancellation to the fresh-plan path") | |
| 497 | + | p.mu.Unlock() | |
| 498 | + | d.cancelStalePreparedGates(sess, tb, &plan) | |
| 499 | + | p.mu.Lock() | |
| 500 | + | require.False(t, p.resizeApplying) | |
| 501 | + | require.Empty(t, p.resizePending) | |
| 502 | + | p.mu.Unlock() | |
| 503 | + | } | |
| 504 | + | ||
| 505 | + | func TestHeadlessResizeRetriesInvalidatedPlan(t *testing.T) { | |
| 506 | + | pty := &transactionalResizePTY{} | |
| 507 | + | d, sess, _, _ := newManualSessionWithPTYs(t, pty) | |
| 508 | + | tb := sess.activeTab() | |
| 509 | + | var invalidate sync.Once | |
| 510 | + | pty.onResize = func() { | |
| 511 | + | invalidate.Do(func() { | |
| 512 | + | tb.mu.Lock() | |
| 513 | + | tb.bumpLayoutGenerationLocked() | |
| 514 | + | tb.mu.Unlock() | |
| 515 | + | }) | |
| 516 | + | } | |
| 517 | + | ||
| 518 | + | require.True(t, d.requestTransactionalResize(sess, nil, domain.Size{Cols: 100, Rows: 30}, true)) | |
| 519 | + | require.Equal(t, []domain.Size{{Cols: 100, Rows: 28}, {Cols: 100, Rows: 28}}, pty.requested()) | |
| 520 | + | tb.mu.Lock() | |
| 521 | + | require.Equal(t, domain.Size{Cols: 100, Rows: 28}, tb.size) | |
| 522 | + | tb.mu.Unlock() | |
| 523 | + | } | |
| 524 | + | ||
| 525 | + | func TestTransactionalResizeRetriesAcceptedFloatingSlotByIdentity(t *testing.T) { | |
| 526 | + | popupPTY := &transactionalResizePTY{errs: []error{errors.New("first popup resize fails")}} | |
| 527 | + | d, sess, ac, _ := newManualSessionWithPTYs(t, &transactionalResizePTY{}) | |
| 528 | + | tb := sess.activeTab() | |
| 529 | + | popup := newPane("popup", popupPTY, domain.Size{Cols: 80, Rows: 23}) | |
| 530 | + | tb.mu.Lock() | |
| 531 | + | tb.floating = floatingSlot{state: floatingVisible, pane: popup, generation: 7} | |
| 532 | + | tb.mu.Unlock() | |
| 533 | + | ||
| 534 | + | rc := d.attachCoordinator(sess, nil, ac, true) | |
| 535 | + | lease := rc.attachmentLease(ac) | |
| 536 | + | epoch := rc.recordResizeRequestForLease(domain.Size{Cols: 80, Rows: 24}, ac, lease) | |
| 537 | + | require.NotZero(t, epoch) | |
| 538 | + | require.True(t, d.runResizeTransaction(sess, ac, lease, epoch)) | |
| 539 | + | first := popupPTY.requested() | |
| 540 | + | require.Len(t, first, 1) | |
| 541 | + | ||
| 542 | + | d.retryResizeMembers(sess, ac, lease, epoch, []resizeMember{{session: sess, tab: tb, pane: popup, isFloating: true, floatingGeneration: 7}}) | |
| 543 | + | retried := popupPTY.requested() | |
| 544 | + | require.Len(t, retried, 2) | |
| 545 | + | require.Equal(t, first[0], retried[1], "retry must use the current validated floating slot geometry") | |
| 546 | + | popup.mu.Lock() | |
| 547 | + | require.False(t, popup.resizeRetry) | |
| 548 | + | popup.mu.Unlock() | |
| 549 | + | ||
| 550 | + | replacementPTY := &transactionalResizePTY{} | |
| 551 | + | replacement := newPane("replacement", replacementPTY, domain.Size{Cols: 80, Rows: 23}) | |
| 552 | + | replacement.resizeRetry = true | |
| 553 | + | tb.mu.Lock() | |
| 554 | + | tb.floating = floatingSlot{state: floatingVisible, pane: replacement, generation: 8} | |
| 555 | + | tb.mu.Unlock() | |
| 556 | + | d.retryResizeMembers(sess, ac, lease, epoch, []resizeMember{{session: sess, tab: tb, pane: popup, isFloating: true, floatingGeneration: 7}}) | |
| 557 | + | require.Empty(t, replacementPTY.requested(), "a replaced floating slot must not inherit an old retry") | |
| 558 | + | } | |
| 559 | + | ||
| 481 | 560 | func TestTransactionalResizeObsoleteTimerCallbacksCommitOnlyLatestEpoch(t *testing.T) { | |
| 482 | 561 | clock := newCoordinatorMockClock(t, 8) | |
| 483 | 562 | pty := &transactionalResizePTY{} | |
| @@ -478,6 +478,85 @@ func TestTransactionalResizeRejectsNewerEpochBeforeSessionPublication(t *testing | |||||
| 478 | require.Equal(t, []domain.Size{{Cols: 100, Rows: 28}, {Cols: 120, Rows: 32}}, second.requested()) | 478 | require.Equal(t, []domain.Size{{Cols: 100, Rows: 28}, {Cols: 120, Rows: 32}}, second.requested()) | ||
| 479 | } | 479 | } | ||
| 480 | 480 | ||||
| 481 | + | func TestStaleRemovedMemberGateIsCanceledOnceByFreshPlan(t *testing.T) { | |||
| 482 | + | pty := &transactionalResizePTY{} | |||
| 483 | + | d, sess, _, _ := newManualSessionWithPTYs(t, pty) | |||
| 484 | + | tb := sess.activeTab() | |||
| 485 | + | p := tb.focusedPane() | |||
| 486 | + | p.resizeApplying = true | |||
| 487 | + | p.resizePending = []byte("x") | |||
| 488 | + | plan := preparedTabLayout{members: []resizeMember{{session: sess, tab: tb, pane: p, rect: p.rect, screenResized: true}}} | |||
| 489 | + | tb.mu.Lock() | |||
| 490 | + | delete(tb.panes, p.id) | |||
| 491 | + | tb.bumpLayoutGenerationLocked() | |||
| 492 | + | tb.mu.Unlock() | |||
| 493 | + | ||||
| 494 | + | d.finishPreparedTabMembers(&plan, false) | |||
| 495 | + | p.mu.Lock() | |||
| 496 | + | require.True(t, p.resizeApplying, "stale finalization must leave cancellation to the fresh-plan path") | |||
| 497 | + | p.mu.Unlock() | |||
| 498 | + | d.cancelStalePreparedGates(sess, tb, &plan) | |||
| 499 | + | p.mu.Lock() | |||
| 500 | + | require.False(t, p.resizeApplying) | |||
| 501 | + | require.Empty(t, p.resizePending) | |||
| 502 | + | p.mu.Unlock() | |||
| 503 | + | } | |||
| 504 | + | ||||
| 505 | + | func TestHeadlessResizeRetriesInvalidatedPlan(t *testing.T) { | |||
| 506 | + | pty := &transactionalResizePTY{} | |||
| 507 | + | d, sess, _, _ := newManualSessionWithPTYs(t, pty) | |||
| 508 | + | tb := sess.activeTab() | |||
| 509 | + | var invalidate sync.Once | |||
| 510 | + | pty.onResize = func() { | |||
| 511 | + | invalidate.Do(func() { | |||
| 512 | + | tb.mu.Lock() | |||
| 513 | + | tb.bumpLayoutGenerationLocked() | |||
| 514 | + | tb.mu.Unlock() | |||
| 515 | + | }) | |||
| 516 | + | } | |||
| 517 | + | ||||
| 518 | + | require.True(t, d.requestTransactionalResize(sess, nil, domain.Size{Cols: 100, Rows: 30}, true)) | |||
| 519 | + | require.Equal(t, []domain.Size{{Cols: 100, Rows: 28}, {Cols: 100, Rows: 28}}, pty.requested()) | |||
| 520 | + | tb.mu.Lock() | |||
| 521 | + | require.Equal(t, domain.Size{Cols: 100, Rows: 28}, tb.size) | |||
| 522 | + | tb.mu.Unlock() | |||
| 523 | + | } | |||
| 524 | + | ||||
| 525 | + | func TestTransactionalResizeRetriesAcceptedFloatingSlotByIdentity(t *testing.T) { | |||
| 526 | + | popupPTY := &transactionalResizePTY{errs: []error{errors.New("first popup resize fails")}} | |||
| 527 | + | d, sess, ac, _ := newManualSessionWithPTYs(t, &transactionalResizePTY{}) | |||
| 528 | + | tb := sess.activeTab() | |||
| 529 | + | popup := newPane("popup", popupPTY, domain.Size{Cols: 80, Rows: 23}) | |||
| 530 | + | tb.mu.Lock() | |||
| 531 | + | tb.floating = floatingSlot{state: floatingVisible, pane: popup, generation: 7} | |||
| 532 | + | tb.mu.Unlock() | |||
| 533 | + | ||||
| 534 | + | rc := d.attachCoordinator(sess, nil, ac, true) | |||
| 535 | + | lease := rc.attachmentLease(ac) | |||
| 536 | + | epoch := rc.recordResizeRequestForLease(domain.Size{Cols: 80, Rows: 24}, ac, lease) | |||
| 537 | + | require.NotZero(t, epoch) | |||
| 538 | + | require.True(t, d.runResizeTransaction(sess, ac, lease, epoch)) | |||
| 539 | + | first := popupPTY.requested() | |||
| 540 | + | require.Len(t, first, 1) | |||
| 541 | + | ||||
| 542 | + | d.retryResizeMembers(sess, ac, lease, epoch, []resizeMember{{session: sess, tab: tb, pane: popup, isFloating: true, floatingGeneration: 7}}) | |||
| 543 | + | retried := popupPTY.requested() | |||
| 544 | + | require.Len(t, retried, 2) | |||
| 545 | + | require.Equal(t, first[0], retried[1], "retry must use the current validated floating slot geometry") | |||
| 546 | + | popup.mu.Lock() | |||
| 547 | + | require.False(t, popup.resizeRetry) | |||
| 548 | + | popup.mu.Unlock() | |||
| 549 | + | ||||
| 550 | + | replacementPTY := &transactionalResizePTY{} | |||
| 551 | + | replacement := newPane("replacement", replacementPTY, domain.Size{Cols: 80, Rows: 23}) | |||
| 552 | + | replacement.resizeRetry = true | |||
| 553 | + | tb.mu.Lock() | |||
| 554 | + | tb.floating = floatingSlot{state: floatingVisible, pane: replacement, generation: 8} | |||
| 555 | + | tb.mu.Unlock() | |||
| 556 | + | d.retryResizeMembers(sess, ac, lease, epoch, []resizeMember{{session: sess, tab: tb, pane: popup, isFloating: true, floatingGeneration: 7}}) | |||
| 557 | + | require.Empty(t, replacementPTY.requested(), "a replaced floating slot must not inherit an old retry") | |||
| 558 | + | } | |||
| 559 | + | ||||
| 481 | func TestTransactionalResizeObsoleteTimerCallbacksCommitOnlyLatestEpoch(t *testing.T) { | 560 | func TestTransactionalResizeObsoleteTimerCallbacksCommitOnlyLatestEpoch(t *testing.T) { | ||
| 482 | clock := newCoordinatorMockClock(t, 8) | 561 | clock := newCoordinatorMockClock(t, 8) | ||
| 483 | pty := &transactionalResizePTY{} | 562 | pty := &transactionalResizePTY{} | ||
M internal/usecase/layout/ops.go +4 -1
| @@ -55,7 +55,10 @@ func (t *Tree) ResizeFocus(axis Axis, delta int, area domain.Rect) error { | |||
| 55 | 55 | ||
| 56 | 56 | requested := delta | |
| 57 | 57 | if requested < 0 { | |
| 58 | - | requested = -requested | |
| 58 | + | // -(MinInt) overflows. Splitting the negation keeps the magnitude | |
| 59 | + | // representable even for a deliberately hostile caller. | |
| 60 | + | requested = -(requested + 1) | |
| 61 | + | requested++ | |
| 59 | 62 | } | |
| 60 | 63 | if requested == 0 { | |
| 61 | 64 | return ErrTooSmall | |
| @@ -55,7 +55,10 @@ func (t *Tree) ResizeFocus(axis Axis, delta int, area domain.Rect) error { | |||||
| 55 | 55 | ||||
| 56 | requested := delta | 56 | requested := delta | ||
| 57 | if requested < 0 { | 57 | if requested < 0 { | ||
| 58 | - | requested = -requested | 58 | + | // -(MinInt) overflows. Splitting the negation keeps the magnitude |
| 59 | + | // representable even for a deliberately hostile caller. | |||
| 60 | + | requested = -(requested + 1) | |||
| 61 | + | requested++ | |||
| 59 | } | 62 | } | ||
| 60 | if requested == 0 { | 63 | if requested == 0 { | ||
| 61 | return ErrTooSmall | 64 | return ErrTooSmall | ||
M internal/usecase/layout/resize_test.go +10 -0
| @@ -289,6 +289,16 @@ func TestEqualizeFailureIsTransactional(t *testing.T) { | |||
| 289 | 289 | require.Equal(t, before, tr) | |
| 290 | 290 | } | |
| 291 | 291 | ||
| 292 | + | func TestResizeFocusMinIntDoesNotOverflow(t *testing.T) { | |
| 293 | + | t.Parallel() | |
| 294 | + | ||
| 295 | + | tr := &Tree{Root: split(Horizontal, NewLeaf("a"), NewLeaf("b")), Focus: "a"} | |
| 296 | + | before := tr.Clone() | |
| 297 | + | minInt := -int(^uint(0)>>1) - 1 | |
| 298 | + | require.ErrorIs(t, tr.ResizeFocus(Width, minInt, domain.Rect{Width: 41, Height: 2}), ErrTooSmall) | |
| 299 | + | require.Equal(t, before, tr, "an unrepresentable shrink magnitude must remain transactional") | |
| 300 | + | } | |
| 301 | + | ||
| 292 | 302 | func TestCanResize(t *testing.T) { | |
| 293 | 303 | t.Parallel() | |
| 294 | 304 | ||
| @@ -289,6 +289,16 @@ func TestEqualizeFailureIsTransactional(t *testing.T) { | |||||
| 289 | require.Equal(t, before, tr) | 289 | require.Equal(t, before, tr) | ||
| 290 | } | 290 | } | ||
| 291 | 291 | ||||
| 292 | + | func TestResizeFocusMinIntDoesNotOverflow(t *testing.T) { | |||
| 293 | + | t.Parallel() | |||
| 294 | + | ||||
| 295 | + | tr := &Tree{Root: split(Horizontal, NewLeaf("a"), NewLeaf("b")), Focus: "a"} | |||
| 296 | + | before := tr.Clone() | |||
| 297 | + | minInt := -int(^uint(0)>>1) - 1 | |||
| 298 | + | require.ErrorIs(t, tr.ResizeFocus(Width, minInt, domain.Rect{Width: 41, Height: 2}), ErrTooSmall) | |||
| 299 | + | require.Equal(t, before, tr, "an unrepresentable shrink magnitude must remain transactional") | |||
| 300 | + | } | |||
| 301 | + | ||||
| 292 | func TestCanResize(t *testing.T) { | 302 | func TestCanResize(t *testing.T) { | ||
| 293 | t.Parallel() | 303 | t.Parallel() | ||
| 294 | 304 | ||||