fix(core): learn a pane's window when it is created, not after - #1115
Conversation
tmux announces a pane's death only by closing its window, so a pane that does not know which window it is in cannot notice its own ending. That mapping was built by asking: a `display-message` round trip issued after `new-window` had already returned, travelling a serialized control-mode channel behind whatever else was in flight. A program that ended inside that gap had its `%window-close` arrive with nothing to match it against, and the announcement is one-shot — no later wait brings it back. `new-window` is already asked to answer (`-P -F`), and asking it for the window as well as the pane costs nothing. The mapping is then written from the answer that created the pane, with no gap to lose a death in. `adopt` is handed a pane id out of the database and nothing else, so it keeps asking as before; so does a multiplexer that answers with the pane id alone. Measured on tmux 3.2a, with a 1.5s delay standing in for a round trip queued under load and a program that ends inside it: asking separately fails the spawn outright (`can't find pane: %2`), answering with both ids passes. The timings Thurbeen#1113 raised to buy room for that round trip go back to what they were written as — `sleep 8` and a 20s deadline here, `sleep 3` next door.
Greptile SummaryThis PR obtains both pane and window IDs from the existing
Confidence Score: 4/5The PR should not merge until immediately exiting panes cannot lose their one-shot window-close notification before registration. Removing the extra lookup substantially narrows the race, but Files Needing Attention: src/agent/tmux.rs
|
| Filename | Overview |
|---|---|
| src/agent/tmux.rs | Threads the window ID from new-window into registration, but registration still occurs after the child has started and the command response has returned. |
| src/agent/control_mode/mod.rs | Adds strict @<digits> window-ID validation; existing unmatched window-close handling exposes the remaining registration race. |
| tests/program_pane_exit.rs | Restores a one-second child lifetime and ten-second deadline without deterministically covering an exit before registration. |
| tests/program_restart_exit.rs | Restores a one-second child lifetime for restart/exit coverage. |
Sequence Diagram
sequenceDiagram
participant Main as Spawn thread
participant Tmux
participant Reader as Control reader
participant Map as pane_windows
Main->>Tmux: new-window -P -F pane_id window_id
Tmux->>Tmux: Start child
alt Child exits before response is registered
Tmux-->>Reader: "%window-close @N"
Reader->>Map: "Find panes mapped to @N"
Map-->>Reader: None
Reader->>Reader: Discard one-shot close
end
Tmux-->>Main: "%N @N"
Main->>Map: "Register %N → @N"
Note over Main,Map: Too late if the close was already discarded
Reviews (1): Last reviewed commit: "fix(core): learn a pane's window when it..." | Re-trigger Greptile
LeTuR
left a comment
There was a problem hiding this comment.
This is the supporting fix in the split #1107's review asked for. Nothing in it blocks from my side; merging is LeTuR's call.
- tmux 3.2 floor: checked on real tmux 3.2 (built from the release tarball) and 3.5a in control mode.
-P -F '#{pane_id} #{window_id}'answers%N @M, and a window named'x %9 @9 y'does not change that answer. - #1107 retention: both
set-window-options still run in the same command list asnew-window. On 3.2,show-window-optionson the new window readsremain-on-exit offandwindow-size manual. - CI on
ed80bfa8: 13 of 22 checks passed and 9 were skipped by path filters (lint, install scripts, website, SonarQube). None failed, was cancelled or was held. Nextest ran 2643 tests with 0 skipped, both program-exit tests included, on Ubuntu's tmux 3.4. No CI job runs tmux 3.2, which is why I checked it by hand. psmux was not tested. - Security: no security findings. The format is a constant with no names in it, both ids are validated before use, and the new check on the
display-messageanswer tightens theadoptpath. - Your two questions, my recommendation with LeTuR deciding: the recently-closed set may not be needed on the spawn path (see the Greptile thread). Threading the window id through
DISCOVER_FORMATand restore belongs in a separate change.
— LeTuR's agent
Your nit on #1107, taken up:
new-windowis already asked to answer, so it may as well answer with the window too.The defect this closes
tmux announces a pane's death only by closing its window, so
pane_windowsis what lets a pane notice its own ending. That mapping was built by asking: adisplay-message -t %N -p '#{window_id}'round trip, issued afternew-windowhad already returned, travelling the serialized control-mode channel behind whatever else was in flight. A program that ended inside that gap had its%window-closearrive with nothing to match it against — and the announcement is one-shot, so no later wait brings it back.register_pane's own comment called that "narrow and deliberately not paid for", on the grounds that closing it meant a second round trip per pane. That was the wrong price: it takes none.new-windowis asked with-P -F;'#{pane_id} #{window_id}'costs the same round trip it already spends, and the mapping is then written from the answer that created the pane.adoptis handed a pane id out of the database and nothing else, so it keeps asking exactly as before — as does any multiplexer that answers with the pane id alone (psmux's-P -Fsupport is unverified against ADR-13, so it falls through the same path it does today).Measured, not reasoned
The mechanism, A/B. A 1.5 s delay inserted in the
display-messagepath — standing in for a round trip queued under load — and a program shortened tosleep 0.2so it ends inside it. Same test, same instrumentation, one difference:main)resize-window -t %2 -x 80 -y 23: can't find pane: %2Being honest about what that shows: B fails loudly, because the pane is gone before
connect_panefinishes attaching. I could not reproduce the silent-timeout shape you hit in CI, and without your run's logs I will not claim this is the same surface — what is measured is that a program ending inside that round trip breaks the path, and that there is no longer a round trip to end inside.The suite, before and after. Six full parallel
cargo nextest run --allruns each, this machine, tmux 3.2a:main's code): the two program tests passed 6/6shared_tests::a_host_with_sharing_off_is_used_the_old_way,spawn::tests::resolve_host_accepts_the_backend_name_the_interface_carries,cli::automations::tests::tick_reports_fired_and_skipped_arrays,shared_sessions::sync_with_no_shareable_host_configured_is_an_empty_report— all four reproduce onmainhere)So the flake never reproduced on this machine, at either timing. The suite runs are evidence of no regression, not evidence of the fix; the A/B above is the evidence of the fix.
The timings from #1113 go back
sleep 8and the 20 s deadline inprogram_pane_exit.rs,sleep 3inprogram_restart_exit.rs. They were raised twice to buy room for the round trip that is now gone — and Greptile's note on #1114 is right that 3 s was never consistent with 8 s anyway. Their comments are rewritten to say what the second is actually for.What this does not do — and a question
The gap is now local work rather than a queued round trip, but it is not zero: the mapping is still written inside
register_pane, a few instructions afternew-windowreturns, and the reader thread could in principle process%window-closein between.Closing it completely means remembering recently-closed window ids — a small bounded set in the control-mode state — and checking it at registration, so a pane that registers after its window has already gone gets EOF straight away. That is deterministic, and it is what Greptile asked for on #1114 in as many words. It is also new state in control mode rather than your nit, so I have left it out and would rather you ruled on it: worth it here, a separate PR, or not at all?
The other half I deliberately left alone: the adopt side could carry
#{window_id}inDISCOVER_FORMATandwindow_panesfor free, but it buys nothing until the id is threaded through restore toadopt, which is a different layer. Say if you want it in the same change.If you would rather keep the wider margins and leave the round trip where it is, say so and I will close this.