Skip to content

Commit beaa754

Browse files
21pages21pages
authored
fix stale primary display selection (rustdesk#15460)
* fix stale primary display selection Signed-off-by: 21pages <sunboeasy@gmail.com> * fix stale display selection during login and switching - resolve the primary display from the refreshed login snapshot - defer display enumeration until authentication succeeds - read Wayland displays and primary index from the same cache snapshot - reject stale monitor and camera indices during display switching Signed-off-by: 21pages <sunboeasy@gmail.com> * fix inconsistent display snapshots during login - return displays from the same enumeration used to select the primary - avoid re-reading the shared display cache after updating it - use the same converted snapshot during Wayland initialization Signed-off-by: 21pages <sunboeasy@gmail.com> * avoid cloning unchanged display snapshots Signed-off-by: 21pages <sunboeasy@gmail.com> * fix invalid display subset handling Signed-off-by: 21pages <sunboeasy@gmail.com> * minimize code churn in switch_display_to Signed-off-by: 21pages <sunboeasy@gmail.com> --------- Signed-off-by: 21pages <sunboeasy@gmail.com>
1 parent 929e989 commit beaa754

4 files changed

Lines changed: 143 additions & 85 deletions

File tree

‎src/server.rs‎

Lines changed: 15 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -357,15 +357,13 @@ impl Server {
357357
}
358358
}
359359

