fix(daemon): address lifecycle review findings
- commit
- 212c6b7f7a2afc535634ca7b0b67b4bc73ff6ca4
- author
- brice · · 2026-07-28 17:22 UTC
- parent
- 4441ea3
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email
8 files · +33 -10
M internal/usecase/daemon/input.go +6 -0
| @@ -384,6 +384,12 @@ func (h daemonKeyHandler) Action(action keys.Action) { | |||
| 384 | 384 | case keys.ActionOpenPalette: | |
| 385 | 385 | h.d.enterPalette(sess, h.ac) | |
| 386 | 386 | case keys.ActionJumpAttention: | |
| 387 | + | if effect == nil { | |
| 388 | + | if err := h.d.jumpAttention(sess, h.ac); err != nil { | |
| 389 | + | h.d.reportError(sess, err) | |
| 390 | + | } | |
| 391 | + | return | |
| 392 | + | } | |
| 387 | 393 | effect.bindActionEnd(h.d, "jump-attention") | |
| 388 | 394 | token := h.roleToken | |
| 389 | 395 | token.effect = effect | |
| @@ -384,6 +384,12 @@ func (h daemonKeyHandler) Action(action keys.Action) { | |||||
| 384 | case keys.ActionOpenPalette: | 384 | case keys.ActionOpenPalette: | ||
| 385 | h.d.enterPalette(sess, h.ac) | 385 | h.d.enterPalette(sess, h.ac) | ||
| 386 | case keys.ActionJumpAttention: | 386 | case keys.ActionJumpAttention: | ||
| 387 | + | if effect == nil { | |||
| 388 | + | if err := h.d.jumpAttention(sess, h.ac); err != nil { | |||
| 389 | + | h.d.reportError(sess, err) | |||
| 390 | + | } | |||
| 391 | + | return | |||
| 392 | + | } | |||
| 387 | effect.bindActionEnd(h.d, "jump-attention") | 393 | effect.bindActionEnd(h.d, "jump-attention") | ||
| 388 | token := h.roleToken | 394 | token := h.roleToken | ||
| 389 | token.effect = effect | 395 | token.effect = effect | ||
M internal/usecase/daemon/layout_mutation.go +6 -1
| @@ -69,7 +69,12 @@ func (d *Daemon) mutateTargetLayoutChanged(target daemonActionTarget, requirePan | |||
| 69 | 69 | d.mu.Unlock() | |
| 70 | 70 | ||
| 71 | 71 | if !d.applyTabLayout(target.session, target.tab) { | |
| 72 | - | return false, layout.ErrNotFound | |
| 72 | + | target.tab.mu.Lock() | |
| 73 | + | applied := target.tab.tree == candidate | |
| 74 | + | target.tab.mu.Unlock() | |
| 75 | + | if !applied { | |
| 76 | + | return false, layout.ErrNotFound | |
| 77 | + | } | |
| 73 | 78 | } | |
| 74 | 79 | return true, nil | |
| 75 | 80 | } | |
| @@ -69,7 +69,12 @@ func (d *Daemon) mutateTargetLayoutChanged(target daemonActionTarget, requirePan | |||||
| 69 | d.mu.Unlock() | 69 | d.mu.Unlock() | ||
| 70 | 70 | ||||
| 71 | if !d.applyTabLayout(target.session, target.tab) { | 71 | if !d.applyTabLayout(target.session, target.tab) { | ||
| 72 | - | return false, layout.ErrNotFound | 72 | + | target.tab.mu.Lock() |
| 73 | + | applied := target.tab.tree == candidate | |||
| 74 | + | target.tab.mu.Unlock() | |||
| 75 | + | if !applied { | |||
| 76 | + | return false, layout.ErrNotFound | |||
| 77 | + | } | |||
| 73 | } | 78 | } | ||
| 74 | return true, nil | 79 | return true, nil | ||
| 75 | } | 80 | } | ||
M internal/usecase/daemon/move.go +5 -1
| @@ -154,7 +154,11 @@ func (d *Daemon) movePane(req movePaneRequest) (result error) { | |||
| 154 | 154 | } | |
| 155 | 155 | source.mu.Unlock() | |
| 156 | 156 | d.mu.Unlock() | |
| 157 | - | frozen = freezeRoleEffectGatesWith(roleEffectFreezeOptions{interrupts: interrupts, nonblocking: true}, participants...) | |
| 157 | + | frozen = freezeRoleEffectGatesWith(roleEffectFreezeOptions{interrupts: interrupts, nonblocking: true, afterFrozen: func(ac *attachedClient) { | |
| 158 | + | if d.afterRoleEffectGateFrozen != nil { | |
| 159 | + | d.afterRoleEffectGateFrozen("move-pane", ac) | |
| 160 | + | } | |
| 161 | + | }}, participants...) | |
| 158 | 162 | if !frozen.acquired || !frozen.drained { | |
| 159 | 163 | frozen.unfreeze() | |
| 160 | 164 | return errMovePaneInvalid | |
| @@ -154,7 +154,11 @@ func (d *Daemon) movePane(req movePaneRequest) (result error) { | |||||
| 154 | } | 154 | } | ||
| 155 | source.mu.Unlock() | 155 | source.mu.Unlock() | ||
| 156 | d.mu.Unlock() | 156 | d.mu.Unlock() | ||
| 157 | - | frozen = freezeRoleEffectGatesWith(roleEffectFreezeOptions{interrupts: interrupts, nonblocking: true}, participants...) | 157 | + | frozen = freezeRoleEffectGatesWith(roleEffectFreezeOptions{interrupts: interrupts, nonblocking: true, afterFrozen: func(ac *attachedClient) { |
| 158 | + | if d.afterRoleEffectGateFrozen != nil { | |||
| 159 | + | d.afterRoleEffectGateFrozen("move-pane", ac) | |||
| 160 | + | } | |||
| 161 | + | }}, participants...) | |||
| 158 | if !frozen.acquired || !frozen.drained { | 162 | if !frozen.acquired || !frozen.drained { | ||
| 159 | frozen.unfreeze() | 163 | frozen.unfreeze() | ||
| 160 | return errMovePaneInvalid | 164 | return errMovePaneInvalid | ||
M internal/usecase/daemon/pane_lifetime_test.go +1 -1
| @@ -311,7 +311,7 @@ func TestPaneProcessLifetimeIsDaemonRooted(t *testing.T) { | |||
| 311 | 311 | require.NotNil(t, p) | |
| 312 | 312 | pty, openCtx := factory.opened(0) | |
| 313 | 313 | require.NotNil(t, p.ctx) | |
| 314 | - | require.Equal(t, openCtx, p.ctx) | |
| 314 | + | require.Same(t, openCtx, p.ctx) | |
| 315 | 315 | require.NotNil(t, p.cancel) | |
| 316 | 316 | ||
| 317 | 317 | cancelSession() | |
| @@ -311,7 +311,7 @@ func TestPaneProcessLifetimeIsDaemonRooted(t *testing.T) { | |||||
| 311 | require.NotNil(t, p) | 311 | require.NotNil(t, p) | ||
| 312 | pty, openCtx := factory.opened(0) | 312 | pty, openCtx := factory.opened(0) | ||
| 313 | require.NotNil(t, p.ctx) | 313 | require.NotNil(t, p.ctx) | ||
| 314 | - | require.Equal(t, openCtx, p.ctx) | 314 | + | require.Same(t, openCtx, p.ctx) |
| 315 | require.NotNil(t, p.cancel) | 315 | require.NotNil(t, p.cancel) | ||
| 316 | 316 | ||||
| 317 | cancelSession() | 317 | cancelSession() | ||
M internal/usecase/daemon/render_resize_deadline.go +1 -1
| @@ -112,7 +112,7 @@ type resizeRetryReservation struct { | |||
| 112 | 112 | func (c *renderCoordinator) reserveResizeRetryForLease(epoch uint64, source *attachedClient, lease *attachmentLease, run func()) (*resizeRetryReservation, bool) { | |
| 113 | 113 | c.mu.Lock() | |
| 114 | 114 | defer c.mu.Unlock() | |
| 115 | - | if !c.leaseCurrentLocked(lease, false) || lease.attachment != source || c.resize.lease != lease || c.resize.source != source || c.resize.epoch != epoch || c.resize.committed != epoch || c.resize.retryAttempts >= maxResizeRetryAttempts { | |
| 115 | + | if c.torndown || !c.leaseCurrentLocked(lease, false) || lease.attachment != source || c.resize.lease != lease || c.resize.source != source || c.resize.epoch != epoch || c.resize.committed != epoch || c.resize.retryAttempts >= maxResizeRetryAttempts { | |
| 116 | 116 | return nil, false | |
| 117 | 117 | } | |
| 118 | 118 | c.resize.retryAttempts++ | |
| @@ -112,7 +112,7 @@ type resizeRetryReservation struct { | |||||
| 112 | func (c *renderCoordinator) reserveResizeRetryForLease(epoch uint64, source *attachedClient, lease *attachmentLease, run func()) (*resizeRetryReservation, bool) { | 112 | func (c *renderCoordinator) reserveResizeRetryForLease(epoch uint64, source *attachedClient, lease *attachmentLease, run func()) (*resizeRetryReservation, bool) { | ||
| 113 | c.mu.Lock() | 113 | c.mu.Lock() | ||
| 114 | defer c.mu.Unlock() | 114 | defer c.mu.Unlock() | ||
| 115 | - | if !c.leaseCurrentLocked(lease, false) || lease.attachment != source || c.resize.lease != lease || c.resize.source != source || c.resize.epoch != epoch || c.resize.committed != epoch || c.resize.retryAttempts >= maxResizeRetryAttempts { | 115 | + | if c.torndown || !c.leaseCurrentLocked(lease, false) || lease.attachment != source || c.resize.lease != lease || c.resize.source != source || c.resize.epoch != epoch || c.resize.committed != epoch || c.resize.retryAttempts >= maxResizeRetryAttempts { |
| 116 | return nil, false | 116 | return nil, false | ||
| 117 | } | 117 | } | ||
| 118 | c.resize.retryAttempts++ | 118 | c.resize.retryAttempts++ | ||
M internal/usecase/daemon/resume.go +1 -1
| @@ -78,10 +78,10 @@ func (d *Daemon) parkAttachmentAs(sess *session, ac *attachedClient, role attach | |||
| 78 | 78 | oldSame *parkedAttachment | |
| 79 | 79 | ) | |
| 80 | 80 | if old := d.parked[token]; old != nil { | |
| 81 | - | retirement := d.retireParkedAttachmentLocked(token, old) | |
| 82 | 81 | if old.ac == ac { | |
| 83 | 82 | oldSame = old | |
| 84 | 83 | } else { | |
| 84 | + | retirement := d.retireParkedAttachmentLocked(token, old) | |
| 85 | 85 | oldRetirement = &retirement | |
| 86 | 86 | } | |
| 87 | 87 | } | |
| @@ -78,10 +78,10 @@ func (d *Daemon) parkAttachmentAs(sess *session, ac *attachedClient, role attach | |||||
| 78 | oldSame *parkedAttachment | 78 | oldSame *parkedAttachment | ||
| 79 | ) | 79 | ) | ||
| 80 | if old := d.parked[token]; old != nil { | 80 | if old := d.parked[token]; old != nil { | ||
| 81 | - | retirement := d.retireParkedAttachmentLocked(token, old) | |||
| 82 | if old.ac == ac { | 81 | if old.ac == ac { | ||
| 83 | oldSame = old | 82 | oldSame = old | ||
| 84 | } else { | 83 | } else { | ||
| 84 | + | retirement := d.retireParkedAttachmentLocked(token, old) | |||
| 85 | oldRetirement = &retirement | 85 | oldRetirement = &retirement | ||
| 86 | } | 86 | } | ||
| 87 | } | 87 | } | ||
M internal/usecase/daemon/role_effect_gate_test.go +12 -4
| @@ -1178,10 +1178,17 @@ func TestRoleEffectGateAdmittedSnatchedPanelFinishesBeforePromotion(t *testing.T | |||
| 1178 | 1178 | token := replaced.displaced | |
| 1179 | 1179 | ||
| 1180 | 1180 | admitted := make(chan struct{}) | |
| 1181 | - | release := make(chan struct{}) | |
| 1181 | + | releaseGate := make(chan struct{}) | |
| 1182 | + | release := releaseTestGate(t, releaseGate) | |
| 1183 | + | afterFrozen := make(chan struct{}) | |
| 1182 | 1184 | d.afterRoleEffectAdmitted = func(attachmentRoleToken) { | |
| 1183 | 1185 | close(admitted) | |
| 1184 | - | <-release | |
| 1186 | + | <-releaseGate | |
| 1187 | + | } | |
| 1188 | + | d.afterRoleEffectGateFrozen = func(action string, ac *attachedClient) { | |
| 1189 | + | if action == "" && ac == old { | |
| 1190 | + | close(afterFrozen) | |
| 1191 | + | } | |
| 1185 | 1192 | } | |
| 1186 | 1193 | effectDone := make(chan struct{}) | |
| 1187 | 1194 | wantSize := domain.Size{Cols: 100, Rows: 30} | |
| @@ -1199,12 +1206,13 @@ func TestRoleEffectGateAdmittedSnatchedPanelFinishesBeforePromotion(t *testing.T | |||
| 1199 | 1206 | }) | |
| 1200 | 1207 | transitionDone <- err | |
| 1201 | 1208 | }() | |
| 1209 | + | <-afterFrozen | |
| 1202 | 1210 | select { | |
| 1203 | 1211 | case err := <-transitionDone: | |
| 1204 | 1212 | t.Fatalf("promotion overtook admitted snatched panel: %v", err) | |
| 1205 | - | case <-time.After(20 * time.Millisecond): | |
| 1213 | + | default: | |
| 1206 | 1214 | } | |
| 1207 | - | close(release) | |
| 1215 | + | release() | |
| 1208 | 1216 | <-effectDone | |
| 1209 | 1217 | require.NoError(t, <-transitionDone) | |
| 1210 | 1218 | old.sendMu.Lock() | |
| @@ -1178,10 +1178,17 @@ func TestRoleEffectGateAdmittedSnatchedPanelFinishesBeforePromotion(t *testing.T | |||||
| 1178 | token := replaced.displaced | 1178 | token := replaced.displaced | ||
| 1179 | 1179 | ||||
| 1180 | admitted := make(chan struct{}) | 1180 | admitted := make(chan struct{}) | ||
| 1181 | - | release := make(chan struct{}) | 1181 | + | releaseGate := make(chan struct{}) |
| 1182 | + | release := releaseTestGate(t, releaseGate) | |||
| 1183 | + | afterFrozen := make(chan struct{}) | |||
| 1182 | d.afterRoleEffectAdmitted = func(attachmentRoleToken) { | 1184 | d.afterRoleEffectAdmitted = func(attachmentRoleToken) { | ||
| 1183 | close(admitted) | 1185 | close(admitted) | ||
| 1184 | - | <-release | 1186 | + | <-releaseGate |
| 1187 | + | } | |||
| 1188 | + | d.afterRoleEffectGateFrozen = func(action string, ac *attachedClient) { | |||
| 1189 | + | if action == "" && ac == old { | |||
| 1190 | + | close(afterFrozen) | |||
| 1191 | + | } | |||
| 1185 | } | 1192 | } | ||
| 1186 | effectDone := make(chan struct{}) | 1193 | effectDone := make(chan struct{}) | ||
| 1187 | wantSize := domain.Size{Cols: 100, Rows: 30} | 1194 | wantSize := domain.Size{Cols: 100, Rows: 30} | ||
| @@ -1199,12 +1206,13 @@ func TestRoleEffectGateAdmittedSnatchedPanelFinishesBeforePromotion(t *testing.T | |||||
| 1199 | }) | 1206 | }) | ||
| 1200 | transitionDone <- err | 1207 | transitionDone <- err | ||
| 1201 | }() | 1208 | }() | ||
| 1209 | + | <-afterFrozen | |||
| 1202 | select { | 1210 | select { | ||
| 1203 | case err := <-transitionDone: | 1211 | case err := <-transitionDone: | ||
| 1204 | t.Fatalf("promotion overtook admitted snatched panel: %v", err) | 1212 | t.Fatalf("promotion overtook admitted snatched panel: %v", err) | ||
| 1205 | - | case <-time.After(20 * time.Millisecond): | 1213 | + | default: |
| 1206 | } | 1214 | } | ||
| 1207 | - | close(release) | 1215 | + | release() |
| 1208 | <-effectDone | 1216 | <-effectDone | ||
| 1209 | require.NoError(t, <-transitionDone) | 1217 | require.NoError(t, <-transitionDone) | ||
| 1210 | old.sendMu.Lock() | 1218 | old.sendMu.Lock() | ||
M internal/usecase/daemon/transactional_resize.go +1 -1
| @@ -291,7 +291,7 @@ func (d *Daemon) applyTabLayoutTransactionWithNotice(sess *session, tb *tab, rep | |||
| 291 | 291 | failed = append(failed, member) | |
| 292 | 292 | } | |
| 293 | 293 | } | |
| 294 | - | if reportFailure && len(failed) != 0 { | |
| 294 | + | if reportFailure && len(failed) != 0 && resizeMembersOwnerCurrent(failed) { | |
| 295 | 295 | d.notify(sess, domain.NoticeWarn, domain.NoticeResizeFailed, | |
| 296 | 296 | "pane resize failed; retrying in background", failed[len(failed)-1].err) | |
| 297 | 297 | } | |
| @@ -291,7 +291,7 @@ func (d *Daemon) applyTabLayoutTransactionWithNotice(sess *session, tb *tab, rep | |||||
| 291 | failed = append(failed, member) | 291 | failed = append(failed, member) | ||
| 292 | } | 292 | } | ||
| 293 | } | 293 | } | ||
| 294 | - | if reportFailure && len(failed) != 0 { | 294 | + | if reportFailure && len(failed) != 0 && resizeMembersOwnerCurrent(failed) { |
| 295 | d.notify(sess, domain.NoticeWarn, domain.NoticeResizeFailed, | 295 | d.notify(sess, domain.NoticeWarn, domain.NoticeResizeFailed, | ||
| 296 | "pane resize failed; retrying in background", failed[len(failed)-1].err) | 296 | "pane resize failed; retrying in background", failed[len(failed)-1].err) | ||
| 297 | } | 297 | } | ||