mirror of
https://github.com/rustdesk/rustdesk.git
synced 2026-09-08 05:20:59 +03:00
drm: stop routing gates from paying for the availability probe
A Major finding I skipped twice, and the file already argued against itself: wayland.rs's own NOTE says re-probing _drm from the async enumeration path blocks the executor long enough to trip 'deadline has elapsed' and spiral into a restart loop -- and then six routing gates called is_available(), which runs query_displays() inline whenever the state is Unknown (cold start, or a NEGATIVE_TTL expiry mid-session). ensure_inited, is_inited, get_displays_and_primary and clear() are exactly the paths the NOTE names. is_available_cached() is a single mutex read: KNOWN-available or not. The six gates use it, which is safe because they are routing decisions, not capability ones -- a cold cache answers 'not DRM' and the caller takes the PipeWire path it would have taken anyway. Switching all seven, which is what the finding literally suggested, would have introduced a worse bug: warm_availability calls query_displays() directly, so is_available() would have had ZERO callers and nothing would ever probe lazily again. A --server that started before the root service would then never see DRM for the rest of its life. get_capturer_for_display keeps the probing form -- it is sync, on the plain video thread, it is the capture-build path where a definitive answer is the point, and it is what makes a cold cache recoverable.
This commit is contained in:
@@ -340,7 +340,7 @@ fn check_get_displays_changed_msg() -> Option<Message> {
|
||||
// from the DRM display list here. Without this the display service broadcasts an empty
|
||||
// list that overwrites the login peer-info displays and the client shows "No displays".
|
||||
#[cfg(feature = "drm")]
|
||||
if super::drm_capturer::is_available() {
|
||||
if super::drm_capturer::is_available_cached() {
|
||||
if let Some(displays) = super::drm_capturer::get_display_infos() {
|
||||
SYNC_DISPLAYS.lock().unwrap().check_changed(&displays);
|
||||
}
|
||||
|
||||
@@ -1075,10 +1075,33 @@ impl Drop for UinputRefreshGuard {
|
||||
}
|
||||
}
|
||||
|
||||
/// Cache-only view of the same verdict: is DRM capture KNOWN to be available right now. Never probes,
|
||||
/// never blocks, never expires a stale negative -- one mutex read.
|
||||
///
|
||||
/// This is the form the routing gates must use. `is_available()` below can run the blocking
|
||||
/// `query_displays()` inline whenever the state is `Unknown` (a cold start, or a `NEGATIVE_TTL`
|
||||
/// expiry mid-session), which is seconds of IPC. Paying that inside `wayland::clear()`,
|
||||
/// `is_inited()`, the display enumeration or `get_displays_and_primary()` is exactly the stall
|
||||
/// `wayland.rs`'s own NOTE says must never happen there: it blocks the executor long enough to trip
|
||||
/// "deadline has elapsed" and spiral into a video-service restart loop. The comment was right and the
|
||||
/// code used the probing accessor anyway.
|
||||
///
|
||||
/// The distinction is safe because these are ROUTING decisions, not capability decisions: with the
|
||||
/// cache cold they answer "not DRM" and the caller takes the PipeWire path it would have taken
|
||||
/// anyway, and `warm_availability` seeds the cache at `--server` start so a genuine DRM host answers
|
||||
/// true from the first connection. Only the capture-build path (`get_capturer_info`) still uses the
|
||||
/// probing form, where paying for a definitive answer is the entire point.
|
||||
pub(super) fn is_available_cached() -> bool {
|
||||
matches!(&*DRM_STATE.lock().unwrap(), ProbeState::Available(..))
|
||||
}
|
||||
|
||||
/// Whether the root service offers DRM/KMS capture. The positive result and a definitive negative
|
||||
/// (connected, but no displays) are cached; a transient probe error stays `Unknown` for a few
|
||||
/// retries. Normally the cache is warmed at `--server` startup (`warm_availability`), so the first
|
||||
/// client connection hits the fast `Available` path.
|
||||
///
|
||||
/// MAY BLOCK for seconds (see `is_available_cached`). Use it only where a definitive verdict is worth
|
||||
/// that, never as a routing gate.
|
||||
pub(super) fn is_available() -> bool {
|
||||
// Fast path under the lock: read the cached verdict, expiring a stale negative so a host that had
|
||||
// no displays at probe time can still enable DRM once displays appear (without a --server
|
||||
|
||||
@@ -178,7 +178,7 @@ pub(super) async fn ensure_inited() -> ResultType<()> {
|
||||
// IPC, so there is no PipeWire recorder to initialize here. But we still must set the uinput
|
||||
// desktop rect (check_init does this on the PipeWire path, and the DRM path skips check_init).
|
||||
#[cfg(feature = "drm")]
|
||||
if super::drm_capturer::is_available() {
|
||||
if super::drm_capturer::is_available_cached() {
|
||||
update_uinput_resolution().await;
|
||||
return Ok(());
|
||||
}
|
||||
@@ -190,7 +190,7 @@ pub(super) fn is_inited() -> Option<Message> {
|
||||
None
|
||||
} else {
|
||||
#[cfg(feature = "drm")]
|
||||
if super::drm_capturer::is_available() {
|
||||
if super::drm_capturer::is_available_cached() {
|
||||
return None;
|
||||
}
|
||||
if CAP_DISPLAY_INFO.read().unwrap().is_empty() {
|
||||
@@ -320,7 +320,7 @@ pub(super) async fn check_init() -> ResultType<()> {
|
||||
|
||||
pub(super) async fn get_displays_and_primary() -> ResultType<(Vec<DisplayInfo>, usize)> {
|
||||
#[cfg(feature = "drm")]
|
||||
if super::drm_capturer::is_available() {
|
||||
if super::drm_capturer::is_available_cached() {
|
||||
if let Some(displays) = super::drm_capturer::get_display_infos() {
|
||||
// DRM connector order is not the compositor's primary; resolve the real primary from
|
||||
// the compositor layout (matched by normalized connector name), not a hardcoded index 0.
|
||||
@@ -351,7 +351,7 @@ pub fn clear() {
|
||||
// against STALE geometry after a monitor hotplug/rotation/scale change. Invalidate it on teardown
|
||||
// so the next session re-reads fresh geometry (lazily, on the next enumeration) and self-heals.
|
||||
#[cfg(feature = "drm")]
|
||||
if super::drm_capturer::is_available() {
|
||||
if super::drm_capturer::is_available_cached() {
|
||||
scrap::wayland::display::clear_wayland_displays_cache();
|
||||
}
|
||||
// NOTE: intentionally do NOT reset the DRM probe cache here. `clear()` runs on every capturer
|
||||
@@ -397,6 +397,11 @@ pub(super) fn get_capturer_for_display(
|
||||
// render-node-absent seat or a convert failure on the unprivileged side) must NOT propagate out
|
||||
// and restart-loop this per-display video service. Instead fall THROUGH to PipeWire for just this
|
||||
// display; the other DRM outputs keep streaming over DRM.
|
||||
// The ONE gate that keeps the probing form on purpose: this runs on the plain video thread,
|
||||
// not an async executor, and it is the capture-build path, so a definitive verdict is worth
|
||||
// seconds here. It is also what makes a cold cache recoverable at all -- warm_availability
|
||||
// gives up after its attempts, so if EVERY gate were cache-only a --server that started
|
||||
// before the root service would never see DRM again for the rest of its life.
|
||||
#[cfg(feature = "drm")]
|
||||
if super::drm_capturer::is_available() {
|
||||
match super::drm_capturer::get_capturer_info(display_idx) {
|
||||
@@ -436,7 +441,7 @@ pub(super) fn get_capturer_for_display(
|
||||
// that display) and is served normally. On a pure-PipeWire host is_available() is false and
|
||||
// this guard is skipped, preserving upstream behavior exactly.
|
||||
#[cfg(feature = "drm")]
|
||||
if super::drm_capturer::is_available() {
|
||||
if super::drm_capturer::is_available_cached() {
|
||||
if let Some(advertised) = super::drm_capturer::get_display_infos()
|
||||
.and_then(|l| l.get(display_idx).cloned())
|
||||
{
|
||||
|
||||
Reference in New Issue
Block a user