From 8f6b9f0d65a8fec611a54ed053936cdb035c8082 Mon Sep 17 00:00:00 2001 From: Anthony Eid <56899983+Anthony-Eid@users.noreply.github.com> Date: Sat, 12 Jul 2025 19:16:35 -0400 Subject: [PATCH] debugger: Allow users to shutdown debug sessions while they're booting (#34362) 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 --- 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(-) diff --git a/crates/debugger_ui/src/debugger_panel.rs b/crates/debugger_ui/src/debugger_panel.rs index 184aedafc2..37064d5d5d 100644 --- a/crates/debugger_ui/src/debugger_panel.rs +++ b/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 diff --git a/crates/debugger_ui/src/session/running.rs b/crates/debugger_ui/src/session/running.rs index d308fc9bd2..264d46370f 100644 --- a/crates/debugger_ui/src/session/running.rs +++ b/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) { diff --git a/crates/project/src/debugger/session.rs b/crates/project/src/debugger/session.rs index 59feb504c5..f526d5a3fe 100644 --- a/crates/project/src/debugger/session.rs +++ b/crates/project/src/debugger/session.rs @@ -134,8 +134,8 @@ pub struct Watcher { pub presentation_hint: Option, } -pub enum Mode { - Building, +pub enum SessionState { + Building(Option>>), Running(RunningMode), } @@ -560,15 +560,15 @@ impl RunningMode { } } -impl Mode { +impl SessionState { pub(super) fn request_dap(&self, request: R) -> Task> where ::Response: 'static, ::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, 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> { 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, ) -> Task> { 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, ) { 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( 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> { 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 diff --git a/crates/terminal/src/pty_info.rs b/crates/terminal/src/pty_info.rs index d9515afbf7..802470493c 100644 --- a/crates/terminal/src/pty_info.rs +++ b/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 { let process = self.refresh()?; let cwd = process.cwd().map_or(PathBuf::new(), |p| p.to_owned()); diff --git a/crates/terminal/src/terminal.rs b/crates/terminal/src/terminal.rs index 032a750d1a..72307b697a 100644 --- a/crates/terminal/src/terminal.rs +++ b/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() }