From 3b7e2cebd9303a43eecb9bc96921548df846f45d Mon Sep 17 00:00:00 2001 From: fufesou Date: Sat, 31 Oct 2026 19:59:06 +0800 Subject: [PATCH] fix: restore readiness after service restart and macOS update (#16780) * fix: restore readiness after service restart and macOS update Wake rendezvous retry waits when a manual restart is requested. For app-only macOS updates with an in-process backend, wait for the old process to exit before relaunching the updated app so it cannot attach to the departing backend. Signed-off-by: fufesou * test: cover service restart and macOS update readiness Signed-off-by: fufesou * test: remove macOS relaunch script regression test Signed-off-by: fufesou * docs: explain macOS update relaunch ordering Signed-off-by: fufesou * fix: isolate macOS update relaunch process group Signed-off-by: fufesou * docs: clarify macOS relaunch process group isolation Signed-off-by: fufesou * fix: clarify macOS update restart failure Signed-off-by: fufesou --------- Signed-off-by: fufesou --- src/platform/macos.rs | 43 ++++++++++++++++++++++++++++++++++++++ src/rendezvous_mediator.rs | 41 ++++++++++++++++++++++++++++++++---- 2 files changed, 80 insertions(+), 4 deletions(-) diff --git a/src/platform/macos.rs b/src/platform/macos.rs index ddc190ccc..d321d12f5 100644 --- a/src/platform/macos.rs +++ b/src/platform/macos.rs @@ -1015,6 +1015,11 @@ fn update_me_from_source(update_source: UpdateSource) -> ResultType<()> { } Ok(output) => log_update_cleanup_warning(&output), } + if crate::common::is_server_running() { + // This process hosts the backend. A new GUI could reuse our IPC and + // skip starting its own backend, leaving it "Not ready" when we exit. + return relaunch_after_update_exit(&app_name); + } } match Command::new("open") .arg("-n") @@ -1034,6 +1039,44 @@ fn update_me_from_source(update_source: UpdateSource) -> ResultType<()> { Ok(()) } +fn relaunch_after_update_exit(app_name: &str) -> ResultType<()> { + // The update caller cleans up and quits this GUI after we return. Waiting + // in a separate process lets that caller finish instead of blocking its exit. + // The shell's kill -0 only checks whether our PID exists; it does not kill us. + const RELAUNCH: &str = r#" +poll_interval=0.1 +while /bin/kill -0 "$1" 2>/dev/null; do + /bin/sleep "$poll_interval" +done +if ! output=$(/usr/bin/open -n "$2" 2>&1); then + /usr/bin/logger -t "$0" "Failed to relaunch updated app: $output" + exit 1 +fi +"#; + Command::new("/bin/sh") + .args(["-c", RELAUNCH, app_name]) + .arg(std::process::id().to_string()) + .arg(format!("/Applications/{}.app", app_name)) + .stdin(Stdio::null()) + .stdout(Stdio::null()) + .stderr(Stdio::null()) + // Children normally inherit their parent's process group, allowing a + // supervisor to signal the whole group. If launchd cleans up the old + // app's group on exit, it could kill this helper before it runs `open`. + // Passing 0 creates a new group whose ID is the child's own PID, so + // cleanup of the old group does not affect the helper or its children. + // It remains our child; ordinary parent exit alone does not kill it. + .process_group(0) + .spawn() + .map_err(|err| { + anyhow!( + "The new version is installed, but automatic restart failed. Please quit and reopen the application. Error: {}", + err + ) + })?; + Ok(()) +} + pub fn update_from_dmg(dmg_path: &str) -> ResultType<()> { let update_temp_dir = get_update_temp_dir_string(); println!("Starting update from DMG: {}", dmg_path); diff --git a/src/rendezvous_mediator.rs b/src/rendezvous_mediator.rs index 042d62b6c..b1e523da3 100644 --- a/src/rendezvous_mediator.rs +++ b/src/rendezvous_mediator.rs @@ -23,7 +23,7 @@ use hbb_common::{ socket_client::{self, connect_tcp, is_ipv4, new_direct_udp_for, new_udp_for}, tokio::{ self, select, - sync::{mpsc, Mutex}, + sync::{mpsc, Mutex, Notify}, time::interval, }, udp::FramedSocket, @@ -208,6 +208,7 @@ impl IceRoute { static SHOULD_EXIT: AtomicBool = AtomicBool::new(false); static MANUAL_RESTARTED: AtomicBool = AtomicBool::new(false); +static MANUAL_RESTART_NOTIFY: Notify = Notify::const_new(); static SENT_REGISTER_PK: AtomicBool = AtomicBool::new(false); pub(crate) static NEEDS_DEPLOY: AtomicBool = AtomicBool::new(false); #[cfg(target_os = "android")] @@ -256,6 +257,13 @@ pub(crate) fn reset_needs_deploy_notification() { NOTIFIED_NEEDS_DEPLOY.store(false, Ordering::SeqCst); } +async fn wait_for_retry(delay: f32, restarted: tokio::sync::futures::Notified<'_>) { + select! { + _ = sleep(delay) => {} + _ = restarted => {} + } +} + #[derive(Clone)] pub struct RendezvousMediator { addr: TargetAddr<'static>, @@ -268,6 +276,7 @@ impl RendezvousMediator { pub fn restart() { SHOULD_EXIT.store(true, Ordering::SeqCst); MANUAL_RESTARTED.store(true, Ordering::SeqCst); + MANUAL_RESTART_NOTIFY.notify_waiters(); log::info!("server restart"); } @@ -339,10 +348,12 @@ impl RendezvousMediator { } Config::reset_online(); let timeout = *timeout.read().unwrap(); + // Subscribe before checking the flag so a restart cannot miss the wait. + let restarted = MANUAL_RESTART_NOTIFY.notified(); if !MANUAL_RESTARTED.load(Ordering::SeqCst) { let elapsed = conn_start_time.elapsed().as_millis() as u64; if elapsed < timeout { - sleep(((timeout - elapsed) / 1000) as _).await; + wait_for_retry(((timeout - elapsed) / 1000) as _, restarted).await; } } else { // https://github.com/rustdesk/rustdesk/issues/12233 @@ -1719,11 +1730,33 @@ impl Drop for CheckIfResendPk { #[cfg(test)] mod tests { use super::{ - connection_meta, mpsc, tokio, AnswererSlot, IceRoute, IntoTargetAddr, PunchHoleSent, - RendezvousMediator, RendezvousMessage, MAX_WEBRTC_ANSWERERS, TCP_PUNCHES, UDP_PUNCHES, + connection_meta, mpsc, tokio, wait_for_retry, AnswererSlot, Duration, IceRoute, + IntoTargetAddr, Ordering, PunchHoleSent, RendezvousMediator, RendezvousMessage, + CONNECT_TIMEOUT, MANUAL_RESTARTED, MANUAL_RESTART_NOTIFY, MAX_WEBRTC_ANSWERERS, + SHOULD_EXIT, TCP_PUNCHES, UDP_PUNCHES, }; use hbb_common::protobuf::Message as _; + #[tokio::test] + async fn manual_restart_interrupts_retry_wait() { + const WAKE_TIMEOUT: Duration = Duration::from_secs(1); + let waiting = wait_for_retry( + (CONNECT_TIMEOUT / 1000) as _, + MANUAL_RESTART_NOTIFY.notified(), + ); + tokio::pin!(waiting); + assert!(hbb_common::futures::poll!(waiting.as_mut()).is_pending()); + + RendezvousMediator::restart(); + let result = tokio::time::timeout(WAKE_TIMEOUT, waiting).await; + MANUAL_RESTARTED.store(false, Ordering::SeqCst); + SHOULD_EXIT.store(false, Ordering::SeqCst); + assert!( + result.is_ok(), + "restart must interrupt the pending retry delay" + ); + } + // The places are the bound and they come back on drop: a punch past the limit is declined // while the ones in flight are left alone, and the next one in is admitted only once a // place has actually been given back.