From 76c885771a4f580330dae305d9b06b9551f5b19d Mon Sep 17 00:00:00 2001 From: Smit Barmase Date: Thu, 22 Jan 2026 23:19:34 +0530 Subject: [PATCH] terminal: Give child processes time to exit on their own (#47408) Closes #44003 Supersedes https://github.com/zed-industries/zed/pull/45777 Co-authored-by: Lukas Release Notes: - Fixed an issue where the terminal would sometimes fail to write shell history. --------- Co-authored-by: Lukas Wirth --- crates/acp_thread/src/acp_thread.rs | 5 ++ crates/agent_servers/src/acp.rs | 1 + crates/terminal/src/terminal.rs | 74 ++++++++++++++----- .../src/terminal_path_like_target.rs | 12 ++- 4 files changed, 71 insertions(+), 21 deletions(-) diff --git a/crates/acp_thread/src/acp_thread.rs b/crates/acp_thread/src/acp_thread.rs index 545f21b922cff308844025aa1f9e5ada82c73728..268d43477e617716bcd68c062b07aa7c11b0e95d 100644 --- a/crates/acp_thread/src/acp_thread.rs +++ b/crates/acp_thread/src/acp_thread.rs @@ -2633,6 +2633,7 @@ mod tests { ::terminal::terminal_settings::AlternateScroll::On, None, 0, + cx.background_executor(), ) .unwrap(); builder.subscribe(cx) @@ -2706,6 +2707,7 @@ mod tests { ::terminal::terminal_settings::AlternateScroll::On, None, 0, + cx.background_executor(), ) .unwrap(); builder.subscribe(cx) @@ -4080,6 +4082,7 @@ mod tests { ::terminal::terminal_settings::AlternateScroll::On, None, 0, + cx.background_executor(), ) .unwrap(); builder.subscribe(cx) @@ -4125,6 +4128,7 @@ mod tests { ::terminal::terminal_settings::AlternateScroll::On, None, 0, + cx.background_executor(), ) .unwrap(); builder.subscribe(cx) @@ -4184,6 +4188,7 @@ mod tests { ::terminal::terminal_settings::AlternateScroll::On, None, 0, + cx.background_executor(), ) .unwrap(); builder.subscribe(cx) diff --git a/crates/agent_servers/src/acp.rs b/crates/agent_servers/src/acp.rs index cf589b983a1bc5fd87bf4fc740f700b08b5da433..872922741f976d010fb997d3651ea4143a658906 100644 --- a/crates/agent_servers/src/acp.rs +++ b/crates/agent_servers/src/acp.rs @@ -1196,6 +1196,7 @@ impl acp::Client for ClientDelegate { AlternateScroll::On, None, 0, + cx.background_executor(), )?; let lower = cx.new(|cx| builder.subscribe(cx)); thread.on_terminal_provider_event( diff --git a/crates/terminal/src/terminal.rs b/crates/terminal/src/terminal.rs index 0dcbfecb5af682698d7a3af2824e9fb6f8cc0811..f9d727a72dbf1d045e1bd5f8a07c1ffa1755d98b 100644 --- a/crates/terminal/src/terminal.rs +++ b/crates/terminal/src/terminal.rs @@ -60,14 +60,14 @@ use std::{ path::PathBuf, process::ExitStatus, sync::Arc, - time::Instant, + time::{Duration, Instant}, }; use thiserror::Error; use gpui::{ - App, AppContext as _, Bounds, ClipboardItem, Context, EventEmitter, Hsla, Keystroke, Modifiers, - MouseButton, MouseDownEvent, MouseMoveEvent, MouseUpEvent, Pixels, Point, Rgba, - ScrollWheelEvent, Size, Task, TouchPhase, Window, actions, black, px, + App, AppContext as _, BackgroundExecutor, Bounds, ClipboardItem, Context, EventEmitter, Hsla, + Keystroke, Modifiers, MouseButton, MouseDownEvent, MouseMoveEvent, MouseUpEvent, Pixels, Point, + Rgba, ScrollWheelEvent, Size, Task, TouchPhase, Window, actions, black, px, }; use crate::mappings::{colors::to_alac_rgb, keys::to_esc_str}; @@ -346,6 +346,7 @@ impl TerminalBuilder { alternate_scroll: AlternateScroll, max_scroll_history_lines: Option, window_id: u64, + background_executor: &BackgroundExecutor, ) -> Result { // Create a display-only terminal (no actual PTY). let default_cursor_style = AlacCursorStyle::from(cursor_shape); @@ -409,6 +410,7 @@ impl TerminalBuilder { }, child_exited: None, event_loop_task: Task::ready(Ok(())), + background_executor: background_executor.clone(), }; Ok(TerminalBuilder { @@ -434,6 +436,7 @@ impl TerminalBuilder { activation_script: Vec, ) -> Task> { let version = release_channel::AppVersion::global(cx); + let background_executor = cx.background_executor().clone(); let fut = async move { // Remove SHLVL so the spawned shell initializes it to 1, matching // the behavior of standalone terminal emulators like iTerm2/Kitty/Alacritty. @@ -636,6 +639,7 @@ impl TerminalBuilder { }, child_exited: None, event_loop_task: Task::ready(Ok(())), + background_executor, }; if !activation_script.is_empty() && no_task { @@ -858,6 +862,7 @@ pub struct Terminal { activation_script: Vec, child_exited: Option, event_loop_task: Task>, + background_executor: BackgroundExecutor, } struct CopyTemplate { @@ -2353,9 +2358,18 @@ unsafe fn append_text_to_term(term: &mut Term, text_lines: &[&str]) impl Drop for Terminal { fn drop(&mut self) { - if let TerminalType::Pty { pty_tx, info } = &mut self.terminal_type { - info.kill_child_process(); + if let TerminalType::Pty { pty_tx, mut info } = + std::mem::replace(&mut self.terminal_type, TerminalType::DisplayOnly) + { pty_tx.0.send(Msg::Shutdown).ok(); + + let timer = self.background_executor.timer(Duration::from_millis(100)); + self.background_executor + .spawn(async move { + timer.await; + info.kill_child_process(); + }) + .detach(); } } } @@ -2545,9 +2559,15 @@ mod tests { }); let terminal = cx.new(|cx| { - TerminalBuilder::new_display_only(CursorShape::default(), AlternateScroll::On, None, 0) - .unwrap() - .subscribe(cx) + TerminalBuilder::new_display_only( + CursorShape::default(), + AlternateScroll::On, + None, + 0, + cx.background_executor(), + ) + .unwrap() + .subscribe(cx) }); terminal.update(cx, |terminal, cx| { @@ -2921,9 +2941,15 @@ mod tests { #[gpui::test] async fn test_write_output_converts_lf_to_crlf(cx: &mut TestAppContext) { let terminal = cx.new(|cx| { - TerminalBuilder::new_display_only(CursorShape::default(), AlternateScroll::On, None, 0) - .unwrap() - .subscribe(cx) + TerminalBuilder::new_display_only( + CursorShape::default(), + AlternateScroll::On, + None, + 0, + cx.background_executor(), + ) + .unwrap() + .subscribe(cx) }); // Test simple LF conversion @@ -2962,9 +2988,15 @@ mod tests { #[gpui::test] async fn test_write_output_preserves_existing_crlf(cx: &mut TestAppContext) { let terminal = cx.new(|cx| { - TerminalBuilder::new_display_only(CursorShape::default(), AlternateScroll::On, None, 0) - .unwrap() - .subscribe(cx) + TerminalBuilder::new_display_only( + CursorShape::default(), + AlternateScroll::On, + None, + 0, + cx.background_executor(), + ) + .unwrap() + .subscribe(cx) }); // Test that existing CRLF doesn't get doubled @@ -2997,9 +3029,15 @@ mod tests { #[gpui::test] async fn test_write_output_preserves_bare_cr(cx: &mut TestAppContext) { let terminal = cx.new(|cx| { - TerminalBuilder::new_display_only(CursorShape::default(), AlternateScroll::On, None, 0) - .unwrap() - .subscribe(cx) + TerminalBuilder::new_display_only( + CursorShape::default(), + AlternateScroll::On, + None, + 0, + cx.background_executor(), + ) + .unwrap() + .subscribe(cx) }); // Test that bare CR (without LF) is preserved diff --git a/crates/terminal_view/src/terminal_path_like_target.rs b/crates/terminal_view/src/terminal_path_like_target.rs index bdf72f9c72e4863497279141d6401c473cdb6d68..fb99457eb498d65820f942f98543dfcf5ab1e597 100644 --- a/crates/terminal_view/src/terminal_path_like_target.rs +++ b/crates/terminal_view/src/terminal_path_like_target.rs @@ -556,9 +556,15 @@ mod tests { app_cx.add_window_view(|window, cx| Workspace::test_new(project.clone(), window, cx)); let terminal = app_cx.new(|cx| { - TerminalBuilder::new_display_only(CursorShape::default(), AlternateScroll::On, None, 0) - .expect("Failed to create display-only terminal") - .subscribe(cx) + TerminalBuilder::new_display_only( + CursorShape::default(), + AlternateScroll::On, + None, + 0, + cx.background_executor(), + ) + .expect("Failed to create display-only terminal") + .subscribe(cx) }); let workspace_a = workspace.clone();