fix(render): prune closed pane captures
- commit
- fd5be11aed09c455c53ccf5d3c65d36d1b9cd485
- author
- brice · · 2026-07-13 06:36 UTC
- parent
- d8754cd
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email
5 files · +73 -3
M internal/usecase/daemon/client.go +24 -0
| @@ -114,6 +114,30 @@ func (ac *attachedClient) clearPreviousSession() { | |||
| 114 | 114 | } | |
| 115 | 115 | } | |
| 116 | 116 | ||
| 117 | + | // pruneCaptureFrames releases snapshots for panes that have left their owner. | |
| 118 | + | // Callers must not hold daemon, session, tab, or pane locks. | |
| 119 | + | func (ac *attachedClient) pruneCaptureFrames(panes ...*pane) { | |
| 120 | + | if ac == nil || len(panes) == 0 { | |
| 121 | + | return | |
| 122 | + | } | |
| 123 | + | ac.sendMu.Lock() | |
| 124 | + | for _, p := range panes { | |
| 125 | + | delete(ac.captureFrames, p) | |
| 126 | + | } | |
| 127 | + | ac.sendMu.Unlock() | |
| 128 | + | } | |
| 129 | + | ||
| 130 | + | // clearCaptureFrames releases all pane snapshots when the attachment no longer | |
| 131 | + | // owns a session. | |
| 132 | + | func (ac *attachedClient) clearCaptureFrames() { | |
| 133 | + | if ac == nil { | |
| 134 | + | return | |
| 135 | + | } | |
| 136 | + | ac.sendMu.Lock() | |
| 137 | + | ac.captureFrames = nil | |
| 138 | + | ac.sendMu.Unlock() | |
| 139 | + | } | |
| 140 | + | ||
| 117 | 141 | func (ac *attachedClient) getTheme() themeui.Theme { | |
| 118 | 142 | ac.themeMu.Lock() | |
| 119 | 143 | defer ac.themeMu.Unlock() | |
| @@ -114,6 +114,30 @@ func (ac *attachedClient) clearPreviousSession() { | |||||
| 114 | } | 114 | } | ||
| 115 | } | 115 | } | ||
| 116 | 116 | ||||
| 117 | + | // pruneCaptureFrames releases snapshots for panes that have left their owner. | |||
| 118 | + | // Callers must not hold daemon, session, tab, or pane locks. | |||
| 119 | + | func (ac *attachedClient) pruneCaptureFrames(panes ...*pane) { | |||
| 120 | + | if ac == nil || len(panes) == 0 { | |||
| 121 | + | return | |||
| 122 | + | } | |||
| 123 | + | ac.sendMu.Lock() | |||
| 124 | + | for _, p := range panes { | |||
| 125 | + | delete(ac.captureFrames, p) | |||
| 126 | + | } | |||
| 127 | + | ac.sendMu.Unlock() | |||
| 128 | + | } | |||
| 129 | + | ||||
| 130 | + | // clearCaptureFrames releases all pane snapshots when the attachment no longer | |||
| 131 | + | // owns a session. | |||
| 132 | + | func (ac *attachedClient) clearCaptureFrames() { | |||
| 133 | + | if ac == nil { | |||
| 134 | + | return | |||
| 135 | + | } | |||
| 136 | + | ac.sendMu.Lock() | |||
| 137 | + | ac.captureFrames = nil | |||
| 138 | + | ac.sendMu.Unlock() | |||
| 139 | + | } | |||
| 140 | + | ||||
| 117 | func (ac *attachedClient) getTheme() themeui.Theme { | 141 | func (ac *attachedClient) getTheme() themeui.Theme { | ||
| 118 | ac.themeMu.Lock() | 142 | ac.themeMu.Lock() | ||
| 119 | defer ac.themeMu.Unlock() | 143 | defer ac.themeMu.Unlock() | ||
M internal/usecase/daemon/floating.go +4 -0
| @@ -427,6 +427,9 @@ func (d *Daemon) reapFloating(sess *session, tb *tab, p *pane, generation uint64 | |||
| 427 | 427 | sess.mu.Lock() | |
| 428 | 428 | ac := sess.client | |
| 429 | 429 | sess.mu.Unlock() | |
| 430 | + | if ac != nil { | |
| 431 | + | ac.pruneCaptureFrames(p) | |
| 432 | + | } | |
| 430 | 433 | copyCleared := ac != nil && ac.overlays.clearCopyModeForPane(p) | |
| 431 | 434 | if ac != nil && (visible || copyCleared) { | |
| 432 | 435 | d.invalidateRender(sess, ac, true, "floating.go") | |
| @@ -443,6 +446,7 @@ func (d *Daemon) teardownFloating(tb *tab, ac *attachedClient) { | |||
| 443 | 446 | tb.mu.Unlock() | |
| 444 | 447 | if ac != nil { | |
| 445 | 448 | ac.overlays.clearCopyModeForPane(p) | |
| 449 | + | ac.pruneCaptureFrames(p) | |
| 446 | 450 | } | |
| 447 | 451 | closeFloatingPane(p) | |
| 448 | 452 | } | |
| @@ -427,6 +427,9 @@ func (d *Daemon) reapFloating(sess *session, tb *tab, p *pane, generation uint64 | |||||
| 427 | sess.mu.Lock() | 427 | sess.mu.Lock() | ||
| 428 | ac := sess.client | 428 | ac := sess.client | ||
| 429 | sess.mu.Unlock() | 429 | sess.mu.Unlock() | ||
| 430 | + | if ac != nil { | |||
| 431 | + | ac.pruneCaptureFrames(p) | |||
| 432 | + | } | |||
| 430 | copyCleared := ac != nil && ac.overlays.clearCopyModeForPane(p) | 433 | copyCleared := ac != nil && ac.overlays.clearCopyModeForPane(p) | ||
| 431 | if ac != nil && (visible || copyCleared) { | 434 | if ac != nil && (visible || copyCleared) { | ||
| 432 | d.invalidateRender(sess, ac, true, "floating.go") | 435 | d.invalidateRender(sess, ac, true, "floating.go") | ||
| @@ -443,6 +446,7 @@ func (d *Daemon) teardownFloating(tb *tab, ac *attachedClient) { | |||||
| 443 | tb.mu.Unlock() | 446 | tb.mu.Unlock() | ||
| 444 | if ac != nil { | 447 | if ac != nil { | ||
| 445 | ac.overlays.clearCopyModeForPane(p) | 448 | ac.overlays.clearCopyModeForPane(p) | ||
| 449 | + | ac.pruneCaptureFrames(p) | |||
| 446 | } | 450 | } | ||
| 447 | closeFloatingPane(p) | 451 | closeFloatingPane(p) | ||
| 448 | } | 452 | } | ||
M internal/usecase/daemon/input_test.go +37 -0
| @@ -121,6 +121,43 @@ func TestSwitchTabFirstFrameDoesNotReuseSamePaneIDCapture(t *testing.T) { | |||
| 121 | 121 | require.NotContains(t, strings.Join(frameRows(terminal.Frame), "\n"), "source", "the first target-tab frame must not retain source pane cells") | |
| 122 | 122 | } | |
| 123 | 123 | ||
| 124 | + | func TestClosePanePrunesAttachedCaptureFrameAndKeepsSurvivor(t *testing.T) { | |
| 125 | + | d, sess, ac, _ := newManualSessionWithPTYs(t, nil) | |
| 126 | + | tb := sess.tabs[0] | |
| 127 | + | survivor := tb.panes["pane-1"] | |
| 128 | + | closed := newPane("pane-2", nil, domain.Size{Cols: 40, Rows: 23}) | |
| 129 | + | tb.mu.Lock() | |
| 130 | + | 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"} | |
| 131 | + | tb.panes[closed.id] = closed | |
| 132 | + | tb.mu.Unlock() | |
| 133 | + | ||
| 134 | + | cachedSurvivor := capturedPaneRenderState{title: "survivor"} | |
| 135 | + | ac.captureFrames = map[*pane]capturedPaneRenderState{ | |
| 136 | + | survivor: cachedSurvivor, | |
| 137 | + | closed: {title: "closed"}, | |
| 138 | + | } | |
| 139 | + | ||
| 140 | + | require.NoError(t, d.closePane(sess, tb, closed.id, ac, false)) | |
| 141 | + | require.NotContains(t, ac.captureFrames, closed, "closed pane must not remain strongly retained by its attachment") | |
| 142 | + | require.Equal(t, cachedSurvivor, ac.captureFrames[survivor], "surviving pane keeps its incremental capture") | |
| 143 | + | } | |
| 144 | + | ||
| 145 | + | func TestCloseTabPrunesAttachedCaptureFrames(t *testing.T) { | |
| 146 | + | d, sess, ac, _ := newManualSessionWithPTYs(t, nil, nil) | |
| 147 | + | closedTab, survivorTab := sess.tabs[0], sess.tabs[1] | |
| 148 | + | closed := closedTab.panes["pane-1"] | |
| 149 | + | survivor := survivorTab.panes["pane-1"] | |
| 150 | + | cachedSurvivor := capturedPaneRenderState{title: "survivor"} | |
| 151 | + | ac.captureFrames = map[*pane]capturedPaneRenderState{ | |
| 152 | + | closed: {title: "closed"}, | |
| 153 | + | survivor: cachedSurvivor, | |
| 154 | + | } | |
| 155 | + | ||
| 156 | + | d.closeTab(sess, closedTab, false) | |
| 157 | + | require.NotContains(t, ac.captureFrames, closed, "closed tab panes must not remain strongly retained by its attachment") | |
| 158 | + | require.Equal(t, cachedSurvivor, ac.captureFrames[survivor], "surviving tab pane keeps its incremental capture") | |
| 159 | + | } | |
| 160 | + | ||
| 124 | 161 | func TestAltCForwardsToPTY(t *testing.T) { | |
| 125 | 162 | writes := make(chan []byte, 2) | |
| 126 | 163 | p, releasePTY := newBlockingPTYWithWrites(t, writes) | |
| @@ -121,6 +121,43 @@ func TestSwitchTabFirstFrameDoesNotReuseSamePaneIDCapture(t *testing.T) { | |||||
| 121 | require.NotContains(t, strings.Join(frameRows(terminal.Frame), "\n"), "source", "the first target-tab frame must not retain source pane cells") | 121 | require.NotContains(t, strings.Join(frameRows(terminal.Frame), "\n"), "source", "the first target-tab frame must not retain source pane cells") | ||
| 122 | } | 122 | } | ||
| 123 | 123 | ||||
| 124 | + | func TestClosePanePrunesAttachedCaptureFrameAndKeepsSurvivor(t *testing.T) { | |||
| 125 | + | d, sess, ac, _ := newManualSessionWithPTYs(t, nil) | |||
| 126 | + | tb := sess.tabs[0] | |||
| 127 | + | survivor := tb.panes["pane-1"] | |||
| 128 | + | closed := newPane("pane-2", nil, domain.Size{Cols: 40, Rows: 23}) | |||
| 129 | + | tb.mu.Lock() | |||
| 130 | + | 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"} | |||
| 131 | + | tb.panes[closed.id] = closed | |||
| 132 | + | tb.mu.Unlock() | |||
| 133 | + | ||||
| 134 | + | cachedSurvivor := capturedPaneRenderState{title: "survivor"} | |||
| 135 | + | ac.captureFrames = map[*pane]capturedPaneRenderState{ | |||
| 136 | + | survivor: cachedSurvivor, | |||
| 137 | + | closed: {title: "closed"}, | |||
| 138 | + | } | |||
| 139 | + | ||||
| 140 | + | require.NoError(t, d.closePane(sess, tb, closed.id, ac, false)) | |||
| 141 | + | require.NotContains(t, ac.captureFrames, closed, "closed pane must not remain strongly retained by its attachment") | |||
| 142 | + | require.Equal(t, cachedSurvivor, ac.captureFrames[survivor], "surviving pane keeps its incremental capture") | |||
| 143 | + | } | |||
| 144 | + | ||||
| 145 | + | func TestCloseTabPrunesAttachedCaptureFrames(t *testing.T) { | |||
| 146 | + | d, sess, ac, _ := newManualSessionWithPTYs(t, nil, nil) | |||
| 147 | + | closedTab, survivorTab := sess.tabs[0], sess.tabs[1] | |||
| 148 | + | closed := closedTab.panes["pane-1"] | |||
| 149 | + | survivor := survivorTab.panes["pane-1"] | |||
| 150 | + | cachedSurvivor := capturedPaneRenderState{title: "survivor"} | |||
| 151 | + | ac.captureFrames = map[*pane]capturedPaneRenderState{ | |||
| 152 | + | closed: {title: "closed"}, | |||
| 153 | + | survivor: cachedSurvivor, | |||
| 154 | + | } | |||
| 155 | + | ||||
| 156 | + | d.closeTab(sess, closedTab, false) | |||
| 157 | + | require.NotContains(t, ac.captureFrames, closed, "closed tab panes must not remain strongly retained by its attachment") | |||
| 158 | + | require.Equal(t, cachedSurvivor, ac.captureFrames[survivor], "surviving tab pane keeps its incremental capture") | |||
| 159 | + | } | |||
| 160 | + | ||||
| 124 | func TestAltCForwardsToPTY(t *testing.T) { | 161 | func TestAltCForwardsToPTY(t *testing.T) { | ||
| 125 | writes := make(chan []byte, 2) | 162 | writes := make(chan []byte, 2) | ||
| 126 | p, releasePTY := newBlockingPTYWithWrites(t, writes) | 163 | p, releasePTY := newBlockingPTYWithWrites(t, writes) | ||
M internal/usecase/daemon/pane_actions.go +1 -0
| @@ -255,6 +255,7 @@ func (d *Daemon) closePane(sess *session, tb *tab, id layout.PaneID, ac *attache | |||
| 255 | 255 | ||
| 256 | 256 | if ac != nil { | |
| 257 | 257 | ac.overlays.clearCopyModeForPane(p) | |
| 258 | + | ac.pruneCaptureFrames(p) | |
| 258 | 259 | } | |
| 259 | 260 | ||
| 260 | 261 | if rc := sess.renderCoordinator(); rc != nil { | |
| @@ -255,6 +255,7 @@ func (d *Daemon) closePane(sess *session, tb *tab, id layout.PaneID, ac *attache | |||||
| 255 | 255 | ||||
| 256 | if ac != nil { | 256 | if ac != nil { | ||
| 257 | ac.overlays.clearCopyModeForPane(p) | 257 | ac.overlays.clearCopyModeForPane(p) | ||
| 258 | + | ac.pruneCaptureFrames(p) | |||
| 258 | } | 259 | } | ||
| 259 | 260 | ||||
| 260 | if rc := sess.renderCoordinator(); rc != nil { | 261 | if rc := sess.renderCoordinator(); rc != nil { | ||
M internal/usecase/daemon/session.go +7 -3
| @@ -632,14 +632,17 @@ func (d *Daemon) closeTab(sess *session, tb *tab, repaint bool) { | |||
| 632 | 632 | markSnapshotDirty(sess) | |
| 633 | 633 | ||
| 634 | 634 | d.clearDestroyedTabPreview(tb) | |
| 635 | + | tb.mu.Lock() | |
| 636 | + | panes := tb.panesSnapshot() | |
| 637 | + | tb.mu.Unlock() | |
| 638 | + | if ac != nil { | |
| 639 | + | ac.pruneCaptureFrames(panes...) | |
| 640 | + | } | |
| 635 | 641 | if tb.cancel != nil { | |
| 636 | 642 | tb.cancel() | |
| 637 | 643 | } | |
| 638 | 644 | d.teardownFloating(tb, ac) | |
| 639 | 645 | if rc := sess.renderCoordinator(); rc != nil { | |
| 640 | - | tb.mu.Lock() | |
| 641 | - | panes := tb.panesSnapshot() | |
| 642 | - | tb.mu.Unlock() | |
| 643 | 646 | for _, p := range panes { | |
| 644 | 647 | rc.noteSyncPaneRemoved(p) | |
| 645 | 648 | } | |
| @@ -742,6 +745,7 @@ func (d *Daemon) killSession(sess *session, reason uint8, purge bool) error { | |||
| 742 | 745 | d.unregisterPreview(ac) | |
| 743 | 746 | ac.clearPreviousSession() | |
| 744 | 747 | ac.setSession(nil) | |
| 748 | + | ac.clearCaptureFrames() | |
| 745 | 749 | } | |
| 746 | 750 | ||
| 747 | 751 | // Prevent queued launches from entering Open, then cancel the parent | |
| @@ -632,14 +632,17 @@ func (d *Daemon) closeTab(sess *session, tb *tab, repaint bool) { | |||||
| 632 | markSnapshotDirty(sess) | 632 | markSnapshotDirty(sess) | ||
| 633 | 633 | ||||
| 634 | d.clearDestroyedTabPreview(tb) | 634 | d.clearDestroyedTabPreview(tb) | ||
| 635 | + | tb.mu.Lock() | |||
| 636 | + | panes := tb.panesSnapshot() | |||
| 637 | + | tb.mu.Unlock() | |||
| 638 | + | if ac != nil { | |||
| 639 | + | ac.pruneCaptureFrames(panes...) | |||
| 640 | + | } | |||
| 635 | if tb.cancel != nil { | 641 | if tb.cancel != nil { | ||
| 636 | tb.cancel() | 642 | tb.cancel() | ||
| 637 | } | 643 | } | ||
| 638 | d.teardownFloating(tb, ac) | 644 | d.teardownFloating(tb, ac) | ||
| 639 | if rc := sess.renderCoordinator(); rc != nil { | 645 | if rc := sess.renderCoordinator(); rc != nil { | ||
| 640 | - | tb.mu.Lock() | |||
| 641 | - | panes := tb.panesSnapshot() | |||
| 642 | - | tb.mu.Unlock() | |||
| 643 | for _, p := range panes { | 646 | for _, p := range panes { | ||
| 644 | rc.noteSyncPaneRemoved(p) | 647 | rc.noteSyncPaneRemoved(p) | ||
| 645 | } | 648 | } | ||
| @@ -742,6 +745,7 @@ func (d *Daemon) killSession(sess *session, reason uint8, purge bool) error { | |||||
| 742 | d.unregisterPreview(ac) | 745 | d.unregisterPreview(ac) | ||
| 743 | ac.clearPreviousSession() | 746 | ac.clearPreviousSession() | ||
| 744 | ac.setSession(nil) | 747 | ac.setSession(nil) | ||
| 748 | + | ac.clearCaptureFrames() | |||
| 745 | } | 749 | } | ||
| 746 | 750 | ||||
| 747 | // Prevent queued launches from entering Open, then cancel the parent | 751 | // Prevent queued launches from entering Open, then cancel the parent | ||