From 23bbe7d4008a93a71efe393b8a348ab6a0cf5e2b Mon Sep 17 00:00:00 2001 From: l0ng-ai <24760907+l0ng-ai@users.noreply.github.com> Date: Sat, 15 Aug 2026 17:18:33 +0800 Subject: [PATCH] diag(tree-sync): name the pane a refused tree operation strands MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `TabCreate`, `PaneSplit` and `PaneReplace` each carry a pane the window has already spawned — the shell is running on the machine before the tree is told about it. When the tree refuses one of those, nothing ever takes the pane and the re-pull leaves the window without the tab it was for, so the shell goes on running with nothing referencing it. The log said only that an operation was refused, which is the one thing that does not point at the leak. Reproducible against a running instance: with a window open on a workspace, `tty7 tab new` and `tty7 tab close` back to back. The GUI restores the new tab, finds its pane already hung up, spawns a replacement, and `PaneReplace` is refused because the tab is gone. Four cycles in five leak a shell; leave half a second between the two and none do. Killing the GUI and repeating the loop leaks nothing — the CLI half is correct throughout. Not swept here, and the comment says why: at the point of the refusal the window still holds a view for the pane and only drops it once the re-pull lands, so hanging it up here would kill a pane that is still on screen. The sweep belongs after the pull settles and has to test the whole machine tree rather than this workspace's mirror, since a pane belonging to another workspace on the same host is not this window's to end. That change wants a GUI it can be watched in; this one only makes the leak say its own name, and `tty7 pane ls --all` already points at the recovery. `seeded_pane` is pinned by a test, because the set of requests that name a pane into existence is exactly the set a refusal can strand one from — a new one added without it would leak silently. 2933 tests pass. --- src/ui/tree_sync.rs | 109 ++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 109 insertions(+) diff --git a/src/ui/tree_sync.rs b/src/ui/tree_sync.rs index b9d69b0f..379b4a65 100644 --- a/src/ui/tree_sync.rs +++ b/src/ui/tree_sync.rs @@ -1463,6 +1463,38 @@ fn pump(cx: &mut App, client_ws: WorkspaceId) { Ok(()) => pump(cx, client_ws), Err((op, e)) => { log::warn!("tree operation {op:?} failed: {e}; re-pulling the tree"); + if let Some(pane) = seeded_pane(&op) { + // Known leak, and this line is how anyone hitting it + // finds out. `TabCreate`, `PaneSplit` and `PaneReplace` + // each name a pane this window has *already* spawned — + // the shell is running on the machine before the tree + // is told about it. When the op is refused the tree + // never takes the pane, the re-pull below leaves the + // window without the tab it was for, and the shell goes + // on running with nothing referencing it. + // + // Reproducible: with a window open, `tty7 tab new` and + // `tty7 tab close` back to back. The GUI restores the + // new tab, finds its pane already hung up, spawns a + // replacement, and `PaneReplace` is refused because the + // tab is gone. Four cycles in five leak; leave half a + // second between the two and none do. + // + // Not swept here on purpose. At this point the window + // still holds a view for the pane and only drops it + // once the re-pull lands, so hanging it up here would + // kill a pane that is still on screen. The sweep + // belongs after the pull settles, and has to test the + // whole machine tree rather than this workspace's + // mirror — a pane of another workspace on the same host + // is not this window's to end. Until then + // `tty7 pane close --orphans` is the recovery, and + // `tty7 pane ls --all` already points at it. + log::warn!( + "pane {pane} was spawned for that operation and nothing holds it \ + now; it will show up in `tty7 pane ls --all` as an orphan" + ); + } desync(cx, client_ws, "an operation was refused"); } } @@ -1471,6 +1503,22 @@ fn pump(cx: &mut App, client_ws: WorkspaceId) { .detach(); } +/// The pane an operation carries that this window has already spawned. +/// +/// These three requests are the only ones that name a pane into existence: +/// the shell is running on the machine before the tree is told about it, which +/// is exactly what makes a refused operation leave something behind. Every +/// other request only ever moves, renames or removes panes the tree already +/// knows, and refusing one of those strands nothing. +fn seeded_pane(op: &ControlRequest) -> Option { + match op { + ControlRequest::TabCreate { pane, .. } => Some(pane.pane), + ControlRequest::PaneSplit { new, .. } => Some(new.pane), + ControlRequest::PaneReplace { new, .. } => Some(new.pane), + _ => None, + } +} + fn desync(cx: &mut App, client_ws: WorkspaceId, why: &str) { log::info!("resynchronizing workspace {client_ws} with its machine ({why})"); let Some(state) = cx.default_global::().windows.get_mut(&client_ws) else { @@ -2567,6 +2615,67 @@ fn set_gui_ratio(pane: &mut Pane, path: &[Side], ratio: f32) -> bool { mod tests { use super::*; + /// Pins which requests carry an already-spawned pane, because that is the + /// set a refused operation can strand a shell from. A new request that + /// names a pane into existence has to be added here too, or its refusal + /// leaks silently. + #[test] + fn only_the_requests_that_spawn_a_pane_report_a_seed() { + let ws = WorkspaceId::new(); + let seed = |pane| PaneSeed { + pane, + cwd: None, + ssh_spec: None, + agent: None, + shell: None, + }; + + assert_eq!( + seeded_pane(&ControlRequest::TabCreate { + workspace: ws, + at: None, + pane: seed(7), + tab: None, + }), + Some(7) + ); + assert_eq!( + seeded_pane(&ControlRequest::PaneReplace { + workspace: ws, + old: 7, + new: seed(8), + }), + Some(8) + ); + assert_eq!( + seeded_pane(&ControlRequest::PaneSplit { + workspace: ws, + pane: 7, + axis: TreeAxis::Vertical, + ratio: 0.5, + new: seed(9), + first: false, + }), + Some(9) + ); + + // Removing or renaming names no new pane, so a refusal strands nothing. + assert_eq!( + seeded_pane(&ControlRequest::TabClose { + workspace: ws, + tab: TabId::new(), + }), + None + ); + assert_eq!( + seeded_pane(&ControlRequest::PaneClose { + workspace: ws, + pane: 7, + }), + None + ); + } + #[test] fn note_instance_reports_only_a_real_change() { let mut seen = String::new();