Merge pull request #2022 from zed-industries/restart-lsp-after-invalid-version-reported

Max Brunsfeld created 3 years ago

Fix crash when restarting a language server after it reports an unknown buffer version

Change summary

crates/project/src/project.rs       | 33 +++++++++-----------
crates/project/src/project_tests.rs | 49 +++++++++++++++++++++++++++++++
2 files changed, 64 insertions(+), 18 deletions(-)

Detailed changes

crates/project/src/project.rs

@@ -2081,6 +2081,7 @@ impl Project {
                                         .buffer_snapshots
                                         .entry(buffer.remote_id())
                                         .or_insert_with(|| vec![(0, buffer.text_snapshot())]);
+
                                     let (version, initial_snapshot) = versions.last().unwrap();
                                     let uri = lsp::Url::from_file_path(file.abs_path(cx)).unwrap();
                                     language_server
@@ -2617,6 +2618,7 @@ impl Project {
             worktree_id: worktree.read(cx).id(),
             path: relative_path.into(),
         };
+
         if let Some(buffer) = self.get_open_buffer(&project_path, cx) {
             self.update_buffer_diagnostics(&buffer, diagnostics.clone(), version, cx)?;
         }
@@ -6124,25 +6126,20 @@ impl Project {
                 .buffer_snapshots
                 .get_mut(&buffer_id)
                 .ok_or_else(|| anyhow!("no snapshot found for buffer {}", buffer_id))?;
-            let mut found_snapshot = None;
-            snapshots.retain(|(snapshot_version, snapshot)| {
-                if snapshot_version + OLD_VERSIONS_TO_RETAIN < version {
-                    false
-                } else {
-                    if *snapshot_version == version {
-                        found_snapshot = Some(snapshot.clone());
-                    }
-                    true
-                }
+            let found_snapshot = snapshots
+                .binary_search_by_key(&version, |e| e.0)
+                .map(|ix| snapshots[ix].1.clone())
+                .map_err(|_| {
+                    anyhow!(
+                        "snapshot not found for buffer {} at version {}",
+                        buffer_id,
+                        version
+                    )
+                })?;
+            snapshots.retain(|(snapshot_version, _)| {
+                snapshot_version + OLD_VERSIONS_TO_RETAIN >= version
             });
-
-            found_snapshot.ok_or_else(|| {
-                anyhow!(
-                    "snapshot not found for buffer {} at version {}",
-                    buffer_id,
-                    version
-                )
-            })
+            Ok(found_snapshot)
         } else {
             Ok((buffer.read(cx)).text_snapshot())
         }

crates/project/src/project_tests.rs

@@ -806,6 +806,55 @@ async fn test_restarting_server_with_diagnostics_running(cx: &mut gpui::TestAppC
     });
 }
 
+#[gpui::test]
+async fn test_restarted_server_reporting_invalid_buffer_version(cx: &mut gpui::TestAppContext) {
+    cx.foreground().forbid_parking();
+
+    let mut language = Language::new(
+        LanguageConfig {
+            path_suffixes: vec!["rs".to_string()],
+            ..Default::default()
+        },
+        None,
+    );
+    let mut fake_servers = language
+        .set_fake_lsp_adapter(Arc::new(FakeLspAdapter {
+            name: "the-lsp",
+            ..Default::default()
+        }))
+        .await;
+
+    let fs = FakeFs::new(cx.background());
+    fs.insert_tree("/dir", json!({ "a.rs": "" })).await;
+
+    let project = Project::test(fs, ["/dir".as_ref()], cx).await;
+    project.update(cx, |project, _| project.languages.add(Arc::new(language)));
+
+    let buffer = project
+        .update(cx, |project, cx| project.open_local_buffer("/dir/a.rs", cx))
+        .await
+        .unwrap();
+
+    // Before restarting the server, report diagnostics with an unknown buffer version.
+    let fake_server = fake_servers.next().await.unwrap();
+    fake_server.notify::<lsp::notification::PublishDiagnostics>(lsp::PublishDiagnosticsParams {
+        uri: lsp::Url::from_file_path("/dir/a.rs").unwrap(),
+        version: Some(10000),
+        diagnostics: Vec::new(),
+    });
+    cx.foreground().run_until_parked();
+
+    project.update(cx, |project, cx| {
+        project.restart_language_servers_for_buffers([buffer.clone()], cx);
+    });
+    let mut fake_server = fake_servers.next().await.unwrap();
+    let notification = fake_server
+        .receive_notification::<lsp::notification::DidOpenTextDocument>()
+        .await
+        .text_document;
+    assert_eq!(notification.version, 0);
+}
+
 #[gpui::test]
 async fn test_toggling_enable_language_server(
     deterministic: Arc<Deterministic>,