fix(cmux-tui): when PTYs are low, closed tabs' terminals waiting out the reap grace give theirs back first (STATUS 7938)

Merged
LA
3d ago
+252-3

Refs STATUS 7938 (VIDEO EXTRA: 50 tabs opened took the daemon tree from 4 to 208 /dev/ptmx handles; still 208 10 s after closing all tabs and 5 s after the workspace close).

Cause (measured)

Diag step b01dd8303d952fd412d5199f (12 tabs, reap grace 2 s, lsof per process of the daemon and its terminal hosts):

momenthandles by pid
anchor terminal onlydaemon 1, host 3
12 more tabsdaemon 13, each host 3
tabs closed, after the gracedaemon 1, anchor host 3
workspace closed, after the gracenone

Each terminal holds 4 handles: the daemon's PTY custody copy (surface/rehost.rs) and 3 in its terminal host; all for one PTY. A closed tab's terminal is not ended at once: it lives out the reap grace (mux/terminal_reap.rs; the app starts the daemon with 30 s, DaemonLauncher.terminalReapGraceSeconds) so the tab can come back with its shell, and it keeps its PTY meanwhile. The journey sampled at 10 s and 15 s, inside that grace. So the daemon does not keep them forever.

The defect: under a burst of closes these closed terminals hold the machine's PTYs (511 on macOS), and the PTY reclaim (mux/pty_reclaim.rs) parked only idle shells that a tab still shows (pick_reclaim skips terminals with no surface). It never ended them, so new tabs were refused with "PTY capacity exhausted" while dozens of closed terminals held PTYs.

Fix

When PTYs are low, a reclaim pass first ends the terminals of closed tabs that wait out the reap grace, longest closed first, through the reaper's own guarded close (reap_unplaced_terminal: never a kept terminal or one a client is attached to; screen archived as at a normal reap). Only if that does not free enough does it park idle shells, as before. A unit-test seam (set_pty_usage_for_test, cfg(test)) sets the PTY usage a pass reads.

Tests

  • mux::terminal_reap::tests::a_reclaim_pass_ends_closed_terminals_waiting_out_the_grace_when_ptys_are_low: red at 2d56ba72eeb (step 10c30572474d25f48d777e71: "PTYs were low and a closed tab's terminal still held one"); green at 9fe65a6dbe7 (terminal_reap 14/14, step 0bcc1ee25caf2bd52ee52d55).
  • terminal_host_recovery::pty_release::closed_tabs_give_back_their_ptys_once_past_the_reap_grace (real daemon and hosts, lsof): passes on the base too (step c0c3da228496e059df4a2945); it pins the measured rule that closed tabs give every handle back after the grace.
  • At the head 9f9eff28b96: clippy passed (f0a43c84668924058979223f), mux:: 560/560 (45f3ee6e14aa84a4c4dfbb5d), pty_release 1/1 (39cb392e4df8c1ed51c98bbb); fmt passed at 9fe65a6dbe7 (76732778f5803eac3bbe50f5).

Observed / Not run

Observed: the steps above on fleet Macs; the per-process counts in the table.

Not run:

  • Ending up to 128 closed terminals in one pass runs them one after another on the reaper thread (review follow-up: use the reaper's parallel path).
  • The VIDEO EXTRA burst journey on a real app at PTY exhaustion (it needs about 120 open tabs on the host); a GUI run.

Changelog

  • Fixed: cmux-next no longer refuses new terminals with "PTY capacity exhausted" after many tabs were closed quickly: closed tabs' terminals give their PTYs back first when PTYs run low.

Autofix with [code]smith
Open in [code]smith · Need help on this PR? Tag @codesmith-bot with what you need.


Summary by cubic

Fixes cmux-tui refusing new tabs with "PTY capacity exhausted" after many tabs were closed quickly.

A closed tab's terminal lives out the reap grace period (30 s by default) so the tab can come back, and keeps its PTY during that time. With a burst of closes, these terminals held the machine's PTYs (511 on macOS) because the reclaim pass only parked idle shells that a tab still showed. Now, when PTYs run low, the reclaim pass first ends the closed terminals waiting out their grace, longest closed first, through the reaper's guarded close (never a kept terminal or one a client is attached to). It parks idle shells only if that does not free enough.

Tests

  • Adds set_pty_usage_for_test so a unit test can set the PTY usage a reclaim pass reads.
  • Adds an integration test that runs a real daemon and checks /dev/ptmx handle counts return to baseline after closed tabs pass the reap grace.