terminal: Give child processes time to exit on their own (#47408)

Smit Barmase and Lukas Wirth created 

Closes #44003

Supersedes https://github.com/zed-industries/zed/pull/45777

Co-authored-by: Lukas <lukas@zed.dev>

Release Notes:

- Fixed an issue where the terminal would sometimes fail to write shell
history.

---------

Co-authored-by: Lukas Wirth <me@lukaswirth.dev>

Change summary

crates/acp_thread/src/acp_thread.rs                   |  5 
crates/agent_servers/src/acp.rs                       |  1 
crates/terminal/src/terminal.rs                       | 74 +++++++++---
crates/terminal_view/src/terminal_path_like_target.rs | 12 +
4 files changed, 71 insertions(+), 21 deletions(-)

Detailed changes

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)

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(

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<usize>,
         window_id: u64,
+        background_executor: &BackgroundExecutor,
     ) -> Result<TerminalBuilder> {
         // 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<String>,
     ) -> Task<Result<TerminalBuilder>> {
         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<String>,
     child_exited: Option<ExitStatus>,
     event_loop_task: Task<Result<(), anyhow::Error>>,
+    background_executor: BackgroundExecutor,
 }
 
 struct CopyTemplate {
@@ -2353,9 +2358,18 @@ unsafe fn append_text_to_term(term: &mut Term<ZedListener>, 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

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();