Skip to content

Commit 001c387

Browse files
VeykrilVeykril
committed
Don't retry position relient requests and version resolve data
1 parent efb390b commit 001c387

File tree

8 files changed

+116
-129
lines changed

8 files changed

+116
-129
lines changed

‎src/tools/rust-analyzer/crates/rust-analyzer/src/dispatch.rs

Lines changed: 12 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -95,45 +95,8 @@ impl RequestDispatcher<'_> {
9595
self
9696
}
9797

98-
/// Dispatches a non-latency-sensitive request onto the thread pool
99-
/// without retrying it if it panics.
100-
pub(crate) fn on_no_retry<R>(
101-
&mut self,
102-
f: fn(GlobalStateSnapshot, R::Params) -> anyhow::Result<R::Result>,
103-
) -> &mut Self
104-
where
105-
R: lsp_types::request::Request + 'static,
106-
R::Params: DeserializeOwned + panic::UnwindSafe + Send + fmt::Debug,
107-
R::Result: Serialize,
108-
{
109-
let (req, params, panic_context) = match self.parse::<R>() {
110-
Some(it) => it,
111-
None => return self,
112-
};
113-
114-
self.global_state.task_pool.handle.spawn(ThreadIntent::Worker, {
115-
let world = self.global_state.snapshot();
116-
move || {
117-
let result = panic::catch_unwind(move || {
118-
let _pctx = stdx::panic_context::enter(panic_context);
119-
f(world, params)
120-
});
121-
match thread_result_to_response::<R>(req.id.clone(), result) {
122-
Ok(response) => Task::Response(response),
123-
Err(_) => Task::Response(lsp_server::Response::new_err(
124-
req.id,
125-
lsp_server::ErrorCode::ContentModified as i32,
126-
"content modified".to_owned(),
127-
)),
128-
}
129-
}
130-
});
131-
132-
self
133-
}
134-
13598
/// Dispatches a non-latency-sensitive request onto the thread pool.
136-
pub(crate) fn on<R>(
99+
pub(crate) fn on<const ALLOW_RETRYING: bool, R>(
137100
&mut self,
138101
f: fn(GlobalStateSnapshot, R::Params) -> anyhow::Result<R::Result>,
139102
) -> &mut Self
@@ -142,11 +105,11 @@ impl RequestDispatcher<'_> {
142105
R::Params: DeserializeOwned + panic::UnwindSafe + Send + fmt::Debug,
143106
R::Result: Serialize,
144107
{
145-
self.on_with_thread_intent::<true, R>(ThreadIntent::Worker, f)
108+
self.on_with_thread_intent::<true, ALLOW_RETRYING, R>(ThreadIntent::Worker, f)
146109
}
147110

148111
/// Dispatches a latency-sensitive request onto the thread pool.
149-
pub(crate) fn on_latency_sensitive<R>(
112+
pub(crate) fn on_latency_sensitive<const ALLOW_RETRYING: bool, R>(
150113
&mut self,
151114
f: fn(GlobalStateSnapshot, R::Params) -> anyhow::Result<R::Result>,
152115
) -> &mut Self
@@ -155,7 +118,7 @@ impl RequestDispatcher<'_> {
155118
R::Params: DeserializeOwned + panic::UnwindSafe + Send + fmt::Debug,
156119
R::Result: Serialize,
157120
{
158-
self.on_with_thread_intent::<true, R>(ThreadIntent::LatencySensitive, f)
121+
self.on_with_thread_intent::<true, ALLOW_RETRYING, R>(ThreadIntent::LatencySensitive, f)
159122
}
160123

161124
/// Formatting requests should never block on waiting a for task thread to open up, editors will wait
@@ -170,7 +133,7 @@ impl RequestDispatcher<'_> {
170133
R::Params: DeserializeOwned + panic::UnwindSafe + Send + fmt::Debug,
171134
R::Result: Serialize,
172135
{
173-
self.on_with_thread_intent::<false, R>(ThreadIntent::LatencySensitive, f)
136+
self.on_with_thread_intent::<false, false, R>(ThreadIntent::LatencySensitive, f)
174137
}
175138

176139
pub(crate) fn finish(&mut self) {
@@ -185,7 +148,7 @@ impl RequestDispatcher<'_> {
185148
}
186149
}
187150

188-
fn on_with_thread_intent<const MAIN_POOL: bool, R>(
151+
fn on_with_thread_intent<const MAIN_POOL: bool, const ALLOW_RETRYING: bool, R>(
189152
&mut self,
190153
intent: ThreadIntent,
191154
f: fn(GlobalStateSnapshot, R::Params) -> anyhow::Result<R::Result>,
@@ -215,7 +178,12 @@ impl RequestDispatcher<'_> {
215178
});
216179
match thread_result_to_response::<R>(req.id.clone(), result) {
217180
Ok(response) => Task::Response(response),
218-
Err(_) => Task::Retry(req),
181+
Err(_cancelled) if ALLOW_RETRYING => Task::Retry(req),
182+
Err(_cancelled) => Task::Response(lsp_server::Response::new_err(
183+
req.id,
184+
lsp_server::ErrorCode::ContentModified as i32,
185+
"content modified".to_owned(),
186+
)),
219187
}
220188
});
221189

