Skip to content

Commit d273439

Browse files
YaLTeRautozimu
authored andcommitted
Fix threading issues in documentHighlight
Due to code being split between critical sections, when many documentHighlight requests were processed at once, it was possible (and very easy) for some highlights to not get cleaned up properly and be left hanging in the editor forever. By putting all state reading and updating into a single critical section, this is no longer possible as state updates are now fully coherent.
1 parent 996206f commit d273439

File tree

1 file changed

+52
-41
lines changed

1 file changed

+52
-41
lines changed

src/language_server_protocol.rs

Lines changed: 52 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -362,46 +362,51 @@ impl LanguageClient {
362362

363363
let buffer = self.vim()?.get_bufnr(&filename, params)?;
364364

365-
let source = if let Some(hs) = self.get(|state| state.document_highlight_source)? {
366-
if hs.buffer == buffer {
367-
// If we want to highlight in the same buffer as last time, we can reuse
368-
// the previous source.
369-
Some(hs.source)
365+
// The following code needs to be inside the critical section as a whole to update
366+
// everything correctly and not leave hanging highlights.
367+
self.update(|state| {
368+
let source = if let Some(hs) = state.document_highlight_source {
369+
if hs.buffer == buffer {
370+
// If we want to highlight in the same buffer as last time, we can reuse
371+
// the previous source.
372+
Some(hs.source)
373+
} else {
374+
// Clear the highlight in the previous buffer.
375+
state.vim.rpcclient.notify(
376+
"nvim_buf_clear_highlight",
377+
json!([hs.buffer, hs.source, 0, -1]),
378+
)?;
379+
380+
None
381+
}
370382
} else {
371-
// Clear the highlight in the previous buffer.
372-
self.vim()?.rpcclient.notify(
373-
"nvim_buf_clear_highlight",
374-
json!([hs.buffer, hs.source, 0, -1]),
375-
)?;
376-
377383
None
378-
}
379-
} else {
380-
None
381-
};
384+
};
382385

383-
let source = match source {
384-
Some(source) => source,
385-
None => {
386-
// Create a new source.
387-
let source = self.vim()?.rpcclient.call(
388-
"nvim_buf_add_highlight",
389-
json!([buffer, 0, "Error", 1, 1, 1]),
390-
)?;
391-
self.update(|state| {
386+
let source = match source {
387+
Some(source) => source,
388+
None => {
389+
// Create a new source.
390+
let source = state.vim.rpcclient.call(
391+
"nvim_buf_add_highlight",
392+
json!([buffer, 0, "Error", 1, 1, 1]),
393+
)?;
392394
state.document_highlight_source = Some(HighlightSource { buffer, source });
393-
Ok(())
394-
})?;
395-
source
396-
}
397-
};
395+
source
396+
}
397+
};
398398

399-
self.vim()?
400-
.rpcclient
401-
.notify("nvim_buf_clear_highlight", json!([buffer, source, 0, -1]))?;
402-
self.vim()?
403-
.rpcclient
404-
.notify("s:AddHighlights", json!([source, highlights]))?;
399+
state
400+
.vim
401+
.rpcclient
402+
.notify("nvim_buf_clear_highlight", json!([buffer, source, 0, -1]))?;
403+
state
404+
.vim
405+
.rpcclient
406+
.notify("s:AddHighlights", json!([source, highlights]))?;
407+
408+
Ok(())
409+
})?;
405410
}
406411

407412
info!("End {}", lsp::request::DocumentHighlightRequest::METHOD);
@@ -411,12 +416,18 @@ impl LanguageClient {
411416
pub fn languageClient_clearDocumentHighlight(&self, _params: &Value) -> Fallible<()> {
412417
info!("Begin {}", NOTIFICATION__ClearDocumentHighlight);
413418

414-
let buffer_source = self.update(|state| Ok(state.document_highlight_source.take()))?;
415-
if let Some(HighlightSource { buffer, source }) = buffer_source {
416-
self.vim()?
417-
.rpcclient
418-
.notify("nvim_buf_clear_highlight", json!([buffer, source, 0, -1]))?;
419-
}
419+
// The following code needs to be inside the critical section as a whole to update
420+
// everything correctly and not leave hanging highlights.
421+
self.update(|state| {
422+
if let Some(HighlightSource { buffer, source }) = state.document_highlight_source.take()
423+
{
424+
state
425+
.vim
426+
.rpcclient
427+
.notify("nvim_buf_clear_highlight", json!([buffer, source, 0, -1]))?;
428+
}
429+
Ok(())
430+
})?;
420431

421432
info!("End {}", NOTIFICATION__ClearDocumentHighlight);
422433
Ok(())

0 commit comments

Comments
 (0)