From 7f50fe3ea0d38c0e5106b0428c9aaeb4f10a5f25 Mon Sep 17 00:00:00 2001 From: Saverio Miroddi Date: Sun, 22 May 2022 18:03:38 +0200 Subject: [PATCH 1/5] Remove KEYBOARD_HOOKED unsafe code, by using AtomicBool For supported types, static R/W globals unsafe code can be replaced by safe `Atomic*` types. The pattern of usage is simple: - AtomicBool#swap is used to fetch the old `KEYBOARD_HOOKED` value, while setting it to true; - if the old value was true, there is effectively no change to `KEYBOARD_HOOKED`, and the flow exits from the enclosing function; - if the old value was false, execute the function (the new `KEYBOARD_HOOKED` has been set to true by swap()). The most conservative ordering is used, as the context is not performance-sensitive. Atomics are not supported on every platform, but the project assumes x86-64, which supports them. --- src/ui/remote.rs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/ui/remote.rs b/src/ui/remote.rs index ac32726aa..b4881954b 100644 --- a/src/ui/remote.rs +++ b/src/ui/remote.rs @@ -1,7 +1,10 @@ use std::{ collections::HashMap, ops::Deref, - sync::{Arc, Mutex, RwLock}, + sync::{ + atomic::{AtomicBool, Ordering}, + Arc, Mutex, RwLock, + }, }; use sciter::{ @@ -64,7 +67,7 @@ fn get_key_state(key: enigo::Key) -> bool { } static mut IS_IN: bool = false; -static mut KEYBOARD_HOOKED: bool = false; +static KEYBOARD_HOOKED: AtomicBool = AtomicBool::new(false); static mut SERVER_KEYBOARD_ENABLED: bool = true; static mut SERVER_FILE_TRANSFER_ENABLED: bool = true; static mut SERVER_CLIPBOARD_ENABLED: bool = true; @@ -249,12 +252,9 @@ impl Handler { if self.is_port_forward() || self.is_file_transfer() { return; } - if unsafe { KEYBOARD_HOOKED } { + if KEYBOARD_HOOKED.swap(true, Ordering::SeqCst) { return; } - unsafe { - KEYBOARD_HOOKED = true; - } log::info!("keyboard hooked"); let mut me = self.clone(); let peer = self.peer_platform(); From c7f452752d72834dddeb090889b7a0b789a0a3c9 Mon Sep 17 00:00:00 2001 From: Saverio Miroddi Date: Tue, 24 May 2022 18:02:33 +0200 Subject: [PATCH 2/5] Remove SERVER_CLIPBOARD_ENABLED unsafe code, by using AtomicBool --- src/ui/remote.rs | 20 +++++++++----------- 1 file changed, 9 insertions(+), 11 deletions(-) diff --git a/src/ui/remote.rs b/src/ui/remote.rs index b4881954b..3340632e4 100644 --- a/src/ui/remote.rs +++ b/src/ui/remote.rs @@ -70,7 +70,7 @@ static mut IS_IN: bool = false; static KEYBOARD_HOOKED: AtomicBool = AtomicBool::new(false); static mut SERVER_KEYBOARD_ENABLED: bool = true; static mut SERVER_FILE_TRANSFER_ENABLED: bool = true; -static mut SERVER_CLIPBOARD_ENABLED: bool = true; +static SERVER_CLIPBOARD_ENABLED: AtomicBool = AtomicBool::new(true); #[cfg(windows)] static mut IS_ALT_GR: bool = false; @@ -1382,7 +1382,7 @@ impl Remote { Ok((mut peer, direct)) => { unsafe { SERVER_KEYBOARD_ENABLED = true; - SERVER_CLIPBOARD_ENABLED = true; + SERVER_CLIPBOARD_ENABLED.store(true, Ordering::SeqCst); SERVER_FILE_TRANSFER_ENABLED = true; } self.handler @@ -1464,7 +1464,7 @@ impl Remote { } unsafe { SERVER_KEYBOARD_ENABLED = false; - SERVER_CLIPBOARD_ENABLED = false; + SERVER_CLIPBOARD_ENABLED.store(false, Ordering::SeqCst); SERVER_FILE_TRANSFER_ENABLED = false; } } @@ -1518,7 +1518,7 @@ impl Remote { } _ => {} } - if !unsafe { SERVER_CLIPBOARD_ENABLED } + if !SERVER_CLIPBOARD_ENABLED.load(Ordering::SeqCst) || !unsafe { SERVER_KEYBOARD_ENABLED } || lc.read().unwrap().disable_clipboard { @@ -1712,7 +1712,7 @@ impl Remote { job.is_last_job = false; allow_err!( peer.send(&fs::new_send(id, job.remote.clone(), job.file_num, job.show_hidden)) - .await + .await ); } } else { @@ -1947,7 +1947,7 @@ impl Remote { let json_str = serde_json::to_string(&job.gen_meta()).unwrap(); transfer_metas.write_jobs.push(json_str); } - log::info!("meta: {:?}",transfer_metas); + log::info!("meta: {:?}", transfer_metas); config.transfer = transfer_metas; self.handler.save_config(config); true @@ -1978,7 +1978,7 @@ impl Remote { self.check_clipboard_file_context(); if !(self.handler.is_file_transfer() || self.handler.is_port_forward() - || !unsafe { SERVER_CLIPBOARD_ENABLED } + || !SERVER_CLIPBOARD_ENABLED.load(Ordering::SeqCst) || !unsafe { SERVER_KEYBOARD_ENABLED } || self.handler.lc.read().unwrap().disable_clipboard) { @@ -2033,7 +2033,7 @@ impl Remote { if self.handler.peer_platform() == "Windows" { fs::transform_windows_path(&mut entries); } - } + } let mut m = make_fd(fd.id, &entries, fd.id > 0); if fd.id <= 0 { m.set_item("path", fd.path); @@ -2188,9 +2188,7 @@ impl Remote { .call2("setPermission", &make_args!("keyboard", p.enabled)); } Permission::Clipboard => { - unsafe { - SERVER_CLIPBOARD_ENABLED = p.enabled; - } + SERVER_CLIPBOARD_ENABLED.store(p.enabled, Ordering::SeqCst); self.handler .call2("setPermission", &make_args!("clipboard", p.enabled)); } From 45bb271c883e304071b653017153343761026445 Mon Sep 17 00:00:00 2001 From: Saverio Miroddi Date: Tue, 24 May 2022 18:12:28 +0200 Subject: [PATCH 3/5] Remove SERVER_KEYBOARD_ENABLED unsafe code, by using AtomicBool --- src/ui/remote.rs | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-) diff --git a/src/ui/remote.rs b/src/ui/remote.rs index 3340632e4..bfac5c74e 100644 --- a/src/ui/remote.rs +++ b/src/ui/remote.rs @@ -68,7 +68,7 @@ fn get_key_state(key: enigo::Key) -> bool { static mut IS_IN: bool = false; static KEYBOARD_HOOKED: AtomicBool = AtomicBool::new(false); -static mut SERVER_KEYBOARD_ENABLED: bool = true; +static SERVER_KEYBOARD_ENABLED: AtomicBool = AtomicBool::new(true); static mut SERVER_FILE_TRANSFER_ENABLED: bool = true; static SERVER_CLIPBOARD_ENABLED: AtomicBool = AtomicBool::new(true); #[cfg(windows)] @@ -266,7 +266,7 @@ impl Handler { std::env::set_var("KEYBOARD_ONLY", "y"); // pass to rdev use rdev::{EventType::*, *}; let func = move |evt: Event| { - if unsafe { !IS_IN || !SERVER_KEYBOARD_ENABLED } { + if unsafe { !IS_IN || !SERVER_KEYBOARD_ENABLED.load(Ordering::SeqCst) } { return; } let (key, down) = match evt.event_type { @@ -1381,7 +1381,7 @@ impl Remote { match Client::start(&self.handler.id, key, token, conn_type).await { Ok((mut peer, direct)) => { unsafe { - SERVER_KEYBOARD_ENABLED = true; + SERVER_KEYBOARD_ENABLED.store(true, Ordering::SeqCst); SERVER_CLIPBOARD_ENABLED.store(true, Ordering::SeqCst); SERVER_FILE_TRANSFER_ENABLED = true; } @@ -1463,7 +1463,7 @@ impl Remote { stop.send(()).ok(); } unsafe { - SERVER_KEYBOARD_ENABLED = false; + SERVER_KEYBOARD_ENABLED.store(false, Ordering::SeqCst); SERVER_CLIPBOARD_ENABLED.store(false, Ordering::SeqCst); SERVER_FILE_TRANSFER_ENABLED = false; } @@ -1519,7 +1519,7 @@ impl Remote { _ => {} } if !SERVER_CLIPBOARD_ENABLED.load(Ordering::SeqCst) - || !unsafe { SERVER_KEYBOARD_ENABLED } + || !SERVER_KEYBOARD_ENABLED.load(Ordering::SeqCst) || lc.read().unwrap().disable_clipboard { continue; @@ -1979,7 +1979,7 @@ impl Remote { if !(self.handler.is_file_transfer() || self.handler.is_port_forward() || !SERVER_CLIPBOARD_ENABLED.load(Ordering::SeqCst) - || !unsafe { SERVER_KEYBOARD_ENABLED } + || !SERVER_KEYBOARD_ENABLED.load(Ordering::SeqCst) || self.handler.lc.read().unwrap().disable_clipboard) { let txt = self.old_clipboard.lock().unwrap().clone(); @@ -2181,9 +2181,7 @@ impl Remote { log::info!("Change permission {:?} -> {}", p.permission, p.enabled); match p.permission.enum_value_or_default() { Permission::Keyboard => { - unsafe { - SERVER_KEYBOARD_ENABLED = p.enabled; - } + SERVER_KEYBOARD_ENABLED.store(p.enabled, Ordering::SeqCst); self.handler .call2("setPermission", &make_args!("keyboard", p.enabled)); } From 0c0051d59acbf10d360e03aff44d0c36dd779b79 Mon Sep 17 00:00:00 2001 From: Saverio Miroddi Date: Tue, 24 May 2022 18:15:46 +0200 Subject: [PATCH 4/5] Remove SERVER_FILE_TRANSFER_ENABLED unsafe code, by using AtomicBool --- src/ui/remote.rs | 24 +++++++++--------------- 1 file changed, 9 insertions(+), 15 deletions(-) diff --git a/src/ui/remote.rs b/src/ui/remote.rs index bfac5c74e..c535f6b52 100644 --- a/src/ui/remote.rs +++ b/src/ui/remote.rs @@ -69,7 +69,7 @@ fn get_key_state(key: enigo::Key) -> bool { static mut IS_IN: bool = false; static KEYBOARD_HOOKED: AtomicBool = AtomicBool::new(false); static SERVER_KEYBOARD_ENABLED: AtomicBool = AtomicBool::new(true); -static mut SERVER_FILE_TRANSFER_ENABLED: bool = true; +static SERVER_FILE_TRANSFER_ENABLED: AtomicBool = AtomicBool::new(true); static SERVER_CLIPBOARD_ENABLED: AtomicBool = AtomicBool::new(true); #[cfg(windows)] static mut IS_ALT_GR: bool = false; @@ -1380,11 +1380,9 @@ impl Remote { }; match Client::start(&self.handler.id, key, token, conn_type).await { Ok((mut peer, direct)) => { - unsafe { - SERVER_KEYBOARD_ENABLED.store(true, Ordering::SeqCst); - SERVER_CLIPBOARD_ENABLED.store(true, Ordering::SeqCst); - SERVER_FILE_TRANSFER_ENABLED = true; - } + SERVER_KEYBOARD_ENABLED.store(true, Ordering::SeqCst); + SERVER_CLIPBOARD_ENABLED.store(true, Ordering::SeqCst); + SERVER_FILE_TRANSFER_ENABLED.store(true, Ordering::SeqCst); self.handler .call("setConnectionType", &make_args!(peer.is_secured(), direct)); @@ -1462,11 +1460,9 @@ impl Remote { if let Some(stop) = stop_clipboard { stop.send(()).ok(); } - unsafe { - SERVER_KEYBOARD_ENABLED.store(false, Ordering::SeqCst); - SERVER_CLIPBOARD_ENABLED.store(false, Ordering::SeqCst); - SERVER_FILE_TRANSFER_ENABLED = false; - } + SERVER_KEYBOARD_ENABLED.store(false, Ordering::SeqCst); + SERVER_CLIPBOARD_ENABLED.store(false, Ordering::SeqCst); + SERVER_FILE_TRANSFER_ENABLED.store(false, Ordering::SeqCst); } fn handle_job_status(&mut self, id: i32, file_num: i32, err: Option) { @@ -2195,9 +2191,7 @@ impl Remote { .call2("setPermission", &make_args!("audio", p.enabled)); } Permission::File => { - unsafe { - SERVER_FILE_TRANSFER_ENABLED = p.enabled; - } + SERVER_FILE_TRANSFER_ENABLED.store(p.enabled, Ordering::SeqCst); if !p.enabled && self.handler.is_file_transfer() { return true; } @@ -2258,7 +2252,7 @@ impl Remote { fn check_clipboard_file_context(&mut self) { #[cfg(windows)] { - let enabled = unsafe { SERVER_FILE_TRANSFER_ENABLED } + let enabled = SERVER_FILE_TRANSFER_ENABLED.load(Ordering::SeqCst) && self.handler.lc.read().unwrap().enable_file_transfer; if enabled == self.clipboard_file_context.is_none() { self.clipboard_file_context = if enabled { From 230f74da2ec700b3183b0af4c611263d7579ac64 Mon Sep 17 00:00:00 2001 From: Saverio Miroddi Date: Tue, 24 May 2022 18:18:38 +0200 Subject: [PATCH 5/5] Remove IS_IN unsafe code, by using AtomicBool --- src/ui/remote.rs | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/src/ui/remote.rs b/src/ui/remote.rs index c535f6b52..80d85acd7 100644 --- a/src/ui/remote.rs +++ b/src/ui/remote.rs @@ -66,7 +66,7 @@ fn get_key_state(key: enigo::Key) -> bool { ENIGO.lock().unwrap().get_key_state(key) } -static mut IS_IN: bool = false; +static IS_IN: AtomicBool = AtomicBool::new(false); static KEYBOARD_HOOKED: AtomicBool = AtomicBool::new(false); static SERVER_KEYBOARD_ENABLED: AtomicBool = AtomicBool::new(true); static SERVER_FILE_TRANSFER_ENABLED: AtomicBool = AtomicBool::new(true); @@ -266,7 +266,7 @@ impl Handler { std::env::set_var("KEYBOARD_ONLY", "y"); // pass to rdev use rdev::{EventType::*, *}; let func = move |evt: Event| { - if unsafe { !IS_IN || !SERVER_KEYBOARD_ENABLED.load(Ordering::SeqCst) } { + if !IS_IN.load(Ordering::SeqCst) || !SERVER_KEYBOARD_ENABLED.load(Ordering::SeqCst) { return; } let (key, down) = match evt.event_type { @@ -865,17 +865,13 @@ impl Handler { fn enter(&mut self) { #[cfg(windows)] crate::platform::windows::stop_system_key_propagate(true); - unsafe { - IS_IN = true; - } + IS_IN.store(true, Ordering::SeqCst); } fn leave(&mut self) { #[cfg(windows)] crate::platform::windows::stop_system_key_propagate(false); - unsafe { - IS_IN = false; - } + IS_IN.store(false, Ordering::SeqCst); } fn send_mouse(