360-
pub fn try_add_primay_video_service(&mut self) {
361-
let primary_video_service_name = video_service::get_service_name(
362-
VideoSource::Monitor,
363-
*display_service::PRIMARY_DISPLAY_IDX,
364-
);
365-
if !self.contains(&primary_video_service_name) {
360+
pub fn try_add_monitor_service(&mut self, display_idx: usize) {
361+
let monitor_service_name =
362+
video_service::get_service_name(VideoSource::Monitor, display_idx);
363+
if !self.contains(&monitor_service_name) {
366364
self.add_service(Box::new(video_service::new(
367365
VideoSource::Monitor,
368-
*display_service::PRIMARY_DISPLAY_IDX,
366+
display_idx,
369367
)));
370368
}
371369
}
@@ -381,14 +379,17 @@ impl Server {
381379
self.connections.insert(conn.id(), conn);
382380
}
383381

384-
pub fn add_connection(&mut self, conn: ConnInner, noperms: &Vec<&'static str>) {
385-
let primary_video_service_name = video_service::get_service_name(
386-
VideoSource::Monitor,
387-
*display_service::PRIMARY_DISPLAY_IDX,
388-
);
382+
pub fn add_monitor_connection(
383+
&mut self,
384+
conn: ConnInner,
385+
noperms: &Vec<&'static str>,
386+
display_idx: usize,
387+
) {
388+
let monitor_service_name =
389+
video_service::get_service_name(VideoSource::Monitor, display_idx);
389390
for s in self.services.values() {
390391
let name = s.name();
391-
if Self::is_video_service_name(&name) && name != primary_video_service_name {
392+
if Self::is_video_service_name(&name) && name != monitor_service_name {
392393
continue;
393394
}
394395
if !noperms.contains(&(&name as _)) {
@@ -783,8 +784,7 @@ async fn sync_and_watch_config_dir(sync_done_tx: Option<tokio::sync::oneshot::Se
783784
loop {
784785
sleep(CONFIG_SYNC_INTERVAL_SECS).await;
785786
let cfg = (Config::get(), Config2::get());
786-
let should_sync =
787-
cfg != cfg0 || (is_root_config_empty && !cfg.0.is_empty());
787+
let should_sync = cfg != cfg0 || (is_root_config_empty && !cfg.0.is_empty());
788788
if should_sync {
789789
if is_root_config_empty {
790790
log::info!("root config is empty, sync our config to root");

‎src/server/connection.rs‎

Lines changed: 79 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -503,7 +503,9 @@ impl Connection {
503503
tx_video: Some(tx_video),
504504
},
505505
require_2fa: crate::auth_2fa::get_2fa(None),
506-
display_idx: *display_service::PRIMARY_DISPLAY_IDX,
506+
// Defer display enumeration until login succeeds. Monitor login replaces this
507+
// with the primary index returned with the refreshed display snapshot.
508+
display_idx: 0,
507509
stream,
508510
server,
509511
hash,
@@ -1891,13 +1893,15 @@ impl Connection {
18911893
Err(err) => {
18921894
res.set_error(format!("{}", err));
18931895
}
1894-
Ok(displays) => {
1896+
Ok((displays, primary_display_idx)) => {
18951897
// For compatibility with old versions, we need to send the displays to the peer.
18961898
// But the displays may be updated later, before creating the video capturer.
18971899
#[cfg(target_os = "macos")]
18981900
{
18991901
self.retina.set_displays(&displays);
19001902
}
1903+
// A separate primary lookup here could race with display hot-plug.
1904+
self.display_idx = primary_display_idx;
19011905
pi.displays = displays;
19021906
pi.current_display = self.display_idx as _;
19031907
#[cfg(not(any(target_os = "android", target_os = "ios")))]
@@ -2006,8 +2010,8 @@ impl Connection {
20062010
#[cfg(not(any(target_os = "android", target_os = "ios")))]
20072011
let _h = try_start_record_cursor_pos();
20082012
self.auto_disconnect_timer = Self::get_auto_disconenct_timer();
2009-
s.try_add_primay_video_service();
2010-
s.add_connection(self.inner.clone(), &noperms);
2013+
s.try_add_monitor_service(self.display_idx);
2014+
s.add_monitor_connection(self.inner.clone(), &noperms, self.display_idx);
20112015
}
20122016
}
20132017
}
@@ -4150,7 +4154,9 @@ impl Connection {
41504154
let display_idx = s.display as usize;
41514155
if self.display_idx != display_idx {
41524156
if let Some(server) = self.server.upgrade() {
4153-
self.switch_display_to(display_idx, server.clone());
4157+
if !self.switch_display_to(display_idx, server.clone()) {
4158+
return;
4159+
}
41544160

41554161
#[cfg(not(any(target_os = "android", target_os = "ios")))]
41564162
if !self.view_camera && s.width != 0 && s.height != 0 {
@@ -4177,6 +4183,13 @@ impl Connection {
41774183
}
41784184
}
41794185

4186+
fn video_source_count(video_source: VideoSource) -> usize {
4187+
match video_source {
4188+
VideoSource::Monitor => display_service::get_sync_displays().len(),
4189+
VideoSource::Camera => camera::Cameras::get_sync_cameras().len(),
4190+
}
4191+
}
4192+
41804193
fn video_source(&self) -> VideoSource {
41814194
if self.view_camera {
41824195
VideoSource::Camera
@@ -4185,18 +4198,28 @@ impl Connection {
41854198
}
41864199
}
41874200

4188-
fn switch_display_to(&mut self, display_idx: usize, server: Arc<RwLock<Server>>) {
4201+
fn switch_display_to(&mut self, display_idx: usize, server: Arc<RwLock<Server>>) -> bool {
4202+
let source_count = Self::video_source_count(self.video_source());
4203+
if display_idx >= source_count {
4204+
// Do not remap an explicit switch: its resolution belongs to the requested source.
4205+
log::warn!(
4206+
"Ignore switch to invalid {:?} index {}, available source count: {}",
4207+
self.video_source(),
4208+
display_idx,
4209+
source_count
4210+
);
4211+
return false;
4212+
}
4213+
41894214
let new_service_name = video_service::get_service_name(self.video_source(), display_idx);
41904215
let old_service_name =
41914216
video_service::get_service_name(self.video_source(), self.display_idx);
41924217
let mut lock = server.write().unwrap();
4193-
if display_idx != *display_service::PRIMARY_DISPLAY_IDX {
4194-
if !lock.contains(&new_service_name) {
4195-
lock.add_service(Box::new(video_service::new(
4196-
self.video_source(),
4197-
display_idx,
4198-
)));
4199-
}
4218+
if !lock.contains(&new_service_name) {
4219+
lock.add_service(Box::new(video_service::new(
4220+
self.video_source(),
4221+
display_idx,
4222+
)));
42004223
}
42014224
// For versions greater than 1.2.4, a `CaptureDisplays` message will be sent immediately.
42024225
// Unnecessary capturers will be removed then.
@@ -4205,6 +4228,7 @@ impl Connection {
42054228
}
42064229
lock.subscribe(&new_service_name, self.inner.clone(), true);
42074230
self.display_idx = display_idx;
4231+
true
42084232
}
42094233

42104234
#[cfg(windows)]
@@ -4231,26 +4255,61 @@ impl Connection {
42314255

42324256
async fn capture_displays(&mut self, add: &[usize], sub: &[usize], set: &[usize]) {
42334257
let video_source = self.video_source();
4234-
if let Some(sever) = self.server.upgrade() {
4235-
let mut lock = sever.write().unwrap();
4236-
for display in add.iter() {
4258+
let source_count = Self::video_source_count(video_source);
4259+
// Only add/set can create services; sub only narrows existing subscriptions.
4260+
let valid_add = add
4261+
.iter()
4262+
.copied()
4263+
.filter(|display| *display < source_count)
4264+
.collect::<Vec<_>>();
4265+
let valid_sub = sub
4266+
.iter()
4267+
.copied()
4268+
.filter(|display| *display < source_count)
4269+
.collect::<Vec<_>>();
4270+
let valid_set = set
4271+
.iter()
4272+
.copied()
4273+
.filter(|display| *display < source_count)
4274+
.collect::<Vec<_>>();
4275+
let invalid_count =
4276+
add.len() + sub.len() + set.len() - valid_add.len() - valid_sub.len() - valid_set.len();
4277+
if invalid_count != 0 {
4278+
log::warn!(
4279+
"Ignore {} invalid {:?} indices, available source count: {}",
4280+
invalid_count,
4281+
video_source,
4282+
source_count
4283+
);
4284+
}
4285+
// Passing an invalid sub request as an empty exclude list would unsubscribe all services.
4286+
if (!add.is_empty() && valid_add.is_empty())
4287+
|| (add.is_empty() && !sub.is_empty() && valid_sub.is_empty())
4288+
|| (add.is_empty() && sub.is_empty() && !set.is_empty() && valid_set.is_empty())
4289+
{
4290+
return;
4291+
}
4292+
4293+
if let Some(server) = self.server.upgrade() {
4294+
let mut lock = server.write().unwrap();
4295+
for display in valid_add.iter() {
42374296
let service_name = video_service::get_service_name(video_source, *display);
42384297
if !lock.contains(&service_name) {
42394298
lock.add_service(Box::new(video_service::new(video_source, *display)));
42404299
}
42414300
}
4242-
for display in set.iter() {
4301+
for display in valid_set.iter() {
42434302
let service_name = video_service::get_service_name(video_source, *display);
42444303
if !lock.contains(&service_name) {
42454304
lock.add_service(Box::new(video_service::new(video_source, *display)));
42464305
}
42474306
}
42484307
if !add.is_empty() {
4249-
lock.capture_displays(self.inner.clone(), video_source, add, true, false);
4308+
lock.capture_displays(self.inner.clone(), video_source, &valid_add, true, false);
42504309
} else if !sub.is_empty() {
4251-
lock.capture_displays(self.inner.clone(), video_source, sub, false, true);
4310+
lock.capture_displays(self.inner.clone(), video_source, &valid_sub, false, true);
42524311
} else {
4253-
lock.capture_displays(self.inner.clone(), video_source, set, true, true);
4312+
lock.capture_displays(self.inner.clone(), video_source, &valid_set, true, true);
42544313
}
42554314
self.multi_ui_session = lock.get_subbed_displays_count(self.inner.id()) > 1;
42564315
if self.follow_remote_window {

‎src/server/display_service.rs‎

Lines changed: 45 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -25,9 +25,6 @@ struct ChangedResolution {
2525
lazy_static::lazy_static! {
2626
static ref IS_CAPTURER_MAGNIFIER_SUPPORTED: bool = is_capturer_mag_supported();
2727
static ref CHANGED_RESOLUTIONS: Arc<RwLock<HashMap<String, ChangedResolution>>> = Default::default();
28-
// Initial primary display index.
29-
// It should not be updated when displays changed.
30-
pub static ref PRIMARY_DISPLAY_IDX: usize = get_primary();
3128
static ref SYNC_DISPLAYS: Arc<Mutex<SyncDisplaysInfo>> = Default::default();
3229
}
3330

@@ -41,22 +38,14 @@ struct SyncDisplaysInfo {
4138
}
4239

4340
impl SyncDisplaysInfo {
44-
fn check_changed(&mut self, displays: Vec<DisplayInfo>) {
45-
if self.displays.len() != displays.len() {
46-
self.displays = displays;
47-
if !TEMP_IGNORE_DISPLAYS_CHANGED.load(Ordering::Relaxed) {
48-
self.is_synced = false;
49-
}
41+
fn check_changed(&mut self, displays: &[DisplayInfo]) {
42+
if self.displays.as_slice() == displays {
5043
return;
5144
}
52-
for (i, d) in displays.iter().enumerate() {
53-
if d != &self.displays[i] {
54-
self.displays = displays;
55-
if !TEMP_IGNORE_DISPLAYS_CHANGED.load(Ordering::Relaxed) {
56-
self.is_synced = false;
57-
}
58-
return;
59-
}
45+
46+
self.displays = displays.to_vec();
47+
if !TEMP_IGNORE_DISPLAYS_CHANGED.load(Ordering::Relaxed) {
48+
self.is_synced = false;
6049
}
6150
}
6251

@@ -304,6 +293,11 @@ pub(super) fn get_display_info(idx: usize) -> Option<DisplayInfo> {
304293
// Display to DisplayInfo
305294
// The DisplayInfo is be sent to the peer.
306295
pub(super) fn check_update_displays(all: &Vec<Display>) {
296+
let _ = update_sync_displays(all);
297+
}
298+
299+
// Return the converted input snapshot while updating the shared display cache.
300+
pub(super) fn update_sync_displays(all: &Vec<Display>) -> Vec<DisplayInfo> {
307301
// For compatibility: if only one display, scale remains 1.0 and we use the physical size for `uinput`.
308302
// If there are multiple displays, we use the logical size for `uinput` by setting scale to d.scale().
309303
#[cfg(target_os = "linux")]
@@ -346,7 +340,8 @@ pub(super) fn check_update_displays(all: &Vec<Display>) {
346340
}
347341
})
348342
.collect::<Vec<DisplayInfo>>();
349-
SYNC_DISPLAYS.lock().unwrap().check_changed(displays);
343+
SYNC_DISPLAYS.lock().unwrap().check_changed(&displays);
344+
displays
350345
}
351346

352347
pub fn is_inited_msg() -> Option<Message> {
@@ -357,34 +352,38 @@ pub fn is_inited_msg() -> Option<Message> {
357352
None
358353
}
359354

360-
pub async fn update_get_sync_displays_on_login() -> ResultType<Vec<DisplayInfo>> {
355+
// Return the primary index with the refreshed list so login cannot mix display snapshots.
356+
pub async fn update_get_sync_displays_on_login() -> ResultType<(Vec<DisplayInfo>, usize)> {
361357
#[cfg(target_os = "linux")]
362358
{
363359
if !is_x11() {
364-
return super::wayland::get_displays().await;
360+
let (displays, primary_display_idx) =
361+
super::wayland::get_displays_and_primary().await?;
362+
let primary_display_idx =
363+
normalize_primary_display_idx(primary_display_idx, displays.len());
364+
return Ok((displays, primary_display_idx));
365365
}
366366
}
367367
#[cfg(not(windows))]
368368
let displays = display_service::try_get_displays();
369369
#[cfg(windows)]
370370
let displays = display_service::try_get_displays_add_amyuni_headless();
371-
check_update_displays(&displays?);
372-
Ok(SYNC_DISPLAYS.lock().unwrap().displays.clone())
371+
let displays = displays?;
372+
let primary_display_idx = get_primary_2(&displays);
373+
let sync_displays = update_sync_displays(&displays);
374+
let primary_display_idx =
375+
normalize_primary_display_idx(primary_display_idx, sync_displays.len());
376+
Ok((sync_displays, primary_display_idx))
373377
}
374378

375379
#[inline]
376-
pub fn get_primary() -> usize {
377-
#[cfg(target_os = "linux")]
378-
{
379-
if !is_x11() {
380-
return match super::wayland::get_primary() {
381-
Ok(n) => n,
382-
Err(_) => 0,
383-
};
384-
}
380+
fn normalize_primary_display_idx(primary_display_idx: usize, display_len: usize) -> usize {
381+
// Zero is the protocol fallback when the list is empty or its primary index is stale.
382+
if primary_display_idx < display_len {
383+
primary_display_idx
384+
} else {
385+
0
385386
}
386-
387-
try_get_displays().map(|d| get_primary_2(&d)).unwrap_or(0)
388387
}
389388

390389
#[inline]
@@ -486,3 +485,16 @@ pub fn try_get_displays_(add_amyuni_headless: bool) -> ResultType<Vec<Display>>
486485
}
487486
Ok(displays)
488487
}
488+
489+
#[cfg(test)]
490+
mod tests {
491+
use super::normalize_primary_display_idx;
492+
493+
#[test]
494+
fn normalize_primary_display_idx_bounds() {
495+
assert_eq!(normalize_primary_display_idx(0, 0), 0);
496+
assert_eq!(normalize_primary_display_idx(0, 2), 0);
497+
assert_eq!(normalize_primary_display_idx(1, 2), 1);
498+
assert_eq!(normalize_primary_display_idx(2, 2), 0);
499+
}
500+
}

0 commit comments

Comments
 (0)