From b3aa7055e48f6cf15bdc3c093234e889a046b516 Mon Sep 17 00:00:00 2001 From: Thorsten Ball Date: Tue, 22 Oct 2024 16:36:40 +0200 Subject: [PATCH] ssh remoting: Daemonize server process properly by forking and closing stdio (#19544) This fixes the `ssh` proxy process not being notified when the proxy process dies. Turns out that the server would have stdout/stderr/stdin connected to the grand-parent ssh process connected to it and as long as the server kept running (even once it was daemonized into the background) the grand-parent ssh process wouldn't exit. That in turn meant that the Zed client wasn't notified when the proxy process died. Release Notes: - N/A --------- Co-authored-by: Bennet --- Cargo.lock | 2 + crates/remote_server/Cargo.toml | 4 ++ crates/remote_server/src/unix.rs | 69 +++++++++++++++++++++++++++----- 3 files changed, 64 insertions(+), 11 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index fbd6b84f774904fe80d5a0350a26a02f150b4341..5a51a0a330135c34af6f2948935c5b5da8e3ce80 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -9159,6 +9159,7 @@ dependencies = [ "client", "clock", "env_logger", + "fork", "fs", "futures 0.3.30", "git", @@ -9167,6 +9168,7 @@ dependencies = [ "http_client", "language", "languages", + "libc", "log", "lsp", "node_runtime", diff --git a/crates/remote_server/Cargo.toml b/crates/remote_server/Cargo.toml index ffec42e5706d9acb054da72700040680e257c1e6..d2623d5f472d647a8d6b3e21bcf0b2a6ffd8456c 100644 --- a/crates/remote_server/Cargo.toml +++ b/crates/remote_server/Cargo.toml @@ -53,6 +53,10 @@ smol.workspace = true util.workspace = true worktree.workspace = true +[target.'cfg(not(windows))'.dependencies] +fork.workspace = true +libc.workspace = true + [dev-dependencies] client = { workspace = true, features = ["test-support"] } clock = { workspace = true, features = ["test-support"] } diff --git a/crates/remote_server/src/unix.rs b/crates/remote_server/src/unix.rs index abfdb798f423764bb2ecfe587fd9e7fae68c1975..df1f90b0fcbb7d0abeeca382d393167209bcb6cb 100644 --- a/crates/remote_server/src/unix.rs +++ b/crates/remote_server/src/unix.rs @@ -27,6 +27,7 @@ use smol::io::AsyncReadExt; use smol::Async; use smol::{net::unix::UnixListener, stream::StreamExt as _}; +use std::ops::ControlFlow; use std::{ io::Write, mem, @@ -305,10 +306,15 @@ pub fn execute_run( stdout_socket: PathBuf, stderr_socket: PathBuf, ) -> Result<()> { - let log_rx = init_logging_server(log_file)?; - init_panic_hook(); init_paths()?; + match daemonize()? { + ControlFlow::Break(_) => return Ok(()), + ControlFlow::Continue(_) => {} + } + + init_panic_hook(); + let log_rx = init_logging_server(log_file)?; log::info!( "starting up. pid_file: {:?}, stdin_socket: {:?}, stdout_socket: {:?}, stderr_socket: {:?}", pid_file, @@ -322,8 +328,6 @@ pub fn execute_run( let listeners = ServerListeners::new(stdin_socket, stdout_socket, stderr_socket)?; - log::info!("starting headless gpui app"); - let git_hosting_provider_registry = Arc::new(GitHostingProviderRegistry::new()); gpui::App::headless().run(move |cx| { settings::init(cx); @@ -523,7 +527,8 @@ fn spawn_server(paths: &ServerPaths) -> Result<()> { } let binary_name = std::env::current_exe()?; - let server_process = smol::process::Command::new(binary_name) + let mut server_process = std::process::Command::new(binary_name); + server_process .arg("run") .arg("--log-file") .arg(&paths.log_file) @@ -534,12 +539,14 @@ fn spawn_server(paths: &ServerPaths) -> Result<()> { .arg("--stdout-socket") .arg(&paths.stdout_socket) .arg("--stderr-socket") - .arg(&paths.stderr_socket) - .spawn()?; - - log::info!( - "proxy spawned server process. PID: {:?}", - server_process.id() + .arg(&paths.stderr_socket); + + let status = server_process + .status() + .context("failed to launch server process")?; + anyhow::ensure!( + status.success(), + "failed to launch and detach server process" ); let mut total_time_waited = std::time::Duration::from_secs(0); @@ -745,3 +752,43 @@ fn read_proxy_settings(cx: &mut ModelContext<'_, HeadlessProject>) -> Option Result> { + match fork::fork().map_err(|e| anyhow::anyhow!("failed to call fork with error code {}", e))? { + fork::Fork::Parent(_) => { + return Ok(ControlFlow::Break(())); + } + fork::Fork::Child => {} + } + + // Once we've detached from the parent, we want to close stdout/stderr/stdin + // so that the outer SSH process is not attached to us in any way anymore. + unsafe { redirect_standard_streams() }?; + + Ok(ControlFlow::Continue(())) +} + +unsafe fn redirect_standard_streams() -> Result<()> { + let devnull_fd = libc::open(b"/dev/null\0" as *const [u8; 10] as _, libc::O_RDWR); + anyhow::ensure!(devnull_fd != -1, "failed to open /dev/null"); + + let process_stdio = |name, fd| { + let reopened_fd = libc::dup2(devnull_fd, fd); + anyhow::ensure!( + reopened_fd != -1, + format!("failed to redirect {} to /dev/null", name) + ); + Ok(()) + }; + + process_stdio("stdin", libc::STDIN_FILENO)?; + process_stdio("stdout", libc::STDOUT_FILENO)?; + process_stdio("stderr", libc::STDERR_FILENO)?; + + anyhow::ensure!( + libc::close(devnull_fd) != -1, + "failed to close /dev/null fd after redirecting" + ); + + Ok(()) +}