From 041ac56892d27178d71a7cece9190c577d645fde Mon Sep 17 00:00:00 2001 From: "gcp-cherry-pick-bot[bot]" <98988430+gcp-cherry-pick-bot[bot]@users.noreply.github.com> Date: Tue, 7 May 2024 20:16:43 -0600 Subject: [PATCH] Fix race condition in editor show_hover (cherry-pick #11441) (#11444) Cherry-picked Fix race condition in editor show_hover (#11441) The DisplayPoint returned from the position map is only valid at the snapshot in the position map. Before this change we were erroneously using it to index into the current version of the buffer. Release Notes: - Fixed a panic caused by a race condition in hover. Co-authored-by: Conrad Irwin --- crates/editor/src/element.rs | 2 +- crates/editor/src/hover_popover.rs | 26 +++++++++++++++++++------- 2 files changed, 20 insertions(+), 8 deletions(-) diff --git a/crates/editor/src/element.rs b/crates/editor/src/element.rs index ab6480a6c9eda4bcdb4c6523effdbc4a5500823c..5f94328ddf2b10b4a48b1ecbff2a68bd3aefe85a 100644 --- a/crates/editor/src/element.rs +++ b/crates/editor/src/element.rs @@ -629,7 +629,7 @@ impl EditorElement { editor.update_hovered_link(point_for_position, &position_map.snapshot, modifiers, cx); if let Some(point) = point_for_position.as_valid() { - hover_at(editor, Some(point), cx); + hover_at(editor, Some((point, &position_map.snapshot)), cx); Self::update_visible_cursor(editor, point, position_map, cx); } else { hover_at(editor, None, cx); diff --git a/crates/editor/src/hover_popover.rs b/crates/editor/src/hover_popover.rs index 97ee81ff81703910f435f5678c280a0aadc50e93..0133b7203251747948f5024927ec68bbfa6c4528 100644 --- a/crates/editor/src/hover_popover.rs +++ b/crates/editor/src/hover_popover.rs @@ -31,15 +31,20 @@ pub const HOVER_POPOVER_GAP: Pixels = px(10.); /// Bindable action which uses the most recent selection head to trigger a hover pub fn hover(editor: &mut Editor, _: &Hover, cx: &mut ViewContext) { let head = editor.selections.newest_display(cx).head(); - show_hover(editor, head, true, cx); + let snapshot = editor.snapshot(cx); + show_hover(editor, head, &snapshot, true, cx); } /// The internal hover action dispatches between `show_hover` or `hide_hover` /// depending on whether a point to hover over is provided. -pub fn hover_at(editor: &mut Editor, point: Option, cx: &mut ViewContext) { +pub fn hover_at( + editor: &mut Editor, + point: Option<(DisplayPoint, &EditorSnapshot)>, + cx: &mut ViewContext, +) { if EditorSettings::get_global(cx).hover_popover_enabled { - if let Some(point) = point { - show_hover(editor, point, false, cx); + if let Some((point, snapshot)) = point { + show_hover(editor, point, snapshot, false, cx); } else { hide_hover(editor, cx); } @@ -160,6 +165,7 @@ pub fn hide_hover(editor: &mut Editor, cx: &mut ViewContext) -> bool { fn show_hover( editor: &mut Editor, point: DisplayPoint, + snapshot: &EditorSnapshot, ignore_timeout: bool, cx: &mut ViewContext, ) { @@ -167,7 +173,6 @@ fn show_hover( return; } - let snapshot = editor.snapshot(cx); let multibuffer_offset = point.to_offset(&snapshot.display_snapshot, Bias::Left); let (buffer, buffer_position) = if let Some(output) = editor @@ -234,6 +239,7 @@ fn show_hover( return; } } + let snapshot = snapshot.clone(); let task = cx.spawn(|this, mut cx| { async move { @@ -659,7 +665,10 @@ mod tests { fn test() { printˇln!(); } "}); - cx.update_editor(|editor, cx| hover_at(editor, Some(hover_point), cx)); + cx.update_editor(|editor, cx| { + let snapshot = editor.snapshot(cx); + hover_at(editor, Some((hover_point, &snapshot)), cx) + }); assert!(!cx.editor(|editor, _| editor.hover_state.visible())); // After delay, hover should be visible. @@ -705,7 +714,10 @@ mod tests { let mut request = cx .lsp .handle_request::(|_, _| async move { Ok(None) }); - cx.update_editor(|editor, cx| hover_at(editor, Some(hover_point), cx)); + cx.update_editor(|editor, cx| { + let snapshot = editor.snapshot(cx); + hover_at(editor, Some((hover_point, &snapshot)), cx) + }); cx.background_executor .advance_clock(Duration::from_millis(HOVER_DELAY_MILLIS + 100)); request.next().await;