From da98e300cc624b170db1b2df603aa1d8ae993c1b Mon Sep 17 00:00:00 2001 From: Anthony Eid <56899983+Anthony-Eid@users.noreply.github.com> Date: Fri, 2 May 2025 15:24:28 -0400 Subject: [PATCH] debugger: Clear active debug line on thread continued (#29811) I also moved the breakpoint store to session from local mode, because both remote/local modes will need the ability to remove active debug lines. Release Notes: - N/A --- crates/debugger_ui/src/debugger_panel.rs | 9 +-- .../debugger_ui/src/tests/debugger_panel.rs | 27 +++++++++ crates/project/src/debugger/dap_store.rs | 3 +- crates/project/src/debugger/session.rs | 59 +++++++++++-------- crates/project/src/debugger/test.rs | 7 +-- 5 files changed, 65 insertions(+), 40 deletions(-) diff --git a/crates/debugger_ui/src/debugger_panel.rs b/crates/debugger_ui/src/debugger_panel.rs index 7b8a4dfe87..6459c76a01 100644 --- a/crates/debugger_ui/src/debugger_panel.rs +++ b/crates/debugger_ui/src/debugger_panel.rs @@ -353,7 +353,6 @@ impl DebugPanel { }; let dap_store_handle = self.project.read(cx).dap_store().clone(); - let breakpoint_store = self.project.read(cx).breakpoint_store(); let definition = parent_session.read(cx).definition().clone(); let mut binary = parent_session.read(cx).binary().clone(); binary.request_args = request.clone(); @@ -364,13 +363,7 @@ impl DebugPanel { dap_store.new_session(definition.clone(), Some(parent_session.clone()), cx); let task = session.update(cx, |session, cx| { - session.boot( - binary, - worktree, - breakpoint_store, - dap_store_handle.downgrade(), - cx, - ) + session.boot(binary, worktree, dap_store_handle.downgrade(), cx) }); (session, task) })?; diff --git a/crates/debugger_ui/src/tests/debugger_panel.rs b/crates/debugger_ui/src/tests/debugger_panel.rs index 13a469d648..b44a65cd40 100644 --- a/crates/debugger_ui/src/tests/debugger_panel.rs +++ b/crates/debugger_ui/src/tests/debugger_panel.rs @@ -1663,6 +1663,33 @@ async fn test_active_debug_line_setting(executor: BackgroundExecutor, cx: &mut T "Second stacktrace request handler was not called" ); + client + .fake_event(dap::messages::Events::Continued(dap::ContinuedEvent { + thread_id: 0, + all_threads_continued: Some(true), + })) + .await; + + cx.run_until_parked(); + + second_editor.update(cx, |editor, _| { + let active_debug_lines: Vec<_> = editor.highlighted_rows::().collect(); + + assert!( + active_debug_lines.is_empty(), + "There shouldn't be any active debug lines" + ); + }); + + main_editor.update(cx, |editor, _| { + let active_debug_lines: Vec<_> = editor.highlighted_rows::().collect(); + + assert!( + active_debug_lines.is_empty(), + "There shouldn't be any active debug lines" + ); + }); + // Clean up let shutdown_session = project.update(cx, |project, cx| { project.dap_store().update(cx, |dap_store, cx| { diff --git a/crates/project/src/debugger/dap_store.rs b/crates/project/src/debugger/dap_store.rs index f0c90c63a7..d5a0670545 100644 --- a/crates/project/src/debugger/dap_store.rs +++ b/crates/project/src/debugger/dap_store.rs @@ -442,7 +442,6 @@ impl DapStore { }; let dap_store = cx.weak_entity(); - let breakpoint_store = self.breakpoint_store.clone(); let definition = session.read(cx).definition(); cx.spawn({ @@ -456,7 +455,7 @@ impl DapStore { session .update(cx, |session, cx| { - session.boot(binary, worktree, breakpoint_store, dap_store, cx) + session.boot(binary, worktree, dap_store, cx) })? .await } diff --git a/crates/project/src/debugger/session.rs b/crates/project/src/debugger/session.rs index a602cb5e76..9a0cb5e6a4 100644 --- a/crates/project/src/debugger/session.rs +++ b/crates/project/src/debugger/session.rs @@ -126,7 +126,6 @@ pub enum Mode { pub struct LocalMode { client: Arc, binary: DebugAdapterBinary, - pub(crate) breakpoint_store: Entity, tmp_breakpoint: Option, worktree: WeakEntity, executor: BackgroundExecutor, @@ -152,7 +151,6 @@ impl LocalMode { session_id: SessionId, parent_session: Option>, worktree: WeakEntity, - breakpoint_store: Entity, binary: DebugAdapterBinary, messages_tx: futures::channel::mpsc::UnboundedSender, cx: AsyncApp, @@ -178,7 +176,6 @@ impl LocalMode { Ok(Self { client, - breakpoint_store, worktree, tmp_breakpoint: None, binary, @@ -219,10 +216,10 @@ impl LocalMode { &self, abs_path: Arc, reason: BreakpointUpdatedReason, + breakpoint_store: &Entity, cx: &mut App, ) -> Task<()> { - let breakpoints = self - .breakpoint_store + let breakpoints = breakpoint_store .read_with(cx, |store, cx| store.breakpoints_from_path(&abs_path, cx)) .into_iter() .filter(|bp| bp.state.is_enabled()) @@ -271,12 +268,11 @@ impl LocalMode { fn send_source_breakpoints( &self, ignore_breakpoints: bool, + breakpoint_store: &Entity, cx: &App, ) -> Task, anyhow::Error>> { let mut breakpoint_tasks = Vec::new(); - let breakpoints = self - .breakpoint_store - .read_with(cx, |store, cx| store.all_breakpoints(cx)); + let breakpoints = breakpoint_store.read_with(cx, |store, cx| store.all_breakpoints(cx)); for (path, breakpoints) in breakpoints { let breakpoints = if ignore_breakpoints { @@ -314,6 +310,7 @@ impl LocalMode { definition: &DebugTaskDefinition, initialized_rx: oneshot::Receiver<()>, dap_store: WeakEntity, + breakpoint_store: Entity, cx: &App, ) -> Task> { let mut raw = self.binary.request_args.clone(); @@ -354,7 +351,7 @@ impl LocalMode { async move |cx| { initialized_rx.await?; let errors_by_path = cx - .update(|cx| this.send_source_breakpoints(false, cx))? + .update(|cx| this.send_source_breakpoints(false, &breakpoint_store, cx))? .await; dap_store.update(cx, |_, cx| { @@ -513,7 +510,6 @@ pub struct Session { id: SessionId, child_session_ids: HashSet, parent_session: Option>, - ignore_breakpoints: bool, modules: Vec, loaded_sources: Vec, output_token: OutputToken, @@ -525,6 +521,8 @@ pub struct Session { locations: HashMap, is_session_terminated: bool, requests: HashMap>>>>, + pub(crate) breakpoint_store: Entity, + ignore_breakpoints: bool, exception_breakpoints: BTreeMap, background_tasks: Vec>, } @@ -642,14 +640,14 @@ impl Session { cx: &mut App, ) -> Entity { cx.new::(|cx| { - cx.subscribe(&breakpoint_store, |this, _, event, cx| match event { + cx.subscribe(&breakpoint_store, |this, store, event, cx| match event { BreakpointStoreEvent::BreakpointsUpdated(path, reason) => { if let Some(local) = (!this.ignore_breakpoints) .then(|| this.as_local_mut()) .flatten() { local - .send_breakpoints_from_path(path.clone(), *reason, cx) + .send_breakpoints_from_path(path.clone(), *reason, &store, cx) .detach(); }; } @@ -672,7 +670,6 @@ impl Session { child_session_ids: HashSet::default(), parent_session, capabilities: Capabilities::default(), - ignore_breakpoints: false, variables: Default::default(), stack_frames: Default::default(), thread_states: ThreadStates::default(), @@ -685,6 +682,8 @@ impl Session { background_tasks: Vec::default(), locations: Default::default(), is_session_terminated: false, + ignore_breakpoints: false, + breakpoint_store, exception_breakpoints: Default::default(), definition: template, }; @@ -704,7 +703,6 @@ impl Session { &mut self, binary: DebugAdapterBinary, worktree: Entity, - breakpoint_store: Entity, dap_store: WeakEntity, cx: &mut Context, ) -> Task> { @@ -750,7 +748,6 @@ impl Session { id, parent_session, worktree.downgrade(), - breakpoint_store.clone(), binary, message_tx, cx.clone(), @@ -991,6 +988,7 @@ impl Session { &self.definition, initialize_rx, dap_store, + self.breakpoint_store.clone(), cx, ), Mode::Building => Task::ready(Err(anyhow!("cannot initialize, still building"))), @@ -1016,6 +1014,7 @@ impl Session { let task = local_mode.send_breakpoints_from_path( path, BreakpointUpdatedReason::Toggled, + &self.breakpoint_store, cx, ); @@ -1081,13 +1080,20 @@ impl Session { } fn handle_stopped_event(&mut self, event: StoppedEvent, cx: &mut Context) { + // todo(debugger): Find a clean way to get around the clone + let breakpoint_store = self.breakpoint_store.clone(); if let Some((local, path)) = self.as_local_mut().and_then(|local| { let breakpoint = local.tmp_breakpoint.take()?; let path = breakpoint.path.clone(); Some((local, path)) }) { local - .send_breakpoints_from_path(path, BreakpointUpdatedReason::Toggled, cx) + .send_breakpoints_from_path( + path, + BreakpointUpdatedReason::Toggled, + &breakpoint_store, + cx, + ) .detach(); }; @@ -1137,6 +1143,9 @@ impl Session { Events::Continued(event) => { if event.all_threads_continued.unwrap_or_default() { self.thread_states.continue_all_threads(); + self.breakpoint_store.update(cx, |store, cx| { + store.remove_active_position(Some(self.session_id()), cx) + }); } else { self.thread_states .continue_thread(ThreadId(event.thread_id)); @@ -1434,7 +1443,7 @@ impl Session { self.ignore_breakpoints = ignore; if let Some(local) = self.as_local() { - local.send_source_breakpoints(ignore, cx) + local.send_source_breakpoints(ignore, &self.breakpoint_store, cx) } else { // todo(debugger): We need to propagate this change to downstream sessions and send a message to upstream sessions unimplemented!() @@ -1516,7 +1525,12 @@ impl Session { ) -> impl FnOnce(&mut Self, Result, &mut Context) -> Option + 'static { move |this, response, cx| match response.log_err() { - Some(response) => Some(response), + Some(response) => { + this.breakpoint_store.update(cx, |store, cx| { + store.remove_active_position(Some(this.session_id()), cx) + }); + Some(response) + } None => { this.thread_states.stop_thread(thread_id); cx.notify(); @@ -1536,12 +1550,9 @@ impl Session { } fn clear_active_debug_line(&mut self, cx: &mut Context) { - self.as_local() - .expect("Message handler will only run in local mode") - .breakpoint_store - .update(cx, |store, cx| { - store.remove_active_position(Some(self.id), cx) - }); + self.breakpoint_store.update(cx, |store, cx| { + store.remove_active_position(Some(self.id), cx) + }); } pub fn pause_thread(&mut self, thread_id: ThreadId, cx: &mut Context) { diff --git a/crates/project/src/debugger/test.rs b/crates/project/src/debugger/test.rs index d61ec80a67..3b9425e369 100644 --- a/crates/project/src/debugger/test.rs +++ b/crates/project/src/debugger/test.rs @@ -36,12 +36,7 @@ pub fn intercept_debug_sessions) + 'static>( fn register_default_handlers(session: &Session, client: &Arc, cx: &mut App) { client.on_request::(move |_, _| Ok(Default::default())); - let paths = session - .as_local() - .unwrap() - .breakpoint_store - .read(cx) - .breakpoint_paths(); + let paths = session.breakpoint_store.read(cx).breakpoint_paths(); client.on_request::(move |_, args| { let p = Arc::from(Path::new(&args.source.path.unwrap()));