debugger: Allow users to shutdown debug sessions while they're booting (#34362)

Anthony Eid created

This solves problems where users couldn't shut down sessions while
locators or build tasks are running.

I renamed `debugger::Session::Mode` enum to `SessionState` to be more
clear when it's referenced in other crates. I also embedded the boot
task that is created in `SessionState::Building` variant. This allows
sessions to shut down all created threads in their boot process in a
clean and idiomatic way.

Finally, I added a method on terminal that allows killing the active
task.

Release Notes:

- Debugger: Allow shutting down debug sessions while they're booting up

Change summary

crates/debugger_ui/src/debugger_panel.rs  |  67 ++++++++----
crates/debugger_ui/src/session/running.rs |  28 ++++
crates/project/src/debugger/session.rs    | 128 ++++++++++++++----------
crates/terminal/src/pty_info.rs           |   4 
crates/terminal/src/terminal.rs           |   8 +
5 files changed, 156 insertions(+), 79 deletions(-)

Detailed changes

crates/debugger_ui/src/debugger_panel.rs 🔗

@@ -27,7 +27,7 @@ use text::ToPoint as _;
 
 use itertools::Itertools as _;
 use language::Buffer;
-use project::debugger::session::{Session, SessionQuirks, SessionStateEvent};
+use project::debugger::session::{Session, SessionQuirks, SessionState, SessionStateEvent};
 use project::{DebugScenarioContext, Fs, ProjectPath, TaskSourceKind, WorktreeId};
 use project::{Project, debugger::session::ThreadStatus};
 use rpc::proto::{self};
@@ -36,7 +36,7 @@ use std::sync::{Arc, LazyLock};
 use task::{DebugScenario, TaskContext};
 use tree_sitter::{Query, StreamingIterator as _};
 use ui::{ContextMenu, Divider, PopoverMenuHandle, Tooltip, prelude::*};
-use util::{ResultExt, maybe};
+use util::{ResultExt, debug_panic, maybe};
 use workspace::SplitDirection;
 use workspace::item::SaveOptions;
 use workspace::{
@@ -278,22 +278,34 @@ impl DebugPanel {
             }
         });
 
