Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 34 additions & 3 deletions cmux-tui/crates/cmux-tui-core/src/mux/terminal_reap.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,11 @@
//! The reaper is event-driven. It sleeps until the next deadline or until a
//! topology, terminal-registry, or keep change wakes it; it never polls.
//!
//! The grace holds at most [`MAX_TERMINALS_IN_REAP_GRACE`] closed terminals: each keeps its
//! shell and so its PTY (a shell cannot outlive its PTY), and a script that closes several tabs a
//! second held the machine's PTYs until every new tab was refused (STATUS 7998). Past the cap the
//! longest-closed ends at once.
//!
//! The grace runs across owner restarts. The registry keeps the wall-clock
//! time the owner first saw each terminal unplaced
//! (`workspace_registry/terminal_unplaced_store.rs`), and a restarted owner
Expand All @@ -33,6 +38,9 @@ use crate::state::kept_tab_store::{KeptTab, kept_title};
/// Default reap grace period for a terminal with no placement.
pub const DEFAULT_TERMINAL_REAP_GRACE: Duration = Duration::from_secs(30);

/// The most closed terminals that wait out the reap grace with their PTY at once (STATUS 7998).
/// Past it, the reaper ends the longest-closed at once (its screen archived as at any reap).
pub(crate) const MAX_TERMINALS_IN_REAP_GRACE: usize = 16;
/// Largest accepted reap grace period: one week.
pub const MAX_TERMINAL_REAP_GRACE: Duration = Duration::from_secs(7 * 24 * 60 * 60);

Expand Down Expand Up @@ -138,6 +146,20 @@ impl ReapSchedule {
}
}

/// The closed terminals past [`MAX_TERMINALS_IN_REAP_GRACE`], longest closed first: each holds a PTY
/// for its grace, and a burst of closes must not hold the machine's PTYs (STATUS 7998).
fn over_grace_cap(reapable: &HashMap<String, Duration>) -> Vec<String> {
let Some(over) =
reapable.len().checked_sub(MAX_TERMINALS_IN_REAP_GRACE).filter(|over| *over > 0)
else {
return Vec::new();
};
let mut oldest: Vec<(&Duration, &String)> =
reapable.iter().map(|(terminal_id, unplaced_for)| (unplaced_for, terminal_id)).collect();
oldest.sort_by(|a, b| b.0.cmp(a.0).then_with(|| a.1.cmp(b.1)));
oldest.into_iter().take(over).map(|(_, terminal_id)| terminal_id.clone()).collect()
}

fn deadline_after(now: Instant, grace: Duration) -> Instant {
now.checked_add(grace).unwrap_or_else(|| now + MAX_TERMINAL_REAP_GRACE)
}
Expand Down Expand Up @@ -335,15 +357,24 @@ impl Mux {
now: Instant,
) -> Vec<String> {
let grace = self.terminal_reap_grace();
match self.reapable_terminals_unplaced_for(&mut schedule.unplaced_rows_empty) {
Ok(reapable) => schedule.observe_unplaced(now, grace, &reapable),
let over_cap = match self.reapable_terminals_unplaced_for(&mut schedule.unplaced_rows_empty)
{
Ok(reapable) => {
schedule.observe_unplaced(now, grace, &reapable);
over_grace_cap(&reapable)
}
Err(error) => {
self.report_internal_diagnostic(format!("terminal reap scan failed: {error}"));
schedule.defer_due_after_failed_scan(now);
return Vec::new();
}
};
let mut due = schedule.due(now);
for terminal_id in over_cap {
if !due.contains(&terminal_id) {
due.push(terminal_id);
}
}
let due = schedule.due(now);
let outcomes: Vec<anyhow::Result<ReapOutcome>> = if due.len() <= 1 {
due.iter().map(|terminal_id| self.reap_unplaced_terminal(terminal_id)).collect()
} else {
Expand Down
50 changes: 50 additions & 0 deletions cmux-tui/crates/cmux-tui-core/src/mux/terminal_reap/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -333,3 +333,53 @@ fn a_reclaim_pass_ends_closed_terminals_waiting_out_the_grace_when_ptys_are_low(
assert_eq!(lifecycle(&mux, &host_id(&mux, &placed)), TerminalLifecycle::Running);
mux.close_surface(placed.id).unwrap();
}

/// PTY HOLD (VIDEO EXTRA, STATUS 7998): every closed tab's terminal lived out the whole reap grace
/// (30 s in the app) with its PTY. A script closing about 5 tabs a second (the L6 storm) piled up
/// closed-but-held PTYs and reached the machine's limit (511) in 48 s. The grace keeps a recent close
/// undoable; it must not hold an unbounded number of PTYs: past
/// [`MAX_TERMINALS_IN_REAP_GRACE`] closed terminals, the reaper ends the longest-closed at once.
#[test]
fn closed_terminals_past_the_grace_cap_end_at_once() {
let mux = Mux::new_for_test("terminal-reap-cap", SurfaceOptions::default());
let extra = 2;
// Every workspace first: a create after a reap trips the scoped-projection cross-check on the
// base as well (a separate defect, see the PR), so this test closes and reaps only.
let placed = mux.new_workspace(Some("placed".into()), Some((80, 24))).unwrap();
let surfaces: Vec<_> = (0..MAX_TERMINALS_IN_REAP_GRACE + extra)
.map(|index| mux.new_workspace(Some(format!("closed-{index}")), Some((80, 24))).unwrap())
.collect();
let mut schedule = ReapSchedule::default();
let mut closed = Vec::new();
let mut reaped = Vec::new();
// A burst of closes, the reaper woken after each (a topology change wakes it).
for (index, surface) in surfaces.iter().enumerate() {
let terminal_id = host_id(&mux, surface);
close_workspace_of(&mux, surface);
reaped.extend(mux.reap_unplaced_terminals(&mut schedule, Instant::now()));
// Distinct close times: the first closed is the longest closed.
mux.workspace_registry
.lock()
.unwrap()
.backdate_terminal_unplaced(&terminal_id, Duration::from_millis(1_000 - index as u64))
.unwrap();
closed.push(terminal_id);
}
reaped.extend(mux.reap_unplaced_terminals(&mut schedule, Instant::now()));
reaped.sort();
let mut oldest = closed[..extra].to_vec();
oldest.sort();
assert_eq!(
reaped, oldest,
"inside the grace, only the closed terminals past the cap end, oldest first"
);
for terminal_id in &closed[extra..] {
assert_eq!(
lifecycle(&mux, terminal_id),
TerminalLifecycle::Running,
"a recent close stays undoable"
);
}
assert_eq!(lifecycle(&mux, &host_id(&mux, &placed)), TerminalLifecycle::Running);
mux.close_surface(placed.id).unwrap();
}
Loading