From 8c550634179bb1e22a5cc18ebc9c4c4219b51da5 Mon Sep 17 00:00:00 2001 From: Michael Sloan Date: Fri, 18 Apr 2025 15:04:26 -0600 Subject: [PATCH] Fix zed sometimes stopping by using setsid on interactive shells (#29070) For some reason `SIGTTIN` sometimes gets sent to the process group, causing it to stop when run from a terminal. This solves that issue by putting the shell in a new session + progress group. This allows removal of a workaround of using `exit 0;` to restore handling of ctrl-c after exit. In testing this appears to no longer be necessary. Closes #27716 Release Notes: - Fixed Zed sometimes becoming a stopped background process when run from a terminal. --- Cargo.lock | 1 - crates/project/src/environment.rs | 34 +++++++++++++--------------- crates/util/src/util.rs | 37 ++++++++++++++++++++++--------- crates/vim/Cargo.toml | 1 - crates/vim/src/command.rs | 13 +---------- 5 files changed, 42 insertions(+), 44 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 9ce96bae46..1b872d02fa 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -15835,7 +15835,6 @@ dependencies = [ "indoc", "itertools 0.14.0", "language", - "libc", "log", "lsp", "multi_buffer", diff --git a/crates/project/src/environment.rs b/crates/project/src/environment.rs index f79a39d8cc..dfb1114a37 100644 --- a/crates/project/src/environment.rs +++ b/crates/project/src/environment.rs @@ -273,23 +273,14 @@ async fn load_shell_environment( // // In certain shells we need to execute additional_command in order to // trigger the behavior of direnv, etc. - // - // - // The `exit 0` is the result of hours of debugging, trying to find out - // why running this command here, without `exit 0`, would mess - // up signal process for our process so that `ctrl-c` doesn't work - // anymore. - // - // We still don't know why `$SHELL -l -i -c '/usr/bin/env -0'` would - // do that, but it does, and `exit 0` helps. let command = match shell_name { Some("fish") => format!( - "cd '{}'; emit fish_prompt; printf '%s' {MARKER}; /usr/bin/env; exit 0;", + "cd '{}'; emit fish_prompt; printf '%s' {MARKER}; /usr/bin/env;", dir.display() ), _ => format!( - "cd '{}'; printf '%s' {MARKER}; /usr/bin/env; exit 0;", + "cd '{}'; printf '%s' {MARKER}; /usr/bin/env;", dir.display() ), }; @@ -297,16 +288,21 @@ async fn load_shell_environment( // csh/tcsh only supports `-l` if it's the only flag. So this won't be a login shell. // Users must rely on vars from `~/.tcshrc` or `~/.cshrc` and not `.login` as a result. let args = match shell_name { - Some("tcsh") | Some("csh") => vec!["-i", "-c", &command], - _ => vec!["-l", "-i", "-c", &command], + Some("tcsh") | Some("csh") => vec!["-i".to_string(), "-c".to_string(), command], + _ => vec![ + "-l".to_string(), + "-i".to_string(), + "-c".to_string(), + command, + ], }; - let Some(output) = smol::process::Command::new(&shell) - .args(&args) - .output() - .await - .log_err() - else { + let Some(output) = smol::unblock(move || { + util::set_pre_exec_to_start_new_session(std::process::Command::new(&shell).args(&args)) + .output() + }) + .await + .log_err() else { return message( "Failed to spawn login shell to source login environment variables. See logs for details", ); diff --git a/crates/util/src/util.rs b/crates/util/src/util.rs index b4608318e6..e19e8ddc84 100644 --- a/crates/util/src/util.rs +++ b/crates/util/src/util.rs @@ -321,21 +321,16 @@ pub fn load_login_shell_environment() -> Result<()> { .and_then(|home| home.into_string().ok()) .map(|home| format!("cd '{home}';")); - // The `exit 0` is the result of hours of debugging, trying to find out - // why running this command here, without `exit 0`, would mess - // up signal process for our process so that `ctrl-c` doesn't work - // anymore. - // We still don't know why `$SHELL -l -i -c '/usr/bin/env -0'` would - // do that, but it does, and `exit 0` helps. let shell_cmd = format!( - "{}printf '%s' {marker}; /usr/bin/env; exit 0;", + "{}printf '%s' {marker}; /usr/bin/env;", shell_cmd_prefix.as_deref().unwrap_or("") ); - let output = std::process::Command::new(&shell) - .args(["-l", "-i", "-c", &shell_cmd]) - .output() - .context("failed to spawn login shell to source login environment variables")?; + let output = set_pre_exec_to_start_new_session( + std::process::Command::new(&shell).args(["-l", "-i", "-c", &shell_cmd]), + ) + .output() + .context("failed to spawn login shell to source login environment variables")?; if !output.status.success() { Err(anyhow!("login shell exited with error"))?; } @@ -357,6 +352,26 @@ pub fn load_login_shell_environment() -> Result<()> { Ok(()) } +/// Configures the process to start a new session, to prevent interactive shells from taking control +/// of the terminal. +/// +/// For more details: https://registerspill.thorstenball.com/p/how-to-lose-control-of-your-shell +pub fn set_pre_exec_to_start_new_session( + command: &mut std::process::Command, +) -> &mut std::process::Command { + // safety: code in pre_exec should be signal safe. + // https://man7.org/linux/man-pages/man7/signal-safety.7.html + #[cfg(not(target_os = "windows"))] + unsafe { + use std::os::unix::process::CommandExt; + command.pre_exec(|| { + libc::setsid(); + Ok(()) + }); + }; + command +} + /// Parse the result of calling `usr/bin/env` with no arguments pub fn parse_env_output(env: &str, mut f: impl FnMut(String, String)) { let mut current_key: Option = None; diff --git a/crates/vim/Cargo.toml b/crates/vim/Cargo.toml index 00e44c1186..2f4ccccaf8 100644 --- a/crates/vim/Cargo.toml +++ b/crates/vim/Cargo.toml @@ -28,7 +28,6 @@ futures.workspace = true gpui.workspace = true itertools.workspace = true language.workspace = true -libc.workspace = true log.workspace = true multi_buffer.workspace = true nvim-rs = { git = "https://github.com/KillTheMule/nvim-rs", branch = "master", features = ["use_tokio"], optional = true } diff --git a/crates/vim/src/command.rs b/crates/vim/src/command.rs index 264edca0b1..881f3dbb09 100644 --- a/crates/vim/src/command.rs +++ b/crates/vim/src/command.rs @@ -1519,18 +1519,7 @@ impl ShellExec { process.stdin(Stdio::null()); }; - // https://registerspill.thorstenball.com/p/how-to-lose-control-of-your-shell - // - // safety: code in pre_exec should be signal safe. - // https://man7.org/linux/man-pages/man7/signal-safety.7.html - #[cfg(not(target_os = "windows"))] - unsafe { - use std::os::unix::process::CommandExt; - process.pre_exec(|| { - libc::setsid(); - Ok(()) - }); - }; + util::set_pre_exec_to_start_new_session(&mut process); let is_read = self.is_read; let task = cx.spawn_in(window, async move |vim, cx| {