Fix race condition in editor show_hover (#11441)

Conrad Irwin created 2 years ago

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.

Change summary

crates/editor/src/element.rs       |  2 +-
crates/editor/src/hover_popover.rs | 26 +++++++++++++++++++-------
2 files changed, 20 insertions(+), 8 deletions(-)

Detailed changes

crates/editor/src/element.rs

@@ -638,7 +638,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);

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<Editor>) {
     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<DisplayPoint>, cx: &mut ViewContext<Editor>) {
+pub fn hover_at(
+    editor: &mut Editor,
+    point: Option<(DisplayPoint, &EditorSnapshot)>,
+    cx: &mut ViewContext<Editor>,
+) {
     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<Editor>) -> bool {
 fn show_hover(
     editor: &mut Editor,
     point: DisplayPoint,
+    snapshot: &EditorSnapshot,
     ignore_timeout: bool,
     cx: &mut ViewContext<Editor>,
 ) {
@@ -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::<lsp::request::HoverRequest, _, _>(|_, _| 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;