mirror of
https://github.com/rustdesk/rustdesk.git
synced 2026-09-08 05:20:59 +03:00
drm: check the greeter server is alive, not just spawned (M5, M10)
M5: the greeter fallback tested the wrong thing. start_server reports whether the SPAWN succeeded, so a greeter account that cannot actually run the server, a nologin shell or a hardened home, leaves a child that exits at once; the loop sees only that the child is gone and respawns it as the greeter forever, never reaching the root fallback, and the login screen becomes un-remotable on a host where it used to work. It now requires the child to still be alive after a one second grace before accepting it. A server that dies later than that is a different, transient failure and the existing restart throttle already bounds it. While there: the whole greeter branch is now inside the drm cfg, so the drm-off build is upstream's single start_server line again rather than a run_as_greeter variable that is always false. M10: two monitors of the same model and resolution whose names do not normalize to the compositor's matched no output at all, so both kept the DRM origin, which is (0,0) for independent CRTCs. The client stacks them and injected coordinates hit the wrong monitor with certainty. Unmatched connectors now take the next free output in layout order, preferring one of the same physical size, and say so in the log. That is at worst a swap of two identically sized rectangles, and the layout stays coherent. The same pass also stops one output being claimed by two connectors, which the unique-resolution rule allowed. The assignment is now a pure function, so the cases are testable without a compositor: five tests cover the naming difference, the identical-monitor case, the double claim, name match beating the fallback, and more connectors than outputs. 99 tests pass, both configs build.
This commit is contained in:
Mariano Abad
@@ -720,6 +720,40 @@ fn start_server(desktop: Option<&Desktop>, server: &mut Option<Child>) {
|
||||
}
|
||||
}
|
||||
|
||||
/// Whether a just-spawned `--server` is still running after a short grace period, taking ownership of
|
||||
/// the corpse (clearing `server`) when it is not. `start_server` reports only whether the SPAWN
|
||||
/// succeeded, which is not the same question: a child that execs and exits immediately still leaves
|
||||
/// `Some(child)` behind.
|
||||
///
|
||||
/// A child that exits is detected as soon as it does; a healthy one costs the full grace, once per
|
||||
/// start. A server that dies LATER than this is a different (transient) failure, and the restart
|
||||
/// throttle in `should_start_server` already bounds that case.
|
||||
#[cfg(feature = "drm")]
|
||||
fn server_survived_grace(server: &mut Option<Child>) -> bool {
|
||||
const GRACE: Duration = Duration::from_millis(1000);
|
||||
const STEP_MS: u64 = 100;
|
||||
let Some(ps) = server.as_mut() else {
|
||||
return false; // spawn itself failed
|
||||
};
|
||||
let deadline = Instant::now() + GRACE;
|
||||
while Instant::now() < deadline {
|
||||
match ps.try_wait() {
|
||||
Ok(Some(status)) => {
|
||||
log::warn!("--server exited {status} within {GRACE:?} of starting");
|
||||
*server = None;
|
||||
return false;
|
||||
}
|
||||
Ok(None) => sleep_millis(STEP_MS),
|
||||
// We cannot tell; treat it as alive rather than tearing down a possibly healthy child.
|
||||
Err(err) => {
|
||||
log::error!("error waiting on the just-started --server: {err}");
|
||||
return true;
|
||||
}
|
||||
}
|
||||
}
|
||||
true
|
||||
}
|
||||
|
||||
fn stop_server(server: &mut Option<Child>) {
|
||||
if let Some(mut ps) = server.take() {
|
||||
allow_err!(ps.kill());
|
||||
@@ -904,21 +938,25 @@ pub fn start_os_service() {
|
||||
// uid, so this drops to whichever greeter owns seat0. A greeter is_gdm_user does not
|
||||
// recognize (e.g. LightDM) never reaches this branch -- it takes the unprivileged
|
||||
// else-branch below already. A genuine root graphical session (username=="root")
|
||||
// has no lower uid to drop to, so it stays root. Gated on the drm feature so the
|
||||
// non-drm build stays byte-identical to upstream.
|
||||
#[cfg(feature = "drm")]
|
||||
let run_as_greeter = desktop.username != "root" && !desktop.uid.is_empty();
|
||||
// has no lower uid to drop to, so it stays root. The whole branch is gated on the drm
|
||||
// feature, so the drm-off build is upstream's single `start_server(None, ..)` line.
|
||||
#[cfg(not(feature = "drm"))]
|
||||
let run_as_greeter = false;
|
||||
if run_as_greeter {
|
||||
start_server(None, &mut server);
|
||||
#[cfg(feature = "drm")]
|
||||
if desktop.username != "root" && !desktop.uid.is_empty() {
|
||||
start_server(Some(&desktop), &mut server);
|
||||
// If dropping to the greeter uid did not produce a running server (spawn/exec
|
||||
// failure), fall back to a root --server so the login screen stays remotable
|
||||
// instead of looping on a failing greeter spawn. This pays the GPU-in-root
|
||||
// tradeoff only on that failure path, never in the normal greeter case.
|
||||
if server.is_none() {
|
||||
// If dropping to the greeter uid did not produce a RUNNING server, fall back to a
|
||||
// root --server so the login screen stays remotable instead of looping on a
|
||||
// failing greeter spawn. This pays the GPU-in-root tradeoff only on that failure
|
||||
// path, never in the normal greeter case. Liveness, not just spawn success: a
|
||||
// greeter account that cannot actually run it (a nologin shell, a hardened home,
|
||||
// no writable config dir) leaves a child that exits at once, and the loop above
|
||||
// notices only that the child is gone and respawns it, forever, without ever
|
||||
// reaching this fallback -- so the login screen becomes permanently un-remotable
|
||||
// on a host where it used to work.
|
||||
if !server_survived_grace(&mut server) {
|
||||
log::warn!(
|
||||
"greeter --server did not start; falling back to a root --server"
|
||||
"greeter --server did not stay up; falling back to a root --server"
|
||||
);
|
||||
start_server(None, &mut server);
|
||||
}
|
||||
|
||||
@@ -1147,45 +1147,103 @@ pub(super) fn get_primary_index() -> usize {
|
||||
/// (already at 0,0, scale 1.0) needs no augmentation, matching the PipeWire path's logical-scale gate.
|
||||
fn augment_with_wayland_geometry(drm: &[DrmDisplayInfo]) -> Vec<DisplayInfo> {
|
||||
let wl = scrap::wayland::display::get_displays();
|
||||
let multi = drm.len() > 1 && wl.displays.len() > 1;
|
||||
drm.iter()
|
||||
.map(|d| {
|
||||
let mut info = display_info_from_drm(d);
|
||||
if multi {
|
||||
if let Some(w) = match_wayland_display(d, &wl.displays) {
|
||||
info.x = w.x;
|
||||
info.y = w.y;
|
||||
if let Some((lw, lh)) = w.logical_size {
|
||||
if lw > 0 && lh > 0 {
|
||||
info.scale = d.width as f64 / lw as f64;
|
||||
// original_resolution is the logical size (physical / scale).
|
||||
info.original_resolution = super::display_service::get_original_resolution(
|
||||
&d.name,
|
||||
lw as usize,
|
||||
lh as usize,
|
||||
);
|
||||
}
|
||||
}
|
||||
}
|
||||
let mut infos: Vec<DisplayInfo> = drm.iter().map(display_info_from_drm).collect();
|
||||
if drm.len() < 2 || wl.displays.len() < 2 {
|
||||
return infos;
|
||||
}
|
||||
let matched = assign_wayland_outputs(drm, &wl.displays);
|
||||
for (i, info) in infos.iter_mut().enumerate() {
|
||||
let Some(w) = matched[i].map(|j| &wl.displays[j]) else {
|
||||
continue;
|
||||
};
|
||||
info.x = w.x;
|
||||
info.y = w.y;
|
||||
if let Some((lw, lh)) = w.logical_size {
|
||||
if lw > 0 && lh > 0 {
|
||||
info.scale = drm[i].width as f64 / lw as f64;
|
||||
// original_resolution is the logical size (physical / scale).
|
||||
info.original_resolution = super::display_service::get_original_resolution(
|
||||
&drm[i].name,
|
||||
lw as usize,
|
||||
lh as usize,
|
||||
);
|
||||
}
|
||||
info
|
||||
})
|
||||
.collect()
|
||||
}
|
||||
}
|
||||
infos
|
||||
}
|
||||
|
||||
/// Match a DRM display to its compositor output: by normalized connector name first, then by a
|
||||
/// uniquely-matching physical resolution.
|
||||
fn match_wayland_display<'a>(
|
||||
d: &DrmDisplayInfo,
|
||||
wl: &'a [hbb_common::platform::linux::WaylandDisplayInfo],
|
||||
) -> Option<&'a hbb_common::platform::linux::WaylandDisplayInfo> {
|
||||
let dn = normalize_connector(&d.name);
|
||||
if let Some(w) = wl.iter().find(|w| normalize_connector(&w.name) == dn) {
|
||||
return Some(w);
|
||||
/// Which compositor output each DRM connector corresponds to, as an index into `wl` (or `None` when
|
||||
/// there is nothing left to give it). Pure, so the assignment is unit-testable without a compositor.
|
||||
///
|
||||
/// Each output goes to at most one connector, which the per-display rules alone did not guarantee:
|
||||
/// the unique-resolution rule could hand the same output to two connectors of that resolution.
|
||||
///
|
||||
/// Connectors that match nothing take the next free output in layout order, with a warning. Leaving
|
||||
/// them unaugmented is not the safe choice it looks like: DRM reports every connector at origin
|
||||
/// (0,0), so two monitors of the same model and resolution whose names do not normalize to the
|
||||
/// compositor's would both keep (0,0), the client stacks them, and injected coordinates land on the
|
||||
/// wrong monitor with certainty. Positional order is at worst a swap of two identically-sized
|
||||
/// rectangles, and the layout stays coherent either way. A free output of the same physical size is
|
||||
/// preferred, so a mixed layout does not pair a connector with an output it cannot be.
|
||||
fn assign_wayland_outputs(
|
||||
drm: &[DrmDisplayInfo],
|
||||
wl: &[hbb_common::platform::linux::WaylandDisplayInfo],
|
||||
) -> Vec<Option<usize>> {
|
||||
let mut taken = vec![false; wl.len()];
|
||||
let mut matched: Vec<Option<usize>> = vec![None; drm.len()];
|
||||
for (i, d) in drm.iter().enumerate() {
|
||||
if let Some(j) = match_wayland_display(d, wl, &taken) {
|
||||
matched[i] = Some(j);
|
||||
taken[j] = true;
|
||||
}
|
||||
}
|
||||
let same_res: Vec<_> = wl
|
||||
for (i, d) in drm.iter().enumerate() {
|
||||
if matched[i].is_some() {
|
||||
continue;
|
||||
}
|
||||
let free_same_size = wl
|
||||
.iter()
|
||||
.enumerate()
|
||||
.position(|(j, w)| !taken[j] && w.width == d.width as i32 && w.height == d.height as i32);
|
||||
let Some(j) = free_same_size.or_else(|| taken.iter().position(|t| !t)) else {
|
||||
continue; // more connectors than outputs; leave the rest unaugmented
|
||||
};
|
||||
log::warn!(
|
||||
"drm: connector {} matched no compositor output by name or by a unique resolution; \
|
||||
falling back to layout order and taking {} at ({}, {})",
|
||||
d.name,
|
||||
wl[j].name,
|
||||
wl[j].x,
|
||||
wl[j].y
|
||||
);
|
||||
matched[i] = Some(j);
|
||||
taken[j] = true;
|
||||
}
|
||||
matched
|
||||
}
|
||||
|
||||
/// Index of the compositor output for a DRM display: by normalized connector name first, then by a
|
||||
/// uniquely-matching physical resolution. `taken` outputs are skipped so one output cannot be
|
||||
/// claimed twice.
|
||||
fn match_wayland_display(
|
||||
d: &DrmDisplayInfo,
|
||||
wl: &[hbb_common::platform::linux::WaylandDisplayInfo],
|
||||
taken: &[bool],
|
||||
) -> Option<usize> {
|
||||
let dn = normalize_connector(&d.name);
|
||||
if let Some((j, _)) = wl
|
||||
.iter()
|
||||
.filter(|w| w.width == d.width as i32 && w.height == d.height as i32)
|
||||
.enumerate()
|
||||
.find(|(j, w)| !taken[*j] && normalize_connector(&w.name) == dn)
|
||||
{
|
||||
return Some(j);
|
||||
}
|
||||
let same_res: Vec<usize> = wl
|
||||
.iter()
|
||||
.enumerate()
|
||||
.filter(|(j, w)| !taken[*j] && w.width == d.width as i32 && w.height == d.height as i32)
|
||||
.map(|(j, _)| j)
|
||||
.collect();
|
||||
if same_res.len() == 1 {
|
||||
return Some(same_res[0]);
|
||||
@@ -1423,6 +1481,99 @@ mod drm_capturer_tests {
|
||||
assert!(matches!(c.frame(Duration::from_millis(50)), Ok(_)));
|
||||
}
|
||||
|
||||
fn drm_display(name: &str, w: u32, h: u32) -> DrmDisplayInfo {
|
||||
DrmDisplayInfo {
|
||||
name: name.to_owned(),
|
||||
crtc_id: 1,
|
||||
x: 0,
|
||||
y: 0,
|
||||
width: w,
|
||||
height: h,
|
||||
active: true,
|
||||
render_node: String::new(),
|
||||
device: String::new(),
|
||||
}
|
||||
}
|
||||
|
||||
fn wl_display(
|
||||
name: &str,
|
||||
x: i32,
|
||||
y: i32,
|
||||
w: i32,
|
||||
h: i32,
|
||||
) -> hbb_common::platform::linux::WaylandDisplayInfo {
|
||||
hbb_common::platform::linux::WaylandDisplayInfo {
|
||||
name: name.to_owned(),
|
||||
x,
|
||||
y,
|
||||
width: w,
|
||||
height: h,
|
||||
logical_size: Some((w, h)),
|
||||
refresh_rate: 60,
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn outputs_are_matched_by_name_across_the_drm_naming_difference() {
|
||||
let drm = [drm_display("HDMI-A-1", 1920, 1080), drm_display("DP-1", 2560, 1440)];
|
||||
// Deliberately in the other order, and the second entry is the one that matches by name.
|
||||
let wl = [wl_display("DP-1", 1920, 0, 2560, 1440), wl_display("HDMI-1", 0, 0, 1920, 1080)];
|
||||
assert_eq!(assign_wayland_outputs(&drm, &wl), vec![Some(1), Some(0)]);
|
||||
}
|
||||
|
||||
// The M10 case: two monitors of the same model and resolution whose names do not normalize to
|
||||
// the compositor's. Both used to end up unmatched, keeping the DRM origin (0,0), which stacks
|
||||
// them on the client. Every connector must now get a distinct output.
|
||||
#[test]
|
||||
fn identical_monitors_that_match_no_name_take_layout_order() {
|
||||
let drm = [drm_display("DP-1", 1920, 1080), drm_display("DP-2", 1920, 1080)];
|
||||
let wl = [
|
||||
wl_display("Unknown-1", 0, 0, 1920, 1080),
|
||||
wl_display("Unknown-2", 1920, 0, 1920, 1080),
|
||||
];
|
||||
assert_eq!(assign_wayland_outputs(&drm, &wl), vec![Some(0), Some(1)]);
|
||||
}
|
||||
|
||||
// The same aliasing, one step earlier: the unique-resolution rule handed ONE output to both
|
||||
// connectors of that resolution, so two displays claimed the same origin.
|
||||
#[test]
|
||||
fn one_output_is_never_claimed_by_two_connectors() {
|
||||
let drm = [drm_display("DP-1", 1920, 1080), drm_display("DP-2", 1920, 1080)];
|
||||
let wl = [
|
||||
wl_display("Unknown-1", 0, 0, 1920, 1080),
|
||||
wl_display("Unknown-2", 1920, 0, 3840, 2160),
|
||||
];
|
||||
let got = assign_wayland_outputs(&drm, &wl);
|
||||
assert_eq!(got[0], Some(0));
|
||||
assert_ne!(got[0], got[1], "two connectors must not share one output");
|
||||
}
|
||||
|
||||
// A name match must still win over a same-size output that comes earlier in the layout.
|
||||
#[test]
|
||||
fn a_name_match_beats_the_positional_fallback() {
|
||||
let drm = [drm_display("DP-1", 1920, 1080), drm_display("HDMI-A-1", 1920, 1080)];
|
||||
let wl = [
|
||||
wl_display("Unknown-1", 0, 0, 1920, 1080),
|
||||
wl_display("HDMI-1", 1920, 0, 1920, 1080),
|
||||
];
|
||||
assert_eq!(assign_wayland_outputs(&drm, &wl), vec![Some(0), Some(1)]);
|
||||
}
|
||||
|
||||
// More connectors than outputs: the extras stay unaugmented rather than sharing one.
|
||||
#[test]
|
||||
fn extra_connectors_stay_unmatched() {
|
||||
let drm = [
|
||||
drm_display("DP-1", 1920, 1080),
|
||||
drm_display("DP-2", 1920, 1080),
|
||||
drm_display("DP-3", 1920, 1080),
|
||||
];
|
||||
let wl = [
|
||||
wl_display("Unknown-1", 0, 0, 1920, 1080),
|
||||
wl_display("Unknown-2", 1920, 0, 1920, 1080),
|
||||
];
|
||||
assert_eq!(assign_wayland_outputs(&drm, &wl), vec![Some(0), Some(1), None]);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn demote_cooldown_doubles_per_cycle_and_caps() {
|
||||
// First demotion keeps the historical 30 s, so a transient failure still recovers quickly.
|
||||
|
||||
Reference in New Issue
Block a user