Cargo.lock 🔗
@@ -15835,7 +15835,6 @@ dependencies = [
"indoc",
"itertools 0.14.0",
"language",
- "libc",
"log",
"lsp",
"multi_buffer",
Fix zed sometimes stopping by using setsid on interactive shells (#29070)
Michael Sloan created
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.Change summary
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(-)
Detailed changes
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",
);
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<String> = None;
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 }
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| {