-        cx.spawn(async move |_, cx| {
-            if let Err(error) = task.await {
-                log::error!("{error}");
-                session
-                    .update(cx, |session, cx| {
-                        session
-                            .console_output(cx)
-                            .unbounded_send(format!("error: {}", error))
-                            .ok();
-                        session.shutdown(cx)
-                    })?
-                    .await;
+        let boot_task = cx.spawn({
+            let session = session.clone();
+
+            async move |_, cx| {
+                if let Err(error) = task.await {
+                    log::error!("{error}");
+                    session
+                        .update(cx, |session, cx| {
+                            session
+                                .console_output(cx)
+                                .unbounded_send(format!("error: {}", error))
+                                .ok();
+                            session.shutdown(cx)
+                        })?
+                        .await;
+                }
+                anyhow::Ok(())
             }
-            anyhow::Ok(())
-        })
-        .detach_and_log_err(cx);
+        });
+
+        session.update(cx, |session, _| match &mut session.mode {
+            SessionState::Building(state_task) => {
+                *state_task = Some(boot_task);
+            }
+            SessionState::Running(_) => {
+                debug_panic!("Session state should be in building because we are just starting it");
+            }
+        });
     }
 
     pub(crate) fn rerun_last_session(
@@ -826,13 +838,24 @@ impl DebugPanel {
                                             .on_click(window.listener_for(
                                                 &running_state,
                                                 |this, _, _window, cx| {
-                                                    this.stop_thread(cx);
+                                                    if this.session().read(cx).is_building() {
+                                                        this.session().update(cx, |session, cx| {
+                                                            session.shutdown(cx).detach()
+                                                        });
+                                                    } else {
+                                                        this.stop_thread(cx);
+                                                    }
+                                                },
+                                            ))
+                                            .disabled(active_session.as_ref().is_none_or(
+                                                |session| {
+                                                    session
+                                                        .read(cx)
+                                                        .session(cx)
+                                                        .read(cx)
+                                                        .is_terminated()
                                                 },
                                             ))
-                                            .disabled(
-                                                thread_status != ThreadStatus::Stopped
-                                                    && thread_status != ThreadStatus::Running,
-                                            )
                                             .tooltip({
                                                 let focus_handle = focus_handle.clone();
                                                 let label = if capabilities

crates/debugger_ui/src/session/running.rs 🔗

@@ -34,7 +34,7 @@ use loaded_source_list::LoadedSourceList;
 use module_list::ModuleList;
 use project::{
     DebugScenarioContext, Project, WorktreeId,
-    debugger::session::{Session, SessionEvent, ThreadId, ThreadStatus},
+    debugger::session::{self, Session, SessionEvent, SessionStateEvent, ThreadId, ThreadStatus},
     terminals::TerminalKind,
 };
 use rpc::proto::ViewId;
@@ -770,6 +770,15 @@ impl RunningState {
             cx.on_focus_out(&focus_handle, window, |this, _, window, cx| {
                 this.serialize_layout(window, cx);
             }),
+            cx.subscribe(
+                &session,
+                |this, session, event: &SessionStateEvent, cx| match event {
+                    SessionStateEvent::Shutdown if session.read(cx).is_building() => {
+                        this.shutdown(cx);
+                    }
+                    _ => {}
+                },
+            ),
         ];
 
         let mut pane_close_subscriptions = HashMap::default();
@@ -884,6 +893,7 @@ impl RunningState {
         let weak_project = project.downgrade();
         let weak_workspace = workspace.downgrade();
         let is_local = project.read(cx).is_local();
+
         cx.spawn_in(window, async move |this, cx| {
             let DebugScenario {
                 adapter,
@@ -1599,9 +1609,21 @@ impl RunningState {
             })
             .log_err();
 
-        self.session.update(cx, |session, cx| {
+        let is_building = self.session.update(cx, |session, cx| {
             session.shutdown(cx).detach();
-        })
+            matches!(session.mode, session::SessionState::Building(_))
+        });
+
+        if is_building {
+            self.debug_terminal.update(cx, |terminal, cx| {
+                if let Some(view) = terminal.terminal.as_ref() {
+                    view.update(cx, |view, cx| {
+                        view.terminal()
+                            .update(cx, |terminal, _| terminal.kill_active_task())
+                    })
+                }
+            })
+        }
     }
 
     pub fn stop_thread(&self, cx: &mut Context<Self>) {

crates/project/src/debugger/session.rs 🔗

@@ -134,8 +134,8 @@ pub struct Watcher {
     pub presentation_hint: Option<VariablePresentationHint>,
 }
 
-pub enum Mode {
-    Building,
+pub enum SessionState {
+    Building(Option<Task<Result<()>>>),
     Running(RunningMode),
 }
 
@@ -560,15 +560,15 @@ impl RunningMode {
     }
 }
 
-impl Mode {
+impl SessionState {
     pub(super) fn request_dap<R: LocalDapCommand>(&self, request: R) -> Task<Result<R::Response>>
     where
         <R::DapRequest as dap::requests::Request>::Response: 'static,
         <R::DapRequest as dap::requests::Request>::Arguments: 'static + Send,
     {
         match self {
-            Mode::Running(debug_adapter_client) => debug_adapter_client.request(request),
-            Mode::Building => Task::ready(Err(anyhow!(
+            SessionState::Running(debug_adapter_client) => debug_adapter_client.request(request),
+            SessionState::Building(_) => Task::ready(Err(anyhow!(
                 "no adapter running to send request: {request:?}"
             ))),
         }
@@ -577,13 +577,13 @@ impl Mode {
     /// Did this debug session stop at least once?
     pub(crate) fn has_ever_stopped(&self) -> bool {
         match self {
-            Mode::Building => false,
-            Mode::Running(running_mode) => running_mode.has_ever_stopped,
+            SessionState::Building(_) => false,
+            SessionState::Running(running_mode) => running_mode.has_ever_stopped,
         }
     }
 
     fn stopped(&mut self) {
-        if let Mode::Running(running) = self {
+        if let SessionState::Running(running) = self {
             running.has_ever_stopped = true;
         }
     }
@@ -660,7 +660,7 @@ type IsEnabled = bool;
 pub struct OutputToken(pub usize);
 /// Represents a current state of a single debug adapter and provides ways to mutate it.
 pub struct Session {
-    pub mode: Mode,
+    pub mode: SessionState,
     id: SessionId,
     label: Option<SharedString>,
     adapter: DebugAdapterName,
@@ -828,10 +828,9 @@ impl Session {
                 BreakpointStoreEvent::SetDebugLine | BreakpointStoreEvent::ClearDebugLines => {}
             })
             .detach();
-            // cx.on_app_quit(Self::on_app_quit).detach();
 
             let this = Self {
-                mode: Mode::Building,
+                mode: SessionState::Building(None),
                 id: session_id,
                 child_session_ids: HashSet::default(),
                 parent_session,
@@ -869,8 +868,8 @@ impl Session {
 
     pub fn worktree(&self) -> Option<Entity<Worktree>> {
         match &self.mode {
-            Mode::Building => None,
-            Mode::Running(local_mode) => local_mode.worktree.upgrade(),
+            SessionState::Building(_) => None,
+            SessionState::Running(local_mode) => local_mode.worktree.upgrade(),
         }
     }
 
@@ -929,7 +928,18 @@ impl Session {
             )
             .await?;
             this.update(cx, |this, cx| {
-                this.mode = Mode::Running(mode);
+                match &mut this.mode {
+                    SessionState::Building(task) if task.is_some() => {
+                        task.take().unwrap().detach_and_log_err(cx);
+                    }
+                    _ => {
+                        debug_assert!(
+                            this.parent_session.is_some(),
+                            "Booting a root debug session without a boot task"
+                        );
+                    }
+                };
+                this.mode = SessionState::Running(mode);
                 cx.emit(SessionStateEvent::Running);
             })?;
 
@@ -1022,8 +1032,8 @@ impl Session {
 
     pub fn binary(&self) -> Option<&DebugAdapterBinary> {
         match &self.mode {
-            Mode::Building => None,
-            Mode::Running(running_mode) => Some(&running_mode.binary),
+            SessionState::Building(_) => None,
+            SessionState::Running(running_mode) => Some(&running_mode.binary),
         }
     }
 
@@ -1068,26 +1078,26 @@ impl Session {
 
     pub fn is_started(&self) -> bool {
         match &self.mode {
-            Mode::Building => false,
-            Mode::Running(running) => running.is_started,
+            SessionState::Building(_) => false,
+            SessionState::Running(running) => running.is_started,
         }
     }
 
     pub fn is_building(&self) -> bool {
-        matches!(self.mode, Mode::Building)
+        matches!(self.mode, SessionState::Building(_))
     }
 
     pub fn as_running_mut(&mut self) -> Option<&mut RunningMode> {
         match &mut self.mode {
-            Mode::Running(local_mode) => Some(local_mode),
-            Mode::Building => None,
+            SessionState::Running(local_mode) => Some(local_mode),
+            SessionState::Building(_) => None,
         }
     }
 
     pub fn as_running(&self) -> Option<&RunningMode> {
         match &self.mode {
-            Mode::Running(local_mode) => Some(local_mode),
-            Mode::Building => None,
+            SessionState::Running(local_mode) => Some(local_mode),
+            SessionState::Building(_) => None,
         }
     }
 
@@ -1229,7 +1239,7 @@ impl Session {
         let adapter_id = self.adapter().to_string();
         let request = Initialize { adapter_id };
 
-        let Mode::Running(running) = &self.mode else {
+        let SessionState::Running(running) = &self.mode else {
             return Task::ready(Err(anyhow!(
                 "Cannot send initialize request, task still building"
             )));
@@ -1278,10 +1288,12 @@ impl Session {
         cx: &mut Context<Self>,
     ) -> Task<Result<()>> {
         match &self.mode {
-            Mode::Running(local_mode) => {
+            SessionState::Running(local_mode) => {
                 local_mode.initialize_sequence(&self.capabilities, initialize_rx, dap_store, cx)
             }
-            Mode::Building => Task::ready(Err(anyhow!("cannot initialize, still building"))),
+            SessionState::Building(_) => {
+                Task::ready(Err(anyhow!("cannot initialize, still building")))
+            }
         }
     }
 
@@ -1292,7 +1304,7 @@ impl Session {
         cx: &mut Context<Self>,
     ) {
         match &mut self.mode {
-            Mode::Running(local_mode) => {
+            SessionState::Running(local_mode) => {
                 if !matches!(
                     self.thread_states.thread_state(active_thread_id),
                     Some(ThreadStatus::Stopped)
@@ -1316,7 +1328,7 @@ impl Session {
                 })
                 .detach();
             }
-            Mode::Building => {}
+            SessionState::Building(_) => {}
         }
     }
 
@@ -1596,7 +1608,7 @@ impl Session {
 
     fn request_inner<T: LocalDapCommand + PartialEq + Eq + Hash>(
         capabilities: &Capabilities,
-        mode: &Mode,
+        mode: &SessionState,
         request: T,
         process_result: impl FnOnce(
             &mut Self,
@@ -1916,28 +1928,36 @@ impl Session {
         self.thread_states.exit_all_threads();
         cx.notify();
 
-        let task = if self
-            .capabilities
-            .supports_terminate_request
-            .unwrap_or_default()
-        {
-            self.request(
-                TerminateCommand {
-                    restart: Some(false),
-                },
-                Self::clear_active_debug_line_response,
-                cx,
-            )
-        } else {
-            self.request(
-                DisconnectCommand {
-                    restart: Some(false),
-                    terminate_debuggee: Some(true),
-                    suspend_debuggee: Some(false),
-                },
-                Self::clear_active_debug_line_response,
-                cx,
-            )
+        let task = match &mut self.mode {
+            SessionState::Running(_) => {
+                if self
+                    .capabilities
+                    .supports_terminate_request
+                    .unwrap_or_default()
+                {
+                    self.request(
+                        TerminateCommand {
+                            restart: Some(false),
+                        },
+                        Self::clear_active_debug_line_response,
+                        cx,
+                    )
+                } else {
+                    self.request(
+                        DisconnectCommand {
+                            restart: Some(false),
+                            terminate_debuggee: Some(true),
+                            suspend_debuggee: Some(false),
+                        },
+                        Self::clear_active_debug_line_response,
+                        cx,
+                    )
+                }
+            }
+            SessionState::Building(build_task) => {
+                build_task.take();
+                Task::ready(Some(()))
+            }
         };
 
         cx.emit(SessionStateEvent::Shutdown);
@@ -1987,8 +2007,8 @@ impl Session {
 
     pub fn adapter_client(&self) -> Option<Arc<DebugAdapterClient>> {
         match self.mode {
-            Mode::Running(ref local) => Some(local.client.clone()),
-            Mode::Building => None,
+            SessionState::Running(ref local) => Some(local.client.clone()),
+            SessionState::Building(_) => None,
         }
     }
 
@@ -2452,7 +2472,7 @@ impl Session {
     }
 
     pub fn is_attached(&self) -> bool {
-        let Mode::Running(local_mode) = &self.mode else {
+        let SessionState::Running(local_mode) = &self.mode else {
             return false;
         };
         local_mode.binary.request_args.request == StartDebuggingRequestArgumentsRequest::Attach

crates/terminal/src/pty_info.rs 🔗

@@ -121,6 +121,10 @@ impl PtyProcessInfo {
         }
     }
 
+    pub(crate) fn kill_current_process(&mut self) -> bool {
+        self.refresh().map_or(false, |process| process.kill())
+    }
+
     fn load(&mut self) -> Option<ProcessInfo> {
         let process = self.refresh()?;
         let cwd = process.cwd().map_or(PathBuf::new(), |p| p.to_owned());

crates/terminal/src/terminal.rs 🔗

@@ -1824,6 +1824,14 @@ impl Terminal {
         }
     }
 
+    pub fn kill_active_task(&mut self) {
+        if let Some(task) = self.task() {
+            if task.status == TaskStatus::Running {
+                self.pty_info.kill_current_process();
+            }
+        }
+    }
+
     pub fn task(&self) -> Option<&TaskState> {
         self.task.as_ref()
     }