Fix tasks not being stopped on reruns (#29786)

Follow-up of https://github.com/zed-industries/zed/pull/28993

* Tone down tasks' cancellation logging
* Fix task terminals' leak, disallowing to fully cancel the task by
dropping the terminal off the pane:

f619d5f02a/crates/terminal_view/src/terminal_panel.rs (L1464-L1471)

Release Notes:

- Fixed tasks not being stopped on reruns

crates/debugger_ui/src/debugger_panel.rs

@@ -6,7 +6,7 @@ use crate::{
     persistence,
 };
 use crate::{new_session_modal::NewSessionModal, session::DebugSession};
-use anyhow::{Result, anyhow};
+use anyhow::{Context as _, Result, anyhow};
 use collections::{HashMap, HashSet};
 use command_palette_hooks::CommandPaletteFilter;
 use dap::DebugRequest;
@@ -500,7 +500,7 @@ impl DebugPanel {
                     workspace.spawn_in_terminal(task.resolved.clone(), window, cx)
                 })?;
 
-                let exit_status = run_build.await?;
+                let exit_status = run_build.await.transpose()?.context("task cancelled")?;
                 if !exit_status.success() {
                     anyhow::bail!("Build failed");
                 }

crates/terminal/src/terminal.rs

@@ -46,7 +46,7 @@ use smol::channel::{Receiver, Sender};
 use task::{HideStrategy, Shell, TaskId};
 use terminal_settings::{AlternateScroll, CursorShape, TerminalSettings};
 use theme::{ActiveTheme, Theme};
-use util::{ResultExt, paths::home_dir, truncate_and_trailoff};
+use util::{paths::home_dir, truncate_and_trailoff};
 
 use std::{
     cmp::{self, min},
@@ -1851,8 +1851,7 @@ impl Terminal {
         if let Some(task) = self.task() {
             if task.status == TaskStatus::Running {
                 let completion_receiver = task.completion_rx.clone();
-                return cx
+                return cx.spawn(async move |_| completion_receiver.recv().await.ok().flatten());
-                    .spawn(async move |_| completion_receiver.recv().await.log_err().flatten());
             } else if let Ok(status) = task.completion_rx.try_recv() {
                 return Task::ready(status);
             }

crates/terminal_view/src/terminal_panel.rs

@@ -483,7 +483,7 @@ impl TerminalPanel {
         task: &SpawnInTerminal,
         window: &mut Window,
         cx: &mut Context<Self>,
-    ) -> Task<Result<Entity<Terminal>>> {
+    ) -> Task<Result<WeakEntity<Terminal>>> {
         let Ok(is_local) = self
             .workspace
             .update(cx, |workspace, cx| workspace.project().read(cx).is_local())
@@ -552,12 +552,12 @@ impl TerminalPanel {
         cx.spawn(async move |_, _| rx.await?)
     }
 
-    pub fn spawn_in_new_terminal(
+    fn spawn_in_new_terminal(
         &mut self,
         spawn_task: SpawnInTerminal,
         window: &mut Window,
         cx: &mut Context<Self>,
-    ) -> Task<Result<Entity<Terminal>>> {
+    ) -> Task<Result<WeakEntity<Terminal>>> {
         let reveal = spawn_task.reveal;
         let reveal_target = spawn_task.reveal_target;
         let kind = TerminalKind::Task(spawn_task);
@@ -652,7 +652,7 @@ impl TerminalPanel {
         kind: TerminalKind,
         window: &mut Window,
         cx: &mut Context<Workspace>,
-    ) -> Task<Result<Entity<Terminal>>> {
+    ) -> Task<Result<WeakEntity<Terminal>>> {
         if !is_enabled_in_workspace(workspace, cx) {
             return Task::ready(Err(anyhow!(
                 "terminal not yet supported for remote projects"
@@ -680,7 +680,7 @@ impl TerminalPanel {
                 });
                 workspace.add_item_to_active_pane(Box::new(terminal_view), None, true, window, cx);
             })?;
-            Ok(terminal)
+            Ok(terminal.downgrade())
         })
     }
 
@@ -690,7 +690,7 @@ impl TerminalPanel {
         reveal_strategy: RevealStrategy,
         window: &mut Window,
         cx: &mut Context<Self>,
-    ) -> Task<Result<Entity<Terminal>>> {
+    ) -> Task<Result<WeakEntity<Terminal>>> {
         let workspace = self.workspace.clone();
         cx.spawn_in(window, async move |terminal_panel, cx| {
             if workspace.update(cx, |workspace, cx| !is_enabled_in_workspace(workspace, cx))? {
@@ -735,11 +735,12 @@ impl TerminalPanel {
                     pane.add_item(terminal_view, true, focus, None, window, cx);
                 });
 
-                Ok(terminal)
+                Ok(terminal.downgrade())
             })?;
-            terminal_panel.update(cx, |this, cx| {
+            terminal_panel.update(cx, |terminal_panel, cx| {
-                this.pending_terminals_to_add = this.pending_terminals_to_add.saturating_sub(1);
+                terminal_panel.pending_terminals_to_add =
-                this.serialize(cx)
+                    terminal_panel.pending_terminals_to_add.saturating_sub(1);
+                terminal_panel.serialize(cx)
             })?;
             result
         })
@@ -802,7 +803,7 @@ impl TerminalPanel {
         terminal_to_replace: Entity<TerminalView>,
         window: &mut Window,
         cx: &mut Context<Self>,
-    ) -> Task<Result<Entity<Terminal>>> {
+    ) -> Task<Result<WeakEntity<Terminal>>> {
         let reveal = spawn_task.reveal;
         let reveal_target = spawn_task.reveal_target;
         let window_handle = window.window_handle();
@@ -884,7 +885,7 @@ impl TerminalPanel {
                 RevealStrategy::Never => {}
             }
 
-            Ok(new_terminal)
+            Ok(new_terminal.downgrade())
         })
     }
 
@@ -1458,22 +1459,25 @@ impl workspace::TerminalProvider for TerminalProvider {
         task: SpawnInTerminal,
         window: &mut Window,
         cx: &mut App,
-    ) -> Task<Result<ExitStatus>> {
+    ) -> Task<Option<Result<ExitStatus>>> {
-        let this = self.0.clone();
+        let terminal_panel = self.0.clone();
         window.spawn(cx, async move |cx| {
-            let terminal = this
+            let terminal = terminal_panel
                 .update_in(cx, |terminal_panel, window, cx| {
                     terminal_panel.spawn_task(&task, window, cx)
-                })?
+                })
-                .await?;
+                .ok()?
-            let Some(exit_code) = terminal
+                .await;
-                .read_with(cx, |terminal, cx| terminal.wait_for_completed_task(cx))?
+            match terminal {
-                .await
+                Ok(terminal) => {
-            else {
+                    let exit_status = terminal
-                return Err(anyhow!("Task cancelled"));
+                        .read_with(cx, |terminal, cx| terminal.wait_for_completed_task(cx))
-            };
+                        .ok()?
-
+                        .await?;
-            Ok(exit_code)
+                    Some(Ok(exit_status))
+                }
+                Err(e) => Some(Err(e)),
+            }
         })
     }
 }

crates/vim/src/command.rs

@@ -1466,9 +1466,22 @@ impl ShellExec {
                     show_command: false,
                     show_rerun: false,
                 };
-                workspace
+
-                    .spawn_in_terminal(spawn_in_terminal, window, cx)
+                let task_status = workspace.spawn_in_terminal(spawn_in_terminal, window, cx);
-                    .detach_and_log_err(cx);
+                cx.background_spawn(async move {
+                    match task_status.await {
+                        Some(Ok(status)) => {
+                            if status.success() {
+                                log::debug!("Vim shell exec succeeded");
+                            } else {
+                                log::debug!("Vim shell exec failed, code: {:?}", status.code());
+                            }
+                        }
+                        Some(Err(e)) => log::error!("Vim shell exec failed: {e}"),
+                        None => log::debug!("Vim shell exec got cancelled"),
+                    }
+                })
+                .detach();
             });
             return;
         };

