mirror of
https://github.com/rustdesk/rustdesk.git
synced 2026-09-06 08:01:03 +03:00
Linux drop shell from service loop (#15979)
* perf(linux): stop the service loop from forking a shell per environment variable The service loop re-derives the desktop every 500 ms, and every lookup on that path forks. A healthy GNOME session spends ~104 process spawns a second, 8 full `ps -u <uid>` scans and 2 full `ps aux` scans, to re-answer a question whose answer has not changed. `get_env` alone is a `sh -c` pipeline of ~12 processes per variable. `get_envs` already reads `/proc` directly and was documented as the intended replacement, so move the remaining `get_env` callers to it and delete it. The xwayland probe drops from 4 pipelines (~48 processes) to one `/proc` walk, and the pathological walk that #15952 was about drops from ~2900 processes to at most 60 `/proc` walks. `get_cm` and `is_xwayland_running` read `/proc` instead of forking `ps aux` and `pgrep -a`; `get_cm` also called `current_exe()` once per line of `ps` output. Selection semantics are preserved where they were load-bearing: * `get_envs_of_newest` reproduces the `ps ... | tail -1` the removed pipelines used, so a variable the newest matching process does not have means moving on to the next pattern, never on to an older process that may belong to a session which has since logged out. * `get_envs` keeps its own order (readdir) and its all-process ranking, so the existing `get_display_xauth_wayland` caller is unaffected. Only its handling of an exported-but-empty value changes: `DISPLAY=` no longer counts as found, where it used to satisfy a single-name query and return the empty value before a process holding a real one was examined. * `get_envs_where` lets the caller state what a complete answer is. Ranking by how many of the requested names a process carries cannot know that `DISPLAY` is mandatory and the rest interchangeable, so it could rank a process holding three optional values above the one holding the pair that matters. `is_xwayland_running` is scoped to the session's uid. The compositor starts Xwayland as the session user, so another user's Xwayland -- a switched-away session, a second seat -- used to route a pure-Wayland session into the Xwayland probe, which has no display for it to find there. Not addressed: this discovery path has never had any notion of the active session, and filters by uid alone. Constraining candidates to the active session is not possible for the most important one, since `xdg-desktop-portal` and its backends run under `user@<uid>.service`, which spans sessions and carries no `XDG_SESSION_ID`, no session cgroup and no audit sessionid. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Q5egQpH4q4GoXJiuMoTJ5t * fix(linux): the newest-process walk must not answer with a grep or an older PID Three findings from review of the commit before this one. `/proc/<pid>/environ` failing to read left the walk on to the next PID, which in `newest_first` mode is an older process -- possibly of a session that has since logged out -- where the `ps ... | tail -1` pipeline this replaces stopped at the one PID it had already picked. A read that fails is a process carrying none of the requested names, not a process to skip. The `seen` latch that was meant to hold the newest process is deleted: `accept` is reached once per matching process, so returning on the first is what it already did. The regex is matched against the whole `/proc/<pid>/cmdline`, where the pipeline had a `grep -v 'grep'`. A user running `grep Xwayland` is otherwise the newest match for that pattern and answers with whatever environment their shell had -- an X forwarding endpoint over ssh, say. This is the one place the walk still differs from the `get_envs` it grew out of, which never had that filter and could take an ssh `grep` over the portal it was looking for. `get_envs` is left exactly as it was. Its completeness test was every requested name *present*; stating it through `accept` turned it into every name *non-empty* and, with the empty-value change that went with it, moved which process the existing `get_display_xauth_wayland` caller settles on. `accept` is now told the count and asks the question the loop it replaced asked. This supersedes the `get_envs` bullet of the previous commit message: an exported-but-empty value counts as found again, as it always did. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019QgsYAUYKDei1AM5yHJsMX --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -1254,20 +1254,15 @@ pub fn get_active_userid_cached() -> Option<u32> {
|
||||
}
|
||||
|
||||
fn get_cm() -> bool {
|
||||
// We use `CMD_PS` instead of `ps` to suppress some audit messages on some systems.
|
||||
if let Ok(output) = Command::new(CMD_PS.as_str()).args(vec!["aux"]).output() {
|
||||
for line in String::from_utf8_lossy(&output.stdout).lines() {
|
||||
if line.contains(&format!(
|
||||
"{} --cm",
|
||||
std::env::current_exe()
|
||||
.unwrap_or("".into())
|
||||
.to_string_lossy()
|
||||
)) {
|
||||
return true;
|
||||
}
|
||||
}
|
||||
}
|
||||
false
|
||||
// Runs twice a second in the service loop, so walk /proc rather than forking `ps aux`; that
|
||||
// fork is also what the `CMD_PS` audit-message workaround this replaces was for.
|
||||
let cm = format!(
|
||||
"{} --cm",
|
||||
std::env::current_exe()
|
||||
.unwrap_or_default()
|
||||
.to_string_lossy()
|
||||
);
|
||||
any_process(None, "cmdline", |cmdline| cmdline.contains(&cm))
|
||||
}
|
||||
|
||||
pub fn is_login_wayland() -> bool {
|
||||
@@ -1576,6 +1571,34 @@ fn get_envs<'a>(
|
||||
process_pat: &str,
|
||||
names: &[&'a str],
|
||||
) -> std::collections::HashMap<&'a str, String> {
|
||||
get_envs_where(uid, process_pat, names, false, |count| count == names.len())
|
||||
}
|
||||
|
||||
/// The newest process matching `process_pat`, whatever it happens to carry: the semantics of the
|
||||
/// `ps -u <uid> -f | grep <pat> | tail -1` pipeline the callers below used before. A variable this
|
||||
/// process does not have means moving on to the next pattern, never on to an older process that
|
||||
/// may belong to a session which has since logged out.
|
||||
fn get_envs_of_newest<'a>(
|
||||
uid: &str,
|
||||
process_pat: &str,
|
||||
names: &[&'a str],
|
||||
) -> std::collections::HashMap<&'a str, String> {
|
||||
get_envs_where(uid, process_pat, names, true, |_| true)
|
||||
}
|
||||
|
||||
/// `get_envs` with the caller's own process order and its own notion of a complete answer, told
|
||||
/// how many of `names` the process carries: the first process `accept` takes wins outright, and
|
||||
/// the count-based ranking is only the fallback for when no process is accepted at all.
|
||||
fn get_envs_where<'a, F>(
|
||||
uid: &str,
|
||||
process_pat: &str,
|
||||
names: &[&'a str],
|
||||
newest_first: bool,
|
||||
mut accept: F,
|
||||
) -> std::collections::HashMap<&'a str, String>
|
||||
where
|
||||
F: FnMut(usize) -> bool,
|
||||
{
|
||||
// The tie-breaking logic uses a u64 bitmask, limiting us to 64 variables.
|
||||
debug_assert!(
|
||||
names.len() <= 64,
|
||||
@@ -1602,21 +1625,24 @@ fn get_envs<'a>(
|
||||
let mut best_count = 0usize;
|
||||
let mut best_mask: u64 = 0;
|
||||
|
||||
// Iterate /proc to find matching processes
|
||||
// Iterate /proc to find matching processes. `newest_first` is only for `get_envs_of_newest`,
|
||||
// whose callers need the last PID-ordered match their `ps ... | tail -1` pipelines took;
|
||||
// without it the order is whatever readdir returns, which is what `get_envs` has always used.
|
||||
// Neither order identifies the active session -- a user with two live graphical sessions has
|
||||
// one of each, and picking by PID guesses. See `Desktop::refresh` for who owns that question.
|
||||
let Ok(entries) = std::fs::read_dir("/proc") else {
|
||||
return best;
|
||||
};
|
||||
let mut pids: Vec<u32> = entries
|
||||
.flatten()
|
||||
.filter_map(|entry| entry.file_name().to_str()?.parse::<u32>().ok())
|
||||
.collect();
|
||||
if newest_first {
|
||||
pids.sort_unstable_by(|a, b| b.cmp(a));
|
||||
}
|
||||
|
||||
for entry in entries.flatten() {
|
||||
let file_name = entry.file_name();
|
||||
let Some(pid_str) = file_name.to_str() else {
|
||||
continue;
|
||||
};
|
||||
if !pid_str.chars().all(|c| c.is_ascii_digit()) {
|
||||
continue;
|
||||
}
|
||||
|
||||
let proc_path = entry.path();
|
||||
for pid in pids {
|
||||
let proc_path = std::path::Path::new("/proc").join(pid.to_string());
|
||||
|
||||
// Check if process belongs to the specified uid
|
||||
if let Ok(meta) = std::fs::metadata(&proc_path) {
|
||||
@@ -1634,15 +1660,18 @@ fn get_envs<'a>(
|
||||
continue;
|
||||
};
|
||||
let cmdline_str = String::from_utf8_lossy(&cmdline).replace('\0', " ");
|
||||
if !re.is_match(&cmdline_str) {
|
||||
// The `grep -v 'grep'` of the pipeline this replaces. A user grepping for one of these
|
||||
// patterns is otherwise the newest match for it, and answers with whatever environment
|
||||
// their shell had -- an X forwarding endpoint over ssh, say.
|
||||
if cmdline_str.contains("grep") || !re.is_match(&cmdline_str) {
|
||||
continue;
|
||||
}
|
||||
|
||||
// Read environ and extract matching variables
|
||||
let environ_path = proc_path.join("environ");
|
||||
let Ok(environ) = std::fs::read(&environ_path) else {
|
||||
continue;
|
||||
};
|
||||
// Read environ and extract matching variables. A read that fails -- the process exited
|
||||
// between these two reads -- is a process carrying none of `names`, not a process to
|
||||
// skip: skipping it would hand `newest_first` on to an older PID, where the pipeline
|
||||
// this replaces stopped at the single PID its `tail -1` had already picked.
|
||||
let environ = std::fs::read(proc_path.join("environ")).unwrap_or_default();
|
||||
|
||||
let mut found = empty.clone();
|
||||
let mut found_count = 0usize;
|
||||
@@ -1673,14 +1702,14 @@ fn get_envs<'a>(
|
||||
found_mask |= bit;
|
||||
}
|
||||
}
|
||||
|
||||
if found_count == names.len() {
|
||||
return found;
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
if accept(found_count) {
|
||||
return found;
|
||||
}
|
||||
|
||||
if found_count > best_count || (found_count == best_count && found_mask > best_mask) {
|
||||
best = found;
|
||||
best_count = found_count;
|
||||
@@ -1691,29 +1720,37 @@ fn get_envs<'a>(
|
||||
best
|
||||
}
|
||||
|
||||
/// Deprecated: Use `get_envs` instead.
|
||||
///
|
||||
/// https://github.com/rustdesk/rustdesk/discussions/11959
|
||||
///
|
||||
/// **Note**: This function is retained for conservative migration. The plan is to gradually
|
||||
/// transition all callers to `get_envs` after it proves stable and reliable. Once `get_envs`
|
||||
/// is confirmed to work correctly across all use cases, this function will be removed entirely.
|
||||
///
|
||||
/// # Arguments
|
||||
/// * `name` - Environment variable name to retrieve
|
||||
/// * `uid` - User ID to filter processes
|
||||
/// * `process` - Process name pattern to match
|
||||
///
|
||||
/// # Returns
|
||||
/// The environment variable value, or empty string if not found
|
||||
#[inline]
|
||||
fn get_env(name: &str, uid: &str, process: &str) -> String {
|
||||
let cmd = format!("ps -u {} -f | grep -E '{}' | grep -v 'grep' | tail -1 | awk '{{print $2}}' | xargs -I__ cat /proc/__/environ 2>/dev/null | tr '\\0' '\\n' | grep '^{}=' | tail -1 | sed 's/{}=//g'", uid, process, name, name);
|
||||
if let Ok(x) = run_cmds(&cmd) {
|
||||
x.trim_end().to_string()
|
||||
} else {
|
||||
"".to_owned()
|
||||
/// True when `pred` accepts the `/proc/<pid>/<file>` of any process, NULs turned into spaces,
|
||||
/// optionally only of processes owned by `uid`.
|
||||
/// Reads `/proc` directly instead of forking `ps` / `pgrep`, for the service-loop callers below.
|
||||
fn any_process<F: Fn(&str) -> bool>(uid: Option<u32>, file: &str, pred: F) -> bool {
|
||||
let Ok(entries) = std::fs::read_dir("/proc") else {
|
||||
return false;
|
||||
};
|
||||
for entry in entries.flatten() {
|
||||
let file_name = entry.file_name();
|
||||
let Some(pid_str) = file_name.to_str() else {
|
||||
continue;
|
||||
};
|
||||
if !pid_str.chars().all(|c| c.is_ascii_digit()) {
|
||||
continue;
|
||||
}
|
||||
let proc_path = entry.path();
|
||||
if let Some(uid) = uid {
|
||||
use std::os::unix::fs::MetadataExt;
|
||||
match std::fs::metadata(&proc_path) {
|
||||
Ok(meta) if meta.uid() == uid => {}
|
||||
_ => continue,
|
||||
}
|
||||
}
|
||||
let Ok(content) = std::fs::read(proc_path.join(file)) else {
|
||||
continue;
|
||||
};
|
||||
if pred(&String::from_utf8_lossy(&content).replace('\0', " ")) {
|
||||
return true;
|
||||
}
|
||||
}
|
||||
false
|
||||
}
|
||||
|
||||
#[inline]
|
||||
@@ -1931,12 +1968,16 @@ pub fn change_resolution_directly(name: &str, width: usize, height: usize) -> Re
|
||||
Ok(())
|
||||
}
|
||||
|
||||
/// Scoped to `uid`, the user of the session being refreshed: the compositor starts Xwayland as
|
||||
/// that user, so another user's Xwayland -- a switched-away session, a second seat -- answering
|
||||
/// this used to route a pure-Wayland session into the Xwayland probe, which has no display for
|
||||
/// it to find. A uid that cannot be parsed falls back to the unscoped answer.
|
||||
#[inline]
|
||||
pub fn is_xwayland_running() -> bool {
|
||||
if let Ok(output) = run_cmds("pgrep -a Xwayland") {
|
||||
return output.contains("Xwayland");
|
||||
}
|
||||
false
|
||||
pub fn is_xwayland_running(uid: &str) -> bool {
|
||||
// Same test as the `pgrep -a Xwayland` this replaces: the process name, not its command line.
|
||||
any_process(uid.parse::<u32>().ok(), "comm", |comm| {
|
||||
comm.contains("Xwayland")
|
||||
})
|
||||
}
|
||||
|
||||
mod desktop {
|
||||
@@ -1959,10 +2000,14 @@ mod desktop {
|
||||
|
||||
/// A compositor that runs Xwayland without exporting `XAUTHORITY` (wlroots, e.g. Hyprland)
|
||||
/// still hands out a usable session through the Wayland side. Requiring xauth there never
|
||||
/// succeeded, so every refresh ran the retry loop to the end, 240 shell pipelines at a time.
|
||||
/// succeeded, so every refresh ran the retry loop to the end.
|
||||
/// https://github.com/rustdesk/rustdesk/issues/15952
|
||||
fn is_session_env_complete(display: &str, xauth: &str, wl_display: &str, dbus: &str) -> bool {
|
||||
!display.is_empty() && (!xauth.is_empty() || (!wl_display.is_empty() && !dbus.is_empty()))
|
||||
fn is_session_env_complete(envs: &std::collections::HashMap<&str, String>) -> bool {
|
||||
let value = |key: &str| envs.get(key).map_or("", |v| v.as_str());
|
||||
!value(ENV_KEY_DISPLAY).is_empty()
|
||||
&& (!value(ENV_KEY_XAUTHORITY).is_empty()
|
||||
|| (!value(ENV_KEY_WAYLAND_DISPLAY).is_empty()
|
||||
&& !value(ENV_KEY_DBUS_SESSION_BUS_ADDRESS).is_empty()))
|
||||
}
|
||||
|
||||
#[derive(Debug, Clone, Default)]
|
||||
@@ -2037,17 +2082,33 @@ mod desktop {
|
||||
self.dbus.clear();
|
||||
let mut kept = 0u8;
|
||||
for proc in display_proc {
|
||||
let display = get_env(ENV_KEY_DISPLAY, &self.uid, proc);
|
||||
let xauth = get_env(ENV_KEY_XAUTHORITY, &self.uid, proc);
|
||||
let wl_display = get_env(ENV_KEY_WAYLAND_DISPLAY, &self.uid, proc);
|
||||
let dbus = get_env(ENV_KEY_DBUS_SESSION_BUS_ADDRESS, &self.uid, proc);
|
||||
// Take a candidate whole and keep the best seen. Assigning each variable
|
||||
// unconditionally let a pattern that does not run on this desktop blank out
|
||||
// the values an earlier one had answered with, which is how a session with a
|
||||
// working portal ended up starting its `--server` with no compositor and no
|
||||
// bus at all. The Wayland-only rank is what a session whose Xwayland exports
|
||||
// no `XAUTHORITY` can still offer.
|
||||
let complete = is_session_env_complete(&display, &xauth, &wl_display, &dbus);
|
||||
let mut envs = get_envs_of_newest(
|
||||
&self.uid,
|
||||
proc,
|
||||
&[
|
||||
ENV_KEY_DISPLAY,
|
||||
ENV_KEY_XAUTHORITY,
|
||||
ENV_KEY_WAYLAND_DISPLAY,
|
||||
ENV_KEY_DBUS_SESSION_BUS_ADDRESS,
|
||||
],
|
||||
);
|
||||
let complete = is_session_env_complete(&envs);
|
||||
let display = envs.remove(ENV_KEY_DISPLAY).unwrap_or_default();
|
||||
let xauth = envs.remove(ENV_KEY_XAUTHORITY).unwrap_or_default();
|
||||
let wl_display = envs.remove(ENV_KEY_WAYLAND_DISPLAY).unwrap_or_default();
|
||||
let dbus = envs
|
||||
.remove(ENV_KEY_DBUS_SESSION_BUS_ADDRESS)
|
||||
.unwrap_or_default();
|
||||
// Take a candidate whole. Two graphical sessions of one user each answer
|
||||
// some of these, and a display paired with another session's xauth or
|
||||
// compositor is a pair that never existed. So rank candidates rather than
|
||||
// merge them, and keep the best seen: the later patterns are fallbacks.
|
||||
//
|
||||
// The Wayland-only rank matters when `is_xwayland_running` matched some other
|
||||
// user's Xwayland and this session has none of its own. Nothing here can then
|
||||
// answer with a display, and dropping the candidate for that would leave the
|
||||
// child server without the compositor and bus of a session that is perfectly
|
||||
// serveable through them.
|
||||
let rank = if complete {
|
||||
3
|
||||
} else if !wl_display.is_empty() && !dbus.is_empty() {
|
||||
@@ -2091,7 +2152,9 @@ mod desktop {
|
||||
SDDM_GREETER,
|
||||
];
|
||||
for proc in display_proc {
|
||||
self.display = get_env(ENV_KEY_DISPLAY, &self.uid, proc);
|
||||
self.display = get_envs_of_newest(&self.uid, proc, &[ENV_KEY_DISPLAY])
|
||||
.remove(ENV_KEY_DISPLAY)
|
||||
.unwrap_or_default();
|
||||
if !self.display.is_empty() {
|
||||
break;
|
||||
}
|
||||
@@ -2222,7 +2285,9 @@ mod desktop {
|
||||
tray.as_str(),
|
||||
];
|
||||
for proc in display_proc {
|
||||
self.xauth = get_env("XAUTHORITY", &self.uid, proc);
|
||||
self.xauth = get_envs_of_newest(&self.uid, proc, &[ENV_KEY_XAUTHORITY])
|
||||
.remove(ENV_KEY_XAUTHORITY)
|
||||
.unwrap_or_default();
|
||||
if !self.xauth.is_empty() {
|
||||
break;
|
||||
}
|
||||
@@ -2308,7 +2373,7 @@ mod desktop {
|
||||
pub fn refresh(&mut self) {
|
||||
if !self.sid.is_empty() && is_active_and_seat0(&self.sid) {
|
||||
// Xwayland display and xauth may not be available in a short time after login.
|
||||
if is_xwayland_running() && !self.is_login_wayland() {
|
||||
if is_xwayland_running(&self.uid) && !self.is_login_wayland() {
|
||||
self.get_display_xauth_xwayland();
|
||||
} else if self.is_wayland() {
|
||||
self.get_display_xauth_wayland();
|
||||
@@ -2350,7 +2415,7 @@ mod desktop {
|
||||
|
||||
self.get_home();
|
||||
if self.is_wayland() {
|
||||
if is_xwayland_running() {
|
||||
if is_xwayland_running(&self.uid) {
|
||||
self.get_display_xauth_xwayland();
|
||||
} else {
|
||||
self.get_display_xauth_wayland();
|
||||
|
||||
Reference in New Issue
Block a user