From a59eeb8436802d56dd25dc1cd78c4aeab5cfe58c Mon Sep 17 00:00:00 2001 From: Kirill Bulatov Date: Tue, 28 Jul 2026 15:32:28 +0300 Subject: [PATCH] Instantly acknowledge workspace/diagnostic/refresh (#61772) Closes https://github.com/zed-industries/zed/issues/61692 Makes no sense to wait for anything at all when receiving such requests, https://microsoft.github.io/language-server-protocol/specifications/lsp/3.17/specification/#diagnostic_refresh does not imply anything either. Release Notes: - Fixed diagnostics refresh deadlocking certain language servers --- crates/project/src/lsp_store.rs | 16 ++-- .../tests/integration/project_tests.rs | 88 +++++++++++++++++++ 2 files changed, 97 insertions(+), 7 deletions(-) diff --git a/crates/project/src/lsp_store.rs b/crates/project/src/lsp_store.rs index 06db4471ee..7cb9956ef3 100644 --- a/crates/project/src/lsp_store.rs +++ b/crates/project/src/lsp_store.rs @@ -1145,10 +1145,10 @@ impl LocalLspStore { .on_request::({ let this = lsp_store.clone(); move |(), cx| { - let this = this.clone(); + let lsp_store = this.clone(); let mut cx = cx.clone(); async move { - this.update(&mut cx, |lsp_store, cx| { + lsp_store.update(&mut cx, |lsp_store, cx| { lsp_store.pull_workspace_diagnostics(server_id); lsp_store .downstream_client @@ -1160,11 +1160,13 @@ impl LocalLspStore { }) }) .transpose()?; - anyhow::Ok( - lsp_store.pull_document_diagnostics_for_server(server_id, None, cx), - ) - })?? - .await; + // Respond before pulling: awaiting a round-trip to the same server + // here can deadlock servers that bound their request concurrency + // and await this response. + let _ = + lsp_store.pull_document_diagnostics_for_server(server_id, None, cx); + anyhow::Ok(()) + })??; Ok(()) } } diff --git a/crates/project/tests/integration/project_tests.rs b/crates/project/tests/integration/project_tests.rs index 1a5d6d358c..29f76a4595 100644 --- a/crates/project/tests/integration/project_tests.rs +++ b/crates/project/tests/integration/project_tests.rs @@ -4553,6 +4553,94 @@ async fn test_workspace_diagnostics_pull_timeout_releases_waiters(cx: &mut gpui: ); } +#[gpui::test] +async fn test_workspace_diagnostics_refresh_is_answered_before_pulling( + cx: &mut gpui::TestAppContext, +) { + init_test(cx); + + let fs = FakeFs::new(cx.executor()); + fs.insert_tree(path!("/dir"), json!({ "a.rs": "one two three" })) + .await; + + let project = Project::test(fs, [path!("/dir").as_ref()], cx).await; + let language_registry = project.read_with(cx, |project, _| project.languages().clone()); + language_registry.add(rust_lang()); + + let document_pulls_received = Arc::new(atomic::AtomicUsize::new(0)); + let mut fake_servers = language_registry.register_fake_lsp( + "Rust", + FakeLspAdapter { + capabilities: lsp::ServerCapabilities { + diagnostic_provider: Some(lsp::DiagnosticServerCapabilities::Options( + lsp::DiagnosticOptions { + identifier: Some("test-refresh-response-first".to_string()), + inter_file_dependencies: true, + workspace_diagnostics: true, + work_done_progress_options: Default::default(), + }, + )), + ..lsp::ServerCapabilities::default() + }, + initializer: Some(Box::new({ + let document_pulls_received = document_pulls_received.clone(); + move |fake_server| { + // Simulate a server that cannot answer any diagnostic pulls until its + // own workspace/diagnostic/refresh request is answered, e.g. one that + // bounds its request concurrency. + fake_server + .set_request_handler::({ + let document_pulls_received = document_pulls_received.clone(); + move |_, _| { + document_pulls_received.fetch_add(1, atomic::Ordering::Release); + async move { + future::pending::<()>().await; + Err(anyhow::anyhow!("should never respond")) + } + } + }); + fake_server + .set_request_handler::( + move |_, _| async move { + future::pending::<()>().await; + Err(anyhow::anyhow!("should never respond")) + }, + ); + } + })), + ..FakeLspAdapter::default() + }, + ); + + let (_buffer, _handle) = project + .update(cx, |project, cx| { + project.open_local_buffer_with_lsp(path!("/dir/a.rs"), cx) + }) + .await + .unwrap(); + + let fake_server = fake_servers.next().await.unwrap(); + cx.executor().run_until_parked(); + + let refresh_response = cx.executor().spawn(async move { + fake_server + .request::((), DEFAULT_LSP_REQUEST_TIMEOUT) + .await + }); + cx.executor().run_until_parked(); + + refresh_response + .now_or_never() + .expect("workspace/diagnostic/refresh must be answered without awaiting diagnostic pulls from the same server") + .into_response() + .expect("workspace/diagnostic/refresh should succeed"); + assert_eq!( + document_pulls_received.load(atomic::Ordering::Acquire), + 1, + "the refresh should still trigger a document diagnostics pull for the open buffer" + ); +} + #[gpui::test] async fn test_edits_from_lsp2_with_past_version(cx: &mut gpui::TestAppContext) { init_test(cx);