‎src/tools/rust-analyzer/crates/rust-analyzer/src/global_state.rs

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -488,6 +488,10 @@ impl GlobalStateSnapshot {
488488
Ok(res)
489489
}
490490

491+
pub(crate) fn file_version(&self, file_id: FileId) -> Option<i32> {
492+
Some(self.mem_docs.get(self.vfs_read().file_path(file_id))?.version)
493+
}
494+
491495
pub(crate) fn url_file_version(&self, url: &Url) -> Option<i32> {
492496
let path = from_proto::vfs_path(url).ok()?;
493497
Some(self.mem_docs.get(&path)?.version)

‎src/tools/rust-analyzer/crates/rust-analyzer/src/handlers/request.rs

Lines changed: 26 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -954,8 +954,13 @@ pub(crate) fn handle_completion(
954954
};
955955
let line_index = snap.file_line_index(position.file_id)?;
956956

957-
let items =
958-
to_proto::completion_items(&snap.config, &line_index, text_document_position, items);
957+
let items = to_proto::completion_items(
958+
&snap.config,
959+
&line_index,
960+
snap.file_version(position.file_id),
961+
text_document_position,
962+
items,
963+
);
959964

960965
let completion_list = lsp_types::CompletionList { is_incomplete: true, items };
961966
Ok(Some(completion_list.into()))
@@ -1240,8 +1245,11 @@ pub(crate) fn handle_code_action(
12401245
frange,
12411246
)?;
12421247
for (index, assist) in assists.into_iter().enumerate() {
1243-
let resolve_data =
1244-
if code_action_resolve_cap { Some((index, params.clone())) } else { None };
1248+
let resolve_data = if code_action_resolve_cap {
1249+
Some((index, params.clone(), snap.file_version(file_id)))
1250+
} else {
1251+
None
1252+
};
12451253
let code_action = to_proto::code_action(&snap, assist, resolve_data)?;
12461254

12471255
// Check if the client supports the necessary `ResourceOperation`s.
@@ -1280,12 +1288,14 @@ pub(crate) fn handle_code_action_resolve(
12801288
mut code_action: lsp_ext::CodeAction,
12811289
) -> anyhow::Result<lsp_ext::CodeAction> {
12821290
let _p = tracing::span!(tracing::Level::INFO, "handle_code_action_resolve").entered();
1283-
let params = match code_action.data.take() {
1284-
Some(it) => it,
1285-
None => return Err(invalid_params_error("code action without data".to_owned()).into()),
1291+
let Some(params) = code_action.data.take() else {
1292+
return Err(invalid_params_error("code action without data".to_owned()).into());
12861293
};
12871294

12881295
let file_id = from_proto::file_id(&snap, &params.code_action_params.text_document.uri)?;
1296+
if snap.file_version(file_id) != params.version {
1297+
return Err(invalid_params_error("stale code action".to_owned()).into());
1298+
}
12891299
let line_index = snap.file_line_index(file_id)?;
12901300
let range = from_proto::text_range(&line_index, params.code_action_params.range)?;
12911301
let frange = FileRange { file_id, range };
@@ -1411,12 +1421,11 @@ pub(crate) fn handle_code_lens(
14111421

14121422
pub(crate) fn handle_code_lens_resolve(
14131423
snap: GlobalStateSnapshot,
1414-
code_lens: CodeLens,
1424+
mut code_lens: CodeLens,
14151425
) -> anyhow::Result<CodeLens> {
1416-
if code_lens.data.is_none() {
1417-
return Ok(code_lens);
1418-
}
1419-
let Some(annotation) = from_proto::annotation(&snap, code_lens.clone())? else {
1426+
let Some(data) = code_lens.data.take() else { return Ok(code_lens) };
1427+
let resolve = serde_json::from_value::<lsp_ext::CodeLensResolveData>(data)?;
1428+
let Some(annotation) = from_proto::annotation(&snap, code_lens.range, resolve)? else {
14201429
return Ok(code_lens);
14211430
};
14221431
let annotation = snap.analysis.resolve_annotation(annotation)?;
@@ -1522,8 +1531,12 @@ pub(crate) fn handle_inlay_hints_resolve(
15221531

15231532
let Some(data) = original_hint.data.take() else { return Ok(original_hint) };
15241533
let resolve_data: lsp_ext::InlayHintResolveData = serde_json::from_value(data)?;
1525-
let Some(hash) = resolve_data.hash.parse().ok() else { return Ok(original_hint) };
15261534
let file_id = FileId::from_raw(resolve_data.file_id);
1535+
if resolve_data.version != snap.file_version(file_id) {
1536+
tracing::warn!("Inlay hint resolve data is outdated");
1537+
return Ok(original_hint);
1538+
}
1539+
let Some(hash) = resolve_data.hash.parse().ok() else { return Ok(original_hint) };
15271540
anyhow::ensure!(snap.file_exists(file_id), "Invalid LSP resolve data");
15281541

15291542
let line_index = snap.file_line_index(file_id)?;

‎src/tools/rust-analyzer/crates/rust-analyzer/src/lsp/ext.rs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -561,6 +561,7 @@ pub struct CodeAction {
561561
pub struct CodeActionData {
562562
pub code_action_params: lsp_types::CodeActionParams,
563563
pub id: String,
564+
pub version: Option<i32>,
564565
}
565566

566567
#[derive(Debug, Eq, PartialEq, Clone, Default, Deserialize, Serialize)]
@@ -802,13 +803,15 @@ impl Request for OnTypeFormatting {
802803
pub struct CompletionResolveData {
803804
pub position: lsp_types::TextDocumentPositionParams,
804805
pub imports: Vec<CompletionImport>,
806+
pub version: Option<i32>,
805807
}
806808

807809
#[derive(Debug, Serialize, Deserialize)]
808810
pub struct InlayHintResolveData {
809811
pub file_id: u32,
810812
// This is a string instead of a u64 as javascript can't represent u64 fully
811813
pub hash: String,
814+
pub version: Option<i32>,
812815
}
813816

814817
#[derive(Debug, Serialize, Deserialize)]

‎src/tools/rust-analyzer/crates/rust-analyzer/src/lsp/from_proto.rs

Lines changed: 7 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -9,10 +9,8 @@ use syntax::{TextRange, TextSize};
99
use vfs::AbsPathBuf;
1010

1111
use crate::{
12-
from_json,
1312
global_state::GlobalStateSnapshot,
1413
line_index::{LineIndex, PositionEncoding},
15-
lsp::utils::invalid_params_error,
1614
lsp_ext,
1715
};
1816

@@ -105,16 +103,13 @@ pub(crate) fn assist_kind(kind: lsp_types::CodeActionKind) -> Option<AssistKind>
105103

106104
pub(crate) fn annotation(
107105
snap: &GlobalStateSnapshot,
108-
code_lens: lsp_types::CodeLens,
106+
range: lsp_types::Range,
107+
data: lsp_ext::CodeLensResolveData,
109108
) -> anyhow::Result<Option<Annotation>> {
110-
let data =
111-
code_lens.data.ok_or_else(|| invalid_params_error("code lens without data".to_owned()))?;
112-
let resolve = from_json::<lsp_ext::CodeLensResolveData>("CodeLensResolveData", &data)?;
113-
114-
match resolve.kind {
109+
match data.kind {
115110
lsp_ext::CodeLensResolveDataKind::Impls(params) => {
116111
if snap.url_file_version(&params.text_document_position_params.text_document.uri)
117-
!= Some(resolve.version)
112+
!= Some(data.version)
118113
{
119114
return Ok(None);
120115
}
@@ -123,19 +118,19 @@ pub(crate) fn annotation(
123118
let line_index = snap.file_line_index(file_id)?;
124119

125120
Ok(Annotation {
126-
range: text_range(&line_index, code_lens.range)?,
121+
range: text_range(&line_index, range)?,
127122
kind: AnnotationKind::HasImpls { pos, data: None },
128123
})
129124
}
130125
lsp_ext::CodeLensResolveDataKind::References(params) => {
131-
if snap.url_file_version(&params.text_document.uri) != Some(resolve.version) {
126+
if snap.url_file_version(&params.text_document.uri) != Some(data.version) {
132127
return Ok(None);
133128
}
134129
let pos @ FilePosition { file_id, .. } = file_position(snap, params)?;
135130
let line_index = snap.file_line_index(file_id)?;
136131

137132
Ok(Annotation {
138-
range: text_range(&line_index, code_lens.range)?,
133+
range: text_range(&line_index, range)?,
139134
kind: AnnotationKind::HasReferences { pos, data: None },
140135
})
141136
}

‎src/tools/rust-analyzer/crates/rust-analyzer/src/lsp/to_proto.rs

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -225,13 +225,14 @@ pub(crate) fn snippet_text_edit_vec(
225225
pub(crate) fn completion_items(
226226
config: &Config,
227227
line_index: &LineIndex,
228+
version: Option<i32>,
228229
tdpp: lsp_types::TextDocumentPositionParams,
229230
items: Vec<CompletionItem>,
230231
) -> Vec<lsp_types::CompletionItem> {
231232
let max_relevance = items.iter().map(|it| it.relevance.score()).max().unwrap_or_default();
232233
let mut res = Vec::with_capacity(items.len());
233234
for item in items {
234-
completion_item(&mut res, config, line_index, &tdpp, max_relevance, item);
235+
completion_item(&mut res, config, line_index, version, &tdpp, max_relevance, item);
235236
}
236237

237238
if let Some(limit) = config.completion(None).limit {
@@ -246,6 +247,7 @@ fn completion_item(
246247
acc: &mut Vec<lsp_types::CompletionItem>,
247248
config: &Config,
248249
line_index: &LineIndex,
250+
version: Option<i32>,
249251
tdpp: &lsp_types::TextDocumentPositionParams,
250252
max_relevance: u32,
251253
item: CompletionItem,
@@ -328,7 +330,7 @@ fn completion_item(
328330
})
329331
.collect::<Vec<_>>();
330332
if !imports.is_empty() {
331-
let data = lsp_ext::CompletionResolveData { position: tdpp.clone(), imports };
333+
let data = lsp_ext::CompletionResolveData { position: tdpp.clone(), imports, version };
332334
lsp_item.data = Some(to_value(data).unwrap());
333335
}
334336
}
@@ -483,6 +485,7 @@ pub(crate) fn inlay_hint(
483485
to_value(lsp_ext::InlayHintResolveData {
484486
file_id: file_id.index(),
485487
hash: hash.to_string(),
488+
version: snap.file_version(file_id),
486489
})
487490
.unwrap(),
488491
),
@@ -1318,7 +1321,7 @@ pub(crate) fn code_action_kind(kind: AssistKind) -> lsp_types::CodeActionKind {
13181321
pub(crate) fn code_action(
13191322
snap: &GlobalStateSnapshot,
13201323
assist: Assist,
1321-
resolve_data: Option<(usize, lsp_types::CodeActionParams)>,
1324+
resolve_data: Option<(usize, lsp_types::CodeActionParams, Option<i32>)>,
13221325
) -> Cancellable<lsp_ext::CodeAction> {
13231326
let mut res = lsp_ext::CodeAction {
13241327
title: assist.label.to_string(),
@@ -1336,10 +1339,11 @@ pub(crate) fn code_action(
13361339

13371340
match (assist.source_change, resolve_data) {
13381341
(Some(it), _) => res.edit = Some(snippet_workspace_edit(snap, it)?),
1339-
(None, Some((index, code_action_params))) => {
1342+
(None, Some((index, code_action_params, version))) => {
13401343
res.data = Some(lsp_ext::CodeActionData {
13411344
id: format!("{}:{}:{index}", assist.id.0, assist.id.1.name()),
13421345
code_action_params,
1346+
version,
13431347
});
13441348
}
13451349
(None, None) => {

0 commit comments

Comments
 (0)