crates/workspace/src/tasks.rs

@@ -1,7 +1,7 @@
 use std::process::ExitStatus;
 
-use anyhow::{Result, anyhow};
+use anyhow::Result;
-use gpui::{Context, Entity, Task};
+use gpui::{AppContext, Context, Entity, Task};
 use language::Buffer;
 use project::TaskSourceKind;
 use remote::ConnectionState;
@@ -68,9 +68,21 @@ impl Workspace {
         }
 
         if let Some(terminal_provider) = self.terminal_provider.as_ref() {
-            terminal_provider
+            let task_status = terminal_provider.spawn(spawn_in_terminal, window, cx);
-                .spawn(spawn_in_terminal, window, cx)
+            cx.background_spawn(async move {
-                .detach_and_log_err(cx);
+                match task_status.await {
+                    Some(Ok(status)) => {
+                        if status.success() {
+                            log::debug!("Task spawn succeeded");
+                        } else {
+                            log::debug!("Task spawn failed, code: {:?}", status.code());
+                        }
+                    }
+                    Some(Err(e)) => log::error!("Task spawn failed: {e}"),
+                    None => log::debug!("Task spawn got cancelled"),
+                }
+            })
+            .detach();
         }
     }
 
@@ -92,11 +104,11 @@ impl Workspace {
         spawn_in_terminal: SpawnInTerminal,
         window: &mut Window,
         cx: &mut Context<Workspace>,
-    ) -> Task<Result<ExitStatus>> {
+    ) -> Task<Option<Result<ExitStatus>>> {
         if let Some(terminal_provider) = self.terminal_provider.as_ref() {
             terminal_provider.spawn(spawn_in_terminal, window, cx)
         } else {
-            Task::ready(Err(anyhow!("No terminal provider")))
+            Task::ready(None)
         }
     }
 }

crates/workspace/src/workspace.rs

@@ -136,7 +136,7 @@ pub trait TerminalProvider {
         task: SpawnInTerminal,
         window: &mut Window,
         cx: &mut App,
-    ) -> Task<Result<ExitStatus>>;
+    ) -> Task<Option<Result<ExitStatus>>>;
 }
 
 pub trait DebuggerProvider {