diff --git a/crates/terminal/src/pty_info.rs b/crates/terminal/src/pty_info.rs index 576a833f01..8e98143248 100644 --- a/crates/terminal/src/pty_info.rs +++ b/crates/terminal/src/pty_info.rs @@ -5,7 +5,7 @@ use std::{path::PathBuf, sync::Arc}; #[cfg(target_os = "windows")] use windows::Win32::{Foundation::HANDLE, System::Threading::GetProcessId}; -use sysinfo::{Pid, Process, ProcessRefreshKind, RefreshKind, System, UpdateKind}; +use sysinfo::{Pid, Process, ProcessRefreshKind, ProcessesToUpdate, System, UpdateKind}; use crate::{Event, Terminal}; @@ -77,23 +77,38 @@ pub(crate) struct PtyProcessInfo { system: RwLock, refresh_kind: ProcessRefreshKind, pid_getter: ProcessIdGetter, + last_foreground_pid: Mutex>, pub(crate) current: RwLock>, task: Mutex>>, } impl PtyProcessInfo { pub(crate) fn new(pid_getter: ProcessIdGetter) -> PtyProcessInfo { + // Task enumeration is on by default and would retain a `Process` entry + // per thread, each pinning an open `/proc//task//stat` handle + // on Linux (#58651). let process_refresh_kind = ProcessRefreshKind::nothing() .with_cmd(UpdateKind::Always) .with_cwd(UpdateKind::Always) - .with_exe(UpdateKind::Always); - let refresh_kind = RefreshKind::nothing().with_processes(process_refresh_kind); - let system = System::new_with_specifics(refresh_kind); + .with_exe(UpdateKind::Always) + .without_tasks(); + // `System::new_with_specifics` with a process refresh kind would + // snapshot every process on the machine into this terminal's `System`, + // retaining one open procfs handle per process for the lifetime of the + // terminal (#58651). Refresh only the spawned child so that + // `kill_child_process` works before the first foreground refresh. + let mut system = System::new(); + system.refresh_processes_specifics( + ProcessesToUpdate::Some(&[pid_getter.fallback_pid()]), + true, + process_refresh_kind, + ); PtyProcessInfo { system: RwLock::new(system), refresh_kind: process_refresh_kind, pid_getter, + last_foreground_pid: Mutex::new(None), current: RwLock::new(None), task: Mutex::new(None), } @@ -105,16 +120,25 @@ impl PtyProcessInfo { fn refresh(&self) -> Option> { let pid = self.pid_getter.pid()?; - if self.system.write().refresh_processes_specifics( - sysinfo::ProcessesToUpdate::Some(&[pid]), - true, - self.refresh_kind, - ) == 1 - { - RwLockReadGuard::try_map(self.system.read(), |system| system.process(pid)).ok() - } else { - None + let fallback_pid = self.pid_getter.fallback_pid(); + let mut system = self.system.write(); + // sysinfo never evicts processes that are absent from the refreshed pid + // set, so entries for former foreground processes (each pinning an open + // `/proc//stat` handle on Linux) would otherwise accumulate for as + // long as this terminal lives (#58651). Rebuild the `System` whenever + // the foreground process changes to keep the map bounded. + if self.last_foreground_pid.lock().replace(pid) != Some(pid) { + *system = System::new(); } + let pids = [pid, fallback_pid]; + let pids = if pid == fallback_pid { + &pids[..1] + } else { + &pids[..] + }; + system.refresh_processes_specifics(ProcessesToUpdate::Some(pids), true, self.refresh_kind); + drop(system); + RwLockReadGuard::try_map(self.system.read(), |system| system.process(pid)).ok() } fn get_child(&self) -> Option> { @@ -206,3 +230,51 @@ impl PtyProcessInfo { self.pid_getter.pid() } } + +#[cfg(all(test, unix))] +mod tests { + use super::*; + + /// Regression test for : + /// on Linux, sysinfo keeps an open `/proc//stat` handle for every + /// `Process` entry retained in a `System`, and never evicts entries that are + /// absent from the refreshed pid set. The per-terminal `System` must + /// therefore not snapshot every process on the machine, nor accumulate an + /// entry per foreground process that has ever run in this terminal. + #[test] + #[allow( + clippy::disallowed_methods, + reason = "the test needs real short-lived child processes and may block" + )] + fn process_map_stays_bounded() { + let mut info = PtyProcessInfo::new(ProcessIdGetter::new(-1, std::process::id())); + assert!( + info.get_child().is_some(), + "the spawned child must be inspectable for kill_child_process \ + before the first foreground refresh" + ); + assert!(info.load_for_test().is_some()); + let initial_len = info.system.read().processes().len(); + assert!( + initial_len <= 2, + "creating a terminal retained {initial_len} process entries" + ); + + for _ in 0..3 { + let mut child = std::process::Command::new("sleep") + .arg("30") + .spawn() + .expect("failed to spawn child process"); + info.pid_getter = ProcessIdGetter::new(-1, child.id()); + assert!(info.load_for_test().is_some()); + child.kill().expect("failed to kill child process"); + child.wait().expect("failed to wait for child process"); + } + + let churned_len = info.system.read().processes().len(); + assert!( + churned_len <= 2, + "foreground process churn retained {churned_len} process entries" + ); + } +}