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
This commit is contained in:
1 parent
7f82c86cd7
commit
a59eeb8436
2 files changed
+97
-7
No files matched your search
@@ -1145,10 +1145,10 @@ impl LocalLspStore {
|
||||
.on_request::<lsp::request::WorkspaceDiagnosticRefresh, _, _>({
|
||||
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(())
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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::<lsp::request::DocumentDiagnosticRequest, _, _>({
|
||||
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::<lsp::request::WorkspaceDiagnosticRequest, _, _>(
|
||||
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::<lsp::request::WorkspaceDiagnosticRefresh>((), 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);
|
||||
|
||||
Reference in new issue
Block a user